2026-06-14 14:06:45 -04:00
1 changed files with 147 additions and 0 deletions
@@ -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.