feat(notifications): quota pace alerts — 3 triggers, launch-prime, per-app stacking (#633)#786
Merged
Merged
Conversation
Add macOS user notifications that fire when a quota's pace gets worse, at three milestones, deduped once per metric per reset period: - first time under 10% remaining, - pace healthy (blue) -> close-to-limit (yellow), - pace close-to-limit (yellow) -> running-out (red). Master toggle and all three triggers default ON; authorization is requested at first launch. Evaluation runs every refresh tick so pace worsening from elapsed time alone still alerts with the popover closed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up to the quota-pace notifications work (#633): - Drop the master "Quota Notifications" toggle; keep the three triggers, renamed to Almost Out / Cutting It Close / Will Run Out, each with an (i) tooltip. Notification copy now uses title (trigger name), subtitle (provider + metric), and body (the verdict). - Permission UX: warning triangle on the Notifications header plus a conditional action row under the toggles — "Open System Settings" when macOS denied, "Allow Notifications" when still undecided. Authorization is re-checked on NSApplication.didBecomeActiveNotification so it clears when the user returns from System Settings. - Fix the under-10% check to use a display-mode-independent remaining fraction; the old data.fraction meant "used/limit" when the meter showed "used", inverting the alert. - Cold-start prime: record each metric's first real observation as a baseline without firing, so a quota already in a bad state at launch no longer posts two alerts on open; only worsening during the session fires. - Per-app threadIdentifier so simultaneous alerts stack into one banner. 514 tests pass (1 pre-existing skip). Co-authored-by: Cursor <cursoragent@cursor.com>
Temporary #if DEBUG section in Settings with a "Fire" button per quota trigger (Almost Out / Cutting It Close / Will Run Out) plus "All Three", so the banner title/subtitle/body, stacking, and permission flow can be verified on demand without waiting for a real worsening. Posts via AppNotifications.shared.post, bypassing the dedup/prime logic. DEBUG-only — not compiled into release builds. Co-authored-by: Cursor <cursoragent@cursor.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
robinebers has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ensureAuthorization memoizes its result, and post treated a cached false as final — so after the user re-enabled notifications in System Settings, the Settings UI cleared its warning but quota alerts still skipped delivery until the app restarted. Re-read the live authorization status when the cached result is false; if it's now authorized, refresh the cache and proceed. Addresses Cursor Bugbot medium-severity finding on PR #786. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
robinebers has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
…s off When a milestone's toggle was off during a worsening, maybeFire skipped posting but the logic still advanced previousBucket/wasUnderTenPercent, so the crossing was silently consumed — turning the trigger back on while the quota remained in the worse bucket never alerted. Now the recorded bucket/under-10% flag only advance when the worsening was actually alerted (or there was no worsening); a skipped crossing persists so re-enabling the trigger fires on the next evaluation. maybeFire now returns whether it fired. Adds a regression test. Addresses Cursor Bugbot medium-severity finding on PR #786. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
robinebers has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Milestone dedup marked a trigger fired (and advanced previousBucket/ wasUnderTenPercent) before AppNotifications.post actually delivered anything — post returned immediately and could skip (not authorized) or fail (add errored), so the alert was lost for the rest of the reset window. post now awaits delivery and returns Bool; evaluateNotifications commits a milestone's dedup state only when delivery succeeded, and reverts the mark + state advance on failure so it re-fires on the next pass. evaluateNotifications is now async (awaited from the periodic loop); the DEBUG fire buttons wrap in Task. Adds a failed-delivery-retries regression test. Addresses Cursor Bugbot high-severity finding on PR #786. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
robinebers has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
…ggle-on The three quota triggers now default OFF, and the app requests notification authorization the first time a trigger is turned on (from Settings) instead of at launch — so a fresh install stays quiet until the user opts in. Drops the launch-time requestAuthorization call. Also makes the dev build's BUNDLE_ID env-overridable in build_and_run.sh so a fresh bundle id can be used to reset macOS notification permission for re-testing the prompt/deny flow. Tests opt the triggers on explicitly (the store defaults are now off). Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
robinebers has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 31dbeee. Configure here.
…; full-width action button The denied/not-determined explanation moves from a caption under the toggles into a hover tooltip on the Notifications header triangle, and the "Open System Settings" / "Allow Notifications" button becomes full width. Less clutter, same information. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
robinebers has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
…el-band metrics Two Cursor Bugbot findings on PR #786: - High "Dedup before delivery succeeds": the logic marked a milestone fired (inserted into firedMilestones) before AppNotifications.post delivered. Now maybeFire only appends a candidate; the caller inserts the dedup mark after delivery succeeds, so a skipped/failed delivery leaves it un-marked and the milestone re-fires next pass. - Medium "Almost Out skips level meter state": .level metrics (no pace projection, but used/limit known) returned early, so "Almost Out" never fired for them. The under-10% check now runs for .level too — it's a remaining-based trigger, not a pace one — while .noData still skips entirely. .level primes on first observation like any other state. Adds a .level-fires-Almost-Out regression test; renames the stale "level never fires" tests. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
robinebers has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Adds opt-out macOS notifications that alert when a quota's pace worsens — three triggers (Almost Out / Cutting It Close / Will Run Out), deduped once per metric per reset period, primed at launch so an already-bad quota doesn't spam on open, and grouped into one stacked thread. Supersedes #775.
What was happening
What this changes
NSApplication.didBecomeActiveNotificationso it clears when the user returns from System Settings.threadIdentifierso simultaneous alerts group in Notification Center.data.fractionmeant "used/limit" when the meter showed "used", inverting the alert).Heads-up
threadIdentifiergroups them in Notification Center (not as simultaneous on-screen banners) — this is a platform limitation, not a bug.#if DEBUGonly and not compiled into release builds; temporary.Tests
PaceNotificationLogicTests(15),WidgetDataStoreNotificationTests(6),AppNotificationsTests(2)..useddisplay-mode under-10% case and rewrote the cold-start test to assert priming (no fire) + a later worsening firing.swift buildclean.Screenshots
Made with Cursor