luckberg/familysync
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(11): create per-event-reminders phase plan (4 plans, 3 waves)

Browse Source
This commit is contained in:
Lucas Berger
2026-06-13 21:20:09 -04:00
parent a3c4aea9b1
commit cfeb8d8660
5 changed files with 806 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.planning/phases/11-per-event-reminders/11-01-PLAN.md
+207
View File
@@ -0,0 +1,207 @@
---
phase: 11-per-event-reminders
plan: 01
type: tdd
wave: 1
depends_on: []
files_modified:
- apps/api/src/broker/vevent.ts
- apps/api/tests/broker/vevent.test.ts
autonomous: true
requirements: [CAL-13, CAL-14]
must_haves:
truths:
- "A timed reminder serializes to a VALARM with a DURATION trigger and never emits VALUE=TEXT"
- "An all-day reminder serializes to a VALARM with an absolute DATE-TIME (UTC) trigger"
- "An existing single relative VALARM not on the preset list is classified as off-list with its real lead minutes"
- "An absolute-time trigger or multiple VALARMs are classified as custom"
- "computeAlertInstantUtc returns the 9 AM-local instant in UTC, correct across DST boundaries"
artifacts:
- path: "apps/api/src/broker/vevent.ts"
provides: "buildTimedValarm, buildAllDayValarm, classifyValarms, extractValarms, computeAlertInstantUtc, VALARM emission in buildVeventString, extended NewEventParams"
contains: "export function classifyValarms"
- path: "apps/api/tests/broker/vevent.test.ts"
provides: "TDD coverage for all five units (RED-first)"
contains: "TRIGGER:-PT30M"
key_links:
- from: "buildVeventString"
to: "buildTimedValarm / buildAllDayValarm / params.valarms"
via: "vevent.addSubcomponent"
pattern: "addSubcomponent"
- from: "classifyValarms"
to: "PRESET_MINUTES set"
via: "leadMinutes membership test"
pattern: "PRESET_MINUTES"
---
<objective>
Build the pure VALARM serialization + classification layer in `apps/api/src/broker/vevent.ts` — the contract every downstream plumbing plan consumes. Five independently testable units: `buildTimedValarm`, `buildAllDayValarm`, `classifyValarms`, `extractValarms`, and `computeAlertInstantUtc`, plus the VALARM emission branch inside `buildVeventString` and the extended `NewEventParams` interface.
Purpose: Isolate every encoding-sensitive piece (TRIGGER value type, absolute DATE-TIME for all-day, off-list vs custom classification, DST-correct 9 AM-local→UTC math — D-04 9 AM-local fire, D-05 0/1440/2880/10080-minute leads) into pure functions with TDD coverage, so Plan 03 (plumbing) and Plan 02 (scheduler) wire against a proven contract rather than discovering it.
Output: New exported functions + extended interface in vevent.ts; extended vevent.test.ts.
</objective>
<execution_context>
@$HOME/.claude/gsd-core/workflows/execute-plan.md
@$HOME/.claude/gsd-core/templates/summary.md
</execution_context>
<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/STATE.md
@.planning/phases/11-per-event-reminders/11-RESEARCH.md
@.planning/phases/11-per-event-reminders/11-PATTERNS.md
@.planning/phases/11-per-event-reminders/11-VALIDATION.md
</context>
<artifacts_this_phase_produces>
This plan creates (exclude these from any source-drift / "symbol not found" check — they are NEW):
- `buildTimedValarm(leadMinutes)` — vevent.ts
- `buildAllDayValarm(alertInstantUtc)` — vevent.ts
- `classifyValarms(rawVevent)` → `{ kind: 'none' | 'preset' | 'offlist' | 'custom'; leadMinutes? }` — vevent.ts
- `extractValarms(rawVevent)` → `ICAL.Component[]` — vevent.ts
- `computeAlertInstantUtc(eventDateStr, leadDays, tz)` → `Date` — vevent.ts
- `AlarmClassification` exported type — vevent.ts
- `NewEventParams.reminderLeadMinutes`, `NewEventParams.valarms`, `NewEventParams.allDayAlertInstantUtc` — new optional fields
</artifacts_this_phase_produces>
<tasks>
<task type="tdd" tdd="true">
<name>Task 1: VALARM builders (buildTimedValarm, buildAllDayValarm) + buildVeventString emission</name>
<files>apps/api/src/broker/vevent.ts, apps/api/tests/broker/vevent.test.ts</files>
<read_first>
- apps/api/src/broker/vevent.ts lines 21-33 (NewEventParams) and 149-153 (RRULE property construction analog — the resetType/setValue technique that prevents VALUE=TEXT, Pitfall 2)
- apps/api/tests/broker/vevent.test.ts lines 22-156 (existing buildVeventString describe blocks to extend)
- 11-PATTERNS.md § vevent.ts (VALARM emission block to insert after line 153; NewEventParams extension)
- 11-RESEARCH.md Q1 (TRIGGER encoding: timed relative DURATION vs all-day absolute DATE-TIME) and Code Examples (buildTimedValarm / buildAllDayValarm)
</read_first>
<behavior>
- RED: buildTimedValarm(30) → ICAL.Component whose serialized form contains `TRIGGER:-PT30M`, `ACTION:DISPLAY`, `DESCRIPTION:Reminder`, and does NOT contain `VALUE=TEXT`.
- RED: buildTimedValarm(120) → contains `TRIGGER:-PT2H` (or RFC-equivalent `-PT120M`), no `VALUE=TEXT`.
- RED: buildAllDayValarm(new Date('2026-06-14T13:00:00Z')) → serialized TRIGGER is an absolute DATE-TIME `20260614T130000Z` with `VALUE=DATE-TIME`, no DURATION.
- RED: buildVeventString({allDay:false, reminderLeadMinutes:15, ...}) → emitted ICS contains exactly one BEGIN:VALARM block with `TRIGGER:-PT15M`.
- RED: buildVeventString({allDay:false, reminderLeadMinutes:0, ...}) → emitted ICS contains NO VALARM (timed 0 = None per D-06).
- RED: buildVeventString({allDay:true, reminderLeadMinutes:1440, allDayAlertInstantUtc:<Date>, ...}) → emitted ICS contains one VALARM with absolute DATE-TIME trigger.
- RED: buildVeventString({reminderLeadMinutes:null, ...}) → no VALARM.
- RED: buildVeventString({valarms:[<pre-parsed ICAL.Component>], ...}) → that VALARM appears verbatim in the ICS AND no new VALARM is synthesized even if reminderLeadMinutes is also passed (preserve path wins).
- GREEN: implement; REFACTOR: factor shared ACTION/DESCRIPTION setup if duplicated.
</behavior>
<action>
Add `buildTimedValarm(leadMinutes: number): ICAL.Component` and `buildAllDayValarm(alertInstantUtc: Date): ICAL.Component` (both exported). Use `new ICAL.Property('trigger')` + `resetType('duration')` + `setValue(ICAL.Duration.fromSeconds(-leadMinutes * 60))` for timed; `resetType('date-time')` + `setValue(ICAL.Time.fromJSDate(alertInstantUtc, true))` for all-day. NEVER use `addPropertyWithValue('trigger', '-PT15M')` (Pitfall 2 — emits VALUE=TEXT). Each VALARM also gets `addPropertyWithValue('action','DISPLAY')` and `addPropertyWithValue('description','Reminder')`.
Extend `NewEventParams` (after the existing `dtstamp` field) with three optional fields: `reminderLeadMinutes?: number | null` (null = no VALARM; 0 = same-day all-day; positive = timed lead), `valarms?: ICAL.Component[]` (pre-parsed preserve-on-edit components), `allDayAlertInstantUtc?: Date` (9 AM-local-in-UTC for all-day absolute trigger).
Insert the VALARM emission block in `buildVeventString` after the RRULE block (after line 153), before the optional location/description fields. Branch order: (1) if `params.valarms?.length` → addSubcomponent each, return without synthesizing (preserve wins); (2) else if `reminderLeadMinutes != null` and `allDay && allDayAlertInstantUtc` → addSubcomponent(buildAllDayValarm(allDayAlertInstantUtc)); (3) else if `!allDay && reminderLeadMinutes > 0` → addSubcomponent(buildTimedValarm(reminderLeadMinutes)). Note `reminderLeadMinutes === 0` on a timed event emits NO VALARM (D-06).
</action>
<verify>
<automated>pnpm --filter @familysync/api exec vitest run tests/broker/vevent.test.ts</automated>
</verify>
<acceptance_criteria>
- tests/broker/vevent.test.ts asserts the emitted ICS for a 30-min timed lead contains `TRIGGER:-PT30M` and does NOT contain `VALUE=TEXT`.
- tests assert buildAllDayValarm output contains an absolute `VALUE=DATE-TIME` trigger ending in `Z`.
- test asserts a timed event with reminderLeadMinutes=0 emits no `BEGIN:VALARM`.
- test asserts params.valarms preserve path emits the supplied VALARM verbatim and does not double-emit when reminderLeadMinutes is also set.
- `pnpm --filter @familysync/api exec vitest run tests/broker/vevent.test.ts` exits 0.
</acceptance_criteria>
<done>buildTimedValarm, buildAllDayValarm exported and tested; buildVeventString emits the correct VALARM per allDay + NULL-vs-0 rules; preserve path takes precedence.</done>
</task>
<task type="tdd" tdd="true">
<name>Task 2: VALARM classifier + extractor (classifyValarms, extractValarms)</name>
<files>apps/api/src/broker/vevent.ts, apps/api/tests/broker/vevent.test.ts</files>
<read_first>
- apps/api/src/broker/vevent.ts lines 63-79 (extractRruleString — the ICAL.parse try/catch + getFirstSubcomponent structure to mirror for extractValarms)
- 11-RESEARCH.md Q3 (classifyValarms full implementation incl. the `firstValue instanceof ICAL.Time` check for absolute triggers, PRESET_MINUTES set, getAllSubcomponents('valarm'))
- 11-PATTERNS.md § Shared Patterns (ICAL.parse try/catch + getFirstSubcomponent)
</read_first>
<behavior>
- RED: classifyValarms(ICS with no VALARM) → { kind: 'none' }.
- RED: classifyValarms(ICS with single `TRIGGER:-PT15M`) → { kind: 'preset', leadMinutes: 15 }.
- RED: classifyValarms(ICS with single `TRIGGER:-PT45M`) → { kind: 'offlist', leadMinutes: 45 }.
- RED: classifyValarms(ICS with `TRIGGER;VALUE=DATE-TIME:20260615T130000Z`) → { kind: 'custom' }.
- RED: classifyValarms(ICS with two VALARM blocks) → { kind: 'custom' }.
- RED: classifyValarms('not valid ics') → { kind: 'none' } (parse failure safe default).
- RED: extractValarms(ICS with one VALARM) → array length 1 of ICAL.Component; extractValarms(ICS with none) → []; extractValarms('garbage') → [].
- GREEN: implement; REFACTOR: share the parse-to-vevent helper between classifyValarms and extractValarms if it reduces duplication.
</behavior>
<action>
Add exported `AlarmClassification` union type (`{ kind:'none' } | { kind:'preset'; leadMinutes:number } | { kind:'offlist'; leadMinutes:number } | { kind:'custom' }`). Add `classifyValarms(rawVevent: string): AlarmClassification` and `extractValarms(rawVevent: string): ICAL.Component[]`. Mirror extractRruleString's try/catch + `getFirstSubcomponent('vevent')` guard. In classifyValarms: `getAllSubcomponents('valarm')` — length 0 → none; length > 1 → custom; single → read `getFirstProperty('trigger')`, take `getFirstValue()`; if `instanceof ICAL.Time` → custom (absolute); otherwise treat as ICAL.Duration, `Math.round(Math.abs(dur.toSeconds())/60)` → leadMinutes; membership-test against `PRESET_MINUTES = new Set([0,5,10,15,30,60,120,1440,2880,10080])` → preset else offlist. Guard the duration value: if it lacks `toSeconds`, return custom.
</action>
<verify>
<automated>pnpm --filter @familysync/api exec vitest run tests/broker/vevent.test.ts</automated>
</verify>
<acceptance_criteria>
- tests/broker/vevent.test.ts asserts classifyValarms returns offlist with leadMinutes 45 for a single `TRIGGER:-PT45M`.
- test asserts an absolute `VALUE=DATE-TIME` trigger → { kind:'custom' } and two VALARMs → { kind:'custom' }.
- test asserts preset minute 1440 (1 day) and 10080 (1 week) classify as preset.
- test asserts extractValarms round-trips one VALARM as a live ICAL.Component (re-attachable via addSubcomponent).
- vitest run exits 0.
</acceptance_criteria>
<done>classifyValarms distinguishes none/preset/offlist/custom per D-07; extractValarms returns live components for preserve-on-edit; both safe on parse failure.</done>
</task>
<task type="tdd" tdd="true">
<name>Task 3: computeAlertInstantUtc (9 AM-local→UTC, DST-correct)</name>
<files>apps/api/src/broker/vevent.ts, apps/api/tests/broker/vevent.test.ts</files>
<read_first>
- 11-RESEARCH.md Code Examples (computeAllDayAlertUtc / getUtcOffsetMs sketch — note the sketch has a placeholder; implement the offset correctly) and the TDD note requiring DST-boundary test cases
- 11-RESEARCH.md Assumptions Log A2/A5 (server TZ = household TZ; no per-user TZ column)
</read_first>
<behavior>
- RED: computeAlertInstantUtc('2026-06-15', 0, 'America/New_York') → Date equal to `2026-06-15T13:00:00Z` (9 AM EDT, summer offset -4).
- RED: computeAlertInstantUtc('2026-06-15', 1, 'America/New_York') → `2026-06-14T13:00:00Z` (1 day before, still EDT).
- RED: computeAlertInstantUtc('2026-01-15', 0, 'America/New_York') → `2026-01-15T14:00:00Z` (9 AM EST, winter offset -5).
- RED (spring-forward): alert day = 2026-03-08 (US DST starts) → 9 AM local resolves with the correct post-transition offset (-4 → `2026-03-08T13:00:00Z`).
- RED (fall-back): alert day = 2026-11-01 (US DST ends) → 9 AM local resolves with -5 → `2026-11-01T14:00:00Z`.
- GREEN: implement using Intl-based offset for the SPECIFIC alert date (not today's offset). REFACTOR: extract the offset helper if it clarifies.
</behavior>
<action>
Add exported `computeAlertInstantUtc(eventDateStr: string, leadDays: number, tz: string): Date`. Parse `eventDateStr` ('YYYY-MM-DD'), subtract `leadDays` to get the alert date, then compute the UTC instant for 09:00 local in `tz` ON THAT alert date. Derive the offset for that specific date via `Intl.DateTimeFormat` (e.g. format a probe UTC instant in `tz` and measure the wall-clock delta, or use `timeZoneName:'shortOffset'` parsing) — must use the alert date's own offset so DST transitions resolve correctly. Do NOT hardcode a fixed offset and do NOT use `new Date('...T09:00:00')` relying on the process TZ. leadDays is `reminderLeadMinutes / 1440` (0, 1, 2, or 7) — the D-05 minute mapping (0/1440/2880/10080); the 09:00 fire time is D-04. Caller passes `process.env.TZ ?? Intl.DateTimeFormat().resolvedOptions().timeZone` as tz.
</action>
<verify>
<automated>pnpm --filter @familysync/api exec vitest run tests/broker/vevent.test.ts</automated>
</verify>
<acceptance_criteria>
- tests/broker/vevent.test.ts asserts computeAlertInstantUtc('2026-06-15', 0, 'America/New_York').toISOString() === '2026-06-15T13:00:00.000Z'.
- test asserts the winter case ('2026-01-15', 0) → '2026-01-15T14:00:00.000Z'.
- test asserts a spring-forward alert date and a fall-back alert date each resolve to the correct UTC instant using that date's offset (not today's).
- vitest run exits 0.
</acceptance_criteria>
<done>computeAlertInstantUtc returns the correct 9 AM-local UTC instant for same-day and N-day leads, verified at both standard and DST-transition dates.</done>
</task>
</tasks>
<threat_model>
## Trust Boundaries
| Boundary | Description |
|----------|-------------|
| stored rawVevent → classifyValarms/extractValarms | Untrusted iCalendar text (set by external clients: Fastmail/Apple) parsed server-side |
## STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-11-01 | Tampering | classifyValarms/extractValarms parsing rawVevent | mitigate | ICAL.parse wrapped in try/catch returning safe defaults (none / []); no eval, no string splicing — ical.js handles line-folding + escaping |
| T-11-02 | Denial of Service | VALARM serialization | accept | Inputs are bounded integers (lead minutes from a fixed preset set) and a single Date; no unbounded loops; ical.js is the established serializer |
| T-11-SC | Tampering | npm/pip/cargo installs | accept | No new packages this phase (RESEARCH.md § Package Legitimacy Audit: not applicable); nothing to install |
This phase adds NO new security surface: the only new external-input path (parsing stored rawVevent) is already wrapped in try/catch matching the existing extractRruleString idiom.
</threat_model>
<verification>
- `pnpm --filter @familysync/api exec vitest run tests/broker/vevent.test.ts` green.
- `pnpm --filter @familysync/api exec tsc --noEmit` clean (esbuild/vitest can pass while tsc fails — run tsc explicitly).
- No `VALUE=TEXT` substring in any emitted timed-VALARM ICS (asserted in tests).
</verification>
<success_criteria>
- All five units (buildTimedValarm, buildAllDayValarm, classifyValarms, extractValarms, computeAlertInstantUtc) exist, exported, and covered by RED-first tests now GREEN.
- buildVeventString emits the correct VALARM honoring allDay, NULL-vs-0, and preserve precedence.
- tsc --noEmit clean for apps/api.
</success_criteria>
<output>
Create `.planning/phases/11-per-event-reminders/11-01-SUMMARY.md` when done. List every new exported symbol and the exact TRIGGER assertions added.
</output>