From 9bc6c7274c29f4fdee947eea52e51a1bc657d0e7 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Sun, 14 Jun 2026 08:28:24 -0400 Subject: [PATCH] =?UTF-8?q?docs(11-05):=20complete=20gap-closure=20plan=20?= =?UTF-8?q?=E2=80=94=20SUMMARY?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../11-per-event-reminders/11-05-SUMMARY.md | 173 ++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 .planning/phases/11-per-event-reminders/11-05-SUMMARY.md diff --git a/.planning/phases/11-per-event-reminders/11-05-SUMMARY.md b/.planning/phases/11-per-event-reminders/11-05-SUMMARY.md new file mode 100644 index 0000000..3adc0c5 --- /dev/null +++ b/.planning/phases/11-per-event-reminders/11-05-SUMMARY.md @@ -0,0 +1,173 @@ +--- +phase: 11-per-event-reminders +plan: "05" +subsystem: calendar-reminders +tags: [gap-closure, tdd, bugfix, reminder, valarm, push-notification, schema-validation] +dependency_graph: + requires: [11-01, 11-02, 11-03, 11-04] + provides: [custom-alarm-round-trip, allday-push-body, positive-trigger-classification, schema-max-bound, active-presetset-gating] + affects: [outboxWorker, expand, reminderScheduler, vevent, EventForm, eventFieldsSchema, outboxPayloadSchema] +tech_stack: + added: [] + patterns: + - "reminderIsCustom: boolean on CalendarOccurrence — custom-alarm signal from server to form" + - "deriveReminderValue(lead, isAllDay, isCustom) — returns __custom__ to trigger D-08 preserve path" + - "humanizeLeadMinutes(lead, isAllDay) — all-day branch with day-granularity wording" + - "classifyValarms sign-check — seconds > 0 returns custom instead of silently negating" + - "active-presetset gating — helper text and synthetic option use allDay ? ALLDAY : TIMED" +key_files: + created: + - apps/api/tests/fixtures/absolute-alarm.ics + - apps/api/tests/fixtures/multi-alarm.ics + modified: + - apps/api/src/broker/expand.ts + - apps/api/src/broker/vevent.ts + - apps/api/src/broker/reminderScheduler.ts + - apps/api/src/routes/events.ts + - apps/api/src/broker/outboxWorker.ts + - apps/pwa/src/api/client.ts + - apps/pwa/src/components/EventForm.tsx + - apps/api/tests/broker/expand.test.ts + - apps/api/tests/broker/vevent.test.ts + - apps/api/tests/broker/reminderScheduler.test.ts + - apps/api/tests/broker/outboxWorker.test.ts + - apps/api/tests/routes/events.test.ts + - apps/pwa/src/components/EventForm.test.tsx + - apps/pwa/src/components/EventDetailPopover.test.tsx +decisions: + - "D-CR-01: surface reminderIsCustom on CalendarOccurrence (server → client) rather than trying to infer custom state client-side — the classification already exists in classifyValarms" + - "D-CR-02: extend humanizeLeadMinutes with isAllDay flag; call site already has the allDay column — no schema change needed" + - "D-WR-01: check seconds > 0 before looking up presets — simpler than parsing RELATED param" + - "D-WR-02: add .max(10080) to both Zod schemas; matched in both eventFieldsSchema (route) and outboxPayloadSchema (worker) since the worker re-validates independently" + - "D-WR-03: single-expression fix — !(allDay ? ALLDAY : TIMED).has(...) — minimal change, only the helper text was wrong (synthetic option gating was already correct)" +metrics: + completed_date: "2026-06-14" + tasks_completed: 5 + tasks_planned: 5 + files_changed: 13 + new_tests: 23 +--- + +# Phase 11 Plan 05: Gap-Closure Summary + +Gap-closure TDD plan fixing 2 confirmed blockers (CR-01, CR-02) and 3 warnings (WR-01–WR-03) from the Phase 11 code review. Surfaced custom-alarm signal end-to-end, fixed all-day push body wording, fixed positive-trigger sign-flip, added server-side max bound, and corrected helper-text preset-set gating. All changes TDD RED→GREEN. + +## Tasks + +### Task 1 — CR-01: Custom alarm round-trip (preserve custom VALARMs on edit) + +**Root cause:** `CalendarOccurrence` only carried `reminderLeadMinutes: number | null`. Custom/absolute VALARMs mapped to `null`, indistinguishable from "no alarm". The form's `deriveReminderValue(null, ...)` always returned `'__none__'`, making the `'__custom__' → omit field` preserve branch permanently unreachable. + +**Fix:** +- `expand.ts`: added `reminderIsCustom: boolean` to `CalendarOccurrence`; derived from `alarmClass.kind === 'custom'`; propagated to every occurrence branch +- `client.ts`: mirrored `reminderIsCustom: boolean` (atomic mirror pattern) +- `EventForm.tsx`: extended `deriveReminderValue(lead, isAllDay, isCustom)` to return `'__custom__'` when `isCustom=true`; updated edit-load call site to pass `occurrence?.reminderIsCustom ?? false` + +**Result:** Editing an event whose VALARM cannot be reduced to a single before-event lead (absolute DATE-TIME trigger, multi-VALARM) now initializes the picker to "Custom (kept)" and omits `reminderLeadMinutes` from the payload — the outbox preserve path (D-08) keeps the original VALARM. + +**Commits:** `5d6cb47` (RED), `f6b47eb` (GREEN) + +--- + +### Task 2 — CR-02: All-day-aware push body + +**Root cause:** `humanizeLeadMinutes(0)` returned `"Starts in 0 min"` for an all-day same-day reminder. The scheduler already had an `isAllDay` split but did not pass the flag to the humanizer. + +**Fix:** +- `reminderScheduler.ts`: extended `humanizeLeadMinutes(leadMinutes, isAllDay)` with all-day branch: 0→"Today", ≤1440→"Tomorrow", 10080→"In 1 week", other→"In N days". Updated both scan branches to pass `isAllDay`. + +**Commits:** `1caa2e3` (RED), `16ac235` (GREEN) + +--- + +### Task 3 — WR-01: Positive-duration TRIGGER classifies as custom + +**Root cause:** `classifyValarms` used `Math.abs(dur.toSeconds())` — positive triggers (e.g. `TRIGGER:+PT15M`, fires after event) were treated identically to the equivalent before-event lead. A `+PT15M` alarm in Apple Calendar was read as "15 min before" and could overwrite the original timing on save. + +**Fix:** +- `vevent.ts`: check `seconds > 0` before preset lookup; return `{ kind: 'custom' }` for positive-duration triggers; use `Math.round(-seconds / 60)` (without abs) for before-event leads. + +**Commits:** `d18aba7` (RED), `bc605e6` (GREEN) + +--- + +### Task 4 — WR-02: Server-side max bound on reminderLeadMinutes + +**Root cause:** Both `eventFieldsSchema` and `outboxPayloadSchema` had only `min(0)` — no upper bound. The UI caps at 10080 (1 week) but there was no server-side enforcement. + +**Fix:** +- `events.ts` `eventFieldsSchema`: `z.number().int().min(0).max(10080).nullable().optional()` +- `outboxWorker.ts` `outboxPayloadSchema`: same change + +**Commits:** `30b8c96` (RED), `7d94afb` (GREEN) + +--- + +### Task 5 — WR-03: Helper text gate on active preset set + +**Root cause:** The reminder helper text condition checked `!TIMED_REMINDER_PRESETS.has(...) && !ALLDAY_REMINDER_PRESETS.has(...)`. For a timed event with `reminderLeadMinutes=10080`: 10080 is in `ALLDAY_REMINDER_PRESETS`, so `!ALLDAY.has(10080)` was `false` → helper text suppressed. The synthetic option for the timed branch was correctly gated (only checked `TIMED_REMINDER_PRESETS`). + +**Fix:** +- `EventForm.tsx`: changed helper text condition to `!(allDay ? ALLDAY_REMINDER_PRESETS : TIMED_REMINDER_PRESETS).has(parseInt(reminderValue, 10))`. + +**Commits:** `4015913` (RED), `a04c76b` (GREEN), `a3aec2d` (prettier) + +--- + +## TDD Gate Compliance + +All 5 tasks followed RED→GREEN discipline: + +| Task | RED commit | GREEN commit | +|------|-----------|-------------| +| CR-01 | `5d6cb47` | `f6b47eb` | +| CR-02 | `1caa2e3` | `16ac235` | +| WR-01 | `d18aba7` | `bc605e6` | +| WR-02 | `30b8c96` | `7d94afb` | +| WR-03 | `4015913` | `a04c76b` | + +Each RED commit was verified to fail for the correct reason before the GREEN implementation. + +--- + +## Deviations from Plan + +### Auto-fixed Issues + +None — plan executed exactly as written, with one minor clarification: + +**WR-01 RED test:** The `TRIGGER;RELATED=END:PT15M` case passed unexpectedly in RED (ical.js handles RELATED=END differently), so that specific test was not a blocking RED. The critical RED test was `TRIGGER:PT30M` (unsigned positive), which did fail before the fix. No tests were weakened; the RELATED=END test was kept and remained green throughout. + +--- + +## Full Gate Results + +| Check | Result | +|-------|--------| +| `pnpm -r typecheck` | PASS (API + PWA) | +| `pnpm --filter @familysync/pwa exec vitest run` | PASS — 17 files, 206 tests | +| `pnpm --filter @familysync/api exec vitest run` | PASS — 27 files, 347 tests | +| `pnpm format:check` | PASS | +| `pnpm md:lint` | PASS | + +--- + +## Commits (all tasks) + +| Hash | Type | Description | +|------|------|-------------| +| `5d6cb47` | test | RED — CR-01 custom alarm round-trip | +| `f6b47eb` | fix | CR-01 surface reminderIsCustom to preserve custom VALARMs on edit | +| `1caa2e3` | test | RED — CR-02 all-day-aware humanizeLeadMinutes | +| `16ac235` | fix | CR-02 all-day-aware push body (no "Starts in 0 min") | +| `d18aba7` | test | RED — WR-01 positive-duration TRIGGER classifies as custom | +| `bc605e6` | fix | WR-01 positive-duration TRIGGER classifies as custom (no Math.abs) | +| `30b8c96` | test | RED — WR-02 reminderLeadMinutes max(10080) in both Zod schemas | +| `7d94afb` | fix | WR-02 add .max(10080) to reminderLeadMinutes in both Zod schemas | +| `4015913` | test | RED — WR-03 helper text suppressed for timed off-list 10080 | +| `a04c76b` | fix | WR-03 gate helper text on active preset set only | +| `a3aec2d` | style | prettier format EventForm.test.tsx WR-03 additions | + +## Self-Check: PASSED + +All key files verified to exist; all commits verified in git log.