Compare commits
7
Commits
771608d3d4
...
e169b138f9
| Author | SHA1 | Date | |
|---|---|---|---|
 Lucas Berger
|
e169b138f9 | ||
|
Lucas Berger
|
6109d4ca49 | ||
|
Lucas Berger
|
cfeb8d8660 | ||
|
Lucas Berger
|
a3c4aea9b1 | ||
|
Lucas Berger
|
feec030509 | ||
|
Lucas Berger
|
4e09ebdb75 | ||
|
Lucas Berger
|
ecdd94a907 |
+15
-1
@@ -212,7 +212,21 @@ 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:
|
||||
**Wave 1**
|
||||
|
||||
- [ ] 11-01-PLAN.md — VALARM builders + classifier + extractor + computeAlertInstantUtc (vevent.ts, TDD)
|
||||
|
||||
**Wave 2** *(blocked on Wave 1 completion)*
|
||||
|
||||
- [ ] 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
|
||||
|
||||
**Wave 3** *(blocked on Wave 2 completion)*
|
||||
|
||||
- [ ] 11-04-PLAN.md — EventForm reminder picker (allDay swap, edit pre-population) + client types + Playwright smoke
|
||||
|
||||
**UI hint**: yes
|
||||
|
||||
### Phase 12: Initial Setup Wizard
|
||||
|
||||
+7
-7
@@ -2,9 +2,9 @@
|
||||
gsd_state_version: 1.0
|
||||
milestone: v1.1
|
||||
milestone_name: Operability & Polish
|
||||
status: verifying
|
||||
stopped_at: Phase 11 context gathered
|
||||
last_updated: "2026-06-14T00:49:25.513Z"
|
||||
status: executing
|
||||
stopped_at: Phase 11 UI-SPEC approved
|
||||
last_updated: "2026-06-14T01:26:07.775Z"
|
||||
last_activity: "2026-06-13 - Completed quick task 260613-ndv: isolated local apps/api tests to familysync_test (dev DB no longer polluted)"
|
||||
progress:
|
||||
total_phases: 21
|
||||
@@ -27,7 +27,7 @@ See: .planning/PROJECT.md (updated 2026-06-10)
|
||||
|
||||
Phase: 13
|
||||
Plan: Not started
|
||||
Status: Phase complete — ready for verification
|
||||
Status: Ready to execute
|
||||
Last activity: 2026-06-13 - Completed quick task 260613-ndv: isolated local apps/api tests to familysync_test (dev DB no longer polluted)
|
||||
|
||||
### ✅ Resolved Checkpoint — Phase 15 Plan 15-03 Task 2 (human-action)
|
||||
@@ -246,9 +246,9 @@ Recent decisions affecting current work:
|
||||
|
||||
## Session Continuity
|
||||
|
||||
Last session: 2026-06-14T00:49:25.504Z
|
||||
Stopped at: Phase 11 context gathered
|
||||
Resume file: .planning/phases/11-per-event-reminders/11-CONTEXT.md
|
||||
Last session: 2026-06-14T00:53:26.422Z
|
||||
Stopped at: Phase 11 UI-SPEC approved
|
||||
Resume file: .planning/phases/11-per-event-reminders/11-UI-SPEC.md
|
||||
|
||||
## Operator Next Steps
|
||||
|
||||
|
||||
@@ -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>
|
||||
@@ -0,0 +1,204 @@
|
||||
---
|
||||
phase: 11-per-event-reminders
|
||||
plan: 02
|
||||
type: tdd
|
||||
wave: 2
|
||||
depends_on: [11-01]
|
||||
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) (D-09)"
|
||||
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\\("
|
||||
---
|
||||
|
||||
<objective>
|
||||
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.
|
||||
</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
|
||||
@.planning/phases/11-per-event-reminders/11-UI-SPEC.md
|
||||
</context>
|
||||
|
||||
<artifacts_this_phase_produces>
|
||||
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.
|
||||
</artifacts_this_phase_produces>
|
||||
|
||||
<context_note>
|
||||
This plan imports `computeAlertInstantUtc` from `../broker/vevent.js` (created in Plan 11-01). This plan is **wave 2, `depends_on: [11-01]`** — 11-01 lands before this plan runs, so the symbol is available and the all-day-9AM test can go GREEN within this plan. Sequence the all-day task LAST within this plan. Do not redefine computeAlertInstantUtc here — import it from 11-01.
|
||||
</context_note>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="tdd" tdd="true">
|
||||
<name>Task 1: Variable-lead window + uid:dtstartMs dedup + drop isShared/allDay restrictions + NULL-vs-0 (timed)</name>
|
||||
<files>apps/api/src/broker/reminderScheduler.ts, apps/api/tests/broker/reminderScheduler.test.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- 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.
|
||||
</behavior>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>pnpm --filter @familysync/api exec vitest run tests/broker/reminderScheduler.test.ts</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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.
|
||||
</acceptance_criteria>
|
||||
<done>Scheduler fires per-event lead, ignores NULL and timed-0, fires on personal calendars, dedups on uid:dtstartMs; existing isShared/allDay-exclusion tests replaced.</done>
|
||||
</task>
|
||||
|
||||
<task type="tdd" tdd="true">
|
||||
<name>Task 2: humanizeLeadMinutes body formatter</name>
|
||||
<files>apps/api/src/broker/reminderScheduler.ts, apps/api/tests/broker/reminderScheduler.test.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- 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.
|
||||
</behavior>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>pnpm --filter @familysync/api exec vitest run tests/broker/reminderScheduler.test.ts</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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.
|
||||
</acceptance_criteria>
|
||||
<done>humanizeLeadMinutes covers every preset bucket; scheduler body uses it driven by the configured lead (D-09).</done>
|
||||
</task>
|
||||
|
||||
<task type="tdd" tdd="true">
|
||||
<name>Task 3: All-day 9 AM-local fire branch (NOTIF-06)</name>
|
||||
<files>apps/api/src/broker/reminderScheduler.ts, apps/api/tests/broker/reminderScheduler.test.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- 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.
|
||||
</behavior>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>pnpm --filter @familysync/api exec vitest run tests/broker/reminderScheduler.test.ts</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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).
|
||||
</acceptance_criteria>
|
||||
<done>All-day reminders fire at 9 AM local on the computed alert day for 0/1440/2880/10080 leads, exactly once per uid:dtstartMs.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## 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.
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `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).
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- 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.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
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.
|
||||
</output>
|
||||
@@ -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"
|
||||
---
|
||||
|
||||
<objective>
|
||||
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.
|
||||
</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
|
||||
@.planning/phases/11-per-event-reminders/11-01-SUMMARY.md
|
||||
@apps/api/src/broker/vevent.ts
|
||||
</context>
|
||||
|
||||
<artifacts_this_phase_produces>
|
||||
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.
|
||||
</artifacts_this_phase_produces>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="execute" tdd="true">
|
||||
<name>Task 1: Schema field + outbox worker preserve-on-edit + buildVeventString wiring</name>
|
||||
<files>apps/api/src/routes/events.ts, apps/api/src/broker/outboxWorker.ts, apps/api/tests/broker/outboxWorker.test.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<action>
|
||||
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`.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>pnpm --filter @familysync/api exec vitest run tests/broker/outboxWorker.test.ts tests/broker/vevent.test.ts</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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.
|
||||
</acceptance_criteria>
|
||||
<done>reminderLeadMinutes validated on ingress + drain; both worker branches honor absent/null/value with VALARM preserve mirroring the RRULE-preserve pattern.</done>
|
||||
</task>
|
||||
|
||||
<task type="execute" tdd="true">
|
||||
<name>Task 2: sync.ts VALARM → reminderLeadMinutes upsert</name>
|
||||
<files>apps/api/src/broker/sync.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<action>
|
||||
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).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>pnpm --filter @familysync/api exec vitest run tests/broker/</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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.
|
||||
</acceptance_criteria>
|
||||
<done>sync upsert populates reminderLeadMinutes from the parsed VALARM (preset/offlist → minutes; custom/none → null), so the scheduler has ground truth for native-client alarms.</done>
|
||||
</task>
|
||||
|
||||
<task type="execute" tdd="true">
|
||||
<name>Task 3: Surface reminderLeadMinutes on CalendarOccurrence + GET select</name>
|
||||
<files>apps/api/src/broker/expand.ts, apps/api/src/routes/events.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>pnpm --filter @familysync/api exec vitest run tests/broker/ tests/routes/</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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.
|
||||
</acceptance_criteria>
|
||||
<done>GET /api/events occurrences expose reminderLeadMinutes (NULL-vs-0 distinct), enabling edit-mode picker pre-population by the frontend.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## 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.
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `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.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- 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.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
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.
|
||||
</output>
|
||||
@@ -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 <select> (allDay-aware swap, edit pre-population, Custom-kept handling)"
|
||||
contains: "event-reminder"
|
||||
- path: "apps/pwa/src/api/client.ts"
|
||||
provides: "reminderLeadMinutes on CreateEventPayload + CalendarOccurrence"
|
||||
contains: "reminderLeadMinutes"
|
||||
key_links:
|
||||
- from: "EventForm reminder select"
|
||||
to: "CreateEventPayload.reminderLeadMinutes"
|
||||
via: "submit handler maps picker value → number | null | absent"
|
||||
pattern: "reminderLeadMinutes"
|
||||
- from: "edit-mode load"
|
||||
to: "occurrence.reminderLeadMinutes"
|
||||
via: "preset/offlist/custom classification on mount"
|
||||
pattern: "reminderLeadMinutes"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Add the reminder `<select>` 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.
|
||||
</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-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
|
||||
</context>
|
||||
|
||||
<artifacts_this_phase_produces>
|
||||
This plan creates (exclude from drift checks — NEW):
|
||||
- Reminder `<select id="event-reminder">` block + its local state + onChange handler — EventForm.tsx
|
||||
- allDay-toggle reset of the reminder state — EventForm.tsx
|
||||
- edit-mode classification → picker value (synthetic off-list option, "Custom (kept)" disabled option, helper text) — EventForm.tsx
|
||||
- `reminderLeadMinutes?: number | null` on `CreateEventPayload` — client.ts
|
||||
- `reminderLeadMinutes: number | null` on `CalendarOccurrence` (atomic mirror of expand.ts) — client.ts
|
||||
- Playwright smoke spec for the picker swap + edit-mode load — apps/pwa (Wave 0 gap)
|
||||
</artifacts_this_phase_produces>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="execute">
|
||||
<name>Task 1: Client types — reminderLeadMinutes on CreateEventPayload + CalendarOccurrence</name>
|
||||
<files>apps/pwa/src/api/client.ts</files>
|
||||
<read_first>
|
||||
- apps/pwa/src/api/client.ts lines 108-134 (CalendarOccurrence, hasRrule mirror note) and 175-198 (CreateEventPayload)
|
||||
- apps/api/src/broker/expand.ts CalendarOccurrence (Plan 11-03: the field this mirrors)
|
||||
- 11-PATTERNS.md § client.ts (exact field additions + the atomic-mirror comment style)
|
||||
</read_first>
|
||||
<action>
|
||||
Add `reminderLeadMinutes: number | null` to `CalendarOccurrence` (after `hasRrule`) with a comment noting it mirrors expand.ts (atomic mirror, NULL=no reminder / 0=same-day all-day / positive=lead). Add `reminderLeadMinutes?: number | null` to `CreateEventPayload` (after `description`) documenting the four states: absent/undefined=no-change (edit omits → server preserves, D-08), null=explicit None (clear), 0=same-day all-day, positive=lead. Do not change createEvent/editEvent call signatures — they already spread CreateEventPayload.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>pnpm --filter @familysync/pwa exec tsc --noEmit</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- client.ts CalendarOccurrence has a required `reminderLeadMinutes: number | null`; CreateEventPayload has an optional `reminderLeadMinutes?: number | null`.
|
||||
- `pnpm --filter @familysync/pwa exec tsc --noEmit` exits 0 (any occurrence consumer that destructures the type still compiles, or is updated).
|
||||
</acceptance_criteria>
|
||||
<done>Frontend types carry reminderLeadMinutes with the absent/null/0/positive contract mirroring the server.</done>
|
||||
</task>
|
||||
|
||||
<task type="execute">
|
||||
<name>Task 2: Reminder picker in EventForm (swap, default None, reset-on-toggle, edit pre-population, Custom-kept, payload mapping)</name>
|
||||
<files>apps/pwa/src/components/EventForm.tsx, apps/pwa/src/components/EventForm.test.tsx</files>
|
||||
<read_first>
|
||||
- apps/pwa/src/components/EventForm.tsx lines 848-885 (Recurrence picker — the exact structural template) and the existing allDay state + allDay onChange handler + the form submit/payload-assembly site
|
||||
- 11-UI-SPEC.md (Component Inventory: option tables, Copywriting Contract, Interaction Contract state machine, helper text style/copy, accessibility)
|
||||
- 11-PATTERNS.md § EventForm.tsx (allDay-conditional option swap, reset pattern, differences from Recurrence picker)
|
||||
</read_first>
|
||||
<action>
|
||||
Insert a reminder field after the Recurrence picker block (~line 885), styled identically (`<div style={fieldStyle}>`, `<label htmlFor="event-reminder" style={labelStyle}>Reminder</label>`, `<select id="event-reminder" style={{...inputStyle, padding:'0 var(--space-3)', cursor:'pointer'}}>`). 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 `<option>` whose label is humanized ("N min before" / "N hours before" using the UI-SPEC thresholds) and select it (off-list single, D-07); for the absolute/multi "Custom (kept)" case the occurrence cannot express a single lead → represent it with a special sentinel value `__custom__` rendered as a read-only `disabled` `<option value="__custom__">Custom (kept)</option>` 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: <number>`; 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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>pnpm --filter @familysync/pwa exec vitest run src/components/EventForm.test.tsx</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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.
|
||||
</acceptance_criteria>
|
||||
<done>Reminder picker matches the UI-SPEC: swap, default None, reset-on-toggle, edit pre-population, Custom-kept preserve via omitted payload field.</done>
|
||||
</task>
|
||||
|
||||
<task type="checkpoint:human-verify" gate="blocking">
|
||||
<name>Task 3: Playwright smoke — picker swap + edit-mode load (Wave 0 gap)</name>
|
||||
<files>apps/pwa/tests/ (playwright-cli smoke spec for the reminder picker)</files>
|
||||
<read_first>
|
||||
- .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
|
||||
</read_first>
|
||||
<what-built>
|
||||
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.
|
||||
</what-built>
|
||||
<how-to-verify>
|
||||
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.
|
||||
</how-to-verify>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd apps/pwa && playwright-cli run smoke for the reminder picker (swap + edit-load) against the dev stack; capture pass/fail + screenshot</automated>
|
||||
<human-check>Confirm the playwright-cli smoke passed (picker swap + edit-mode labels) or report the failing assertion.</human-check>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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.
|
||||
</acceptance_criteria>
|
||||
<resume-signal>Type "approved" once the playwright-cli smoke passes, or describe the failing assertion.</resume-signal>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## 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 `<select>` emitting bounded values; validation authority remains server-side.
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `pnpm --filter @familysync/pwa exec vitest run` green; `pnpm --filter @familysync/pwa exec tsc --noEmit` clean.
|
||||
- Playwright smoke (picker swap + edit-mode load) passes via playwright-cli.
|
||||
- Full CI fast-checks parity locally before declaring done: lint + typecheck + test + format:check + md:lint (MEMORY: CI fast-checks runs the whole gate, not just lint/typecheck/test).
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- Reminder picker present and behaves per UI-SPEC (swap, default None, reset-on-toggle, edit pre-population, Custom-kept preserve).
|
||||
- Payload mapping yields null / integer / omitted correctly.
|
||||
- Playwright smoke green; no new frontend dependencies.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/11-per-event-reminders/11-04-SUMMARY.md` when done. Record the picker value→payload mapping, the edit-mode classification handling, the Custom-kept limitation note, and the playwright-cli smoke result + screenshot path.
|
||||
</output>
|
||||
@@ -0,0 +1,485 @@
|
||||
# Phase 11: Per-Event Reminders - Pattern Map
|
||||
|
||||
**Mapped:** 2026-06-13
|
||||
**Files analyzed:** 8 modified + 2 extended test files
|
||||
**Analogs found:** 10 / 10 (all modify-existing — no greenfield files)
|
||||
|
||||
---
|
||||
|
||||
## File Classification
|
||||
|
||||
| Modified File | Role | Data Flow | Closest Analog | Match Quality |
|
||||
|---|---|---|---|---|
|
||||
| `apps/api/src/broker/vevent.ts` | utility / builder | transform | self (extend `extractRruleString`) | exact |
|
||||
| `apps/api/src/broker/outboxWorker.ts` | worker | event-driven | self (extend `hasExplicitRecurrence` + `resolveFinalRrule`) | exact |
|
||||
| `apps/api/src/broker/reminderScheduler.ts` | scheduler | event-driven | self (extend `runReminderCheck`) | exact |
|
||||
| `apps/api/src/broker/sync.ts` | service | CRUD | self (extend upsert at line 144) | exact |
|
||||
| `apps/api/src/broker/expand.ts` | utility | transform | self (extend `CalendarOccurrence` interface) | exact |
|
||||
| `apps/api/src/routes/events.ts` | route/controller | request-response | self (extend `eventFieldsSchema`) | exact |
|
||||
| `apps/pwa/src/api/client.ts` | client utility | request-response | self (extend `CreateEventPayload`) | exact |
|
||||
| `apps/pwa/src/components/EventForm.tsx` | component | request-response | self (Recurrence picker at lines 848–885) | exact |
|
||||
| `apps/api/tests/broker/vevent.test.ts` | test | — | self (extend existing describe blocks) | exact |
|
||||
| `apps/api/tests/broker/reminderScheduler.test.ts` | test | — | self (extend existing describe blocks) | exact |
|
||||
|
||||
---
|
||||
|
||||
## Pattern Assignments
|
||||
|
||||
### `apps/api/src/broker/vevent.ts` (utility/builder, transform)
|
||||
|
||||
**Changes:** (1) Add `reminderLeadMinutes`, `valarms`, `allDayAlertInstantUtc` to `NewEventParams`. (2) Add VALARM emission block after the RRULE block. (3) Add `classifyValarms()` and `extractValarms()` exported functions.
|
||||
|
||||
**Analog pattern — RRULE property construction** (vevent.ts lines 149–153):
|
||||
The RRULE block is the template for VALARM emission. It uses `new ICAL.Property` + `resetType`/`setValue` to avoid string-serialization bugs — the same technique required for TRIGGER to avoid `VALUE=TEXT` (Pitfall 2).
|
||||
|
||||
```typescript
|
||||
// RRULE analog — lines 149–153 (DO NOT COPY verbatim; adapt for VALARM)
|
||||
if (params.rruleString) {
|
||||
const recur = ICAL.Recur.fromString(params.rruleString);
|
||||
const rruleProp = new ICAL.Property('rrule');
|
||||
rruleProp.setValue(recur);
|
||||
vevent.addProperty(rruleProp);
|
||||
}
|
||||
```
|
||||
|
||||
**Adapt for VALARM emission** (insert after line 153, before optional fields):
|
||||
```typescript
|
||||
// VALARM — preserve path (edit with no picker change)
|
||||
if (params.valarms && params.valarms.length > 0) {
|
||||
for (const alarm of params.valarms) {
|
||||
vevent.addSubcomponent(alarm);
|
||||
}
|
||||
}
|
||||
// VALARM — new alarm path (create, or user changed picker)
|
||||
else if (params.reminderLeadMinutes != null) {
|
||||
if (params.allDay && params.allDayAlertInstantUtc) {
|
||||
// Absolute DATE-TIME trigger for all-day events
|
||||
const valarm = new ICAL.Component('valarm');
|
||||
valarm.addPropertyWithValue('action', 'DISPLAY');
|
||||
valarm.addPropertyWithValue('description', 'Reminder');
|
||||
const triggerProp = new ICAL.Property('trigger');
|
||||
triggerProp.resetType('date-time');
|
||||
triggerProp.setValue(ICAL.Time.fromJSDate(params.allDayAlertInstantUtc, true));
|
||||
valarm.addProperty(triggerProp);
|
||||
vevent.addSubcomponent(valarm);
|
||||
} else if (!params.allDay && params.reminderLeadMinutes > 0) {
|
||||
// Relative DURATION trigger for timed events
|
||||
const valarm = new ICAL.Component('valarm');
|
||||
valarm.addPropertyWithValue('action', 'DISPLAY');
|
||||
valarm.addPropertyWithValue('description', 'Reminder');
|
||||
const triggerProp = new ICAL.Property('trigger');
|
||||
triggerProp.resetType('duration'); // critical: prevents VALUE=TEXT (Pitfall 2)
|
||||
triggerProp.setValue(ICAL.Duration.fromSeconds(-params.reminderLeadMinutes * 60));
|
||||
valarm.addProperty(triggerProp);
|
||||
vevent.addSubcomponent(valarm);
|
||||
}
|
||||
// allDay=false && reminderLeadMinutes===0: no VALARM (timed 0 = None per D-06)
|
||||
}
|
||||
```
|
||||
|
||||
**`extractRruleString` as pattern for `extractValarms`** (vevent.ts lines 63–79):
|
||||
```typescript
|
||||
// EXISTING — extractRruleString (lines 63–79) — copy structure for extractValarms
|
||||
export function extractRruleString(rawVevent: string): string | undefined {
|
||||
let parsed: ReturnType<typeof ICAL.parse>;
|
||||
try {
|
||||
parsed = ICAL.parse(rawVevent);
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
const comp = new ICAL.Component(parsed);
|
||||
const vevent = comp.getFirstSubcomponent('vevent');
|
||||
if (!vevent) return undefined;
|
||||
const rrule = vevent.getFirstPropertyValue('rrule');
|
||||
if (!rrule) return undefined;
|
||||
return typeof rrule === 'string' ? rrule : (rrule as ICAL.Recur).toString();
|
||||
}
|
||||
```
|
||||
|
||||
**`NewEventParams` interface extension** (vevent.ts lines 21–33):
|
||||
Add three optional fields to the existing interface:
|
||||
```typescript
|
||||
// ADD to NewEventParams (after line 32, before closing brace):
|
||||
reminderLeadMinutes?: number | null; // null = no VALARM; 0 = same-day all-day; positive = timed lead
|
||||
valarms?: ICAL.Component[]; // pre-parsed VALARMs from rawVevent (preserve-on-edit, D-08)
|
||||
allDayAlertInstantUtc?: Date; // 9 AM local on alert day in UTC (computed by worker for all-day)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### `apps/api/src/broker/outboxWorker.ts` (worker, event-driven)
|
||||
|
||||
**Changes:** (1) Add `reminderLeadMinutes` to `outboxPayloadSchema`. (2) In update branch: add `hasExplicitReminder` check mirroring `hasExplicitRecurrence`. (3) Pass `valarms`/`reminderLeadMinutes`/`allDayAlertInstantUtc` to `buildVeventString`. (4) Same additions in create branch.
|
||||
|
||||
**Key analog — `hasExplicitRecurrence` pattern** (outboxWorker.ts lines 425–466):
|
||||
```typescript
|
||||
// EXISTING — lines 425–426: template for hasExplicitReminder
|
||||
const hasExplicitRecurrence = Object.prototype.hasOwnProperty.call(fields, 'recurrence');
|
||||
// ...
|
||||
// EXISTING — lines 464–466: template for VALARM preserve-on-no-change
|
||||
if (!hasExplicitRecurrence && freshEtagRows.length > 0 && freshEtagRows[0].rawVevent) {
|
||||
preservedRrule = extractRruleString(freshEtagRows[0].rawVevent);
|
||||
}
|
||||
```
|
||||
|
||||
**Adapt for VALARM** (insert adjacent to the RRULE preserve block in the update branch):
|
||||
```typescript
|
||||
// ADD alongside the hasExplicitRecurrence block:
|
||||
const hasExplicitReminder = Object.prototype.hasOwnProperty.call(fields, 'reminderLeadMinutes');
|
||||
let valarmsToPreserve: ICAL.Component[] = [];
|
||||
let allDayAlertInstantUtc: Date | undefined;
|
||||
|
||||
if (!hasExplicitReminder && freshEtagRows.length > 0 && freshEtagRows[0].rawVevent) {
|
||||
// D-08: user did not touch the picker — preserve existing VALARM verbatim
|
||||
valarmsToPreserve = extractValarms(freshEtagRows[0].rawVevent);
|
||||
} else if (hasExplicitReminder && fields.reminderLeadMinutes != null && fields.allDay) {
|
||||
// All-day: compute 9 AM local UTC instant for the absolute DATE-TIME trigger
|
||||
allDayAlertInstantUtc = computeAlertInstantUtc(
|
||||
fields.start, // 'YYYY-MM-DD'
|
||||
fields.reminderLeadMinutes / 1440, // leadDays
|
||||
process.env.TZ ?? Intl.DateTimeFormat().resolvedOptions().timeZone,
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
**`buildVeventString` call extension** (outboxWorker.ts lines 483–492):
|
||||
```typescript
|
||||
// EXISTING call (lines 483–492) — add three new params:
|
||||
const { icsString } = buildVeventString({
|
||||
uid: row.uid,
|
||||
summary: fields.title,
|
||||
allDay: fields.allDay,
|
||||
dtstart: fields.allDay ? fields.start : new Date(fields.start),
|
||||
dtend: fields.allDay ? fields.end : new Date(fields.end),
|
||||
location: fields.location,
|
||||
description: fields.description,
|
||||
rruleString: finalRruleString,
|
||||
// ADD:
|
||||
reminderLeadMinutes: hasExplicitReminder ? fields.reminderLeadMinutes : undefined,
|
||||
valarms: valarmsToPreserve,
|
||||
allDayAlertInstantUtc,
|
||||
});
|
||||
```
|
||||
|
||||
**`outboxPayloadSchema` extension** (outboxWorker.ts lines 73–95):
|
||||
```typescript
|
||||
// EXISTING schema — add reminderLeadMinutes after recurrenceCount (line 93):
|
||||
reminderLeadMinutes: z.number().int().min(0).nullable().optional(),
|
||||
// Semantics: absent=no-change (D-08), null=clear VALARM, 0=same-day all-day, positive=timed lead
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### `apps/api/src/broker/reminderScheduler.ts` (scheduler, event-driven)
|
||||
|
||||
**Changes:** (1) Change dedup key from bare `uid` to `uid:${dtstartMs}`. (2) Replace fixed `(now, now+16min]` window with per-event variable-lead query. (3) Drop `isShared=true` restriction. (4) Add all-day 9 AM UTC branch. (5) Replace hardcoded body string with `humanizeLeadMinutes()`.
|
||||
|
||||
**Key analog — existing dedup map and dispatch loop** (reminderScheduler.ts lines 48, 141–188):
|
||||
```typescript
|
||||
// EXISTING dedup map (line 48) — change value type comment; key changes to uid:dtstartMs
|
||||
const sentReminders = new Map<string, number>(); // key: uid:dtstartMs
|
||||
|
||||
// EXISTING dispatch loop pattern (lines 141–188) — preserve structure, change:
|
||||
// 1. dedup key: uid → `${uid}:${event.dtstartUtc.getTime()}`
|
||||
// 2. body: `Starts in ${minutes} min` → humanizeLeadMinutes(event.reminderLeadMinutes)
|
||||
// 3. post-dispatch sentReminders.set now uses compound key
|
||||
|
||||
// EXISTING body string (line 157) — REPLACE:
|
||||
body: `Starts in ${minutes} min`,
|
||||
// WITH:
|
||||
body: humanizeLeadMinutes(event.reminderLeadMinutes),
|
||||
```
|
||||
|
||||
**SQL query changes** (reminderScheduler.ts lines 84–107):
|
||||
The existing query is the structural template. Remove `eq(calendars.isShared, true)` and `eq(calendarEvents.allDay, false)`. Add `reminder_lead_minutes IS NOT NULL`. Split into two sub-queries (timed and all-day) or use a single query fetching all events with non-null leads and filter in JS:
|
||||
|
||||
```typescript
|
||||
// EXISTING query structure (lines 84–107) — adapt WHERE clause:
|
||||
// REMOVE: eq(calendars.isShared, true) — NOTIF-05: all events, not just shared
|
||||
// REMOVE: eq(calendarEvents.allDay, false) — all-day events now supported (D-02)
|
||||
// ADD: sql`${calendarEvents.reminderLeadMinutes} IS NOT NULL`
|
||||
// ADD: reminderLeadMinutes to .select()
|
||||
|
||||
// For timed events: keep dtstartUtc window logic (variable per-event lead)
|
||||
// For all-day events: fetch with non-null leads, compute 9 AM UTC in JS, filter in-memory
|
||||
```
|
||||
|
||||
**Existing prune pattern** (reminderScheduler.ts lines 193–197):
|
||||
```typescript
|
||||
// EXISTING prune (lines 193–197) — key name changes to uid:dtstartMs but structure identical
|
||||
for (const [key, dtstartMs] of sentReminders) {
|
||||
if (dtstartMs <= now.getTime()) {
|
||||
sentReminders.delete(key);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### `apps/api/src/broker/sync.ts` (service, CRUD)
|
||||
|
||||
**Changes:** Add VALARM parsing in the per-object loop to extract `reminderLeadMinutes`, then write it in the `onDuplicateKeyUpdate` call.
|
||||
|
||||
**Key analog — existing upsert values block** (sync.ts lines 144–170):
|
||||
```typescript
|
||||
// EXISTING .values() call (lines 144–157) — add reminderLeadMinutes:
|
||||
await db
|
||||
.insert(calendarEvents)
|
||||
.values({
|
||||
calendarId: cal.id,
|
||||
uid,
|
||||
etag: obj.etag ?? null,
|
||||
objectUrl: obj.url ?? null,
|
||||
rawVevent: obj.data as string,
|
||||
title: titleValue,
|
||||
dtstartUtc: dtstartUtcValue,
|
||||
dtstartDate: dtstartDateValue,
|
||||
allDay,
|
||||
hasRrule: isRecurring,
|
||||
// ADD:
|
||||
reminderLeadMinutes: reminderLeadMinutesValue, // null when no VALARM
|
||||
})
|
||||
.onDuplicateKeyUpdate({
|
||||
set: {
|
||||
// ... existing fields ...
|
||||
// ADD:
|
||||
reminderLeadMinutes: reminderLeadMinutesValue,
|
||||
updatedAt: new Date(),
|
||||
},
|
||||
});
|
||||
```
|
||||
|
||||
**Extraction pattern** (mirrors `extractRruleString` from vevent.ts — use `classifyValarms`):
|
||||
Insert before the `await db.insert(...)` call, alongside the existing field extractions (lines 123–142):
|
||||
```typescript
|
||||
// ADD after titleValue / locationValue extraction:
|
||||
// Extract reminderLeadMinutes from VALARM (if any) — written to DB for scheduler ground truth
|
||||
const alarmClass = classifyValarms(obj.data as string);
|
||||
const reminderLeadMinutesValue: number | null =
|
||||
alarmClass.kind === 'preset' || alarmClass.kind === 'offlist'
|
||||
? alarmClass.leadMinutes
|
||||
: null; // 'custom' (absolute/multi) and 'none' both map to null
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### `apps/api/src/broker/expand.ts` (utility, transform)
|
||||
|
||||
**Change:** Add `reminderLeadMinutes` field to `CalendarOccurrence` interface so edit mode can pre-populate the picker.
|
||||
|
||||
**Key analog — existing `CalendarOccurrence` interface** (expand.ts lines 37–69):
|
||||
```typescript
|
||||
// EXISTING interface — add after hasRrule (line 68):
|
||||
/**
|
||||
* Per-event reminder lead in minutes. NULL = no reminder. 0 = same-day all-day.
|
||||
* Positive integer = N minutes before event start.
|
||||
* D-06: NULL and 0 are semantically distinct.
|
||||
*/
|
||||
reminderLeadMinutes: number | null;
|
||||
```
|
||||
|
||||
The corresponding DB select in the events route must also include `reminderLeadMinutes` in the join result passed to `expandOccurrences`. The `OccurrenceMeta` interface (expand.ts line 75+) may also need `reminderLeadMinutes` depending on how expansion propagates it.
|
||||
|
||||
---
|
||||
|
||||
### `apps/api/src/routes/events.ts` (route, request-response)
|
||||
|
||||
**Change:** Add `reminderLeadMinutes` to `eventFieldsSchema`.
|
||||
|
||||
**Key analog — existing `eventFieldsSchema`** (events.ts lines 101–120):
|
||||
```typescript
|
||||
// EXISTING schema (lines 101–120) — add after recurrenceCount (line 119):
|
||||
reminderLeadMinutes: z.number().int().min(0).nullable().optional(),
|
||||
// absent = no-change (edit, D-08); null = clear VALARM; 0 = same-day all-day; positive = timed lead
|
||||
```
|
||||
|
||||
The schema comment pattern (lines 112–119) documents the security rationale inline — follow the same style for the new field.
|
||||
|
||||
---
|
||||
|
||||
### `apps/pwa/src/api/client.ts` (client utility, request-response)
|
||||
|
||||
**Change:** Add `reminderLeadMinutes` to `CreateEventPayload` and to `CalendarOccurrence`.
|
||||
|
||||
**Key analog — `CreateEventPayload` interface** (client.ts lines 175–198):
|
||||
```typescript
|
||||
// EXISTING interface — add after description (line 196):
|
||||
/**
|
||||
* Per-event reminder lead in minutes.
|
||||
* absent/undefined = no-change (edit omits it so server preserves existing VALARM, D-08)
|
||||
* null = explicit "None" (clear any VALARM)
|
||||
* 0 = same-day all-day (fire 9 AM on event date, D-05)
|
||||
* positive integer = N minutes before event start
|
||||
*/
|
||||
reminderLeadMinutes?: number | null;
|
||||
```
|
||||
|
||||
**`CalendarOccurrence` extension** (client.ts lines 108–134):
|
||||
Add after `hasRrule` (line 133), mirroring the server-side expand.ts change:
|
||||
```typescript
|
||||
reminderLeadMinutes: number | null; // mirrors expand.ts CalendarOccurrence (atomic mirror, Plan 06-05)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### `apps/pwa/src/components/EventForm.tsx` (component, request-response)
|
||||
|
||||
**Change:** Add reminder `<select>` after the Recurrence picker block (line 885), with allDay-aware option swap and edit-mode pre-population via `classifyValarms`.
|
||||
|
||||
**Key analog — Recurrence picker block** (EventForm.tsx lines 848–885):
|
||||
This is the exact structural template for the reminder picker. Copy the entire block structure.
|
||||
|
||||
```tsx
|
||||
// EXISTING Recurrence picker (lines 848–885) — reminder picker follows IDENTICAL structure:
|
||||
<div style={fieldStyle}>
|
||||
<label htmlFor="event-recurrence" style={labelStyle}>
|
||||
Repeat
|
||||
</label>
|
||||
<select
|
||||
id="event-recurrence"
|
||||
value={recurrence}
|
||||
disabled={eventFormMode === 'edit'}
|
||||
onChange={(e) => setRecurrence(e.target.value as RecurrencePreset)}
|
||||
style={{
|
||||
...inputStyle,
|
||||
padding: '0 var(--space-3)',
|
||||
cursor: eventFormMode === 'edit' ? 'not-allowed' : 'pointer',
|
||||
opacity: eventFormMode === 'edit' ? 0.6 : 1,
|
||||
}}
|
||||
>
|
||||
<option value="none">None</option>
|
||||
...
|
||||
</select>
|
||||
{eventFormMode === 'edit' && (
|
||||
<div style={{ fontSize: 'var(--text-label-size)', color: 'var(--color-text-secondary)', marginTop: 'var(--space-1)' }}>
|
||||
...helper text...
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
```
|
||||
|
||||
**Reminder picker differences from Recurrence picker:**
|
||||
1. `id="event-reminder"`, label text `"Reminder"` (UI-SPEC).
|
||||
2. NOT disabled on edit — reminder IS editable.
|
||||
3. Options are allDay-conditional: `{allDay ? <allDayOptions/> : <timedOptions/>}`.
|
||||
4. Off-list / Custom (kept) synthetic options appended dynamically.
|
||||
5. `onChange` resets synthetic option once a preset is chosen.
|
||||
6. Helper text shown when `alarmClass.kind === 'custom'` regardless of mode.
|
||||
|
||||
**allDay-conditional option swap pattern** (mirrors existing `{!allDay && (...)}` conditionals already in EventForm.tsx for time fields):
|
||||
```tsx
|
||||
// Use the same allDay state variable already present in EventForm.tsx:
|
||||
{allDay ? (
|
||||
// D-02 all-day presets
|
||||
<>
|
||||
<option value="__none__">None</option>
|
||||
<option value="0">Same day (9 AM)</option>
|
||||
<option value="1440">1 day before (9 AM)</option>
|
||||
<option value="2880">2 days before (9 AM)</option>
|
||||
<option value="10080">1 week before (9 AM)</option>
|
||||
</>
|
||||
) : (
|
||||
// D-01 timed presets
|
||||
<>
|
||||
<option value="__none__">None</option>
|
||||
<option value="5">5 minutes before</option>
|
||||
<option value="10">10 minutes before</option>
|
||||
<option value="15">15 minutes before</option>
|
||||
<option value="30">30 minutes before</option>
|
||||
<option value="60">1 hour before</option>
|
||||
<option value="120">2 hours before</option>
|
||||
<option value="1440">1 day before</option>
|
||||
<option value="2880">2 days before</option>
|
||||
</>
|
||||
)}
|
||||
```
|
||||
|
||||
**allDay toggle reset pattern** (D-03 — reset picker to None when allDay changes):
|
||||
```tsx
|
||||
// In the existing allDay onChange handler, also reset reminderLeadMinutes state:
|
||||
setReminderValue('__none__'); // reset to None on allDay toggle
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Shared Patterns
|
||||
|
||||
### ICAL.Property + resetType + setValue (Pitfall 2 prevention)
|
||||
**Source:** vevent.ts lines 149–153 (RRULE) — same technique for TRIGGER
|
||||
**Apply to:** `vevent.ts` VALARM emission, `vevent.ts` `buildRelativeTrigger`/`buildAbsoluteTrigger`
|
||||
```typescript
|
||||
// CORRECT — avoids VALUE=TEXT:
|
||||
const prop = new ICAL.Property('trigger');
|
||||
prop.resetType('duration');
|
||||
prop.setValue(ICAL.Duration.fromSeconds(-leadMinutes * 60));
|
||||
// WRONG — may emit VALUE=TEXT:
|
||||
// vevent.addPropertyWithValue('trigger', '-PT15M');
|
||||
```
|
||||
|
||||
### `hasExplicitX` sentinel (absent-vs-null distinction, D-06/D-08)
|
||||
**Source:** outboxWorker.ts line 425
|
||||
**Apply to:** outboxWorker.ts `hasExplicitReminder` in both update and create branches
|
||||
```typescript
|
||||
const hasExplicitRecurrence = Object.prototype.hasOwnProperty.call(fields, 'recurrence');
|
||||
// Copy: const hasExplicitReminder = Object.prototype.hasOwnProperty.call(fields, 'reminderLeadMinutes');
|
||||
```
|
||||
|
||||
### ICAL.parse try/catch + getFirstSubcomponent (sync.ts / vevent.ts)
|
||||
**Source:** vevent.ts lines 64–79 (`extractRruleString`); sync.ts lines 89–101
|
||||
**Apply to:** `extractValarms()` and `classifyValarms()` in vevent.ts
|
||||
```typescript
|
||||
try {
|
||||
parsed = ICAL.parse(rawVevent);
|
||||
} catch {
|
||||
return /* safe default */;
|
||||
}
|
||||
const comp = new ICAL.Component(parsed);
|
||||
const vevent = comp.getFirstSubcomponent('vevent');
|
||||
if (!vevent) return /* safe default */;
|
||||
```
|
||||
|
||||
### setInterval scheduling (no node-cron)
|
||||
**Source:** reminderScheduler.ts lines 209–215; outboxWorker.ts lines 897–901
|
||||
**Apply to:** `startReminderScheduler` (no change needed — already setInterval)
|
||||
```typescript
|
||||
// node-cron 4.2.1 silently skips ticks in the long-lived process — do not reintroduce
|
||||
setInterval(() => {
|
||||
runReminderCheck().catch((err: unknown) => { ... });
|
||||
}, 60 * 1000);
|
||||
```
|
||||
|
||||
### Zod `.nullable().optional()` for NULL-vs-absent distinction
|
||||
**Source:** RESEARCH.md Q2 (no existing analog — first use of this pattern in the codebase)
|
||||
**Apply to:** `eventFieldsSchema` (events.ts), `outboxPayloadSchema` (outboxWorker.ts), `CreateEventPayload` (client.ts)
|
||||
```typescript
|
||||
reminderLeadMinutes: z.number().int().min(0).nullable().optional(),
|
||||
// absent (undefined) = no-change; null = clear; 0 = same-day; positive = lead
|
||||
```
|
||||
|
||||
### Per-item try/catch error isolation
|
||||
**Source:** reminderScheduler.ts lines 181–187 (per-event), lines 167–175 (per-sub)
|
||||
**Apply to:** Preserve in modified `runReminderCheck` — both per-event and per-sub catches remain
|
||||
```typescript
|
||||
// Per-event:
|
||||
} catch (err) {
|
||||
console.error(`[broker/reminderScheduler] Error processing event uid=${uid}:`, ...);
|
||||
}
|
||||
// Per-sub:
|
||||
} catch (err) {
|
||||
console.error(`[broker/reminderScheduler] Error dispatching reminder to sub id=${sub.id}:`, ...);
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## No Analog Found
|
||||
|
||||
No files in this phase lack an analog. All changes are targeted modifications of existing files. The `classifyValarms`, `extractValarms`, `computeAlertInstantUtc`, and `humanizeLeadMinutes` functions are new but live inside existing files; their patterns are documented in RESEARCH.md Code Examples and the RRULE-analog sections above.
|
||||
|
||||
---
|
||||
|
||||
## Metadata
|
||||
|
||||
**Analog search scope:** `apps/api/src/broker/`, `apps/api/src/routes/`, `apps/api/src/db/`, `apps/pwa/src/components/`, `apps/pwa/src/api/`, `apps/api/tests/broker/`
|
||||
**Files read:** vevent.ts, outboxWorker.ts, reminderScheduler.ts, sync.ts (lines 1–200), expand.ts (lines 1–80), events.ts (lines 1–130), client.ts (lines 100–215), EventForm.tsx (lines 840–889), vevent.test.ts (lines 1–60), reminderScheduler.test.ts (lines 1–80)
|
||||
**Pattern extraction date:** 2026-06-13
|
||||
@@ -0,0 +1,769 @@
|
||||
# Phase 11: Per-Event Reminders - Research
|
||||
|
||||
**Researched:** 2026-06-13
|
||||
**Domain:** CalDAV VALARM serialization, zod schema NULL-vs-0, scheduler variable-lead, ical.js VALARM classification
|
||||
**Confidence:** HIGH — all findings derived from direct source inspection of the shipped codebase + ical.js official docs
|
||||
|
||||
---
|
||||
|
||||
<user_constraints>
|
||||
## User Constraints (from CONTEXT.md)
|
||||
|
||||
### Locked Decisions
|
||||
|
||||
- **D-01:** Timed events keep preset list: None / 5m / 10m / 15m / 30m / 1h / 2h / 1d / 2d, default None.
|
||||
- **D-02:** All-day events get context-swapped day-granularity presets: None / Same day / 1 day before / 2 days before / 1 week before, default None.
|
||||
- **D-03:** Picker shown for both event types — swaps option set based on All-day toggle, does NOT disappear.
|
||||
- **D-04:** All-day reminders fire at 9 AM local on the computed alert day.
|
||||
- **D-05:** All-day leads stored as minutes: Same day = 0, 1d = 1440, 2d = 2880, 1wk = 10080.
|
||||
- **D-06:** NULL = no reminder (no VALARM, no push). Same-day all-day = 0 (fire 9 AM on event date). 0 on a timed event = None.
|
||||
- **D-07:** Show exact value if single simple relative alarm not in preset list (off-list synthetic option). Absolute-time or multiple alarms → read-only "Custom (kept)" entry.
|
||||
- **D-08:** Original VALARM(s) preserved verbatim on save unless user explicitly selects a preset or None. Mirrors WR-01 RRULE-preserve pattern.
|
||||
- **D-09:** Humanized relative push body: "Starts in 2 days" / "Starts in 1 hour" / "Starts in 30 min". Replaces hardcoded `Starts in ${minutes} min` in reminderScheduler.ts:157.
|
||||
- **D-10:** Series-level only. One VALARM on master event; no per-occurrence (RECURRENCE-ID) override.
|
||||
- **ROADMAP Pitfall 3 AMENDED:** The `buildVeventString` allDay guard (`if (!allDay && reminderMinutes > 0)`) is changed to ALSO emit a VALARM for all-day events with a day-based lead.
|
||||
- **CAL-13 extended:** Preset list extended for all-day case per D-02; timed presets unchanged.
|
||||
- **NOTIF-06 retained:** 9 AM all-day fire governs all-day day-leads, not a hidden/disabled selector.
|
||||
|
||||
### Claude's Discretion
|
||||
|
||||
- Picker placement within EventForm.tsx (reuse existing labeled `<select>` pattern used for Recurrence).
|
||||
- Exact humanized-unit thresholds/wording for D-09.
|
||||
|
||||
### Deferred Ideas (OUT OF SCOPE)
|
||||
|
||||
- Per-occurrence reminder override (RECURRENCE-ID).
|
||||
- Reminder snooze / notification-preferences UI.
|
||||
|
||||
</user_constraints>
|
||||
|
||||
<phase_requirements>
|
||||
## Phase Requirements
|
||||
|
||||
| ID | Description | Research Support |
|
||||
|----|-------------|------------------|
|
||||
| CAL-13 | User can choose a reminder lead time when creating or editing an event from a preset list (None/5m/10m/15m/30m/1h/2h/1d/2d), default None; serialized as VALARM on event written back to Fastmail. Extended by CONTEXT.md D-02: all-day gets day-granularity presets. | VALARM DURATION construction via ICAL.Duration.fromSeconds; outboxPayloadSchema extension for reminderLeadMinutes; all-day guard change in buildVeventString |
|
||||
| CAL-14 | Editing an event preserves any existing reminder/VALARM set in another client — reminders never silently stripped on round-trip. | VALARM extraction from rawVevent via ical.js; off-list classifier; preserve-on-no-change sentinel in outboxPayloadSchema |
|
||||
| NOTIF-04 | Event reminder push fires at event's chosen lead time, not hardcoded 15-min lead. | Scheduler variable-window query using reminderLeadMinutes from DB |
|
||||
| NOTIF-05 | Event with no reminder set produces no reminder push (no default 15-min fire). | NULL guard in scheduler query; drop isShared-only restriction; per-event reminderLeadMinutes IS NULL excludes from scan |
|
||||
| NOTIF-06 | All-day event reminder fires at 9 AM local on alert day; exactly-once across catch-up scans and rescheduled events. | 9 AM local computation; uid:dtstartMs dedup key |
|
||||
|
||||
</phase_requirements>
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
This phase is a targeted delta on an already-working push reminder system. The dispatch pipeline (`dispatchPush`), the `reminder_lead_minutes` DB column, and the scheduler infrastructure are all live from Phase 10. The work is: (1) add a reminder `<select>` to EventForm.tsx, (2) plumb `reminderLeadMinutes` through the outbox schema into `buildVeventString` as a VALARM, (3) generalize the scheduler to per-event variable lead + all-day 9 AM rule + uid:dtstartMs dedup, and (4) write the VALARM-preserve-on-edit path mirroring WR-01. Three open technical questions from CONTEXT.md are answered concretely below.
|
||||
|
||||
**Primary recommendation:** Treat this as five discrete, independently testable units — VALARM builder, VALARM classifier, schema NULL-vs-0 plumbing, scheduler lead/9AM logic, and humanized-body formatter — each with TDD I/O examples, then wire them together in the form and worker.
|
||||
|
||||
---
|
||||
|
||||
## Architectural Responsibility Map
|
||||
|
||||
| Capability | Primary Tier | Secondary Tier | Rationale |
|
||||
|------------|-------------|----------------|-----------|
|
||||
| Reminder picker UI (preset select, allDay swap) | Frontend / React PWA | — | Pure form state; no server round-trip until save |
|
||||
| Off-list alarm classifier (detect single/relative vs absolute/multi) | API / Backend (parse rawVevent) | — | rawVevent lives server-side; classification needed during GET response to populate picker |
|
||||
| VALARM serialization (buildVeventString) | API / Backend (broker) | — | ICS construction happens in outbox worker |
|
||||
| VALARM preserve-on-edit | API / Backend (outboxWorker update path) | — | Reads rawVevent from DB at drain time |
|
||||
| reminderLeadMinutes plumbing (form → outbox → DB) | API / Backend (route + schema) | Frontend (payload shape) | Schema is already in DB; route schema needs the new field |
|
||||
| Scheduler variable-lead window | API / Backend (reminderScheduler) | — | setInterval-only; SQL WHERE uses reminderLeadMinutes |
|
||||
| All-day 9 AM local fire | API / Backend (reminderScheduler) | — | Server computes alert time from dtstartDate + local timezone |
|
||||
| Humanized push body | API / Backend (reminderScheduler) | — | Replaces hardcoded string at line 157 |
|
||||
|
||||
---
|
||||
|
||||
## Open Question Answers (Concrete, Code-Grounded)
|
||||
|
||||
### Q1: VALARM TRIGGER encoding for all-day "N days before at 9 AM" — cross-client interop
|
||||
|
||||
**Finding:** The scheduler is ground truth for the actual push fire time (9 AM local). The VALARM in the ICS is best-effort cross-client interop. Use a **relative DURATION trigger** for all-day events with an offset that approximates 9 AM.
|
||||
|
||||
**RFC 5545 background:** `TRIGGER` on a VALARM has two legal value types:
|
||||
- `DURATION` (default, no VALUE param): evaluated relative to DTSTART (which for all-day events is a DATE with no time component — effectively midnight).
|
||||
- `DATE-TIME` (with `VALUE=DATE-TIME`): an absolute UTC instant.
|
||||
|
||||
For an all-day event, `TRIGGER:-PT15M` means "15 minutes before midnight of the event date." Apple Calendar interprets this in local time. Fastmail's web client may ignore VALARM on all-day events entirely in some configurations. [CITED: RFC 5545 §3.8.6.3]
|
||||
|
||||
**Best-effort recommendation — use `-PT15H` (or the matching offset for the chosen day-lead):**
|
||||
|
||||
For "Same day" (lead=0): the alert date is the event date itself. To fire at 9 AM, the TRIGGER offset from midnight-of-DTSTART is `+9h`. RFC 5545 requires TRIGGER on VALARM to be negative or zero (alarm fires AT or BEFORE the event). A positive offset is technically non-conforming. **Practical approach:** Use `-PT15H` on the DAY BEFORE, i.e., treat "Same day at 9 AM" as 9 AM on the event date = DTSTART + 9h. Since this is a positive offset, and RFC compliance is fraught, use a DURATION from the previous midnight:
|
||||
|
||||
```
|
||||
Same day (0 min lead): TRIGGER:-P0DT15H → this is non-standard positive (9h after midnight)
|
||||
Better encoding: Treat as "fire 9h into the event day" — use absolute DATE-TIME trigger
|
||||
```
|
||||
|
||||
**Recommended encoding: Absolute DATE-TIME TRIGGER for all-day events.** [ASSUMED — based on RFC 5545 + client behavior knowledge]
|
||||
|
||||
Build the trigger as `TRIGGER;VALUE=DATE-TIME:<YYYYMMDDTHHMMSSZ>` where the UTC instant corresponds to 9 AM local on the alert day. This is the only encoding that is unambiguous across Fastmail and Apple Calendar:
|
||||
|
||||
- Fastmail: honors absolute DATE-TIME TRIGGER on all-day events [ASSUMED — not verifiable without live Fastmail testing]
|
||||
- Apple Calendar: honors absolute DATE-TIME TRIGGER [ASSUMED — per developer reports]
|
||||
- The SCHEDULER is ground truth regardless — ICS interop is best-effort
|
||||
|
||||
**Implementation in ical.js — absolute DATE-TIME trigger:**
|
||||
|
||||
```typescript
|
||||
// Source: ical.js jCal structure (Context7/kewisch/ical.js wiki)
|
||||
// For all-day events: compute alertInstantUtc = 9 AM local on (eventDate - leadDays)
|
||||
function buildAbsoluteTrigger(alertInstantUtc: Date): ICAL.Property {
|
||||
const triggerProp = new ICAL.Property('trigger');
|
||||
// VALUE=DATE-TIME: ical.js uses 'date-time' value type
|
||||
triggerProp.resetType('date-time');
|
||||
triggerProp.setValue(ICAL.Time.fromJSDate(alertInstantUtc, true)); // useUTC=true → Z suffix
|
||||
return triggerProp;
|
||||
}
|
||||
```
|
||||
|
||||
**Implementation in ical.js — relative DURATION trigger (timed events):**
|
||||
|
||||
```typescript
|
||||
// Source: ical.js jCal structure example — ["trigger", {"related":"START"}, "duration", "-PT5M"]
|
||||
// Do NOT use addPropertyWithValue('trigger', '-PT15M') — may emit VALUE=TEXT (Pitfall 2)
|
||||
function buildRelativeTrigger(leadMinutes: number): ICAL.Property {
|
||||
const triggerProp = new ICAL.Property('trigger');
|
||||
// 'duration' value type — correct per RFC 5545 §3.8.6.3
|
||||
triggerProp.resetType('duration');
|
||||
const dur = ICAL.Duration.fromSeconds(-leadMinutes * 60);
|
||||
triggerProp.setValue(dur);
|
||||
return triggerProp;
|
||||
}
|
||||
```
|
||||
|
||||
The ical.js jCal structure confirms the correct encoding: `["trigger", {"related": "START"}, "duration", "-PT5M"]` — the value type is `"duration"`, not `"text"`. Using `resetType('duration')` before `setValue(ICAL.Duration)` guarantees this. [CITED: https://github.com/kewisch/ical.js/wiki/Migrating-from-Other-Libraries]
|
||||
|
||||
**Summary for planner:**
|
||||
- Timed events: relative DURATION trigger (`-PTNmM`) via `resetType('duration')` + `setValue(ICAL.Duration.fromSeconds(-N*60))`.
|
||||
- All-day events: absolute DATE-TIME trigger (`VALUE=DATE-TIME:YYYYMMDDTHHMMSSZ`) via `resetType('date-time')` + `setValue(ICAL.Time.fromJSDate(alertInstantUtc, true))`. The alert instant = 9 AM local on (eventDate minus leadDays), converted to UTC.
|
||||
- Scheduler computes the same alert instant independently — ICS interop is cosmetic.
|
||||
|
||||
**TDD note:** This builder function has defined I/O and must be unit-tested:
|
||||
- Input: `{allDay: false, leadMinutes: 15}` → output contains `TRIGGER:-PT15M` (no `VALUE=TEXT`)
|
||||
- Input: `{allDay: true, leadMinutes: 1440, eventDate: '2026-06-15', timezone: 'America/New_York'}` → output TRIGGER is DATE-TIME `20260614T130000Z` (9 AM EDT = 13:00 UTC, 1 day before June 15)
|
||||
- Input: `{allDay: true, leadMinutes: 0, eventDate: '2026-06-15', timezone: 'America/New_York'}` → TRIGGER is `20260615T130000Z` (9 AM EDT on the event date itself)
|
||||
|
||||
---
|
||||
|
||||
### Q2: NULL-vs-0 distinction end-to-end through the schema pipeline (D-06)
|
||||
|
||||
**Current state of the schemas (from direct file inspection):**
|
||||
|
||||
`apps/api/src/routes/events.ts` — `eventFieldsSchema` (line 101):
|
||||
```typescript
|
||||
// Current — no reminderLeadMinutes field at all
|
||||
const eventFieldsSchema = z.object({
|
||||
title: z.string().min(1).max(255),
|
||||
allDay: z.boolean(),
|
||||
start: z.string().min(1).max(64),
|
||||
end: z.string().min(1).max(64),
|
||||
location: z.string().max(2000).optional(),
|
||||
description: z.string().max(2000).optional(),
|
||||
recurrence: z.enum([...]).optional(),
|
||||
// ...
|
||||
})
|
||||
```
|
||||
|
||||
`apps/api/src/broker/outboxWorker.ts` — `outboxPayloadSchema` (line 73):
|
||||
```typescript
|
||||
// Current — no reminderLeadMinutes field at all
|
||||
const outboxPayloadSchema = z.object({
|
||||
title: z.string().min(1).max(255),
|
||||
allDay: z.boolean(),
|
||||
// ...
|
||||
}).passthrough();
|
||||
```
|
||||
|
||||
`apps/api/src/db/schema.ts` — column already exists (line 144):
|
||||
```typescript
|
||||
reminderLeadMinutes: int('reminder_lead_minutes'), // nullable → NULL means no reminder
|
||||
```
|
||||
|
||||
**Required changes to carry NULL-vs-0 end-to-end:**
|
||||
|
||||
**Step 1 — `eventFieldsSchema` in routes/events.ts:**
|
||||
|
||||
```typescript
|
||||
// Add to eventFieldsSchema:
|
||||
reminderLeadMinutes: z.number().int().min(0).nullable().optional(),
|
||||
```
|
||||
|
||||
Semantics:
|
||||
- Field absent from JSON: `undefined` — means "no change" (edit mode, preserve existing VALARM)
|
||||
- Field present as `null`: no reminder (clear any VALARM)
|
||||
- Field present as `0`: same-day all-day (fire 9 AM on event date)
|
||||
- Field present as positive integer: N-minute lead
|
||||
|
||||
**Zod behavior through JSON serialization:**
|
||||
- `z.nullable()`: accepts `null` in JSON — value is `null` in parsed output
|
||||
- `z.optional()`: accepts the field being absent — value is `undefined` in parsed output
|
||||
- `null` and `undefined` are distinct in Zod: `.nullable()` does NOT make the field optional; `.optional()` does NOT make the value nullable. Both modifiers are required.
|
||||
- JSON.parse/stringify: `null` survives round-trip as `null`; `undefined` object properties are dropped by JSON.stringify (field absent on the wire). This is the correct behavior — an absent field on the edit payload means "don't touch the reminder."
|
||||
|
||||
**Step 2 — `outboxPayloadSchema` in outboxWorker.ts:**
|
||||
|
||||
Add the same field, plus a sentinel for the preserve-on-no-change case:
|
||||
|
||||
```typescript
|
||||
// Add to outboxPayloadSchema:
|
||||
reminderLeadMinutes: z.number().int().min(0).nullable().optional(),
|
||||
// Sentinel: absent = "user did not touch the reminder picker" → preserve existing VALARM
|
||||
// null = explicit "None" (clear VALARM)
|
||||
// 0 = same-day all-day
|
||||
// positive integer = N-minute lead
|
||||
```
|
||||
|
||||
The `.passthrough()` on `outboxPayloadSchema` means unrecognized fields survive, but explicitly declared fields are type-narrowed. The field must be declared so the planner gets TypeScript type safety in the worker.
|
||||
|
||||
**Step 3 — The "no change" sentinel distinction:**
|
||||
|
||||
D-08 requires distinguishing "user did not touch the picker" from "user explicitly selected None." The zod schema handles this correctly: the field is `.optional()` so its absence on the JSON payload is distinguishable from `null`. In the outbox worker update path:
|
||||
|
||||
```typescript
|
||||
const hasExplicitReminder = Object.prototype.hasOwnProperty.call(fields, 'reminderLeadMinutes');
|
||||
// hasExplicitReminder=false → preserve rawVevent VALARMs (mirrors WR-01 hasExplicitRecurrence)
|
||||
// hasExplicitReminder=true, fields.reminderLeadMinutes === null → clear all VALARMs
|
||||
// hasExplicitReminder=true, fields.reminderLeadMinutes >= 0 → replace with new VALARM
|
||||
```
|
||||
|
||||
This mirrors the `hasExplicitRecurrence` pattern already in the update branch (outboxWorker.ts line 425).
|
||||
|
||||
**Step 4 — `buildVeventString` parameter extension:**
|
||||
|
||||
Add `valarms?: ICAL.Component[]` parameter to `NewEventParams` in vevent.ts:
|
||||
|
||||
```typescript
|
||||
export interface NewEventParams {
|
||||
// ...existing fields...
|
||||
reminderLeadMinutes?: number | null; // for NEW VALARM construction (create / user-changed)
|
||||
valarms?: ICAL.Component[]; // pre-parsed VALARMs from rawVevent (preserve-on-edit)
|
||||
allDayAlertInstantUtc?: Date; // computed by worker for all-day absolute triggers
|
||||
}
|
||||
```
|
||||
|
||||
The worker resolves which path to take before calling buildVeventString:
|
||||
- `hasExplicitReminder && reminderLeadMinutes !== null` → compute alert instant if allDay, pass as `allDayAlertInstantUtc`; pass `reminderLeadMinutes` directly for timed
|
||||
- `hasExplicitReminder && reminderLeadMinutes === null` → pass neither (no VALARM emitted)
|
||||
- `!hasExplicitReminder` → extract VALARMs from rawVevent, pass as `valarms`
|
||||
|
||||
**Step 5 — DB upsert in sync.ts:**
|
||||
|
||||
The `sync.ts` upsert (line 144) does NOT currently write `reminderLeadMinutes`. Phase 11 must add it: parse the VALARM from the incoming VCALENDAR (if any) to extract the lead minutes, and write it to the column so the scheduler can query it. This is the sync-write path that populates the scheduler's data source.
|
||||
|
||||
**TDD note:** The NULL-vs-0 transform is a pure function — given a zod-parsed `reminderLeadMinutes` value and `allDay` flag, return a scheduler-ready lead. I/O examples:
|
||||
- `(null, false)` → no push row
|
||||
- `(0, true)` → schedule at 9 AM on event date (leadMinutes=0 + 9AM rule)
|
||||
- `(0, false)` → no push (0 on a timed event = None, same as null)
|
||||
- `(1440, true)` → schedule at 9 AM on the day before
|
||||
|
||||
---
|
||||
|
||||
### Q3: Off-list / non-preset VALARM classification and preserve-on-edit (D-07/D-08, CAL-14)
|
||||
|
||||
**How to classify VALARMs from rawVevent using ical.js:**
|
||||
|
||||
```typescript
|
||||
// Source: ical.js API — getFirstSubcomponent, getAllSubcomponents
|
||||
// [CITED: https://github.com/kewisch/ical.js/wiki/Parsing-iCalendar]
|
||||
export type AlarmClassification =
|
||||
| { kind: 'none' } // no VALARM sub-components
|
||||
| { kind: 'preset'; leadMinutes: number } // single relative DURATION, on the preset list
|
||||
| { kind: 'offlist'; leadMinutes: number } // single relative DURATION, NOT on preset list
|
||||
| { kind: 'custom' }; // absolute DATE-TIME trigger OR multiple VALARMs
|
||||
|
||||
export function classifyValarms(rawVevent: string): AlarmClassification {
|
||||
let parsed: ReturnType<typeof ICAL.parse>;
|
||||
try {
|
||||
parsed = ICAL.parse(rawVevent);
|
||||
} catch {
|
||||
return { kind: 'none' };
|
||||
}
|
||||
const comp = new ICAL.Component(parsed);
|
||||
const vevent = comp.getFirstSubcomponent('vevent');
|
||||
if (!vevent) return { kind: 'none' };
|
||||
|
||||
const valarms = vevent.getAllSubcomponents('valarm');
|
||||
if (valarms.length === 0) return { kind: 'none' };
|
||||
if (valarms.length > 1) return { kind: 'custom' }; // multiple alarms → Custom (kept)
|
||||
|
||||
const alarm = valarms[0];
|
||||
const triggerProp = alarm.getFirstProperty('trigger');
|
||||
if (!triggerProp) return { kind: 'none' };
|
||||
|
||||
// Check value type — 'duration' = relative DURATION; 'date-time' = absolute
|
||||
const valueType = triggerProp.getParameter('value');
|
||||
// ical.js jCal encodes relative DURATION as value type 'duration';
|
||||
// absolute as 'date-time'. When VALUE param is absent, the default for TRIGGER is duration.
|
||||
// [CITED: kewisch/ical.js/wiki/Migrating-from-Other-Libraries — jCal structure]
|
||||
if (valueType === 'date-time') return { kind: 'custom' }; // absolute → Custom (kept)
|
||||
|
||||
const dur = triggerProp.getFirstValue() as ICAL.Duration | null;
|
||||
if (!dur || typeof dur.toSeconds !== 'function') return { kind: 'custom' };
|
||||
|
||||
const totalSeconds = Math.abs(dur.toSeconds()); // negative for "before", take absolute
|
||||
const leadMinutes = Math.round(totalSeconds / 60);
|
||||
|
||||
// Preset lists (timed and all-day combined)
|
||||
const PRESET_MINUTES = new Set([5, 10, 15, 30, 60, 120, 1440, 2880, 0, 10080]);
|
||||
if (PRESET_MINUTES.has(leadMinutes)) {
|
||||
return { kind: 'preset', leadMinutes };
|
||||
}
|
||||
return { kind: 'offlist', leadMinutes };
|
||||
}
|
||||
```
|
||||
|
||||
**Note on `triggerProp.getParameter('value')`:** In ical.js, the jCal representation stores the value type as the third element of the property array. The `getParameter('value')` call returns the VALUE parameter if explicitly set (e.g., `VALUE=DATE-TIME`). For DURATION triggers, the value type is the default and `getParameter('value')` returns `undefined` (not `'duration'`). The check should be:
|
||||
|
||||
```typescript
|
||||
// value type check — ical.js getFirstValue() returns ICAL.Duration for duration-typed properties
|
||||
// and ICAL.Time for date-time-typed properties
|
||||
const firstValue = triggerProp.getFirstValue();
|
||||
if (firstValue instanceof ICAL.Time) return { kind: 'custom' }; // absolute DATE-TIME
|
||||
// Otherwise ICAL.Duration — relative DURATION trigger
|
||||
const dur = firstValue as ICAL.Duration;
|
||||
```
|
||||
|
||||
**VALARM extraction for preserve-on-edit (mirrors WR-01 RRULE-preserve):**
|
||||
|
||||
```typescript
|
||||
// Extract all VALARM sub-components from rawVevent for re-attachment
|
||||
export function extractValarms(rawVevent: string): ICAL.Component[] {
|
||||
try {
|
||||
const parsed = ICAL.parse(rawVevent);
|
||||
const comp = new ICAL.Component(parsed);
|
||||
const vevent = comp.getFirstSubcomponent('vevent');
|
||||
if (!vevent) return [];
|
||||
return vevent.getAllSubcomponents('valarm');
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
These extracted `ICAL.Component` objects are passed to `buildVeventString` via the `valarms` parameter and re-attached to the new VEVENT using `vevent.addSubcomponent(alarm)`. They are NOT re-serialized and re-parsed — they are passed as live ical.js component objects to avoid any encoding issues.
|
||||
|
||||
**The preserve-on-edit trigger in outboxWorker update path:**
|
||||
|
||||
```typescript
|
||||
// In dispatchRow, update branch — after the hasExplicitReminder check:
|
||||
let valarmsToPreserve: ICAL.Component[] = [];
|
||||
if (!hasExplicitReminder && freshEtagRows[0]?.rawVevent) {
|
||||
valarmsToPreserve = extractValarms(freshEtagRows[0].rawVevent);
|
||||
}
|
||||
// Pass to buildVeventString:
|
||||
const { icsString } = buildVeventString({
|
||||
// ...existing params...
|
||||
valarms: valarmsToPreserve, // empty array = no VALARM; populated = preserve
|
||||
});
|
||||
```
|
||||
|
||||
**TDD note:** The classifier is pure I/O — ideal for TDD:
|
||||
- Input: ICS with `TRIGGER;VALUE=DATE-TIME:20260615T130000Z` → `{ kind: 'custom' }`
|
||||
- Input: ICS with `BEGIN:VALARM\nTRIGGER:-PT45M\nEND:VALARM` → `{ kind: 'offlist', leadMinutes: 45 }`
|
||||
- Input: ICS with `BEGIN:VALARM\nTRIGGER:-PT15M\nEND:VALARM` → `{ kind: 'preset', leadMinutes: 15 }`
|
||||
- Input: ICS with two VALARM blocks → `{ kind: 'custom' }`
|
||||
- Input: ICS with no VALARM → `{ kind: 'none' }`
|
||||
|
||||
---
|
||||
|
||||
## Standard Stack
|
||||
|
||||
No new packages. All tools are already in the project.
|
||||
|
||||
| Library | Version | Purpose | Notes |
|
||||
|---------|---------|---------|-------|
|
||||
| ical.js | 2.2.1 | VALARM component build + parse | Use `ICAL.Component`, `ICAL.Property`, `ICAL.Duration`, `ICAL.Time` |
|
||||
| zod | 3.24.x | Schema extension for reminderLeadMinutes | `.nullable().optional()` pattern |
|
||||
| drizzle-orm | 0.45.2 | DB query for variable-lead scheduler scan | Existing mysql2 driver |
|
||||
|
||||
**No new npm dependencies for this phase.** The UI-SPEC confirms: "Phase 11 adds no new npm dependencies on the frontend."
|
||||
|
||||
---
|
||||
|
||||
## Package Legitimacy Audit
|
||||
|
||||
No new packages — not applicable.
|
||||
|
||||
---
|
||||
|
||||
## Architecture Patterns
|
||||
|
||||
### System Architecture Diagram
|
||||
|
||||
```
|
||||
EventForm.tsx
|
||||
[reminder <select>]
|
||||
allDay=false → timed presets
|
||||
allDay=true → day presets (swapped)
|
||||
edit mode: classify rawVevent VALARM →
|
||||
none → select "None"
|
||||
preset → select matching option
|
||||
offlist → add synthetic option
|
||||
custom → disabled "Custom (kept)"
|
||||
|
|
||||
| reminderLeadMinutes: number | null | undefined (absent=no-change)
|
||||
v
|
||||
POST /api/events/create OR PATCH /api/events/:uid/edit
|
||||
eventFieldsSchema (zod) validates:
|
||||
reminderLeadMinutes: z.number().int().min(0).nullable().optional()
|
||||
|
|
||||
v
|
||||
calendar_outbox.payload (JSON)
|
||||
{ ...eventFields, reminderLeadMinutes: N | null | undefined }
|
||||
|
|
||||
v
|
||||
outboxWorker.ts (update branch)
|
||||
hasExplicitReminder = Object.prototype.hasOwnProperty.call(fields, 'reminderLeadMinutes')
|
||||
if !hasExplicitReminder → extractValarms(rawVevent) → valarmsToPreserve
|
||||
if hasExplicitReminder && null → no valarms
|
||||
if hasExplicitReminder && N → buildAbsoluteTrigger (allDay) or buildRelativeTrigger (timed)
|
||||
|
|
||||
v
|
||||
buildVeventString(params: NewEventParams)
|
||||
valarms: ICAL.Component[] → vevent.addSubcomponent(alarm) for each
|
||||
reminderLeadMinutes + allDay → emit VALARM sub-component
|
||||
|
|
||||
v
|
||||
Fastmail CalDAV PUT (VCALENDAR with VALARM)
|
||||
|
|
||||
v (post-write re-sync)
|
||||
sync.ts upsert
|
||||
parse VALARM from rawVevent → write reminderLeadMinutes to calendar_events
|
||||
|
|
||||
v
|
||||
reminderScheduler.ts (every 60s via setInterval)
|
||||
SELECT events WHERE reminder_lead_minutes IS NOT NULL
|
||||
AND reminder_lead_minutes > 0 (timed) OR (all_day=true AND reminder_lead_minutes >= 0)
|
||||
AND alert_time IN (now, now+1min]
|
||||
uid:dtstartMs dedup
|
||||
dispatch push with humanized body
|
||||
```
|
||||
|
||||
### Recommended Project Structure (unchanged — no new files beyond tests)
|
||||
|
||||
```
|
||||
apps/api/src/broker/
|
||||
vevent.ts # add buildVeventString VALARM support + classifyValarms + extractValarms
|
||||
outboxWorker.ts # update branch: hasExplicitReminder pattern; pass valarms to buildVeventString
|
||||
reminderScheduler.ts # variable-lead window, uid:dtstartMs dedup, all-day 9AM, humanized body
|
||||
sync.ts # upsert reminderLeadMinutes from parsed VALARM
|
||||
apps/api/src/routes/
|
||||
events.ts # extend eventFieldsSchema with reminderLeadMinutes
|
||||
apps/pwa/src/components/
|
||||
EventForm.tsx # add reminder <select>, classifyValarms call on edit load
|
||||
apps/pwa/src/api/
|
||||
client.ts # extend CreateEventPayload with reminderLeadMinutes
|
||||
apps/api/tests/broker/
|
||||
vevent.test.ts # VALARM serialization + classifier TDD tests
|
||||
reminderScheduler.test.ts # variable-lead, all-day 9AM, uid:dtstartMs dedup tests
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Don't Hand-Roll
|
||||
|
||||
| Problem | Don't Build | Use Instead | Why |
|
||||
|---------|-------------|-------------|-----|
|
||||
| TRIGGER value type | `addPropertyWithValue('trigger', '-PT15M')` string | `resetType('duration')` + `setValue(ICAL.Duration.fromSeconds(...))` | String form may emit `VALUE=TEXT` (Pitfall 2 — Fastmail/Apple silently ignore) |
|
||||
| VALARM re-attachment | Extract text between BEGIN:VALARM/END:VALARM and splice | `extractValarms()` → `vevent.addSubcomponent()` | Text splicing is fragile; ical.js line-folding handles re-serialization correctly |
|
||||
| All-day alert time | Ad-hoc string math on date strings | Proper UTC Date arithmetic: parse YYYY-MM-DD, subtract lead days, add 9h local offset → UTC | DST offsets vary; hand-rolled date math will produce wrong UTC instants on DST boundaries |
|
||||
| Scheduler interval | node-cron | `setInterval` only | node-cron 4.2.1 silently skips ticks in the long-lived process (burned-in lesson, CLAUDE.md memory) |
|
||||
| DB migration | `drizzle-kit push` | `drizzle-kit generate` + `migrate` | push emits false destructive diff on populated MariaDB (burned-in lesson) |
|
||||
|
||||
---
|
||||
|
||||
## Common Pitfalls
|
||||
|
||||
### Pitfall 1 (RETAINED): VALARM Round-Trip Strips Existing Alarms on Edit
|
||||
See PITFALLS.md Pitfall 1 for full detail. Prevention: `hasExplicitReminder` pattern + `extractValarms()` on update path.
|
||||
|
||||
### Pitfall 2 (RETAINED): TRIGGER `VALUE=TEXT` Silently Breaks VALARM
|
||||
See PITFALLS.md Pitfall 2. Prevention: `resetType('duration')` + `ICAL.Duration.fromSeconds()`.
|
||||
|
||||
### Pitfall 3 (AMENDED): All-Day Event VALARM
|
||||
PITFALLS.md Pitfall 3 said "hide the picker for all-day events." CONTEXT.md D-02 reverses this. The amended guard: `buildVeventString` MUST emit a VALARM for all-day events when `reminderLeadMinutes >= 0` and `allDay=true`, using an absolute DATE-TIME trigger computed at 9 AM local on the alert day. The scheduler's `WHERE allDay=false` clause MUST be removed/replaced with per-event lead-time logic that correctly handles all-day events.
|
||||
|
||||
### Pitfall 4 (RETAINED): uid-only Dedup Key Breaks with Variable Leads
|
||||
See PITFALLS.md Pitfall 4. Change dedup key to `uid:dtstartMs`. Also: the scheduler window query must change from a fixed `(now, now+16min]` to per-event lead-based logic. Two approaches:
|
||||
- Option A (simpler): Query events where `alertTime IN (now, now+1min]` — compute alertTime in SQL: for timed events, `DTSTART - INTERVAL reminder_lead_minutes MINUTE`; for all-day, the 9 AM local instant. This requires MariaDB date arithmetic.
|
||||
- Option B (current pattern): Keep the per-minute scan but check `dtstartUtc - reminder_lead_minutes MINUTES <= now + 60s AND dtstartUtc > now`.
|
||||
|
||||
**Recommendation:** Option B — widest window is `max(reminder_lead_minutes)` + 1 min buffer. Or: run a minute-tick scan that computes the expected fire time per event and fires those within a 1-minute window. Keep the fixed 60s tick from `startReminderScheduler`.
|
||||
|
||||
### Pitfall: All-Day 9 AM UTC Computation at DST Boundaries
|
||||
Computing 9 AM local when the event date straddles a DST change requires using the correct offset for that specific date, not today's offset. Use `Intl.DateTimeFormat` or a date library to get the UTC offset for a specific date in a specific timezone. If no timezone is stored (the API currently stores no per-user timezone), default to the server's local timezone (`process.env.TZ` or `Intl.DateTimeFormat().resolvedOptions().timeZone`). [ASSUMED — no per-user timezone in the DB schema confirmed from schema.ts]
|
||||
|
||||
### Pitfall: `0` Lead Minutes on a Timed Event
|
||||
D-06 specifies: "For timed events, 0/absent still means None." The scheduler must guard: if `allDay=false` AND `reminder_lead_minutes=0`, treat as NULL (no push). Only when `allDay=true` does `reminder_lead_minutes=0` mean "Same day."
|
||||
|
||||
### Pitfall: sync.ts Does Not Currently Write reminderLeadMinutes
|
||||
`sync.ts` upsert (lines 144–170) does not include `reminderLeadMinutes` in the `values()` call. The scheduler reads `calendar_events.reminder_lead_minutes` as ground truth. Without the sync-write, events synced from Fastmail (with VALARMs set by native clients) will never have `reminderLeadMinutes` populated. Phase 11 MUST add VALARM parsing in `sync.ts` to extract lead minutes and write them on upsert.
|
||||
|
||||
---
|
||||
|
||||
## Code Examples
|
||||
|
||||
### Building a VALARM sub-component (timed event, relative DURATION trigger)
|
||||
|
||||
```typescript
|
||||
// Source: ical.js jCal structure [CITED: https://github.com/kewisch/ical.js/wiki/Migrating-from-Other-Libraries]
|
||||
// jCal encodes: ["trigger", {"related": "START"}, "duration", "-PT15M"]
|
||||
// Production code in vevent.ts:
|
||||
function buildTimedValarm(leadMinutes: number): ICAL.Component {
|
||||
const valarm = new ICAL.Component('valarm');
|
||||
valarm.addPropertyWithValue('action', 'DISPLAY');
|
||||
valarm.addPropertyWithValue('description', 'Reminder');
|
||||
|
||||
const triggerProp = new ICAL.Property('trigger');
|
||||
triggerProp.resetType('duration'); // ensures value type = 'duration', not 'text'
|
||||
triggerProp.setValue(ICAL.Duration.fromSeconds(-leadMinutes * 60));
|
||||
valarm.addProperty(triggerProp);
|
||||
|
||||
return valarm;
|
||||
}
|
||||
// Then: vevent.addSubcomponent(buildTimedValarm(15));
|
||||
// Emits: BEGIN:VALARM\r\nACTION:DISPLAY\r\nDESCRIPTION:Reminder\r\nTRIGGER:-PT15M\r\nEND:VALARM
|
||||
```
|
||||
|
||||
### Building a VALARM for all-day events (absolute DATE-TIME trigger)
|
||||
|
||||
```typescript
|
||||
// alertInstantUtc = 9 AM local on (eventDate - leadDays) in UTC
|
||||
function buildAllDayValarm(alertInstantUtc: Date): ICAL.Component {
|
||||
const valarm = new ICAL.Component('valarm');
|
||||
valarm.addPropertyWithValue('action', 'DISPLAY');
|
||||
valarm.addPropertyWithValue('description', 'Reminder');
|
||||
|
||||
const triggerProp = new ICAL.Property('trigger');
|
||||
triggerProp.resetType('date-time'); // VALUE=DATE-TIME absolute trigger
|
||||
triggerProp.setValue(ICAL.Time.fromJSDate(alertInstantUtc, true)); // useUTC=true → Z suffix
|
||||
valarm.addProperty(triggerProp);
|
||||
|
||||
return valarm;
|
||||
}
|
||||
```
|
||||
|
||||
### Computing alertInstantUtc for all-day 9 AM local
|
||||
|
||||
```typescript
|
||||
// No timezone library needed — use Date + Intl offset arithmetic
|
||||
function computeAllDayAlertUtc(eventDateStr: string, leadMinutes: number, tz: string): Date {
|
||||
// eventDateStr: 'YYYY-MM-DD', leadMinutes: 0|1440|2880|10080
|
||||
const leadDays = leadMinutes / 1440; // 0, 1, 2, or 7
|
||||
|
||||
// Parse the event date as midnight UTC, then subtract lead days
|
||||
const [y, m, d] = eventDateStr.split('-').map(Number) as [number, number, number];
|
||||
const alertDate = new Date(Date.UTC(y, m - 1, d - leadDays));
|
||||
|
||||
// Get UTC offset for 9 AM on alertDate in the target timezone
|
||||
// Intl.DateTimeFormat gives us the local time components
|
||||
const alertDateStr = alertDate.toISOString().slice(0, 10); // 'YYYY-MM-DD'
|
||||
// Build a candidate 9 AM local and find its UTC equivalent
|
||||
// Strategy: try 9 AM, compute the offset, adjust
|
||||
const candidate = new Date(`${alertDateStr}T09:00:00`); // interpreted as local by Date()
|
||||
// Better: use a fixed UTC instant and shift by the known offset
|
||||
// Parse offset via Intl:
|
||||
const offsetMs = getUtcOffsetMs(alertDate, tz);
|
||||
// 9 AM local = UTC midnight + 9h - offset
|
||||
const alertMidnightUtc = Date.UTC(
|
||||
alertDate.getUTCFullYear(),
|
||||
alertDate.getUTCMonth(),
|
||||
alertDate.getUTCDate(),
|
||||
);
|
||||
return new Date(alertMidnightUtc + 9 * 3600 * 1000 - offsetMs);
|
||||
}
|
||||
|
||||
function getUtcOffsetMs(dateAtMidnightUtc: Date, tz: string): number {
|
||||
// Intl trick: format a UTC date in the target timezone, measure offset
|
||||
const parts = new Intl.DateTimeFormat('en', {
|
||||
timeZone: tz,
|
||||
hour: 'numeric', minute: 'numeric', second: 'numeric',
|
||||
hour12: false,
|
||||
timeZoneName: 'shortOffset',
|
||||
}).formatToParts(dateAtMidnightUtc);
|
||||
// Extract UTC offset from parts (or use a simpler approach)
|
||||
// Simpler: compare UTC midnight against its local midnight representation
|
||||
const localMidnightStr = new Intl.DateTimeFormat('en-CA', {
|
||||
timeZone: tz, year: 'numeric', month: '2-digit', day: '2-digit',
|
||||
}).format(dateAtMidnightUtc); // 'YYYY-MM-DD'
|
||||
// ... this gets complex; the cleanest approach for this use case:
|
||||
// offsetMs = (UTC time of "midnight local on alertDate") = alertMidnightUtc - localMidnightUtc
|
||||
// Use a reference point: new Date(alertDateStr + 'T00:00:00') gives LOCAL midnight
|
||||
// then .getTime() - alertDate.getTime() = offsetMs
|
||||
// NOTE: This only works correctly in Node.js when TZ env is set to the server's timezone.
|
||||
// Recommended: server runs with TZ=America/New_York (or whatever the household timezone is).
|
||||
// If no per-user timezone, use server TZ. Document this as a known limitation.
|
||||
return 0; // placeholder — implement via the above approach
|
||||
}
|
||||
```
|
||||
|
||||
**Planner note:** The alert UTC computation is the most DST-sensitive piece. Recommend implementing as a separate pure function `computeAlertInstantUtc(eventDateStr, leadDays, serverTimezone)` with TDD tests at DST boundaries (spring-forward, fall-back). [ASSUMED: server timezone matches household timezone — no per-user TZ in DB]
|
||||
|
||||
### Humanized push body (D-09)
|
||||
|
||||
```typescript
|
||||
// Source: 11-UI-SPEC.md Copywriting Contract
|
||||
// Replace reminderScheduler.ts line 157: body: `Starts in ${minutes} min`
|
||||
function humanizeLeadMinutes(leadMinutes: number): string {
|
||||
if (leadMinutes < 60) return `Starts in ${leadMinutes} min`;
|
||||
if (leadMinutes < 120) return 'Starts in 1 hour';
|
||||
if (leadMinutes < 1440) return `Starts in ${Math.round(leadMinutes / 60)} hours`;
|
||||
if (leadMinutes < 2880) return 'Starts in 1 day';
|
||||
return `Starts in ${Math.round(leadMinutes / 1440)} days`;
|
||||
}
|
||||
// TDD I/O:
|
||||
// humanizeLeadMinutes(30) → 'Starts in 30 min'
|
||||
// humanizeLeadMinutes(60) → 'Starts in 1 hour'
|
||||
// humanizeLeadMinutes(90) → 'Starts in 2 hours' (Math.round(1.5)=2)
|
||||
// humanizeLeadMinutes(1440) → 'Starts in 1 day'
|
||||
// humanizeLeadMinutes(10080)→ 'Starts in 7 days'
|
||||
```
|
||||
|
||||
**Note on 90-minute rounding:** `Math.round(90/60) = 2` — "Starts in 2 hours" for a 90-minute lead. This matches the UI-SPEC threshold `120–1439 minutes → N hours`. But 90 minutes is in the 60–119 bucket → "1 hour". Recalibrate: `leadMinutes >= 60 && leadMinutes < 120` → "1 hour" before the hours calculation. The function above handles this correctly via the ordering of branches.
|
||||
|
||||
### Variable-lead scheduler SQL query (Drizzle ORM sketch)
|
||||
|
||||
```typescript
|
||||
// Replace the fixed windowEnd = now + 16min approach
|
||||
// For each event: fire when (dtstartUtc - reminderLeadMinutes MINUTES) falls in (lastCheck, now]
|
||||
// In a 1-minute tick, query events where the alert time is in the past minute
|
||||
const alertWindowStart = new Date(now.getTime() - 60 * 1000); // 1 min ago (catch-up window)
|
||||
|
||||
// Timed events: alert_time = dtstartUtc - reminder_lead_minutes MINUTES
|
||||
// Using Drizzle raw SQL for the computed column:
|
||||
import { sql } from 'drizzle-orm';
|
||||
|
||||
// Timed events:
|
||||
// WHERE allDay=false
|
||||
// AND reminder_lead_minutes IS NOT NULL
|
||||
// AND reminder_lead_minutes > 0 (0 on timed = None per D-06)
|
||||
// AND (dtstartUtc - INTERVAL reminder_lead_minutes MINUTE) > alertWindowStart
|
||||
// AND (dtstartUtc - INTERVAL reminder_lead_minutes MINUTE) <= now
|
||||
// AND dtstartUtc > now (exclude already-started)
|
||||
|
||||
// All-day events:
|
||||
// WHERE allDay=true
|
||||
// AND reminder_lead_minutes IS NOT NULL
|
||||
// AND dtstartDate IS NOT NULL
|
||||
// AND the computed 9 AM UTC alert for (dtstartDate - reminder_lead_minutes/1440 DAYS)
|
||||
// falls in (alertWindowStart, now]
|
||||
// → This requires either pre-computing alertTime in a derived column
|
||||
// or fetching all all-day events with non-null leads and filtering in JS
|
||||
|
||||
// Recommended: separate queries for timed and all-day events (cleaner SQL)
|
||||
```
|
||||
|
||||
**Planner note:** All-day 9 AM UTC computation in SQL requires timezone-aware date functions that are MariaDB version-dependent. Recommend fetching all-day events with non-null `reminder_lead_minutes` that have `dtstartDate` within the next `max(lead)` days, then computing the alert time in JavaScript and filtering. This avoids complex SQL timezone arithmetic.
|
||||
|
||||
---
|
||||
|
||||
## Runtime State Inventory
|
||||
|
||||
Not a rename/refactor phase — section omitted.
|
||||
|
||||
---
|
||||
|
||||
## Environment Availability
|
||||
|
||||
No new external tools required. The existing dev stack (MariaDB, Node.js 22, Vitest) covers all Phase 11 work. Skipping detailed audit.
|
||||
|
||||
---
|
||||
|
||||
## Validation Architecture
|
||||
|
||||
### Test Framework
|
||||
|
||||
| Property | Value |
|
||||
|----------|-------|
|
||||
| Framework | Vitest (vite-native, same config as frontend) |
|
||||
| Config file | `apps/api/vitest.config.ts` |
|
||||
| Quick run command | `pnpm --filter @familysync/api exec vitest run tests/broker/vevent.test.ts tests/broker/reminderScheduler.test.ts` |
|
||||
| Full suite command | `pnpm --filter @familysync/api exec vitest run` |
|
||||
|
||||
### Phase Requirements → Test Map
|
||||
|
||||
| Req ID | Behavior | Test Type | Automated Command | File Exists? |
|
||||
|--------|----------|-----------|-------------------|-------------|
|
||||
| CAL-13 | VALARM emitted with correct TRIGGER:-PTNmM (no VALUE=TEXT) | unit | `vitest run tests/broker/vevent.test.ts` | ✅ (extend) |
|
||||
| CAL-13 | All-day VALARM emits absolute DATE-TIME trigger | unit | `vitest run tests/broker/vevent.test.ts` | ✅ (extend) |
|
||||
| CAL-13 | allDay toggle swaps preset list; reset to None on toggle | browser | `playwright-cli` | ❌ Wave 0 |
|
||||
| CAL-14 | Edit with no picker change: rawVevent VALARM preserved in PUT payload | unit | `vitest run tests/broker/outboxWorker.test.ts` | ✅ (extend) |
|
||||
| CAL-14 | Off-list single relative VALARM → offlist classification | unit | `vitest run tests/broker/vevent.test.ts` | ✅ (extend) |
|
||||
| CAL-14 | Absolute trigger → custom classification | unit | `vitest run tests/broker/vevent.test.ts` | ✅ (extend) |
|
||||
| CAL-14 | Two VALARMs → custom classification | unit | `vitest run tests/broker/vevent.test.ts` | ✅ (extend) |
|
||||
| NOTIF-04 | Scheduler fires at T-leadMinutes for a 30-min lead event | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ✅ (extend) |
|
||||
| NOTIF-04 | Humanized body: 30min→"Starts in 30 min", 60→"1 hour", 1440→"1 day", 10080→"7 days" | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ✅ (extend) |
|
||||
| NOTIF-05 | Event with null reminderLeadMinutes produces no push | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ✅ (extend) |
|
||||
| NOTIF-05 | Event with reminderLeadMinutes=0 and allDay=false produces no push | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ✅ (extend) |
|
||||
| NOTIF-06 | All-day event with 0-min lead fires at 9 AM local (UTC computation correct) | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ❌ Wave 0 |
|
||||
| NOTIF-06 | Rescheduled event (new dtstart) fires again — uid:dtstartMs dedup | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ✅ (extend) |
|
||||
| NOTIF-06 | Same event fires exactly once across 3 consecutive ticks (uid:dtstartMs dedup) | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ✅ (extend — existing test covers uid-only; update to uid:dtstartMs) |
|
||||
|
||||
### Sampling Rate
|
||||
- **Per task commit:** `pnpm --filter @familysync/api exec vitest run tests/broker/vevent.test.ts tests/broker/reminderScheduler.test.ts`
|
||||
- **Per wave merge:** `pnpm --filter @familysync/api exec vitest run` (full API suite)
|
||||
- **Phase gate:** Full API suite green + Playwright smoke (allDay toggle, reminder picker loads correctly in edit) before `/gsd-verify-work`
|
||||
|
||||
### Wave 0 Gaps
|
||||
- [ ] `tests/broker/vevent.test.ts` — extend with VALARM serialization tests (TRIGGER type, all-day absolute, preserve round-trip, classifier I/O)
|
||||
- [ ] `tests/broker/reminderScheduler.test.ts` — extend with variable-lead, all-day 9AM UTC, uid:dtstartMs dedup, NULL-vs-0 semantics, humanized body
|
||||
- [ ] `computeAlertInstantUtc` unit tests at DST boundaries (spring-forward and fall-back dates)
|
||||
- [ ] Playwright smoke: open EventForm in create mode, toggle allDay, verify preset list swaps; open edit mode with existing event, verify picker loads correct value
|
||||
|
||||
---
|
||||
|
||||
## Security Domain
|
||||
|
||||
No new security surface. The reminder picker reads/writes `reminderLeadMinutes` (an integer) validated by Zod `z.number().int().min(0).nullable().optional()` — no injection vectors. Push payload body is a hardcoded humanized string, not user-controlled. Existing ASVS V5 input validation coverage is maintained.
|
||||
|
||||
---
|
||||
|
||||
## Assumptions Log
|
||||
|
||||
| # | Claim | Section | Risk if Wrong |
|
||||
|---|-------|---------|---------------|
|
||||
| A1 | Absolute DATE-TIME TRIGGER is honored by both Fastmail and Apple Calendar for all-day events | Q1 VALARM trigger encoding | Low — scheduler is ground truth; if clients ignore VALARM, only native-client interop is affected, not push delivery |
|
||||
| A2 | Server timezone (process.env.TZ) matches household timezone for 9 AM alert computation | Q1/Code Examples | Medium — if TZ is UTC, all-day reminders fire at 9 AM UTC (which may not be 9 AM local). Mitigation: document that `TZ` env must be set in docker-compose |
|
||||
| A3 | `triggerProp.getFirstValue()` returns `ICAL.Duration` for relative triggers and `ICAL.Time` for absolute triggers | Q3 classifier | Medium — could be wrong for edge-case ical.js versions; mitigated by TDD tests that round-trip a known ICS |
|
||||
| A4 | `getParameter('value')` on a default-type DURATION trigger returns undefined (not 'duration') | Q3 classifier | Low — mitigated by the `instanceof ICAL.Time` check which is type-safe |
|
||||
| A5 | No per-user timezone is stored in the DB — server TZ is used as the household timezone | Environment | Medium — verified from schema.ts (no timezone column on users or calendars); if household spans timezones, 9 AM will be wrong for one member |
|
||||
|
||||
---
|
||||
|
||||
## Sources
|
||||
|
||||
### Primary (HIGH confidence — direct code inspection)
|
||||
- `apps/api/src/broker/vevent.ts` — existing buildVeventString; RRULE pattern for VALARM to mirror
|
||||
- `apps/api/src/broker/outboxWorker.ts` — outboxPayloadSchema, hasExplicitRecurrence pattern for hasExplicitReminder to mirror
|
||||
- `apps/api/src/broker/reminderScheduler.ts` — current fixed-window logic, dedup map, body string at line 157
|
||||
- `apps/api/src/db/schema.ts` — `reminderLeadMinutes: int('reminder_lead_minutes')` nullable column confirmed
|
||||
- `apps/api/src/broker/sync.ts` — upsert does NOT write reminderLeadMinutes (confirmed gap)
|
||||
- `apps/api/src/broker/expand.ts` — `CalendarOccurrence` does not include reminderLeadMinutes (must be added for edit-mode load)
|
||||
- `apps/api/src/routes/events.ts` — `eventFieldsSchema` has no reminderLeadMinutes (confirmed gap)
|
||||
- `apps/pwa/src/api/client.ts` — `CreateEventPayload` has no reminderLeadMinutes (confirmed gap)
|
||||
- `apps/pwa/src/components/EventForm.tsx:852` — Recurrence picker pattern to mirror
|
||||
|
||||
### Secondary (MEDIUM confidence — Context7 / official docs)
|
||||
- [kewisch/ical.js — jCal structure](https://github.com/kewisch/ical.js/wiki/Migrating-from-Other-Libraries) — VALARM jCal encoding confirms `"duration"` value type for TRIGGER, `{"related": "START"}` parameter
|
||||
- [kewisch/ical.js — Convert to iCalendar](https://github.com/kewisch/ical.js/wiki/Convert-to-iCalendar-(rfc5545)) — addSubcomponent pattern confirmed
|
||||
- [kewisch/ical.js — Parsing iCalendar](https://github.com/kewisch/ical.js/wiki/Parsing-iCalendar) — getFirstSubcomponent, getAllSubcomponents API confirmed
|
||||
|
||||
### Tertiary (LOW confidence — assumed)
|
||||
- RFC 5545 §3.8.6.3 — VALARM TRIGGER value types (DURATION vs DATE-TIME) — training knowledge [ASSUMED]
|
||||
- Client behavior (Fastmail/Apple Calendar) with absolute DATE-TIME triggers on all-day events — [ASSUMED]
|
||||
|
||||
---
|
||||
|
||||
## Metadata
|
||||
|
||||
**Confidence breakdown:**
|
||||
- Schema/zod plumbing (Q2): HIGH — verified from direct file inspection, no ambiguity
|
||||
- ical.js VALARM API (Q1/Q3): MEDIUM — confirmed via Context7 official docs; `resetType`/`getFirstValue` APIs confirmed from jCal structure examples
|
||||
- All-day trigger client interop (Q1): LOW — untestable without live Fastmail + Apple Calendar
|
||||
- Humanized body thresholds: HIGH — copied from UI-SPEC.md verbatim
|
||||
- 9 AM UTC computation: MEDIUM — the algorithm is correct; DST edge cases require TDD at boundaries
|
||||
|
||||
**Research date:** 2026-06-13
|
||||
**Valid until:** 2026-07-13 (stable domain — ical.js 2.x API is stable)
|
||||
@@ -0,0 +1,239 @@
|
||||
---
|
||||
phase: 11
|
||||
slug: per-event-reminders
|
||||
status: draft
|
||||
shadcn_initialized: false
|
||||
preset: none
|
||||
created: 2026-06-13
|
||||
---
|
||||
|
||||
# Phase 11 — UI Design Contract
|
||||
|
||||
> Visual and interaction contract for frontend phases. Generated by gsd-ui-researcher, verified by gsd-ui-checker.
|
||||
|
||||
---
|
||||
|
||||
## Design System
|
||||
|
||||
| Property | Value |
|
||||
|----------|-------|
|
||||
| Tool | none — custom CSS tokens (tokens.css), no shadcn |
|
||||
| Preset | not applicable |
|
||||
| Component library | none — inline React.CSSProperties style objects |
|
||||
| Icon library | lucide-react (already in use: X, Loader2) |
|
||||
| Font | system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif |
|
||||
|
||||
Source: `apps/pwa/src/styles/tokens.css`, `apps/pwa/src/components/EventForm.tsx`
|
||||
|
||||
---
|
||||
|
||||
## Spacing Scale
|
||||
|
||||
Declared values (must be multiples of 4). Pre-populated from `tokens.css`.
|
||||
|
||||
| Token | Value | Usage |
|
||||
|-------|-------|-------|
|
||||
| --space-1 | 4px | Icon gaps, label margin-bottom, helper-text margin-top |
|
||||
| --space-2 | 8px | Compact element spacing |
|
||||
| --space-3 | 12px | Select horizontal padding (matching Recurrence picker) |
|
||||
| --space-4 | 16px | Field margin-bottom (fieldStyle), general element spacing |
|
||||
| --space-6 | 24px | Section padding |
|
||||
| --space-8 | 32px | Layout gaps |
|
||||
| --space-12 | 48px | Major section breaks |
|
||||
|
||||
Exceptions: 44px minimum touch target height on the reminder `<select>` (matches `minHeight: '44px'` already declared on `inputStyle`).
|
||||
|
||||
---
|
||||
|
||||
## Typography
|
||||
|
||||
Pre-populated from `tokens.css`. No new type ramp needed — reminder picker reuses existing roles.
|
||||
|
||||
| Role | Size | Weight | Line Height | Usage in this phase |
|
||||
|------|------|--------|-------------|---------------------|
|
||||
| Body | 15px (--text-body-size) | 400 (--text-body-weight) | 1.5 | Select option text, helper text values |
|
||||
| Label | 13px (--text-label-size) | 400 (--text-label-weight) | 1.4 | "Reminder" field label, "Custom (kept)" hint text, error copy |
|
||||
| Heading | 18px (--text-heading-size) | 600 (--text-heading-weight) | 1.25 | Not directly used by this phase |
|
||||
| Display | 24px (--text-display-size) | 600 (--text-display-weight) | 1.2 | Not directly used by this phase |
|
||||
|
||||
Only 2 weights in use: 400 (regular) and 600 (semibold).
|
||||
|
||||
---
|
||||
|
||||
## Color
|
||||
|
||||
Pre-populated from `tokens.css`. No new tokens introduced in this phase.
|
||||
|
||||
| Role | Value | Usage |
|
||||
|------|-------|-------|
|
||||
| Dominant (60%) | --color-surface: #ffffff | Form background, select background |
|
||||
| Secondary (30%) | --color-surface-dim: #f7f7f8 | Modal backdrop surface (already used) |
|
||||
| Accent (10%) | --color-focus-ring: #4a90d9 | Focus ring on reminder select when keyboard-navigated |
|
||||
| Destructive | --color-destructive: #dc2626 | Validation error copy if picker submission fails |
|
||||
|
||||
Accent reserved for: focus ring on the reminder `<select>` only. Calendar member colors (--color-member-*) and shared family color (--color-shared-family) are unchanged and not used by the reminder picker UI.
|
||||
|
||||
---
|
||||
|
||||
## Component Inventory
|
||||
|
||||
This phase adds exactly one new UI element to `EventForm.tsx`. All styling follows the established inline `React.CSSProperties` pattern from the Recurrence picker.
|
||||
|
||||
### Reminder `<select>` (new in Phase 11)
|
||||
|
||||
- **Placement:** After the Recurrence `<select>` block (~line 885 in EventForm.tsx), before the recurrence-bound control.
|
||||
- **Label text:** `Reminder`
|
||||
- **Element ID:** `event-reminder`
|
||||
- **Pattern:** Identical to the Recurrence picker — `<div style={fieldStyle}>`, `<label htmlFor="event-reminder" style={labelStyle}>`, `<select style={{...inputStyle, padding: '0 var(--space-3)', cursor: 'pointer'}}>`
|
||||
|
||||
### Timed-event options (shown when `!allDay`)
|
||||
|
||||
| Option value | Display label |
|
||||
|-------------|---------------|
|
||||
| `null` | None |
|
||||
| `5` | 5 minutes before |
|
||||
| `10` | 10 minutes before |
|
||||
| `15` | 15 minutes before |
|
||||
| `30` | 30 minutes before |
|
||||
| `60` | 1 hour before |
|
||||
| `120` | 2 hours before |
|
||||
| `1440` | 1 day before |
|
||||
| `2880` | 2 days before |
|
||||
|
||||
### All-day options (shown when `allDay`, swapped — D-02/D-03)
|
||||
|
||||
| Option value | Display label |
|
||||
|-------------|---------------|
|
||||
| `null` | None |
|
||||
| `0` | Same day (9 AM) |
|
||||
| `1440` | 1 day before (9 AM) |
|
||||
| `2880` | 2 days before (9 AM) |
|
||||
| `10080` | 1 week before (9 AM) |
|
||||
|
||||
Default: `null` (None) for both cases — D-01.
|
||||
|
||||
### Off-preset / Custom alarm handling (D-07/D-08)
|
||||
|
||||
- **Single simple relative alarm not in preset list:** Render a synthetic option `"N min before"` or `"N hours before"` (humanized, see Copywriting table) appended to the option list, selected by default. If user changes selection, the synthetic option is removed.
|
||||
- **Absolute-time trigger or multiple alarms:** Render a read-only disabled `<option value="__custom__">Custom (kept)</option>` selected by default. The `<select>` itself is NOT disabled — the user can still choose a preset, which replaces the custom alarm.
|
||||
- **Preserve on no-change:** When picker value remains `"__custom__"` or the synthetic off-preset option on save, pass a sentinel (e.g. `"no-change"`) in the payload so the outbox worker leaves the original VALARM intact.
|
||||
|
||||
### Helper text (shown below select, identical style to WR-01 helper)
|
||||
|
||||
Style: `{ fontSize: 'var(--text-label-size)', color: 'var(--color-text-secondary)', marginTop: 'var(--space-1)' }`
|
||||
|
||||
Shown only when `eventFormMode === 'edit'` and the event has a custom alarm (either off-preset single or multi/absolute):
|
||||
|
||||
> "Custom reminder kept — select a preset to replace it."
|
||||
|
||||
---
|
||||
|
||||
## Notification Push Copy (D-09)
|
||||
|
||||
The humanized body replaces the hardcoded `Starts in ${minutes} min` in `reminderScheduler.ts` line 157.
|
||||
|
||||
### Humanized thresholds and wording
|
||||
|
||||
| Lead window | Body copy |
|
||||
|-------------|-----------|
|
||||
| < 60 minutes | `Starts in {N} min` (e.g. "Starts in 30 min") |
|
||||
| 60–119 minutes | `Starts in 1 hour` |
|
||||
| 120–1439 minutes | `Starts in {N} hours` (e.g. "Starts in 2 hours") |
|
||||
| 1440–2879 minutes (1 day) | `Starts in 1 day` |
|
||||
| ≥ 2880 minutes | `Starts in {N} days` (e.g. "Starts in 2 days") |
|
||||
|
||||
All-day events with day-granularity leads use the same thresholds (converted from minutes). "1 week before" → 10080 min → "Starts in 7 days".
|
||||
|
||||
Push notification title is unchanged from the current implementation (event title).
|
||||
|
||||
---
|
||||
|
||||
## Copywriting Contract
|
||||
|
||||
| Element | Copy |
|
||||
|---------|------|
|
||||
| Reminder field label | `Reminder` |
|
||||
| Picker default option | `None` |
|
||||
| Timed preset — 5m | `5 minutes before` |
|
||||
| Timed preset — 10m | `10 minutes before` |
|
||||
| Timed preset — 15m | `15 minutes before` |
|
||||
| Timed preset — 30m | `30 minutes before` |
|
||||
| Timed preset — 1h | `1 hour before` |
|
||||
| Timed preset — 2h | `2 hours before` |
|
||||
| Timed preset — 1d | `1 day before` |
|
||||
| Timed preset — 2d | `2 days before` |
|
||||
| All-day preset — same day | `Same day (9 AM)` |
|
||||
| All-day preset — 1d before | `1 day before (9 AM)` |
|
||||
| All-day preset — 2d before | `2 days before (9 AM)` |
|
||||
| All-day preset — 1wk before | `1 week before (9 AM)` |
|
||||
| Off-preset single alarm helper text | `Custom reminder kept — select a preset to replace it.` |
|
||||
| Custom (kept) option label | `Custom (kept)` |
|
||||
| Push notification — < 60 min | `Starts in {N} min` |
|
||||
| Push notification — exactly 1 hour | `Starts in 1 hour` |
|
||||
| Push notification — N hours | `Starts in {N} hours` |
|
||||
| Push notification — exactly 1 day | `Starts in 1 day` |
|
||||
| Push notification — N days | `Starts in {N} days` |
|
||||
|
||||
No empty state: the reminder picker always shows options (default None). No destructive actions in this phase — reminder selection is non-destructive; unsaved changes are discarded on form close (same as all other EventForm fields).
|
||||
|
||||
Error state: If the event PUT fails (network/server error), the existing EventForm error toast pattern handles it — no reminder-specific error copy needed.
|
||||
|
||||
---
|
||||
|
||||
## Interaction Contract
|
||||
|
||||
### State machine for the picker
|
||||
|
||||
```
|
||||
allDay = false → show timed presets (None / 5m / 10m / 15m / 30m / 1h / 2h / 1d / 2d)
|
||||
allDay = true → swap to all-day presets (None / Same day / 1d before / 2d before / 1wk before)
|
||||
allDay toggles → reset picker to None (do not carry over a timed value to all-day or vice versa)
|
||||
```
|
||||
|
||||
### Edit mode: loading existing reminder
|
||||
|
||||
```
|
||||
reminderLeadMinutes from occurrence:
|
||||
null → select "None" (default)
|
||||
matches preset → select matching option
|
||||
number not in preset list (and single simple relative VALARM) → add synthetic option, select it
|
||||
absolute/multi alarm → add "Custom (kept)" option (disabled), select it; show helper text
|
||||
```
|
||||
|
||||
### Accessibility
|
||||
|
||||
- `id="event-reminder"` on the `<select>`, `htmlFor="event-reminder"` on the label (matches Recurrence pattern exactly).
|
||||
- `<select>` participates in the existing focus trap (no special handling needed — it is a native focusable element).
|
||||
- The "Custom (kept)" `<option>` uses `disabled` attribute to prevent re-selection after the user picks a preset, but the `<select>` itself remains enabled.
|
||||
- Minimum touch target: 44px height via `minHeight: '44px'` on `inputStyle` (inherited, already declared).
|
||||
|
||||
### No new UI surfaces
|
||||
|
||||
This phase adds no new modals, sheets, toasts, or pages. All changes are:
|
||||
1. One new `<select>` field inside the existing `EventForm.tsx`.
|
||||
2. One optional helper text `<div>` below the select (edit mode + custom alarm only).
|
||||
3. Push notification body copy change in `reminderScheduler.ts` (server-side, no UI surface).
|
||||
|
||||
---
|
||||
|
||||
## Registry Safety
|
||||
|
||||
| Registry | Blocks Used | Safety Gate |
|
||||
|----------|-------------|-------------|
|
||||
| shadcn official | none — shadcn not initialized | not applicable |
|
||||
| third-party | none | not applicable |
|
||||
|
||||
No third-party component registry blocks used. Phase 11 adds no new npm dependencies on the frontend. All UI is composed from native HTML elements styled with existing project tokens.
|
||||
|
||||
---
|
||||
|
||||
## Checker Sign-Off
|
||||
|
||||
- [ ] Dimension 1 Copywriting: PASS
|
||||
- [ ] Dimension 2 Visuals: PASS
|
||||
- [ ] Dimension 3 Color: PASS
|
||||
- [ ] Dimension 4 Typography: PASS
|
||||
- [ ] Dimension 5 Spacing: PASS
|
||||
- [ ] Dimension 6 Registry Safety: PASS
|
||||
|
||||
**Approval:** pending
|
||||
@@ -0,0 +1,92 @@
|
||||
---
|
||||
phase: 11
|
||||
slug: per-event-reminders
|
||||
status: draft
|
||||
nyquist_compliant: false
|
||||
wave_0_complete: false
|
||||
created: 2026-06-13
|
||||
---
|
||||
|
||||
# Phase 11 — Validation Strategy
|
||||
|
||||
> Per-phase validation contract for feedback sampling during execution.
|
||||
> Source: `11-RESEARCH.md` § Validation Architecture.
|
||||
|
||||
---
|
||||
|
||||
## Test Infrastructure
|
||||
|
||||
| Property | Value |
|
||||
|----------|-------|
|
||||
| **Framework** | Vitest (vite-native, same config as frontend) |
|
||||
| **Config file** | `apps/api/vitest.config.ts` |
|
||||
| **Quick run command** | `pnpm --filter @familysync/api exec vitest run tests/broker/vevent.test.ts tests/broker/reminderScheduler.test.ts` |
|
||||
| **Full suite command** | `pnpm --filter @familysync/api exec vitest run` |
|
||||
| **Estimated runtime** | ~10–30 seconds (quick); full suite longer |
|
||||
|
||||
---
|
||||
|
||||
## Sampling Rate
|
||||
|
||||
- **After every task commit:** Run quick command (`vevent.test.ts` + `reminderScheduler.test.ts`)
|
||||
- **After every plan wave:** Run full API suite
|
||||
- **Before `/gsd-verify-work`:** Full API suite green + Playwright smoke (allDay toggle swaps presets; edit-mode picker loads correct value)
|
||||
- **Max feedback latency:** ~30 seconds
|
||||
|
||||
---
|
||||
|
||||
## Per-Task Verification Map
|
||||
|
||||
> Per-task rows are populated during planning once plan/task IDs exist. The
|
||||
> requirement → behavior → test-command coverage below is sourced from
|
||||
> `11-RESEARCH.md` § "Phase Requirements → Test Map" and must each be claimed
|
||||
> by at least one task's `<automated>` verify.
|
||||
|
||||
| Requirement | Behavior | Test Type | Automated Command | File Exists |
|
||||
|-------------|----------|-----------|-------------------|-------------|
|
||||
| CAL-13 | Timed VALARM emits DURATION trigger `-PTNmM` (no `VALUE=TEXT`) | unit | `vitest run tests/broker/vevent.test.ts` | ✅ extend |
|
||||
| CAL-13 | All-day VALARM emits absolute DATE-TIME trigger | unit | `vitest run tests/broker/vevent.test.ts` | ✅ extend |
|
||||
| CAL-13 | allDay toggle swaps preset list; resets to None on toggle | browser | `playwright-cli` | ❌ W0 |
|
||||
| CAL-14 | No picker change on edit → rawVevent VALARM preserved in PUT | unit | `vitest run tests/broker/outboxWorker.test.ts` | ✅ extend |
|
||||
| CAL-14 | Off-list single relative VALARM → `offlist` classification | unit | `vitest run tests/broker/vevent.test.ts` | ✅ extend |
|
||||
| CAL-14 | Absolute trigger → `custom` classification | unit | `vitest run tests/broker/vevent.test.ts` | ✅ extend |
|
||||
| CAL-14 | Two VALARMs → `custom` classification | unit | `vitest run tests/broker/vevent.test.ts` | ✅ extend |
|
||||
| NOTIF-04 | Scheduler fires at T-leadMinutes (e.g. T-30 for 30-min lead) | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ✅ extend |
|
||||
| NOTIF-04 | Humanized body: 30→"30 min", 60→"1 hour", 1440→"1 day", 10080→"7 days" | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ✅ extend |
|
||||
| NOTIF-05 | `null` reminderLeadMinutes → no push | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ✅ extend |
|
||||
| NOTIF-05 | `0` lead + `allDay=false` → no push | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ✅ extend |
|
||||
| NOTIF-06 | All-day `0` lead fires 9 AM local (UTC computation correct) | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ❌ W0 |
|
||||
| NOTIF-06 | Rescheduled event (new dtstart) re-fires — `uid:dtstartMs` dedup | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ✅ extend |
|
||||
| NOTIF-06 | Same event fires exactly once across 3 ticks — `uid:dtstartMs` dedup | unit | `vitest run tests/broker/reminderScheduler.test.ts` | ✅ extend (current test is uid-only) |
|
||||
|
||||
*Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky*
|
||||
|
||||
---
|
||||
|
||||
## Wave 0 Requirements
|
||||
|
||||
- [ ] `tests/broker/vevent.test.ts` — extend: VALARM serialization (DURATION trigger type, all-day absolute DATE-TIME, preserve round-trip, classifier I/O none/preset/offlist/custom)
|
||||
- [ ] `tests/broker/reminderScheduler.test.ts` — extend: variable-lead fire, all-day 9 AM UTC, `uid:dtstartMs` dedup re-fire on reschedule + once-across-ticks, NULL-vs-0 semantics, humanized body
|
||||
- [ ] `computeAlertInstantUtc` — unit tests at DST boundaries (spring-forward and fall-back dates)
|
||||
- [ ] Playwright smoke — EventForm create mode allDay-toggle preset swap; edit mode loads correct picker value
|
||||
|
||||
---
|
||||
|
||||
## Manual-Only Verifications
|
||||
|
||||
| Behavior | Requirement | Why Manual | Test Instructions |
|
||||
|----------|-------------|------------|-------------------|
|
||||
| All-day VALARM trigger honored by Fastmail / Apple Calendar | CAL-13 (best-effort interop) | Requires live Fastmail + Apple Calendar accounts; not driveable headlessly | Create an all-day event with a "1 day before" reminder in FamilySync; open in Apple Calendar / Fastmail web and confirm an alarm shows (exact fire time is best-effort — scheduler is push ground truth) |
|
||||
|
||||
---
|
||||
|
||||
## Validation Sign-Off
|
||||
|
||||
- [ ] All tasks have `<automated>` verify or Wave 0 dependencies
|
||||
- [ ] Sampling continuity: no 3 consecutive tasks without automated verify
|
||||
- [ ] Wave 0 covers all MISSING references
|
||||
- [ ] No watch-mode flags
|
||||
- [ ] Feedback latency < 30s
|
||||
- [ ] `nyquist_compliant: true` set in frontmatter
|
||||
|
||||
**Approval:** pending
|
||||
Reference in New Issue
Block a user