Files
familysync/.planning/phases/11-per-event-reminders/11-05-SUMMARY.md
T

8.6 KiB
Raw Blame History

phase, plan, subsystem, tags, dependency_graph, tech_stack, key_files, decisions, metrics
phase plan subsystem tags dependency_graph tech_stack key_files decisions metrics
11-per-event-reminders 05 calendar-reminders
gap-closure
tdd
bugfix
reminder
valarm
push-notification
schema-validation
requires provides affects
11-01
11-02
11-03
11-04
custom-alarm-round-trip
allday-push-body
positive-trigger-classification
schema-max-bound
active-presetset-gating
outboxWorker
expand
reminderScheduler
vevent
EventForm
eventFieldsSchema
outboxPayloadSchema
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
created modified
apps/api/tests/fixtures/absolute-alarm.ics
apps/api/tests/fixtures/multi-alarm.ics
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
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)
completed_date tasks_completed tasks_planned files_changed new_tests
2026-06-14 5 5 13 23

Phase 11 Plan 05: Gap-Closure Summary

Gap-closure TDD plan fixing 2 confirmed blockers (CR-01, CR-02) and 3 warnings (WR-01WR-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.