fix(ics): emit RRULE and EXDATE for recurring events in user feed#386
Merged
Conversation
The user-facing ICS feed at /api/cal/{token}.ics emits recurring events
as single one-off VEVENTs anchored at the master DTSTART. Two gaps
converge:
1. generateUserCalendar does not select recurrenceRule or exceptionDates
from event_states, so the data never reaches the formatter.
2. formatEventsAsIcal does not pass recurrenceRule or exceptionDates to
the IcsEvent passed to ts-ics, so even when present they would be
dropped.
Net effect: a weekly meeting set up in October 2025 appears in client
calendars as a single event in October 2025 and never again.
This change adds JSON parsing + date revival when reading
recurrenceRule and exceptionDates from the DB (stored as JSON strings
of ts-ics shapes), and threads them through to the IcsEvent. Adds
regression tests covering RRULE emission, EXDATE emission, and that
one-off events remain RRULE-less.
ridafkih
approved these changes
May 23, 2026
ridafkih
reviewed
May 23, 2026
| return null; | ||
| } | ||
| try { | ||
| return JSON.parse(value) as T; |
Owner
There was a problem hiding this comment.
We should really be validating this, rather than using type assertions. You can use a predicate to verify the structure.
ridafkih
reviewed
May 23, 2026
| } | ||
| // ts-ics expects Date objects on the in-memory shape (e.g. UNTIL is a {date: Date}). | ||
| // JSON.parse leaves these as ISO strings, so revive them before re-serializing. | ||
| return reviveDates(parsed) as IcsRecurrenceRule; |
ridafkih
reviewed
May 23, 2026
| if (!parsed) { | ||
| return null; | ||
| } | ||
| return reviveDates(parsed) as IcsDateObject[]; |
ridafkih
reviewed
May 23, 2026
| }; | ||
|
|
||
| const reviveDates = (value: unknown): unknown => { | ||
| if (typeof value === "string" && /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?Z$/.test(value)) { |
Owner
There was a problem hiding this comment.
Would prefer if the Regex pattern here was pulled up to the top, but I tend to try to avoid patterns like this as much as possible.
ridafkih
requested changes
May 23, 2026
Owner
ridafkih
left a comment
There was a problem hiding this comment.
Left a few comments above, may address them myself.
ridafkih
added a commit
to agurod42/keeper.sh
that referenced
this pull request
May 26, 2026
- Remove duplicate `recurrenceId?: Date` declarations in EventTimeSlot and the CalDAV ParsedCalendarEvent shape (left over from a rebase). - Add `recurrenceId` to the expected sorted conflict-set keys in write-event-states.test.ts to match the new column added in this PR. - Relax the RRULE assertion in the master/overrides grouping test to a regex — ts-ics emits RRULE properties in alphabetical order so `BYDAY` lands before `FREQ` and the literal substring check fails. - Apply the same oxlint fixes from PR ridafkih#386 (rename `<T>` → `<TValue>` for the id-length rule, capitalize multi-line `//` comments) so this PR's CI passes independently. These will collapse to a no-op once this branch rebases on a merged ridafkih#386. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Rename single-letter generic `<T>` to `<TValue>` to satisfy id-length rule. - Rewrite multi-line `//` comments so each starts with an uppercase letter (or collapsed into a block comment for the test regression note). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ridafkih
added a commit
to agurod42/keeper.sh
that referenced
this pull request
May 26, 2026
The previous fallback to null on JSON.parse or arktype validation failure silently degraded a recurring event into a one-off VEVENT — exactly the bug PR ridafkih#386 was trying to fix. Since the JSONB columns are written by our own ingest pipelines, a validation failure means a bug, a schema drift, or DB corruption, not normal operation. - `parseRecurrenceRule` / `parseExceptionDates` now throw with the offending event_state id and the arktype error summary (or the underlying JSON.parse error as `cause`). - The route is already wrapped in `withWideEvent`, which catches the throw and calls `widelog.errorFields(error)` automatically — so monitoring sees status_code=500, outcome=error, and the full error context. No new logging plumbing required. - Cost: one bad row fails the whole .ics endpoint for that user instead of silently emitting wrong data. Loud failure is preferable to wrong output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ridafkih
approved these changes
May 26, 2026
ridafkih
added a commit
to agurod42/keeper.sh
that referenced
this pull request
May 26, 2026
- Remove duplicate `recurrenceId?: Date` declarations in EventTimeSlot and the CalDAV ParsedCalendarEvent shape (left over from a rebase). - Add `recurrenceId` to the expected sorted conflict-set keys in write-event-states.test.ts to match the new column added in this PR. - Relax the RRULE assertion in the master/overrides grouping test to a regex — ts-ics emits RRULE properties in alphabetical order so `BYDAY` lands before `FREQ` and the literal substring check fails. - Apply the same oxlint fixes from PR ridafkih#386 (rename `<T>` → `<TValue>` for the id-length rule, capitalize multi-line `//` comments) so this PR's CI passes independently. These will collapse to a no-op once this branch rebases on a merged ridafkih#386. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ridafkih
added a commit
to agurod42/keeper.sh
that referenced
this pull request
May 26, 2026
The previous fallback to null on JSON.parse or arktype validation failure silently degraded a recurring event into a one-off VEVENT — exactly the bug PR ridafkih#386 was trying to fix. Since the JSONB columns are written by our own ingest pipelines, a validation failure means a bug, a schema drift, or DB corruption, not normal operation. - `parseRecurrenceRule` / `parseExceptionDates` now throw with the offending event_state id and the arktype error summary (or the underlying JSON.parse error as `cause`). - The route is already wrapped in `withWideEvent`, which catches the throw and calls `widelog.errorFields(error)` automatically — so monitoring sees status_code=500, outcome=error, and the full error context. No new logging plumbing required. - Cost: one bad row fails the whole .ics endpoint for that user instead of silently emitting wrong data. Loud failure is preferable to wrong output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ridafkih
added a commit
that referenced
this pull request
May 26, 2026
…387) * fix(ics): emit RRULE and EXDATE for recurring events in user feed The user-facing ICS feed at /api/cal/{token}.ics emits recurring events as single one-off VEVENTs anchored at the master DTSTART. Two gaps converge: 1. generateUserCalendar does not select recurrenceRule or exceptionDates from event_states, so the data never reaches the formatter. 2. formatEventsAsIcal does not pass recurrenceRule or exceptionDates to the IcsEvent passed to ts-ics, so even when present they would be dropped. Net effect: a weekly meeting set up in October 2025 appears in client calendars as a single event in October 2025 and never again. This change adds JSON parsing + date revival when reading recurrenceRule and exceptionDates from the DB (stored as JSON strings of ts-ics shapes), and threads them through to the IcsEvent. Adds regression tests covering RRULE emission, EXDATE emission, and that one-off events remain RRULE-less. * fix(ics): emit recurring events as master + RECURRENCE-ID overrides When a CalDAV/ICS source serializes a recurring event with modified instances (e.g. an Outlook weekly meeting where one occurrence was moved to a different time), each override arrives as a VEVENT with the same UID as the master plus a RECURRENCE-ID pointing at the occurrence being replaced. Keeper parsed each override into its own event_states row but dropped the RECURRENCE-ID, then emitted them back into the user feed as standalone VEVENTs with brand new UIDs. Calendar clients then showed BOTH the original occurrence (from the master's RRULE expansion) AND the override as a separate event — visible duplicate. This change: 1. Adds a `recurrenceId timestamp` column to event_states (migration `0070_event_states_recurrence_id.sql`). Existing rows have NULL; subsequent syncs populate it. 2. Updates the CalDAV and ICS parsers to read `event.recurrenceId?.value?.date` from ts-ics. The ts-ics IcsRecurrenceId type is `{ range?, value: IcsDateObject }`, so the parsed event exposes the date at `.value.date`, not `.date`. 3. Threads `recurrenceId` through `SyncableEvent`, `SourceEvent`, `EventTimeSlot`, `ParsedCalendarEvent`, the shared ingest flush in services/cron, the CalDAV provider DB write, and the `EVENT_STATE_CONFLICT_SET` so re-syncs update the field. 4. Updates the user-facing ICS feed generator to group rows by `sourceEventUid`. For each group, the master (row with `recurrenceRule != null` and `recurrenceId == null`) is emitted with its own UID; each override reuses the master's UID and emits the `RECURRENCE-ID` it parsed. ts-ics's `IcsRecurrenceId` shape is `{ value: { date } }`. Adds regression tests covering master+override grouping and standalone events without sourceEventUid. Builds on the RRULE/EXDATE fix in the parent commit on this branch. * refactor(api): single-pass grouping + granular helpers for ical feed Addresses the review feedback that the recurring-event grouping was doing too many iterations and would benefit from being split into well-named helpers. - Replace two-pass `groupRecurringEvents` (bucket, then findIndex + splice to reorder master) with single-pass `groupEventsBySourceUid` that identifies the master inline by promoting it as soon as one is seen. - Introduce explicit `EventGroup { master, overrides }` type so the master is a typed field instead of "group[0] after you reordered correctly." - Split out `isRecurringMaster`, `applyMasterRecurrence`, `applyOverrideRecurrence`, `buildIcsEventsForGroup`, `shouldIncludeEvent` so each step in the feed-emission pipeline has a single, named purpose. No behavioral change: tests pin RRULE on master, RECURRENCE-ID on overrides, and standalone UIDs for events without sourceEventUid. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: fix CI failures (duplicate types, test assertions, lint) - Remove duplicate `recurrenceId?: Date` declarations in EventTimeSlot and the CalDAV ParsedCalendarEvent shape (left over from a rebase). - Add `recurrenceId` to the expected sorted conflict-set keys in write-event-states.test.ts to match the new column added in this PR. - Relax the RRULE assertion in the master/overrides grouping test to a regex — ts-ics emits RRULE properties in alphabetical order so `BYDAY` lands before `FREQ` and the literal substring check fails. - Apply the same oxlint fixes from PR #386 (rename `<T>` → `<TValue>` for the id-length rule, capitalize multi-line `//` comments) so this PR's CI passes independently. These will collapse to a no-op once this branch rebases on a merged #386. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): pure construction for ical event building Replaces the mutate-and-return helpers (`applyMasterRecurrence`, `applyOverrideRecurrence`) with pure builders that construct IcsEvent objects via conditional spread. Also rewrites `buildBaseIcsEvent` so description/location are part of the literal instead of post-construction mutation. No behavioral change. The conditional-spread `&&` form is the same idiom already used elsewhere in this file (e.g. `...(isAllDay && { type: "DATE" })`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): validate stored ICS json with arktype schemas Replaces the hand-rolled JSON.parse + recursive reviveDates + `as IcsRecurrenceRule` / `as IcsDateObject[]` pipeline with arktype schemas in @keeper.sh/data-schemas. The schemas use `string.date.iso.parse` morphs which validate AND convert ISO strings back into Date instances in a single pass — ts-ics requires Date objects on its in-memory shape. - `icsDateObjectSchema`, `icsRecurrenceRuleSchema`, `icsExceptionDatesSchema` live alongside the other domain schemas (google, outlook, caldav, etc.). - `services/api/src/utils/ical.ts` loses `reviveDates`, the generic `parseStoredJsonField<T>`, and the three `as` assertions. Parsing is now: JSON.parse → schema validation → typed result or null on error. - Invalid stored payloads now fail closed (return null) instead of silently typecasting garbage as IcsRecurrenceRule. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api): throw on invalid stored ICS recurrence/exception data The previous fallback to null on JSON.parse or arktype validation failure silently degraded a recurring event into a one-off VEVENT — exactly the bug PR #386 was trying to fix. Since the JSONB columns are written by our own ingest pipelines, a validation failure means a bug, a schema drift, or DB corruption, not normal operation. - `parseRecurrenceRule` / `parseExceptionDates` now throw with the offending event_state id and the arktype error summary (or the underlying JSON.parse error as `cause`). - The route is already wrapped in `withWideEvent`, which catches the throw and calls `widelog.errorFields(error)` automatically — so monitoring sees status_code=500, outcome=error, and the full error context. No new logging plumbing required. - Cost: one bad row fails the whole .ics endpoint for that user instead of silently emitting wrong data. Loud failure is preferable to wrong output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Rida F'kih <github.com@rida.dev> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Rida F'kih <hello@rida.dev>
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.
Summary
The user-facing ICS feed at
/api/cal/{token}.icsemits recurring events as single one-offVEVENTs anchored at the masterDTSTART. Two gaps converge:generateUserCalendar(inservices/api/src/utils/ical.ts) does not selectrecurrenceRuleorexceptionDatesfromevent_states, so the data never reaches the formatter.formatEventsAsIcal(inservices/api/src/utils/ical-format.ts) does not threadrecurrenceRuleorexceptionDatesinto theIcsEventit passes tots-ics, so even if they arrive they get dropped.Net effect: a weekly meeting set up in October 2025 appears in client calendars as a single event in October 2025 and never again — every recurring event becomes a single occurrence in the distant past.
Fix
recurrenceRuleandexceptionDatesto theevent_statesSELECT ingenerateUserCalendar.JSON.parses the stored JSON strings (the providers store these viaJSON.stringify(...)) and revives any ISO-string dates back intoDateobjects —ts-icsexpectsDateinstances on the in-memory shape (recurrenceRule.until.date, eachexceptionDates[i].date).recurrenceRuleandexceptionDatesto theCalendarEventinterface inical-format.ts, and sets them on the emittedIcsEventwhen present.Repro / tested
Against a CalDAV calendar with an
RRULE:FREQ=WEEKLYmaster set up months ago, the feed previously had zeroRRULE:lines despite many recurring events being stored. After the fix the same calendar producesRRULE:for each master plusEXDATEfor events with skipped occurrences.Added unit tests in
services/api/tests/utils/ical.test.ts:emits RRULE for events with a recurrenceRule— covers the basic case.emits EXDATE for events with exceptionDates— covers skipped occurrences.does not emit RRULE for one-off events— guards against accidentally emitting RRULE/EXDATE when neither is set.Notes
recurrenceRuleandexceptionDatesand persist them toevent_statesviaserializeOptionalJson/stringifyIfPresent. This change wires the read path so the feed actually emits them.