diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 0ab1431..0a254e3 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -212,7 +212,14 @@ Plans: - **uid:dtstartMs dedup** (Pitfall 4): change the scheduler dedup key from bare `uid` to compound `uid:dtstartMs` and widen the scan to a variable per-event window so long leads fire and rescheduled events re-fire; keep `eventFieldsSchema` and `outboxPayloadSchema` in sync (IN-03). - Hard constraints: `setInterval` only; scheduler reads `reminder_lead_minutes` from the DB (ground truth), not the outbox payload; drop the `isShared`-only reminder restriction (a user who set an alarm wants it regardless of calendar). -**Plans**: TBD +**Plans**: 4 plans (3 waves) + +Plans: +- [ ] 11-01-PLAN.md — VALARM builders + classifier + extractor + computeAlertInstantUtc (vevent.ts, TDD) +- [ ] 11-02-PLAN.md — Variable-lead scheduler: uid:dtstartMs dedup, drop isShared, all-day 9 AM, humanized body (TDD) +- [ ] 11-03-PLAN.md — Backend plumbing: schema field, outbox preserve-on-edit, sync upsert, occurrence surfacing +- [ ] 11-04-PLAN.md — EventForm reminder picker (allDay swap, edit pre-population) + client types + Playwright smoke + **UI hint**: yes ### Phase 12: Initial Setup Wizard diff --git a/.planning/phases/11-per-event-reminders/11-01-PLAN.md b/.planning/phases/11-per-event-reminders/11-01-PLAN.md new file mode 100644 index 0000000..5cacd94 --- /dev/null +++ b/.planning/phases/11-per-event-reminders/11-01-PLAN.md @@ -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" +--- + + +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. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.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 + + + +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 + + + + + + Task 1: VALARM builders (buildTimedValarm, buildAllDayValarm) + buildVeventString emission + apps/api/src/broker/vevent.ts, apps/api/tests/broker/vevent.test.ts + + - 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) + + + - 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:, ...}) → emitted ICS contains one VALARM with absolute DATE-TIME trigger. + - RED: buildVeventString({reminderLeadMinutes:null, ...}) → no VALARM. + - RED: buildVeventString({valarms:[], ...}) → 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. + + + 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). + + + pnpm --filter @familysync/api exec vitest run tests/broker/vevent.test.ts + + + - 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. + + buildTimedValarm, buildAllDayValarm exported and tested; buildVeventString emits the correct VALARM per allDay + NULL-vs-0 rules; preserve path takes precedence. + + + + Task 2: VALARM classifier + extractor (classifyValarms, extractValarms) + apps/api/src/broker/vevent.ts, apps/api/tests/broker/vevent.test.ts + + - 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) + + + - 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. + + + 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. + + + pnpm --filter @familysync/api exec vitest run tests/broker/vevent.test.ts + + + - 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. + + classifyValarms distinguishes none/preset/offlist/custom per D-07; extractValarms returns live components for preserve-on-edit; both safe on parse failure. + + + + Task 3: computeAlertInstantUtc (9 AM-local→UTC, DST-correct) + apps/api/src/broker/vevent.ts, apps/api/tests/broker/vevent.test.ts + + - 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) + + + - 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. + + + 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. + + + pnpm --filter @familysync/api exec vitest run tests/broker/vevent.test.ts + + + - 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. + + computeAlertInstantUtc returns the correct 9 AM-local UTC instant for same-day and N-day leads, verified at both standard and DST-transition dates. + + + + + +## 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. + + + +- `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). + + + +- 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. + + + +Create `.planning/phases/11-per-event-reminders/11-01-SUMMARY.md` when done. List every new exported symbol and the exact TRIGGER assertions added. + diff --git a/.planning/phases/11-per-event-reminders/11-02-PLAN.md b/.planning/phases/11-per-event-reminders/11-02-PLAN.md new file mode 100644 index 0000000..3fabe0d --- /dev/null +++ b/.planning/phases/11-per-event-reminders/11-02-PLAN.md @@ -0,0 +1,204 @@ +--- +phase: 11-per-event-reminders +plan: 02 +type: tdd +wave: 1 +depends_on: [] +files_modified: + - apps/api/src/broker/reminderScheduler.ts + - apps/api/tests/broker/reminderScheduler.test.ts +autonomous: true +requirements: [NOTIF-04, NOTIF-05, NOTIF-06] +must_haves: + truths: + - "A reminder fires at the event's chosen lead time (e.g. T-30 for a 30-min lead), not a hardcoded 15-min lead" + - "An event with NULL reminderLeadMinutes produces no push" + - "A timed event with reminderLeadMinutes=0 produces no push; an all-day event with 0 fires at 9 AM on the event date" + - "Reminders fire on personal calendars too (the isShared-only restriction is dropped)" + - "The same event fires exactly once across consecutive ticks; a rescheduled event (new dtstart) re-fires — dedup key is uid:dtstartMs" + - "Push body is humanized to the largest sensible unit (30 min / 1 hour / 1 day / 7 days)" + artifacts: + - path: "apps/api/src/broker/reminderScheduler.ts" + provides: "humanizeLeadMinutes, variable-lead query, uid:dtstartMs dedup, all-day 9 AM branch, NULL-vs-0 guard, dropped isShared restriction" + contains: "humanizeLeadMinutes" + - path: "apps/api/tests/broker/reminderScheduler.test.ts" + provides: "TDD coverage for variable-lead, all-day 9 AM, dedup, NULL-vs-0, humanized body" + contains: "uid:dtstartMs" + key_links: + - from: "runReminderCheck" + to: "calendarEvents.reminderLeadMinutes" + via: "SQL WHERE reminder_lead_minutes IS NOT NULL" + pattern: "reminderLeadMinutes" + - from: "notification.body" + to: "humanizeLeadMinutes" + via: "function call replacing the hardcoded string" + pattern: "humanizeLeadMinutes\\(" +--- + + +Generalize the reminder scheduler in `apps/api/src/broker/reminderScheduler.ts` from the fixed shared-timed-15-min scan to a per-event variable-lead scheduler that reads `reminder_lead_minutes` from the DB as ground truth: variable per-event window, `uid:dtstartMs` compound dedup, dropped `isShared`-only restriction, an all-day 9 AM-local branch (D-04), NULL-vs-0 semantics, and a humanized push body (`humanizeLeadMinutes`). + +Purpose: Satisfy NOTIF-04 (fire at chosen lead), NOTIF-05 (no fire when no reminder / timed-0), and NOTIF-06 (all-day 9 AM, exactly-once dedup across catch-up + reschedule). The dispatch plumbing (dispatchPush, per-event/per-sub try/catch, deep-link navigate, prune) is preserved — only the query window, dedup key, lead source, fire-time computation, and body text change. +Output: Rewritten runReminderCheck logic + new humanizeLeadMinutes; extended reminderScheduler.test.ts. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.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 +@.planning/phases/11-per-event-reminders/11-UI-SPEC.md + + + +This plan creates (exclude from drift checks — NEW): +- `humanizeLeadMinutes(leadMinutes)` → string — reminderScheduler.ts +- Compound dedup key format `uid:dtstartMs` (replaces bare uid) — reminderScheduler.ts +- All-day 9 AM scheduler branch reading reminderLeadMinutes — reminderScheduler.ts +NOTE: `computeAlertInstantUtc` is created by Plan 11-01 in vevent.ts and imported here — it is NOT new to this plan. + + + +This plan imports `computeAlertInstantUtc` from `../broker/vevent.js` (created in Plan 11-01). Until Plan 11-01 merges, the all-day-9AM test (Wave 0 gap) cannot pass — this plan is wave 1 alongside 11-01 but the all-day task's automated verify depends on 11-01's symbol. Sequence the all-day task LAST within this plan; if 11-01 has not yet landed in the shared tree, the executor imports the symbol signature as specified in 11-01's artifacts list and the test goes RED until both merge. Do not redefine computeAlertInstantUtc here. + + + + + + Task 1: Variable-lead window + uid:dtstartMs dedup + drop isShared/allDay restrictions + NULL-vs-0 (timed) + apps/api/src/broker/reminderScheduler.ts, apps/api/tests/broker/reminderScheduler.test.ts + + - apps/api/src/broker/reminderScheduler.ts (full) — the existing query (lines 84-107), byUid grouping (109-138), dispatch+dedup loop (141-188), prune (193-197) + - apps/api/tests/broker/reminderScheduler.test.ts — existing describe blocks: shared+timed filtering (76), catch-up window single-fire across 3 ticks (167-228), fan-out (265), mark-sent-after-dispatch (306), prune (362) + - 11-RESEARCH.md Pitfall 4 (uid:dtstartMs dedup, variable window Option B) and "Pitfall: 0 Lead Minutes on a Timed Event" + - 11-PATTERNS.md § reminderScheduler.ts (dedup key change, SQL WHERE changes, prune key change) + + + - RED: a timed event with reminderLeadMinutes=30 dispatches exactly when now is in (dtstart - 30min - 60s, dtstart - 30min] window; does NOT dispatch a 30-min-lead event 5 min before start with no other coverage — i.e. fire time is lead-driven, not a fixed 16-min window. Replace the existing single-fire test to use a per-event lead. + - RED: a personal (isShared=false) timed event with a non-null lead DOES dispatch (drop the isShared restriction — NOTIF-05 corollary; the existing "does not dispatch for non-shared" test must be inverted/replaced). + - RED: an event with reminderLeadMinutes=NULL produces zero dispatches. + - RED: a timed event with reminderLeadMinutes=0 produces zero dispatches (0 on timed = None, D-06). + - RED: dedup — the same event (same uid + same dtstartMs) fires exactly once across 3 consecutive ticks inside its fire window; an event whose dtstart is rescheduled to a new instant (new dtstartMs, same uid) fires again (new compound key). The existing "single-fire across three ticks" test is updated to assert the uid:dtstartMs key. + - GREEN: implement; REFACTOR: keep per-event and per-sub try/catch intact. + + + Change `sentReminders` key from bare `uid` to `` `${uid}:${dtstartMs}` `` (template using `event.dtstartUtc.getTime()`); update the dedup check, the `sentReminders.set(...)` call, and the prune loop to use the compound key while still pruning on stored dtstartMs <= now. In the query: REMOVE `eq(calendars.isShared, true)` and `eq(calendarEvents.allDay, false)`; ADD `reminderLeadMinutes` to the `.select()` and a `sql\`${calendarEvents.reminderLeadMinutes} IS NOT NULL\`` predicate. For TIMED events compute fire time per-event as `dtstartUtc - reminderLeadMinutes minutes` and fire when that instant falls in `(now - 60s, now]` (catch-up of one missed tick) while `dtstartUtc > now`; widen the SQL pre-filter window to `dtstartUtc <= now + max-lead` (use the max preset, 2880 min) so long-lead events enter the JS filter, then apply the precise per-event fire-time check in JS. Guard: timed events with `reminderLeadMinutes === 0` are skipped (treated as None per D-06) — only all-day uses 0. Keep dispatchPush fan-out, per-sub try/catch, mark-sent-after-dispatch (WR-01), and the navigate deep-link unchanged. + + + pnpm --filter @familysync/api exec vitest run tests/broker/reminderScheduler.test.ts + + + - reminderScheduler.test.ts asserts a 30-min-lead timed event fires in the lead-driven window (NOTIF-04), and a NULL-lead event and a timed-0-lead event both produce zero dispatches (NOTIF-05). + - test asserts a personal (isShared=false) event with a non-null lead DOES dispatch (restriction dropped). + - test asserts the dedup key is `uid:dtstartMs`: same compound key fires once across 3 ticks; a new dtstartMs re-fires (NOTIF-06 reschedule). + - vitest run exits 0. + + Scheduler fires per-event lead, ignores NULL and timed-0, fires on personal calendars, dedups on uid:dtstartMs; existing isShared/allDay-exclusion tests replaced. + + + + Task 2: humanizeLeadMinutes body formatter + apps/api/src/broker/reminderScheduler.ts, apps/api/tests/broker/reminderScheduler.test.ts + + - apps/api/src/broker/reminderScheduler.ts line 157 (hardcoded `Starts in ${minutes} min` to replace) + - 11-UI-SPEC.md § Notification Push Copy (humanized thresholds table) and 11-RESEARCH.md Code Examples (humanizeLeadMinutes incl. the 90-min/1-hour ordering note) + + + - RED: humanizeLeadMinutes(30) → 'Starts in 30 min' + - RED: humanizeLeadMinutes(59) → 'Starts in 59 min' + - RED: humanizeLeadMinutes(60) → 'Starts in 1 hour' + - RED: humanizeLeadMinutes(90) → 'Starts in 1 hour' (60–119 bucket, per UI-SPEC) + - RED: humanizeLeadMinutes(120) → 'Starts in 2 hours' + - RED: humanizeLeadMinutes(1440) → 'Starts in 1 day' + - RED: humanizeLeadMinutes(2880) → 'Starts in 2 days' + - RED: humanizeLeadMinutes(10080) → 'Starts in 7 days' + - GREEN: implement; the notification body in runReminderCheck calls humanizeLeadMinutes(event.reminderLeadMinutes) instead of the actual minutes-to-start string. + + + Add `humanizeLeadMinutes(leadMinutes: number): string` with branch order: `< 60` → `Starts in ${N} min`; `< 120` → `Starts in 1 hour`; `< 1440` → `Starts in ${Math.round(N/60)} hours`; `< 2880` → `Starts in 1 day`; else `Starts in ${Math.round(N/1440)} days`. Replace the `body: \`Starts in ${minutes} min\`` line with `body: humanizeLeadMinutes(event.reminderLeadMinutes)` — drive off the EVENT's configured lead (the DB ground-truth value carried into the byUid record), not the live minutes-to-start delta. Carry `reminderLeadMinutes` into the byUid grouped record so the body has the configured lead available. Title and navigate unchanged. + + + pnpm --filter @familysync/api exec vitest run tests/broker/reminderScheduler.test.ts + + + - reminderScheduler.test.ts asserts humanizeLeadMinutes maps 30→"30 min", 60→"1 hour", 90→"1 hour", 120→"2 hours", 1440→"1 day", 10080→"7 days". + - a dispatch test asserts the dispatched notification.body equals the humanized string for the event's configured lead (e.g. a 1440 all-day lead → "Starts in 1 day"). + - vitest run exits 0. + + humanizeLeadMinutes covers every preset bucket; scheduler body uses it driven by the configured lead (D-09). + + + + Task 3: All-day 9 AM-local fire branch (NOTIF-06) + apps/api/src/broker/reminderScheduler.ts, apps/api/tests/broker/reminderScheduler.test.ts + + - apps/api/src/broker/reminderScheduler.ts (the query + JS filter from Task 1 — extend with an all-day branch) + - 11-RESEARCH.md "Pitfall: All-Day 9 AM UTC Computation at DST Boundaries" and "Variable-lead scheduler SQL query" (all-day: fetch with non-null leads, compute 9 AM UTC in JS) ; 11-01 artifacts for computeAlertInstantUtc signature + - 11-PATTERNS.md § reminderScheduler.ts (all-day handling) + + + - RED: an all-day event (allDay=true, dtstartDate='2026-06-15') with reminderLeadMinutes=0 dispatches when now is ~9 AM local on 2026-06-15 (computeAlertInstantUtc('2026-06-15',0,tz) in the fire window), and does NOT dispatch at midnight. + - RED: an all-day event with reminderLeadMinutes=1440 dispatches at 9 AM local on the day before (2026-06-14). + - RED: an all-day event with reminderLeadMinutes=10080 dispatches at 9 AM local 7 days before. + - RED: all-day dedup uses the same uid:dtstartMs (use dtstartDate-derived ms or the computed alert ms consistently as the dtstart component) so it fires once across ticks. + - GREEN: implement using the imported computeAlertInstantUtc; REFACTOR. + + + Import `computeAlertInstantUtc` from `./vevent.js` (Plan 11-01). Add an all-day branch to the scan: fetch all-day events (allDay=true) with non-null `reminderLeadMinutes` whose `dtstartDate` is within the next `max-lead-days` (7) of now; for each compute `alertInstant = computeAlertInstantUtc(dtstartDate, reminderLeadMinutes / 1440, tz)` where `tz = process.env.TZ ?? Intl.DateTimeFormat().resolvedOptions().timeZone`; fire when `alertInstant` falls in `(now - 60s, now]`. All-day reminderLeadMinutes=0 IS valid (same-day 9 AM; D-04 fire time, D-05 minute mapping 0/1440/2880/10080, D-06 NULL-vs-0) — do NOT apply the timed-0 skip to all-day rows. Use the all-day event's stable dtstart component for the `uid:dtstartMs` dedup key (e.g. the dtstartDate's UTC-midnight ms, consistent between ticks). Keep dispatch/fan-out/body (humanizeLeadMinutes) identical to the timed path. + + + pnpm --filter @familysync/api exec vitest run tests/broker/reminderScheduler.test.ts + + + - reminderScheduler.test.ts asserts an all-day 0-lead event fires at the computeAlertInstantUtc(date,0,tz) instant (≈9 AM local) and not at midnight (NOTIF-06). + - test asserts a 1440 all-day lead fires 9 AM the prior day and a 10080 lead fires 7 days prior. + - test asserts the all-day event dedups once across consecutive ticks via uid:dtstartMs. + - vitest run exits 0 (requires Plan 11-01's computeAlertInstantUtc merged). + + All-day reminders fire at 9 AM local on the computed alert day for 0/1440/2880/10080 leads, exactly once per uid:dtstartMs. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| DB calendar_events → scheduler | reminder_lead_minutes is a bounded int already validated on write; read path, no external input | +| scheduler → browser push | dispatchPush (existing, unchanged) signs VAPID payloads; body is a hardcoded humanized string | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-11-03 | Information Disclosure | Dropping the isShared restriction | accept | Personal-event reminders fan out to all push subscriptions in a 2-member (expanding) household by design (NOTIF-05 corollary); body carries only event title + relative time, no sensitive field; same exposure model as the existing shared path | +| T-11-04 | Denial of Service | Variable-lead scan window | mitigate | Window capped at max preset lead (2880 min timed / 7 days all-day); JS-side per-event filter bounds work; setInterval-only (node-cron forbidden) | +| T-11-05 | Repudiation | Exactly-once delivery | mitigate | uid:dtstartMs dedup + mark-sent-after-dispatch (WR-01) preserved; prune prevents unbounded Map growth | +| T-11-SC | Tampering | npm installs | accept | No new packages this phase | + +No new security surface: the only behavioral expansion (personal-calendar reminders) is an authorized requirement (NOTIF-05 corollary), not a new untrusted-input path. + + + +- `pnpm --filter @familysync/api exec vitest run tests/broker/reminderScheduler.test.ts` green. +- `pnpm --filter @familysync/api exec tsc --noEmit` clean. +- Grep confirms `setInterval` retained and no `node-cron` import added (hard project rule). + + + +- Scheduler reads reminder_lead_minutes as ground truth; fires per-event lead; ignores NULL and timed-0; fires personal events; dedups on uid:dtstartMs. +- All-day events fire at 9 AM local; humanized body for every preset bucket. +- All NOTIF-04/05/06 rows in 11-VALIDATION.md Per-Task Verification Map are claimed by an automated test here. + + + +Create `.planning/phases/11-per-event-reminders/11-02-SUMMARY.md` when done. Record the new dedup key format, the dropped predicates, and the humanizeLeadMinutes branch thresholds. + diff --git a/.planning/phases/11-per-event-reminders/11-03-PLAN.md b/.planning/phases/11-per-event-reminders/11-03-PLAN.md new file mode 100644 index 0000000..c3d3c82 --- /dev/null +++ b/.planning/phases/11-per-event-reminders/11-03-PLAN.md @@ -0,0 +1,198 @@ +--- +phase: 11-per-event-reminders +plan: 03 +type: execute +wave: 2 +depends_on: [11-01] +files_modified: + - apps/api/src/routes/events.ts + - apps/api/src/broker/outboxWorker.ts + - apps/api/src/broker/sync.ts + - apps/api/src/broker/expand.ts + - apps/api/tests/broker/outboxWorker.test.ts +autonomous: true +requirements: [CAL-13, CAL-14] +must_haves: + truths: + - "reminderLeadMinutes round-trips end to end through eventFieldsSchema → outbox payload → buildVeventString → Fastmail PUT" + - "Editing an event with no picker change preserves the existing VALARM verbatim (never stripped)" + - "Explicit null reminderLeadMinutes clears the VALARM; explicit value replaces it" + - "An all-day event written with a day-lead gets an absolute DATE-TIME VALARM computed at 9 AM local (D-04) from the D-05 minute mapping" + - "Events synced from Fastmail with a native VALARM have reminderLeadMinutes populated in the DB (scheduler ground truth)" + - "GET /api/events occurrences carry reminderLeadMinutes for edit-mode pre-population (NULL-vs-0 distinct)" + artifacts: + - path: "apps/api/src/routes/events.ts" + provides: "reminderLeadMinutes in eventFieldsSchema + GET occurrence select" + contains: "reminderLeadMinutes" + - path: "apps/api/src/broker/outboxWorker.ts" + provides: "outboxPayloadSchema field + hasExplicitReminder preserve path + buildVeventString wiring (both branches)" + contains: "hasExplicitReminder" + - path: "apps/api/src/broker/sync.ts" + provides: "VALARM parse + reminderLeadMinutes upsert" + contains: "reminderLeadMinutes" + - path: "apps/api/src/broker/expand.ts" + provides: "reminderLeadMinutes on CalendarOccurrence" + contains: "reminderLeadMinutes" + key_links: + - from: "outboxWorker update branch" + to: "extractValarms(rawVevent) / computeAlertInstantUtc" + via: "hasExplicitReminder gate" + pattern: "hasExplicitReminder" + - from: "sync.ts upsert" + to: "classifyValarms(rawVevent)" + via: "reminderLeadMinutesValue derivation" + pattern: "classifyValarms" + - from: "GET /api/events select" + to: "expandOccurrences → CalendarOccurrence.reminderLeadMinutes" + via: "DB column surfaced through expansion" + pattern: "reminderLeadMinutes" +--- + + +Plumb `reminderLeadMinutes` end to end through the backend: extend `eventFieldsSchema` (route ingress) and `outboxPayloadSchema` (outbox drain) with `z.number().int().min(0).nullable().optional()`; add the `hasExplicitReminder` preserve-on-no-change path in the outbox update AND create branches (mirroring `hasExplicitRecurrence`); wire `valarms` / `reminderLeadMinutes` / `allDayAlertInstantUtc` into the `buildVeventString` calls; write `reminderLeadMinutes` into the DB on sync (so the scheduler has ground truth for native-client VALARMs); and surface `reminderLeadMinutes` on `CalendarOccurrence` + the GET select for edit-mode pre-population. + +Purpose: Carry the NULL-vs-0-vs-absent distinction (D-06) all the way through (CAL-13), and preserve existing VALARMs verbatim when the user does not touch the picker (CAL-14, D-08). Consumes the builders/classifier/extractor/computeAlertInstantUtc from Plan 11-01. +Output: Extended schemas + worker branches + sync upsert + occurrence interface; extended outboxWorker.test.ts. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.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 +@.planning/phases/11-per-event-reminders/11-01-SUMMARY.md +@apps/api/src/broker/vevent.ts + + + +This plan creates (exclude from drift checks — NEW): +- `reminderLeadMinutes` field on `eventFieldsSchema` (events.ts) +- `reminderLeadMinutes` field on `outboxPayloadSchema` (outboxWorker.ts) +- `hasExplicitReminder` const + `valarmsToPreserve` / `allDayAlertInstantUtc` locals in both outbox branches (outboxWorker.ts) +- `reminderLeadMinutesValue` derivation + upsert write (sync.ts) +- `reminderLeadMinutes` field on `CalendarOccurrence` (expand.ts) + in the GET select / expandOccurrences propagation (events.ts) +CONSUMES from Plan 11-01 (NOT new here): buildTimedValarm, buildAllDayValarm, classifyValarms, extractValarms, computeAlertInstantUtc, extended NewEventParams. + + + + + + Task 1: Schema field + outbox worker preserve-on-edit + buildVeventString wiring + apps/api/src/routes/events.ts, apps/api/src/broker/outboxWorker.ts, apps/api/tests/broker/outboxWorker.test.ts + + - apps/api/src/routes/events.ts lines 101-120 (eventFieldsSchema — add field after recurrenceCount) + - apps/api/src/broker/outboxWorker.ts lines 73-95 (outboxPayloadSchema), 425-466 (update branch hasExplicitRecurrence + freshEtagRows rawVevent read + preservedRrule), 483-492 (update buildVeventString call), 543-560 (create branch hasExplicitRecurrence) + - apps/api/tests/broker/outboxWorker.test.ts lines 246-375 (ICS-building + CR-01 _preservedRrule preserve tests to mirror) + - 11-RESEARCH.md Q2 (NULL-vs-0 zod semantics, hasExplicitReminder sentinel, NewEventParams extension, worker path resolution) ; 11-PATTERNS.md § outboxWorker.ts (exact insertion points + the buildVeventString call extension) + - apps/api/src/broker/vevent.ts (Plan 11-01: extractValarms, computeAlertInstantUtc, NewEventParams valarms/reminderLeadMinutes/allDayAlertInstantUtc) + + + Add `reminderLeadMinutes: z.number().int().min(0).nullable().optional()` to BOTH `eventFieldsSchema` (events.ts, after recurrenceCount) and `outboxPayloadSchema` (outboxWorker.ts, after recurrenceCount) with a comment documenting the four states: absent=no-change (D-08), null=clear VALARM, 0=same-day all-day, positive=timed/day lead. Import `extractValarms` and `computeAlertInstantUtc` from `./vevent.js`. + In the outbox UPDATE branch: add `const hasExplicitReminder = Object.prototype.hasOwnProperty.call(fields, 'reminderLeadMinutes')` adjacent to the existing hasExplicitRecurrence line. Resolve VALARM handling before the buildVeventString call: if `!hasExplicitReminder` and a freshEtagRows rawVevent exists → `valarmsToPreserve = extractValarms(freshEtagRows[0].rawVevent)`; else if `hasExplicitReminder && fields.reminderLeadMinutes != null && fields.allDay` → `allDayAlertInstantUtc = computeAlertInstantUtc(fields.start, fields.reminderLeadMinutes / 1440 /* D-05 leadDays */, process.env.TZ ?? Intl.DateTimeFormat().resolvedOptions().timeZone)`. Extend the update buildVeventString call with `reminderLeadMinutes: hasExplicitReminder ? fields.reminderLeadMinutes : undefined`, `valarms: valarmsToPreserve`, `allDayAlertInstantUtc`. + In the CREATE branch: a create always carries an explicit picker value (no rawVevent source), so compute allDayAlertInstantUtc the same way when `fields.reminderLeadMinutes != null && fields.allDay`, and pass `reminderLeadMinutes: fields.reminderLeadMinutes` + `allDayAlertInstantUtc` to its buildVeventString call (no valarms — create has no source to preserve). + Verify TS narrowing: `fields.reminderLeadMinutes` is `number | null | undefined` from the schema; the buildVeventString param accepts `number | null`. + + + pnpm --filter @familysync/api exec vitest run tests/broker/outboxWorker.test.ts tests/broker/vevent.test.ts + + + - outboxWorker.test.ts asserts: an UPDATE row with NO reminderLeadMinutes field whose stored rawVevent has a VALARM → emitted ICS still contains that VALARM (CAL-14 preserve, mirrors the CR-01 _preservedRrule test). + - test asserts an UPDATE/CREATE row with reminderLeadMinutes=15 (timed) → emitted ICS contains `TRIGGER:-PT15M`. + - test asserts an UPDATE/CREATE row with reminderLeadMinutes=null → emitted ICS contains no VALARM (clear). + - test asserts an all-day CREATE row with reminderLeadMinutes=1440 → emitted ICS contains an absolute `VALUE=DATE-TIME` trigger. + - both vitest files exit 0; `tsc --noEmit` clean. + + reminderLeadMinutes validated on ingress + drain; both worker branches honor absent/null/value with VALARM preserve mirroring the RRULE-preserve pattern. + + + + Task 2: sync.ts VALARM → reminderLeadMinutes upsert + apps/api/src/broker/sync.ts + + - apps/api/src/broker/sync.ts lines 120-172 (field extraction block + insert/onDuplicateKeyUpdate values) + - 11-RESEARCH.md "Pitfall: sync.ts Does Not Currently Write reminderLeadMinutes" + 11-PATTERNS.md § sync.ts (classifyValarms-based reminderLeadMinutesValue derivation + upsert additions) + - apps/api/src/broker/vevent.ts (Plan 11-01: classifyValarms / AlarmClassification) + + + Import `classifyValarms` from `./vevent.js`. After the titleValue/locationValue extraction in the per-object loop, derive `const reminderLeadMinutesValue: number | null = (alarmClass.kind === 'preset' || alarmClass.kind === 'offlist') ? alarmClass.leadMinutes : null` where `alarmClass = classifyValarms(obj.data as string)`. 'custom' (absolute/multi) and 'none' both map to null (the scheduler cannot resolve a single relative lead from those — D-07/NOTIF-05). Add `reminderLeadMinutes: reminderLeadMinutesValue` to BOTH the `.values({...})` object and the `.onDuplicateKeyUpdate({ set: {...} })` object so re-synced events keep the column current. Do NOT add or change any DB DDL — the `reminder_lead_minutes` column already exists and is nullable (no migration this phase; never drizzle-kit push). + + + pnpm --filter @familysync/api exec vitest run tests/broker/ + + + - A sync test (extend an existing sync/broker test or add one) asserts that ingesting a VCALENDAR with a single `TRIGGER:-PT30M` VALARM writes reminderLeadMinutes=30 to calendar_events. + - test asserts ingesting a VCALENDAR with no VALARM writes reminderLeadMinutes=null. + - test asserts ingesting a VCALENDAR with an absolute DATE-TIME or two VALARMs writes reminderLeadMinutes=null (custom → null). + - full broker test dir exits 0; `tsc --noEmit` clean. + + sync upsert populates reminderLeadMinutes from the parsed VALARM (preset/offlist → minutes; custom/none → null), so the scheduler has ground truth for native-client alarms. + + + + Task 3: Surface reminderLeadMinutes on CalendarOccurrence + GET select + apps/api/src/broker/expand.ts, apps/api/src/routes/events.ts + + - apps/api/src/broker/expand.ts lines 37-69 (CalendarOccurrence interface), 169-179 (expandOccurrences signature), 255-307 (where occurrence objects are built — both non-recurring and recurring branches set hasRrule) + - apps/api/src/routes/events.ts lines 166-230 (GET select + expandOccurrences call site) + - 11-PATTERNS.md § expand.ts (add field after hasRrule; note OccurrenceMeta may also need it depending on propagation) + + + Add `reminderLeadMinutes: number | null` to the `CalendarOccurrence` interface in expand.ts (after `hasRrule`), documenting NULL=no reminder / 0=same-day all-day / positive=lead (D-06). Add `reminderLeadMinutes` to the GET `.select({...})` in events.ts (line 166 block). Pass it from each selected row through `expandOccurrences(...)` so every emitted occurrence carries the master event's lead (series-level, D-10 — all occurrences inherit the master's reminderLeadMinutes). Set `reminderLeadMinutes` in both occurrence-construction branches in expand.ts (non-recurring and recurring), captured once like `isRecurring`. If the value flows through `OccurrenceMeta`, extend that interface too. Do NOT change the DB schema. + + + pnpm --filter @familysync/api exec vitest run tests/broker/ tests/routes/ + + + - An expand test asserts CalendarOccurrence.reminderLeadMinutes carries the master event's value (e.g. a master with reminderLeadMinutes=30 → every expanded occurrence has 30; series-level D-10). + - test asserts a NULL-lead master yields occurrences with reminderLeadMinutes=null and a 0-lead all-day master yields 0 (NULL-vs-0 preserved through expansion). + - `tsc --noEmit` clean for apps/api (the new field is required on CalendarOccurrence, forcing every construction site to set it). + - test dirs exit 0. + + GET /api/events occurrences expose reminderLeadMinutes (NULL-vs-0 distinct), enabling edit-mode picker pre-population by the frontend. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| client → POST/PATCH /api/events | Untrusted JSON; reminderLeadMinutes crosses here, validated by eventFieldsSchema | +| stored outbox payload → drain | Re-validated by outboxPayloadSchema (IN-03 defense-in-depth) | +| Fastmail rawVevent → sync upsert | External iCalendar parsed by classifyValarms (try/catch safe) | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-11-06 | Tampering | reminderLeadMinutes ingress | mitigate | `z.number().int().min(0).nullable().optional()` at eventFieldsSchema AND re-validated at outboxPayloadSchema — bounded integer, no injection vector; this IS the ASVS V5 input-validation coverage | +| T-11-07 | Tampering | sync parsing native VALARM | mitigate | classifyValarms wraps ICAL.parse in try/catch; only a bounded integer (minutes) is extracted; no string interpolated into SQL (Drizzle parameterized) | +| T-11-08 | Spoofing/IDOR | preserve-on-edit reads rawVevent | mitigate | freshEtagRows read is already scoped to the writing member's calendar (CR-02 join on userId+calendarUrl); unchanged — VALARM extraction rides the same scoped query | +| T-11-SC | Tampering | npm installs | accept | No new packages this phase | + +No new security surface: the only new ingress field is a bounded integer; existing ASVS V5 input-validation coverage is maintained (the new zod field is the validation). Push body remains hardcoded/humanized (Plan 02), not user-controlled. + + + +- `pnpm --filter @familysync/api exec vitest run tests/broker/ tests/routes/` green. +- `pnpm --filter @familysync/api exec tsc --noEmit` clean (the required CalendarOccurrence field forces all construction sites). +- Grep confirms no `drizzle-kit push` introduced and no schema.ts DDL change. + + + +- reminderLeadMinutes carried through schemas, worker (both branches), sync, and occurrences with NULL/0/value/absent semantics intact. +- Edit with no picker change preserves the VALARM (CAL-14); explicit null clears; explicit value (timed or all-day) emits the correct trigger (CAL-13). +- sync populates the scheduler's ground-truth column from native VALARMs. + + + +Create `.planning/phases/11-per-event-reminders/11-03-SUMMARY.md` when done. Record the schema field, the hasExplicitReminder branch behavior, the sync derivation rule, and the occurrence-propagation path. + diff --git a/.planning/phases/11-per-event-reminders/11-04-PLAN.md b/.planning/phases/11-per-event-reminders/11-04-PLAN.md new file mode 100644 index 0000000..157829e --- /dev/null +++ b/.planning/phases/11-per-event-reminders/11-04-PLAN.md @@ -0,0 +1,189 @@ +--- +phase: 11-per-event-reminders +plan: 04 +type: execute +wave: 3 +depends_on: [11-03] +files_modified: + - apps/pwa/src/api/client.ts + - apps/pwa/src/components/EventForm.tsx +autonomous: false +requirements: [CAL-13, CAL-14] +must_haves: + truths: + - "When creating/editing a timed event the user picks a reminder lead from the timed preset list (default None)" + - "When the event is all-day the picker swaps to day-granularity presets (None / Same day / 1d / 2d / 1wk), default None" + - "Toggling all-day resets the picker to None (no carry-over between preset sets)" + - "Edit mode pre-populates the picker from the occurrence: none→None, preset→matching option, off-list single→synthetic option, absolute/multi→read-only Custom (kept)" + - "Leaving a Custom (kept) selection on save preserves the original VALARM (absent reminderLeadMinutes on the payload)" + - "Selecting None sends explicit null; selecting a preset sends the integer" + artifacts: + - path: "apps/pwa/src/components/EventForm.tsx" + provides: "Reminder ` to `EventForm.tsx` per the UI-SPEC: allDay-aware preset swap (D-02/D-03), default None (D-01), reset-on-allDay-toggle, and edit-mode pre-population (none / preset / synthetic off-list / read-only "Custom (kept)") driven by `occurrence.reminderLeadMinutes`. Extend `CreateEventPayload` and `CalendarOccurrence` in `client.ts` with `reminderLeadMinutes`, and map the picker value to the payload (None→null, preset→integer, Custom-kept/unchanged→omit field for server-side preserve, D-08). Verify with a Playwright smoke (Wave 0 gap). + +Purpose: Deliver the user-facing reminder choice (CAL-13) and the preserve-on-no-change behavior at the UI layer (CAL-14). Consumes the occurrence shape from Plan 11-03. +Output: Reminder picker + payload mapping; extended client types; Playwright smoke spec/run. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/11-per-event-reminders/11-UI-SPEC.md +@.planning/phases/11-per-event-reminders/11-PATTERNS.md +@.planning/phases/11-per-event-reminders/11-VALIDATION.md +@.planning/phases/11-per-event-reminders/11-03-SUMMARY.md +@apps/pwa/src/components/EventForm.tsx +@apps/pwa/src/api/client.ts + + + +This plan creates (exclude from drift checks — NEW): +- Reminder ``). The select is NOT disabled in edit mode (reminders are editable, unlike Repeat). Render options conditionally on the existing `allDay` state: timed presets (None=`__none__`, 5/10/15/30/60/120/1440/2880 with the exact labels in the Copywriting Contract) when `!allDay`; all-day presets (None, 0=`Same day (9 AM)`, 1440=`1 day before (9 AM)`, 2880=`2 days before (9 AM)`, 10080=`1 week before (9 AM)`) when `allDay`. Default selected value `__none__` (D-01). + Add a `reminderValue` state (string). In the existing allDay onChange handler, also `setReminderValue('__none__')` so toggling all-day resets the picker (D-03 — no carry-over). + Edit-mode pre-population on mount/when the occurrence loads, from `occurrence.reminderLeadMinutes`: null → `__none__`; a value matching a preset for the current allDay set → that option; a positive value NOT in the preset set → append a synthetic `` selected by default, with the select itself still enabled. Show the helper text `Custom reminder kept — select a preset to replace it.` (style: fontSize var(--text-label-size), color var(--color-text-secondary), marginTop var(--space-1)) only in edit mode when the value is `__custom__` (or the synthetic off-list option). Selecting any real preset removes the synthetic/custom option from selection. + Payload mapping in the submit handler: `__none__` → `reminderLeadMinutes: null` (explicit clear); a numeric preset/synthetic value → `reminderLeadMinutes: `; if the selection is still `__custom__` (unchanged) → OMIT `reminderLeadMinutes` from the payload entirely so the server preserves the original VALARM (D-08 — absent = no-change). Render all option labels as plain-text JSX children (XSS guard T-03-15). Determining "Custom (kept)" vs off-list synthetic from the occurrence: since the occurrence only carries a number-or-null, the frontend treats null as None and any number as preset-or-offlist; the `__custom__` state is reached only when the occurrence signals an unresolvable alarm — if the occurrence reminderLeadMinutes is null but the event is known to carry a kept custom alarm, follow the UI-SPEC: with only number|null available, map null→None and rely on the server-side preserve (absent payload) — document this limitation in the SUMMARY. + + + pnpm --filter @familysync/pwa exec vitest run src/components/EventForm.test.tsx + + + - A component test asserts: in create mode with allDay=false the picker shows the timed preset labels and defaults to None; toggling allDay swaps to the day-granularity labels and resets selection to None. + - test asserts edit mode with occurrence.reminderLeadMinutes=30 (timed) selects "30 minutes before"; =1440 all-day selects "1 day before (9 AM)"; =45 (off-list) shows a synthetic "45 min before" option selected. + - test asserts the submitted payload: None→reminderLeadMinutes:null; a preset→the integer; an unchanged Custom-kept→field omitted (Object.prototype.hasOwnProperty is false). + - pwa vitest + `tsc --noEmit` exit 0. + + Reminder picker matches the UI-SPEC: swap, default None, reset-on-toggle, edit pre-population, Custom-kept preserve via omitted payload field. + + + + Task 3: Playwright smoke — picker swap + edit-mode load (Wave 0 gap) + apps/pwa/tests/ (playwright-cli smoke spec for the reminder picker) + + - .claude/skills/playwright-cli/SKILL.md (browser-driving usage; project convention: prefer playwright-cli over manual human verification for desktop/Chromium checks) + - 11-VALIDATION.md § Wave 0 Requirements (Playwright smoke) and § Manual-Only Verifications (device-only items NOT covered here) + - MEMORY: dev stack bring-up (dev compose exposes 3306, API `dev` needs separate build, DEV_AUTH_BYPASS=true → Dev User id 1); Dev user 1 has no calendars — verify form-rendering/picker behavior, not live event create + + + A Playwright smoke (driven via the playwright-cli skill, Chromium/desktop) that: opens the New Event form, asserts the Reminder select defaults to None and shows timed presets; toggles All-day and asserts the option set swaps to the day-granularity presets and selection resets to None. Plus an edit-mode load assertion against a route-mocked occurrence carrying reminderLeadMinutes (e.g. 30 → "30 minutes before"; 1440 all-day → "1 day before (9 AM)"), since dev user 1 has no real calendars. + Automate first: bring up the host-side dev stack per the documented command (DEV_AUTH_BYPASS=true, DB_HOST=localhost), build the API dev bundle, then run the smoke headlessly. This is autonomous via playwright-cli — the human only confirms the run results if the harness cannot self-assert. + + + 1. Start the dev stack (two terminals or background): API dev (with DEV_AUTH_BYPASS=true DB_HOST=localhost, env-sourced) + PWA dev server. + 2. Drive Chromium via playwright-cli to the calendar, open New Event, assert: Reminder select present (id=event-reminder), value None, timed option labels visible. + 3. Toggle All-day; assert the swapped day-granularity labels are present and the selection is None. + 4. With a page.route mock returning an occurrence whose reminderLeadMinutes=30 (timed) and a second whose reminderLeadMinutes=1440 (all-day), open edit and assert the picker shows the matching labels. + 5. Confirm no console errors and the screenshot shows the picker rendered per the UI-SPEC. + + + Automate the verification end-to-end via the playwright-cli skill before pausing for human confirmation: (1) bring up the host-side dev stack per docs/deployment.md (env-sourced, DEV_AUTH_BYPASS=true DB_HOST=localhost; rebuild the API dev bundle since dist can be stale) plus the PWA dev server; (2) drive Chromium with playwright-cli to open the New Event form and assert the Reminder select (id=event-reminder) defaults to None and shows the timed preset labels; (3) toggle All-day and assert the option set swaps to the day-granularity labels and the selection resets to None; (4) use page.route to mock occurrences with reminderLeadMinutes=30 (timed) and =1440 (all-day) and assert edit mode loads "30 minutes before" and "1 day before (9 AM)" respectively (dev user 1 has no real calendars, so mock rather than create live); (5) capture a screenshot and the pass/fail of each assertion. Only after the smoke runs do you surface the result for human sign-off. Do NOT ask the human to perform steps the playwright-cli skill can drive. + + + cd apps/pwa && playwright-cli run smoke for the reminder picker (swap + edit-load) against the dev stack; capture pass/fail + screenshot + Confirm the playwright-cli smoke passed (picker swap + edit-mode labels) or report the failing assertion. + + + - playwright-cli smoke asserts the timed→all-day option swap and reset-to-None (CAL-13 browser row in 11-VALIDATION.md Per-Task Verification Map). + - playwright-cli smoke asserts edit-mode picker loads the correct label for a route-mocked occurrence (30→"30 minutes before", 1440 all-day→"1 day before (9 AM)"). + - run completes with zero console errors; screenshot attached to the SUMMARY. + + Type "approved" once the playwright-cli smoke passes, or describe the failing assertion. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| EventForm select value → payload | User-chosen value mapped to a bounded integer / null / omitted; re-validated server-side (Plan 03 eventFieldsSchema) | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-11-09 | Tampering | reminder picker → payload | mitigate | Client emits only null / a preset integer / omitted field; the server is the real boundary (eventFieldsSchema validates `z.number().int().min(0).nullable().optional()`); client trust is not relied upon | +| T-11-10 | XSS | option labels + helper text | mitigate | All labels/helper text rendered as plain-text JSX children, no dangerouslySetInnerHTML (T-03-15 precedent) | +| T-11-SC | Tampering | npm installs | accept | No new frontend npm dependencies (UI-SPEC: "Phase 11 adds no new npm dependencies on the frontend") | + +No new security surface on the frontend: the picker is a native `