Feat/calendar reminders - #4079
Draft
yevhen-sychov wants to merge 7 commits into
Draft
Conversation
Calendar events carry reminder information that noctalia currently discards: VALARM triggers in CalDAV/ICS feeds and reminder overrides in the Google API. Introduce the data model and configuration surface for surfacing them, ahead of the parsing and notification work. CalendarEvent gains reminderLeadSeconds, holding offsets before the event start rather than absolute instants. Recurrence expansion builds each occurrence by copying the base event and overwriting only start/end, so relative leads are inherited by every instance for free. It also matches the Google API's native "minutes before start" model. [calendar.reminders] configures the behaviour: whether to honour per-event reminders, the fallback lead for events carrying none, and the time of day for a single digest covering all-day events. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cUUcFnV6JYbw8LwVS3k3J
The rule deciding when a reminder fires is the part most worth testing, so keep it a pure function: planReminders() takes a snapshot, the config, the set of already-fired reminders and the current time, and returns what to notify. No NotificationManager, ConfigService or D-Bus involved, mirroring how google_calendar_list.h is structured. A reminder fires when it is due, has not fired before, and is still relevant -- meaning its event has not started, or it came due within kLateGrace. The grace window is what makes an at-start reminder (TRIGGER:PT0S) possible at all: it comes due exactly at the event start, so requiring the event to be in the future would make it unsatisfiable. It also lets a wakeup delayed by suspend deliver an alert that is only moments late, while reminders for events that began long ago stay dropped. Fired reminders are identified by (uid, start, lead). The event start is part of the key because recurrence instances share a UID. The account deliberately is not, so one meeting subscribed through two accounts notifies once. Keys are pruned once their event is past the grace window, which bounds the set without a horizon watermark -- a watermark would silently swallow a reminder for a freshly accepted invitation whose lead time has already elapsed. When several reminders for one event come due together, as happens on a cold start where every lead is overdue, only the one closest to the event notifies. Grouping is by (start, title) so duplicates collapse even when two providers disagree on the UID. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cUUcFnV6JYbw8LwVS3k3J
The parser documented that VALARM was ignored, so reminders set in a calendar app never reached noctalia. Read them into reminderLeadSeconds. Extraction happens in baseEventFromComponent, which both the plain and the recurring path go through, so expanded occurrences inherit the leads without touching addRecurrence. collectVEvents is deliberately left alone: VALARMs are children of the VEVENT and are reached directly, and making the collector descend into VEVENTs would let a nested component register as a top-level event. DISPLAY and AUDIO alarms are honoured, as is a VALARM with no ACTION at all. EMAIL is skipped because the server delivers it, PROCEDURE asks us to run a script, and NONE is not actionable. Two details the RFC forces: - Triggers are validated with icaltriggertype_is_bad_trigger rather than is_null_trigger, because the latter also reports true for the perfectly valid TRIGGER:PT0S, which must survive as a zero lead. - RELATED=END anchors the trigger to DTEND, so the lead before DTSTART shrinks by the event's own duration. A short end-relative trigger on a long event therefore lands after the start and is dropped. Absolute triggers on recurring events are skipped: such a trigger fires once for the whole series, so replicating its offset onto every occurrence would be wrong. A VALARM's REPEAT/DURATION snooze ladder is ignored, as honouring it would multiply toasts with no dismissal semantics to stop them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cUUcFnV6JYbw8LwVS3k3J
Google returns reminder settings per event, but the client dropped them. Explicit overrides win; useDefault falls back to the calendar's own defaults, which the events.list response already carries at the top level. Reading them from there rather than from calendarList keeps the resolution in one payload, so it cannot go stale against a separately fetched list. The request needs no change, as it sends no fields mask and already receives the reminders object. Reminder methods are filtered to popup and display, matching the VALARM ACTION policy: email reminders are delivered by Google itself. The helpers live in a header of their own so they can be tested without an HttpClient, following google_calendar_list.h. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cUUcFnV6JYbw8LwVS3k3J
The encrypted event cache is what the shell reads at startup before the first network sync completes, so without reminders in it a restart would miss any reminder due before that sync lands. Reads use a defaulted lookup, so an existing cache loads with no reminders and is corrected on the next sync -- no format version bump needed. Leads are normalised on read as well as on write, so a corrupted or hand-edited cache cannot inflate the fired set or the notification burst. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cUUcFnV6JYbw8LwVS3k3J
…ctions Internal notifications are toast-only: they are never written to history, and their actions are routed to the D-Bus callback. Both rules suit every current caller -- battery warnings re-fire, calendar credential failures resurface in settings, the config migration reminder re-arms on a timer -- but not a one-shot alert whose entire value is a moment in time and which has no other surface. Two narrow additions rather than a blanket change of internal behaviour: - NotificationRequest::persistInHistory opts a notification into history. Filters still apply, so a user can silence an opted-in source by app name, and Urgency::Low remains excluded regardless. The flag round-trips through the history store so restored entries keep it. addInternal() is left alone; callers wanting this use addOrReplace(). - Actions on notifications of internal origin are dispatched in-process through setInternalActionCallback instead of the external callback. An internally generated notification has no D-Bus owner, so emitting ActionInvoked for it signalled into the void; this also makes internal notifications able to act on a click at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cUUcFnV6JYbw8LwVS3k3J
Completes the feature: a monitor turns the planner's decisions into notifications, and the shell wakes up in time to deliver them. CalendarReminderMonitor is level-triggered like BatteryWarningMonitor, so it is safe to call on every tick, on snapshot change and on config reload. Fired reminders persist through the state store, keyed so a restart does not re-notify what already fired, and the monitor is seeded from the encrypted cache before the first sync so reminders missed while the shell was down still fire. It carries its own poll source rather than extending CalendarService's. That method only advertises a deadline while a refresh is pending, and refresh_minutes reaches 240, which would leave reminders up to four hours late. When nothing is armed it reports no timeout at all, so an idle shell gains no wakeups. Deadlines are held in wall-clock time and converted on each call, because a monotonic timeout does not advance across suspend; the five minute ceiling bounds how late a resumed session can deliver. All-day events are surfaced only through a once-a-day digest instead of a burst at midnight. Its instant is resolved through the local time zone with choose::earliest, so a digest time falling in a DST gap does not throw. Reminders stay on screen until dismissed and persist in history: one that fades while the user is away from the desk is exactly the case this feature exists to prevent. Events with a resolved meeting link get a default action, so clicking the toast opens the link -- resolveEventLink already restricts those to http(s), which is what makes them safe to hand to xdg-open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cUUcFnV6JYbw8LwVS3k3J
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
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.
What's missing today
Calendar accounts sync and events are displayed, but nothing ever tells you an event is about to start. The reminder data is discarded before it can be used:
ical_parser.hdocuments that VALARM is ignored, so a reminder set in a calendar app never reaches the shellCalendarEventhas no field to carry onereminders/defaultRemindersCalendarServicealready holds aNotificationManager*, but only for OAuth and keyring failures.What this adds
Reminder notifications for upcoming events, honouring the reminder you actually set in your calendar app, with a configurable fallback for events that carry none.
DISPLAY/AUDIOactions, relative and absolute triggers,RELATED=ENDoverrides, anduseDefaultresolved against the calendar's own defaultsNew config, all under
[calendar.reminders]:Please look closely at this part
One change is outside the calendar module and deserves review:
src/notification/gains two narrow opt-ins rather than a blanket change to internal-notification behaviour.NotificationRequest::persistInHistory— internal notifications are toast-only today, which is right for every current caller (battery warnings re-fire, credential failures resurface in settings, the migration reminder re-arms). A calendar reminder is the first whose entire value is a moment in time with no other surface. Filters still apply andUrgency::Lowstays excluded.ActionInvokedwas being emitted into the void; this also makes internal notifications able to respond to a click at all.Happy to split this into its own PR if you'd prefer it reviewed separately.
Design decisions worth knowing
Lead offsets, not absolute instants. Recurrence expansion builds each occurrence by copying the base event and overwriting only start/end, so relative leads are inherited by every instance with no change to
addRecurrence. It also matches Google's native model and keeps the cache small.A pure planner plus a thin monitor.
planReminders()is a free function over a snapshot, the config, the fired set and a caller-supplied "now" — noNotificationManager,ConfigServiceor D-Bus — which is what makes the firing rules testable.CalendarReminderMonitoris the stateful shell, modelled onBatteryWarningMonitor.One firing rule covers in-session firing, startup catch-up and dropping stale reminders: due, not already fired, and still relevant — meaning the event has not started, or the reminder came due within a five minute grace. That grace is what makes an at-start reminder (
TRIGGER:PT0S) possible at all; it comes due exactly at the start, so requiring a future event would make it unsatisfiable.Its own poll source.
CalendarService::pollTimeoutMs()only advertises a deadline while a refresh is pending, andrefresh_minutesreaches 240, which would leave reminders up to four hours late. When nothing is armed the new source reports no timeout, so an idle shell gains no wakeups. Deadlines are kept in wall-clock time, since a monotonicpoll()timeout does not advance across suspend.Bounded state. Fired reminders are keyed by
(uid, start, lead)and pruned once past the grace window, which bounds the set without a horizon watermark — a watermark would silently swallow a reminder for a freshly accepted invitation whose lead has already elapsed. Reminders are capped per event, per catch-up pass, and at rest.Some deliberate omissions, all documented in the code: a VALARM's
REPEAT/DURATIONsnooze ladder is ignored, absolute triggers on recurring events are skipped (they fire once for a series, not per occurrence), and snooze/actions beyond opening the link are out of scope.Testing
Three new test binaries and extensions to two existing ones: VALARM trigger forms including the
TRIGGER:PT0SandRELATED=ENDedge cases, Google reminder resolution, the firing rules under an injected clock, and the notification opt-ins.just testis green, and each of the 7 commits builds on its own, so the history bisects.Verified live against a real account, which turned up two bugs now fixed and covered by regression tests: at-start reminders could never fire, and a meeting carrying two reminders notified twice on a cold start.
Not covered by automated tests: the toast rendering and the browser actually opening, both of which need a running compositor.
🤖 Generated with Claude Code