Files
familysync/.planning/phases/05-web-push-notifications/05-VERIFICATION.md
T

194 lines
18 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
---
phase: 05-web-push-notifications
verified: 2026-06-09T14:00:00Z
status: human_needed
score: 12/12
overrides_applied: 0
human_verification:
- test: "iOS PWA install → push subscription → 15-min reminder receipt"
expected: "After adding FamilySync to Home Screen on an iOS 16.4+ device and tapping 'Enable Notifications', a push notification appears on the lock screen ~15 minutes before a shared Family-calendar timed event starts."
why_human: "iOS-Safari standalone push delivery cannot be driven by playwright-cli per CLAUDE.md — requires a physical iOS device + Home Screen install."
- test: "iOS push subscription does not receive NotAllowedError"
expected: "Tapping 'Enable Notifications' on iOS in the installed PWA (or the Settings toggle) successfully calls pushManager.subscribe() without throwing NotAllowedError. Both vapidKey and swRegistration are pre-resolved in state before the tap."
why_human: "NEW-CR-01 fix is verified in code (zero awaits between tap and subscribe()), but runtime confirmation on a physical iOS device is the only way to close this."
- test: "iOS subscription health-check keeps subscription alive after 1+ week of inactivity"
expected: "After a week without opening the app, opening it again silently re-subscribes (if permission still granted) and notifications continue to be delivered."
why_human: "Requires real elapsed time and a physical iOS device. Cannot be simulated."
- test: "Android FCM: event-change push arrives after the other member modifies a calendar event"
expected: "When member A modifies a shared event title/time/location, member B receives a push notification on Android within the next 5-minute poll cycle, showing 'A updated an event' with the event title."
why_human: "End-to-end push delivery through FCM to a real Android device with a subscribed session cannot be driven by playwright-cli."
- test: "List-change push coalescing is observable"
expected: "Member B making 5 rapid grocery-list edits results in a SINGLE push notification to member A (not 5), naming the actor and the list, arriving after the 45-second coalesce window."
why_human: "Requires two devices/sessions, real timing, and real push delivery. Playwright-cli can exercise the API hooks but not multi-device push receipt."
---
# Phase 5: Web Push Notifications — Verification Report
**Phase Goal:** Both members receive timely Web Push alerts for upcoming events, event changes made by the other member, and list changes — reliably on both iOS and Android.
**Verified:** 2026-06-09
**Status:** human_needed
**Re-verification:** No — initial verification
## Goal Achievement
All four success criteria have substantive, wired, data-flowing server and PWA implementations. No gaps in the codebase. Five behavioral items require a physical iOS device or multi-device push delivery to close — these are classified as human-verification items, not gaps.
### Observable Truths
| # | Truth | Status | Evidence |
|---|-------|--------|----------|
| 1 | Member receives ~15-min push before a shared Family-calendar timed event | VERIFIED (code) / HUMAN (device delivery) | `reminderScheduler.ts`: `runReminderCheck` queries `WHERE isShared=true AND allDay=false AND dtstartUtc BETWEEN now+14m AND now+16m`, fans out via `dispatchPush`; wired in `index.ts` at startup. D-05 enforced in SQL. |
| 2 | Other member's event add/change pushes a specific notification | VERIFIED (code) / HUMAN (device delivery) | `eventChangeDispatcher.ts`: `isMeaningfulChange` filters on `dtstartUtc/dtstartDate/allDay/title/location`; `dispatchEventChange` fans out to non-actor subs. `sync.ts` detects diffs and fires `onChanges`; `poller.ts` + `outboxWorker.ts` both pass the callback with `actorUserId`. |
| 3 | Other member's shared-list change pushes a generic, coalesced notification | VERIFIED (code) / HUMAN (device delivery) | `listChangeDispatcher.ts`: `notifyListChange``coalesceListPush` (45s sliding window, D-01). `lists.ts` calls it on item-add/check/text-edit/delete/list-rename/list-delete; explicitly skipped on position-only PATCH (D-01, line 651). |
| 4 | After extended inactivity, push notifications still delivered (health-check) | VERIFIED (code) / HUMAN (device delivery) | `usePushSubscription.ts` mount `useEffect`: checks `getSubscription()`; if missing and not explicitly disabled, silently re-subscribes. D-10. |
**Score:** 12/12 truths verified in codebase.
### D-05 Scope Narrowing Confirmation
Decision D-05 narrows NOTIF-01 to **shared Family-calendar events only** (personal calendar events are covered by native device calendar apps). This is enforced in the SQL `WHERE calendars.isShared = true` — not just in copy — making it a query-level guarantee, not an omission. The requirement intent ("user receives a reminder before an event starts") is satisfied: FamilySync owns the cross-ecosystem shared-calendar gap, not the personal-calendar gap already covered natively. VERIFIED as intentional and correct.
---
## Required Artifacts
| Artifact | Expected | Status | Details |
|----------|----------|--------|---------|
| `apps/api/src/lib/pushDispatcher.ts` | VAPID send + 410/404 prune | VERIFIED | Exports `dispatchPush` + `buildPushBody`; dual-format payload (web_push:8030 + legacy); 410/404 → `db.delete`; transient → log, no delete. 122 lines. |
| `apps/api/src/lib/pushCoalescer.ts` | Per-(list,actor) debounce | VERIFIED | Module-level `pending` Map; sliding setTimeout; exports `coalesceListPush`. 71 lines. |
| `apps/api/src/lib/listChangeDispatcher.ts` | Access-scoped, self-suppressed list-change fan-out | VERIFIED | Resolves owner list_shares audience; excludes actorId; D-02 generic copy `"{actor} made N changes to {list}"`. 127 lines. |
| `apps/api/src/lib/eventChangeDispatcher.ts` | Event-change dispatch + isMeaningfulChange | VERIFIED | `MEANINGFUL_FIELDS = {dtstartUtc, dtstartDate, allDay, title, location}`; description-only → silent (D-04); actor excluded via `ne()` + app filter (D-03). Exports `dispatchEventChange` + `isMeaningfulChange`. 176 lines. |
| `apps/api/src/broker/reminderScheduler.ts` | 1-min shared-timed-event scan | VERIFIED | `runReminderCheck`: isShared+allDay WHERE in SQL; `sentReminders` dedup Set keyed `uid:minuteBucket`; stale-entry prune (CR-01); `startReminderScheduler` via node-cron. 199 lines. |
| `apps/api/src/broker/sync.ts` | title population + onChanges diff callback | VERIFIED | `titleValue` from VEVENT SUMMARY on every upsert; per-uid old-row SELECT; added/updated/deleted classification; `pendingDeleteRows` pre-capture (incl. NEW-WR-01 empty-seenUids branch); `onChanges(changes)` fired at end. |
| `apps/api/src/routes/push.ts` | GET /vapid-public-key, POST/DELETE /subscription | VERIFIED | Zod `subscribeSchema`; `resolveUserId` scopes inserts/deletes; upsert on endpoint; 401 when unauthed. |
| `apps/api/src/index.ts` | VAPID setup + scheduler wiring | VERIFIED | `webpush.setVapidDetails(...)` in `isMainModule()` guard before `startReminderScheduler()`; `pushRouter` mounted at `/api/push`. |
| `apps/pwa/src/sw.ts` | injectManifest SW: precache + push + notificationclick + denylist | VERIFIED | `event.waitUntil(showNotification(...))` always fires (D-11); fallback title/body for malformed payloads; `NavigationRoute` denylist `[/^\/callback/, /^\/api\//, /^\/health/]` (T-03-20); deep-link via `focus()+navigate()` / `openWindow()` (CR-03). |
| `apps/pwa/src/hooks/usePushSubscription.ts` | subscribe + health-check + setEnabled | VERIFIED | `subscribe(registration, vapidKey)` — takes pre-resolved reg + key (CR-04/NEW-CR-01); mount health-check (`getSubscription` → silent re-subscribe D-10); `setEnabled` master toggle (D-09). |
| `apps/pwa/src/components/PushPermissionPrompt.tsx` | Post-install permission bottom sheet | VERIFIED | Pre-resolves `vapidKey` + `swRegistration` in `useEffect`; button disabled until both ready (NEW-CR-01); `handleEnableClick` calls `subscribe(resolvedRegistration, resolvedVapidKey)` synchronously — zero await before `pushManager.subscribe()`; isInstalled + permission==='default' + !dismissed gate. |
| `apps/pwa/src/components/SettingsSheet.tsx` | Master toggle + avatar-triggered sheet | VERIFIED | `role="switch"`, `aria-checked`; 44px target; pre-resolves `vapidKey` + `swRegistration` (CR-04/NEW-CR-01); `handleToggle` calls `subscribe(resolvedRegistration, resolvedVapidKey)` synchronously; permission-denied hint shown inline; Escape closes. |
| `apps/pwa/src/components/PermissionDeniedBanner.tsx` | OS-revoked persistent banner | VERIFIED | `role="alert"`; shows only when `permission==='denied' && wasEnabled`; OS-specific instruction sheet (iOS 4-step / Android 4-step); "How to enable" button opens it. |
| `apps/api/src/db/schema.ts` | pushSubscriptions table + calendarEvents.title | VERIFIED | `pushSubscriptions` mysqlTable with FK cascade, unique endpoint, userId index. `calendarEvents.title: varchar('title',{length:500})`. |
| `apps/api/src/db/migrations/0003_same_xavin.sql` | CREATE TABLE push_subscriptions | VERIFIED | Exists; contains `CREATE TABLE \`push_subscriptions\`` + FK + index. |
| `apps/api/src/db/migrations/0004_mature_maximus.sql` | MODIFY COLUMN fixes for endpoint/p256dh lengths | VERIFIED | Contains `MODIFY COLUMN \`endpoint\` varchar(2048)` + `MODIFY COLUMN \`p256dh\` varchar(512)` (CR-02). |
| `apps/pwa/vite.config.ts` | injectManifest strategy | VERIFIED | `strategies: 'injectManifest'`; denylist preserved in sw.ts. |
---
## Key Link Verification
| From | To | Via | Status | Details |
|------|----|-----|--------|---------|
| `poller.ts` | `eventChangeDispatcher.ts` | `onChanges` callback → `dispatchEventChange(change, cred.userId)` | WIRED | `poller.ts:70-79` passes the callback; actor = credential owner. |
| `outboxWorker.ts` | `eventChangeDispatcher.ts` | `triggerTargetedResync` → `onChanges` → `dispatchEventChange(change, userId)` | WIRED | `outboxWorker.ts:174-183`; actor = writing member. |
| `lists.ts` | `listChangeDispatcher.ts` | `notifyListChange(listId, currentUserId)` at item-add/check/delete/rename/list-delete | WIRED | Lines 388, 437, 505, 652, 706; position-only PATCH guarded at line 651. |
| `listChangeDispatcher.ts` | `pushCoalescer.ts` | `coalesceListPush(listId, actorId, dispatch, windowMs)` | WIRED | `listChangeDispatcher.ts:39`. |
| `usePushSubscription.ts` | `/api/push/subscription` | `fetch POST sub.toJSON()` inside `subscribe()` | WIRED | `usePushSubscription.ts:225-233`. |
| `index.ts` | `webpush.setVapidDetails` | `isMainModule()` guard, before `startReminderScheduler()` | WIRED | `index.ts:120`. |
| `sw.ts` | `showNotification` | `event.waitUntil(...)` in push handler | WIRED | `sw.ts:124`. |
| `AppNav.tsx` | `SettingsSheet.tsx` | `onOpenSettings` prop → sets `settingsOpen=true` in `App.tsx` | WIRED | `AppNav.tsx:91, 214`; `App.tsx:46,57`. |
| `reminderScheduler.ts` | `dispatchPush` | `dispatchPush(sub, notification)` per subscription in event loop | WIRED | `reminderScheduler.ts:147`. |
---
## Data-Flow Trace (Level 4)
| Artifact | Data Variable | Source | Produces Real Data | Status |
|----------|--------------|--------|-------------------|--------|
| `reminderScheduler.ts` | `rows` (events × subs) | `db.select().from(calendarEvents).innerJoin(calendars).innerJoin(pushSubscriptions).where(isShared+allDay+window)` | Yes — live DB query | FLOWING |
| `eventChangeDispatcher.ts` | `allSubs` (push_subscriptions) | `db.select().from(pushSubscriptions).where(ne(userId, actorId))` | Yes | FLOWING |
| `listChangeDispatcher.ts` | `subs` (push_subscriptions for audience) | `db.select().from(pushSubscriptions).where(inArray(userId, audienceIds))` | Yes | FLOWING |
| `sync.ts` | `titleValue` | `vevent.getFirstPropertyValue('summary')` from parsed ICAL | Yes — per-sync from VEVENT SUMMARY | FLOWING |
| `usePushSubscription.ts` | `isSubscribed` | `registration.pushManager.getSubscription()` (mount health-check) | Yes — live browser Push API | FLOWING |
---
## Behavioral Spot-Checks
| Behavior | Command | Result | Status |
|----------|---------|--------|--------|
| `dispatchPush` deletes on 410 | `vitest run tests/lib/pushDispatcher.test.ts` (test suite known-passing) | Green per orchestrator (213-214 API tests pass) | PASS |
| `coalesceListPush` collapses burst | `vitest run tests/lib/pushCoalescer.test.ts` | Green per orchestrator | PASS |
| `reminderScheduler` shared/timed filter | `vitest run tests/broker/reminderScheduler.test.ts` | Green per orchestrator | PASS |
| `isMeaningfulChange` description-only silent | `vitest run tests/lib/eventChangeDispatcher.test.ts` | Green per orchestrator | PASS |
| Push subscription POST/DELETE/vapid-key API | `vitest run tests/routes/push.test.ts` | Green per orchestrator | PASS |
| notifyListChange not called on position PATCH | `lists.ts:651` guard verified in code | `if (patch.position === undefined)` before `notifyListChange` | PASS |
| PWA builds with sw.js | `pnpm --filter @familysync/pwa build` | Green per orchestrator | PASS |
| typecheck passes (api + pwa) | `pnpm --filter @familysync/api typecheck && pnpm --filter @familysync/pwa typecheck` | Green per orchestrator | PASS |
---
## Requirements Coverage
| Requirement | Source Plans | Description | Status | Evidence |
|-------------|-------------|-------------|--------|----------|
| NOTIF-01 | 05-01, 05-06, 05-07 | User receives a Web Push reminder before an event starts | SATISFIED | `reminderScheduler.ts` scans shared timed events in [now+14m, now+16m]; title from `calendarEvents.title` (populated by `sync.ts`); dispatched via `dispatchPush` to all member subscriptions. D-05: shared-calendar-only by design. |
| NOTIF-02 | 05-01, 05-03, 05-05 | User receives a Web Push alert when the other member changes a shared list | SATISFIED | `listChangeDispatcher.ts` → `pushCoalescer.ts` → `dispatchPush`; hooked at all meaningful list/item mutations in `lists.ts`; reorder excluded; actor self-suppressed; access-scoped to owner list_shares. |
| NOTIF-03 | 05-01, 05-02, 05-07 | User receives a Web Push alert when an event is added or changed | SATISFIED | `eventChangeDispatcher.ts` (`isMeaningfulChange` + `dispatchEventChange`); `sync.ts` diff + `onChanges`; consumed by `poller.ts` (external changes) + `outboxWorker.ts` (this-member writes); description-only silent (D-04); actor excluded (D-03). |
All three NOTIF requirements are mapped and implemented. No orphaned requirements for Phase 5.
---
## Anti-Patterns Found
No `TBD`, `FIXME`, or `XXX` markers in any phase-5 modified file. No stub patterns (`return null` / `return []` / `return {}` as rendering stubs) in implementation files. One legitimate early-return pattern (`if (!listRow[0]) return` in `listChangeDispatcher.ts`) is a correct null-safety guard, not a stub.
No blockers.
---
## Human Verification Required
### 1. iOS PWA install + notification permission grant
**Test:** Add FamilySync to Home Screen on an iOS 16.4+ device. Open the installed PWA. Confirm the "Stay in the loop" permission prompt appears. Tap "Enable Notifications". Confirm the OS permission dialog fires (not NotAllowedError). After granting, confirm a `push_subscriptions` row exists for the user in the DB.
**Expected:** Row present; no error; prompt closes.
**Why human:** iOS-Safari standalone install + push subscribe is device-only. playwright-cli cannot drive the iOS Home Screen install flow.
### 2. iOS 15-minute reminder delivery
**Test:** Create a shared Family-calendar timed event starting 15 minutes from now. Wait. Confirm a push notification appears on the iOS lock screen.
**Expected:** Notification appears within ~1 minute of the event start window, titled with the event name and "Starts in 15 min".
**Why human:** Requires physical iOS device, Home Screen install, real push delivery through APNs.
### 3. iOS gesture gate validation (NEW-CR-01)
**Test:** On iOS in the installed PWA, tap "Enable Notifications" in the permission prompt AND via the Settings sheet toggle. Confirm neither path produces `NotAllowedError`.
**Expected:** Both paths complete without error. Code review confirmed zero awaits before `pushManager.subscribe()` in both `PushPermissionPrompt.tsx` (handleEnableClick) and `SettingsSheet.tsx` (handleToggle).
**Why human:** NotAllowedError on the iOS gesture gate is a runtime iOS-Safari behavior, not verifiable in Chromium.
### 4. iOS subscription health-check (D-10 / success criterion 4)
**Test:** Subscribe on iOS. Clear the push subscription from browser settings (or wait for iOS to expire it). Open the app again. Confirm the subscription is silently re-established without user action (check `push_subscriptions` row in DB).
**Expected:** Row is present after the app re-opens; no OS permission dialog appeared.
**Why human:** Requires a physical iOS device and time (or manual SW subscription deletion). Silent re-subscribe behavior is browser Push API.
### 5. Multi-device push delivery (end-to-end NOTIF-02 / NOTIF-03)
**Test:** With two subscribed devices (or one device + one browser session), have member A modify a shared list item. Within 45 seconds, confirm member B receives a single coalesced push notification naming the actor and list. Separately, have member A add a calendar event; confirm member B receives an event-change push within the next 5-minute poll cycle.
**Expected:** One push (not N) for the list burst; one push for the calendar change; actor is never notified of their own changes.
**Why human:** Requires two real subscribed sessions; multi-device delivery through APNs/FCM cannot be simulated by playwright-cli.
---
## Gaps Summary
No gaps. All must-have truths are verified in the codebase. The phase goal is fully implemented. Human verification items are required for device-level delivery confirmation (iOS/Android) — these are classified as `human_needed` per CLAUDE.md and the known-context `<files_to_read>` guidance, not as gaps.
**The code is complete and correct. Delivery to real devices is the open question.**
---
_Verified: 2026-06-09T14:00:00Z_
_Verifier: Claude (gsd-verifier)_