From b869fe0a9357e87d70fd6407f4e71886e379bc10 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Wed, 10 Jun 2026 11:04:04 -0400 Subject: [PATCH] docs(06-02): complete RRULE UNTIL/COUNT serialization plan --- .planning/ROADMAP.md | 6 +- .planning/STATE.md | 11 +- .../phases/06-ux-polish/06-02-SUMMARY.md | 136 ++++++++++++++++++ 3 files changed, 145 insertions(+), 8 deletions(-) create mode 100644 .planning/phases/06-ux-polish/06-02-SUMMARY.md diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 4ba1bdf..e2d53c4 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -236,7 +236,7 @@ Plans: **Wave 1** *(parallel — exclusive file ownership)* - [x] 06-01-PLAN.md — TDD: duration-preserving end-tracking math (computeNewTimedEnd/computeNewAllDayEnd) in eventDateTime.ts (D-04) -- [ ] 06-02-PLAN.md — TDD: RRULE UNTIL/COUNT serialization + Zod acceptance + FREQ-persistence regression (vevent/outboxWorker/events route) (D-06/D-07) +- [x] 06-02-PLAN.md — TDD: RRULE UNTIL/COUNT serialization + Zod acceptance + FREQ-persistence regression (vevent/outboxWorker/events route) (D-06/D-07) - [ ] 06-03-PLAN.md — TDD: hasRrule on CalendarOccurrence + bounded-expansion lock (expand.ts) (D-06/D-08) - [ ] 06-04-PLAN.md — Spinner/pulse: global @keyframes pulse + remove redundant spin redefinition (D-13) - [ ] 06-05-PLAN.md — Auth gating slice: SessionExpiredError + AuthSplash + global QueryCache/MutationCache error handler; client.ts type mirrors (D-10/D-11, + D-06/D-08 type carriers) @@ -260,7 +260,7 @@ Note: Phase 4 depends only on Phase 1 and can begin as soon as Phase 1 is comple | 3. Event Write-Back + PWA Install | 12/12 | Complete | 2026-06-07 | | 4. Shared Lists + Live Sync | 6/6 | Complete | 2026-06-09 | | 5. Web Push Notifications | 8/8 | Complete | 2026-06-10 | -| 6. UX Polish | 1/6 | In Progress| | +| 6. UX Polish | 2/6 | In Progress| | ## Backlog @@ -268,7 +268,7 @@ Note: Phase 4 depends only on Phase 1 and can begin as soon as Phase 1 is comple **Goal:** [Captured for future planning] Abstract the calendar backend behind a provider interface so Fastmail/CalDAV is one implementation among potentially many. Shipping with a single provider is fine, but the broker, sync, and event-expansion layers should be structured so additional providers (e.g. other CalDAV hosts, Google Calendar, generic ICS feeds) can be added without rework. Captures the "provider" seam as an explicit architectural concern. **Requirements:** TBD -**Plans:** 1/6 plans executed +**Plans:** 2/6 plans executed Plans: diff --git a/.planning/STATE.md b/.planning/STATE.md index 3b70eff..ec1fcfa 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -4,13 +4,13 @@ milestone: v1.0 milestone_name: milestone status: executing stopped_at: Phase 06 UI-SPEC approved -last_updated: "2026-06-10T14:51:02.287Z" +last_updated: "2026-06-10T15:03:08.173Z" last_activity: 2026-06-10 -- Phase 06 execution started progress: total_phases: 17 completed_phases: 5 total_plans: 42 - completed_plans: 37 + completed_plans: 38 percent: 29 --- @@ -26,9 +26,9 @@ See: .planning/PROJECT.md (updated 2026-06-07) ## Current Position Phase: 06 (ux-polish) — EXECUTING -Plan: 2 of 6 +Plan: 3 of 6 Status: Ready to execute -Last activity: 2026-06-10 -- Phase 06 execution started +Last activity: 2026-06-10 -- Completed 06-02: RRULE UNTIL/COUNT serialization + FREQ persistence Progress: [█████████░] 89% @@ -74,6 +74,7 @@ Progress: [█████████░] 89% | Phase 05 P08 | 9 | 3 tasks | 7 files | | Phase 05 P07 | 8 | 1 tasks | 4 files | | Phase 06-ux-polish P01 | 2 | 2 tasks | 2 files | +| Phase 06-ux-polish P02 | 8 | 2 tasks | 4 files | ## Accumulated Context @@ -158,6 +159,6 @@ Recent decisions affecting current work: ## Session Continuity -Last session: 2026-06-10T14:51:02.281Z +Last session: 2026-06-10T15:03:08.167Z Stopped at: Phase 06 UI-SPEC approved Resume file: .planning/phases/06-ux-polish/06-UI-SPEC.md diff --git a/.planning/phases/06-ux-polish/06-02-SUMMARY.md b/.planning/phases/06-ux-polish/06-02-SUMMARY.md new file mode 100644 index 0000000..40a0b35 --- /dev/null +++ b/.planning/phases/06-ux-polish/06-02-SUMMARY.md @@ -0,0 +1,136 @@ +--- +phase: 06-ux-polish +plan: "02" +subsystem: api/broker +tags: [tdd, rrule, recurrence, serialization, ical.js, zod] +dependency_graph: + requires: [] + provides: + - assembleRruleString helper in apps/api/src/broker/outboxWorker.ts + - recurrenceUntil/recurrenceCount fields in outboxPayloadSchema + eventFieldsSchema + affects: + - apps/api/src/broker/outboxWorker.ts + - apps/api/src/routes/events.ts + - apps/api/tests/broker/vevent.test.ts + - apps/api/tests/broker/outboxWorker.test.ts +tech_stack: + added: [] + patterns: + - ical.js ICAL.Recur.fromString + rruleProp.setValue for RRULE serialization + - assembleRruleString count-wins-over-until mutual exclusion (RFC 5545 §3.3.10) + - Series-edit Pitfall 3: strip UNTIL/COUNT via regex before re-applying new bound +key_files: + created: [] + modified: + - apps/api/src/broker/outboxWorker.ts + - apps/api/src/routes/events.ts + - apps/api/tests/broker/vevent.test.ts + - apps/api/tests/broker/outboxWorker.test.ts +decisions: + - "assembleRruleString: COUNT takes precedence over UNTIL (mutual exclusion, RFC 5545 §3.3.10)" + - "Timed UNTIL serializes as YYYYMMDDTHHMMSSZ (end-of-UTC-day T235959Z) per RESEARCH Pitfall 2" + - "hasExplicitRecurrence check gates assembleRruleString; recurrence:'none' explicitly yields undefined (no RRULE)" + - "Series-edit bound-only change: regex strips existing UNTIL/COUNT from preserved RRULE before re-applying new bound" +metrics: + duration_minutes: 8 + completed_date: "2026-06-10" + tasks_completed: 2 + files_modified: 4 +--- + +# Phase 06 Plan 02: RRULE UNTIL/COUNT Serialization + FREQ Persistence Summary + +**One-liner:** RRULE UNTIL/COUNT serialization with value-type-matching (DATE vs DATETIME UTC) via `assembleRruleString`, wired into both create + update outbox branches, with a FREQ=DAILY regression lock. + +## Tasks Completed + +| # | Name | Commit | Type | +|---|------|--------|------| +| 1 | RED — failing tests for UNTIL/COUNT serialization + FREQ-persistence regression | a59455a | test | +| 2 | GREEN — assembleRruleString + Zod schema acceptance, wired into the write path | d2abb91 | feat | + +## What Was Built + +### Task 1: RED +Added failing tests to two files: + +**`vevent.test.ts`** — three new serialization assertions confirming ical.js 2.2.1 handles UNTIL/COUNT correctly via the existing `ICAL.Recur.fromString` path: +- `FREQ=WEEKLY;COUNT=5` → `RRULE:FREQ=WEEKLY;COUNT=5` +- `FREQ=DAILY;UNTIL=20260630` (all-day) → contains `RRULE:FREQ=DAILY;UNTIL=20260630`, does NOT contain `T235959Z` +- `FREQ=WEEKLY;UNTIL=20260630T235959Z` (timed) → `RRULE:FREQ=WEEKLY;UNTIL=20260630T235959Z` + +**`outboxWorker.test.ts`** — two new describe blocks: +- `assembleRruleString (D-06)`: 6 cases covering COUNT wins, UNTIL DATE/DATETIME, COUNT-wins-over-UNTIL mutual exclusion, base preset unchanged +- `FREQ persistence (D-07 regression)`: 1 case asserting daily-recurrence payload emits `RRULE:FREQ=DAILY` + +RED confirmed: `assembleRruleString is not a function` (6 failing tests). + +### Task 2: GREEN + +**`apps/api/src/broker/outboxWorker.ts`:** +- Added `recurrenceUntil: z.string().max(10).optional()` and `recurrenceCount: z.number().int().min(1).optional()` to `outboxPayloadSchema` (T-06-02 mitigations) +- Implemented and exported `assembleRruleString(basePreset, until?, count?, allDay?)` with JSDoc (D-06) +- Wired `assembleRruleString` into both create and update dispatch branches +- Fixed precedence: `hasExplicitRecurrence` checked first (covers `recurrence:'none'` → explicitly yields `undefined`); `preservedRrule` only used when no explicit recurrence +- Series-edit Pitfall 3: when a bound-only change applies to a preserved RRULE, strips `UNTIL/COUNT` via `/;(UNTIL|COUNT)=[^;]*/g` before re-applying + +**`apps/api/src/routes/events.ts`:** +- Added `recurrenceUntil: z.string().max(10).optional()` and `recurrenceCount: z.number().int().min(1).optional()` to `eventFieldsSchema` + +All 39 tests pass. The previously passing CR-01 (`recurrence:'none' wins over _preservedRrule`) was initially broken by the change and auto-fixed (Rule 1 bug: logic precedence error). + +## TDD Gate Compliance + +| Gate | Status | +|------|--------| +| RED commit (`test(06-02):`) | a59455a — exists, confirmed failing | +| GREEN commit (`feat(06-02):`) | d2abb91 — follows RED commit | +| Commit order | test(06-02) precedes feat(06-02) — verified via `git log` | + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Fixed hasExplicitRecurrence precedence for recurrence:'none'** +- **Found during:** Task 2 (GREEN) +- **Issue:** Initial implementation used `if (hasExplicitRecurrence && rruleFromPayload)` — when `recurrence:'none'`, `rruleFromPayload` is `undefined`, so the condition was `false`, incorrectly falling through to `else if (preservedRrule)` and emitting an RRULE even though the user explicitly selected 'none'. Broke existing `CR-01: explicit recurrence:'none' wins` test. +- **Fix:** Changed to `if (hasExplicitRecurrence)` with an inner ternary: if `rruleFromPayload` is truthy, assemble with bound; otherwise `undefined`. Applied identically to both create and update branches. +- **Files modified:** `apps/api/src/broker/outboxWorker.ts` +- **Commit:** d2abb91 (folded into GREEN commit) + +## Verification Evidence + +``` +cd apps/api && pnpm vitest run tests/broker/vevent.test.ts tests/broker/outboxWorker.test.ts +Test Files 2 passed (2) +Tests 39 passed (39) +``` + +``` +grep -n "recurrenceUntil" apps/api/src/routes/events.ts apps/api/src/broker/outboxWorker.ts +events.ts:111: recurrenceUntil: z.string().max(10).optional() +outboxWorker.ts:84: recurrenceUntil: z.string().max(10).optional() +``` + +Git log confirms `test(06-02)` precedes `feat(06-02)`. + +## Known Stubs + +None. All test assertions target exact ICS/RRULE strings verified against ical.js 2.2.1 in RESEARCH. No placeholder data. + +## Threat Flags + +No new threat surface beyond what was planned in T-06-02 / T-06-02b. Both mitigations implemented: +- `z.string().max(10)` on `recurrenceUntil` + `z.number().int().min(1)` on `recurrenceCount` at both route and outbox schema boundaries. +- `assembleRruleString` uses `.replace(/-/g,'')` (digits only) + fixed templates — no raw passthrough to ICS. +- Assembled string passes through `ICAL.Recur.fromString` (parse-rejects malformed RRULE). + +## Self-Check: PASSED + +| Item | Status | +|------|--------| +| SUMMARY.md created | FOUND | +| RED commit a59455a | FOUND | +| GREEN commit d2abb91 | FOUND | +| 39 tests passing | CONFIRMED | +| recurrenceUntil in both schemas | CONFIRMED |