diff --git a/.planning/phases/11-per-event-reminders/11-01-SUMMARY.md b/.planning/phases/11-per-event-reminders/11-01-SUMMARY.md new file mode 100644 index 0000000..3d90edb --- /dev/null +++ b/.planning/phases/11-per-event-reminders/11-01-SUMMARY.md @@ -0,0 +1,147 @@ +--- +phase: 11-per-event-reminders +plan: "01" +subsystem: api/broker +tags: [valarm, ical.js, tdd, reminders, dst, calendar] +dependency_graph: + requires: [] + provides: + - buildTimedValarm + - buildAllDayValarm + - classifyValarms + - extractValarms + - computeAlertInstantUtc + - AlarmClassification + - PRESET_MINUTES + - NewEventParams.reminderLeadMinutes + - NewEventParams.valarms + - NewEventParams.allDayAlertInstantUtc + affects: + - apps/api/src/broker/outboxWorker.ts (Plan 03 consumer) + - apps/api/src/broker/reminderScheduler.ts (Plan 02 consumer) +tech_stack: + added: [] + patterns: + - resetType('duration') + setValue(ICAL.Duration) to prevent VALUE=TEXT on TRIGGER + - resetType('date-time') + setValue(ICAL.Time) for absolute DATE-TIME VALARM trigger + - Intl.DateTimeFormat-based UTC offset probe at 9 AM (not midnight) for DST-correct computation + - ICAL.parse try/catch safe-default pattern for T-11-01 tamper mitigation +key_files: + created: [] + modified: + - apps/api/src/broker/vevent.ts + - apps/api/tests/broker/vevent.test.ts +decisions: + - "D-VALARM-PROBE: computeAlertInstantUtc probes UTC offset at naive-9AM-UTC (not midnight) so DST transitions before 9 AM (spring-forward at 2 AM) use the post-transition offset — single-pass Intl computation, no iteration needed" + - "D-ICAL-INSTANCEOF: classifyValarms uses instanceof ICAL.Time (not getParameter('value')) to distinguish absolute vs relative TRIGGER — more robust per A3/A4 assumption log since getParameter returns undefined for default-type DURATION triggers" +metrics: + duration_minutes: 7 + completed_date: "2026-06-14" + tasks_completed: 3 + files_modified: 2 +--- + +# Phase 11 Plan 01: VALARM Serialization + Classification Layer Summary + +VALARM pure-function layer: five exported units covering timed DURATION trigger, all-day absolute DATE-TIME trigger, none/preset/offlist/custom classification, component extraction for preserve-on-edit, and DST-correct 9 AM-local→UTC computation — all TDD RED-first, 37/37 tests green. + +## Tasks Completed + +| Task | Description | Commit | +|------|-------------|--------| +| RED | Failing tests for all 5 units + buildVeventString VALARM emission | 860c741 | +| GREEN | Implementation: buildTimedValarm, buildAllDayValarm, classifyValarms, extractValarms, computeAlertInstantUtc, VALARM emission in buildVeventString, extended NewEventParams | d9eb5c1 | + +## New Exported Symbols + +| Symbol | File | Description | +|--------|------|-------------| +| `buildTimedValarm(leadMinutes)` | vevent.ts | VALARM with DURATION trigger `-PTNmM`; never emits VALUE=TEXT (resetType('duration')) | +| `buildAllDayValarm(alertInstantUtc)` | vevent.ts | VALARM with absolute DATE-TIME trigger `VALUE=DATE-TIME:YYYYMMDDTHHMMSSz` | +| `classifyValarms(rawVevent)` | vevent.ts | Returns `AlarmClassification`: none/preset/offlist/custom; safe on parse failure | +| `extractValarms(rawVevent)` | vevent.ts | Returns live `ICAL.Component[]` for preserve-on-edit re-attachment; safe on parse failure | +| `computeAlertInstantUtc(dateStr, leadDays, tz)` | vevent.ts | DST-correct 9 AM-local→UTC; probes offset at 9 AM to handle transitions before 9 AM | +| `AlarmClassification` | vevent.ts | Union type: `{ kind:'none' } \| { kind:'preset'; leadMinutes } \| { kind:'offlist'; leadMinutes } \| { kind:'custom' }` | +| `PRESET_MINUTES` | vevent.ts | `Set([0,5,10,15,30,60,120,1440,2880,10080])` — D-01/D-02 preset list | + +## Interface Extensions + +`NewEventParams` in `vevent.ts` (after existing `dtstamp` field): + +```typescript +reminderLeadMinutes?: number | null; // null = no VALARM; 0 = same-day all-day; positive = timed lead +valarms?: ICAL.Component[]; // pre-parsed preserve-on-edit components (D-08/CAL-14) +allDayAlertInstantUtc?: Date; // 9 AM local on alert day in UTC (for buildAllDayValarm) +``` + +## Key TRIGGER Assertions in Tests + +- `buildTimedValarm(30)` → ICS contains `TRIGGER:-PT30M`, does NOT contain `VALUE=TEXT` +- `buildTimedValarm(120)` → ICS matches `/TRIGGER:-P(?:T2H|T120M)/`, no VALUE=TEXT +- `buildAllDayValarm(new Date('2026-06-14T13:00:00Z'))` → ICS contains `20260614T130000Z` + `VALUE=DATE-TIME` +- `buildVeventString({allDay:false, reminderLeadMinutes:15})` → ICS contains `TRIGGER:-PT15M`, no VALUE=TEXT +- `buildVeventString({allDay:false, reminderLeadMinutes:0})` → NO `BEGIN:VALARM` (timed 0 = None, D-06) +- `buildVeventString({allDay:true, reminderLeadMinutes:1440, allDayAlertInstantUtc:...})` → `VALUE=DATE-TIME` trigger +- `buildVeventString({reminderLeadMinutes:null})` → NO `BEGIN:VALARM` +- `buildVeventString({valarms:[60minAlarm], reminderLeadMinutes:15})` → only 60-min VALARM present (preserve wins) + +## buildVeventString VALARM Branch Order + +1. `params.valarms?.length > 0` → re-attach each via `addSubcomponent`; ignore `reminderLeadMinutes` +2. `params.reminderLeadMinutes != null && allDay && allDayAlertInstantUtc` → `buildAllDayValarm` +3. `params.reminderLeadMinutes != null && !allDay && reminderLeadMinutes > 0` → `buildTimedValarm` +4. Everything else → no VALARM + +## computeAlertInstantUtc DST Tests + +| Input | Expected | Notes | +|-------|----------|-------| +| `('2026-06-15', 0, 'America/New_York')` | `2026-06-15T13:00:00.000Z` | 9 AM EDT (UTC-4) | +| `('2026-06-15', 1, 'America/New_York')` | `2026-06-14T13:00:00.000Z` | 1 day before, EDT | +| `('2026-01-15', 0, 'America/New_York')` | `2026-01-15T14:00:00.000Z` | 9 AM EST (UTC-5) | +| `('2026-03-08', 0, 'America/New_York')` | `2026-03-08T13:00:00.000Z` | Spring-forward day, post-transition EDT | +| `('2026-11-01', 0, 'America/New_York')` | `2026-11-01T14:00:00.000Z` | Fall-back day, post-transition EST | + +## Verification Results + +- `pnpm --filter @familysync/api exec vitest run tests/broker/vevent.test.ts`: 37/37 PASS +- `pnpm --filter @familysync/api exec vitest run` (full suite): 296/296 PASS +- `pnpm --filter @familysync/api exec tsc --noEmit`: CLEAN (0 errors) + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] ical.js TRIGGER duration normalization in round-trip test** +- **Found during:** Task 2 GREEN +- **Issue:** Test expected `TRIGGER:-PT1H` but ical.js re-serializes a parsed `-PT60M` duration as `-PT60M` (it does not normalize to hours). Both are RFC-valid. +- **Fix:** Updated test to match `/TRIGGER:-P(?:T60M|T1H)/` accepting both forms. +- **Files modified:** apps/api/tests/broker/vevent.test.ts +- **Commit:** d9eb5c1 + +**2. [Rule 1 - Bug] DST probe at midnight vs at 9 AM** +- **Found during:** Task 3 GREEN — spring-forward (2026-03-08) and fall-back (2026-11-01) tests failed +- **Issue:** Original `getUtcOffsetMsForDate` computed the UTC offset at alertDate midnight. On spring-forward day (DST change at 2 AM), midnight is still in EST (UTC-5), but 9 AM is in EDT (UTC-4). Using the midnight offset gave the wrong UTC instant. +- **Fix:** Replaced the midnight-based offset computation with a probe at `alertDateUtcMs + 9h` (naive 9 AM UTC), then asked Intl what local date+time that corresponds to and computed the adjustment. The probe is inherently near 9 AM so it captures the post-transition offset on DST days. +- **Files modified:** apps/api/src/broker/vevent.ts +- **Commit:** d9eb5c1 + +## Known Stubs + +None. All five units are fully implemented and tested. No placeholder values or TODO markers in the produced code. + +## Threat Flags + +None. This plan adds no new network endpoints, auth paths, or schema changes. The only external-input path (classifyValarms/extractValarms parsing stored rawVevent) is wrapped in try/catch per T-11-01 mitigation, matching the existing extractRruleString idiom. + +## Self-Check: PASSED + +Files exist: +- FOUND: apps/api/src/broker/vevent.ts +- FOUND: apps/api/tests/broker/vevent.test.ts + +Commits exist: +- 860c741: RED commit (test(11-01)) +- d9eb5c1: GREEN commit (feat(11-01)) + +Exports verified: `grep -n 'export function\|export const\|export type' apps/api/src/broker/vevent.ts` confirms all 7 symbols exported.