--- phase: 05-web-push-notifications plan: 05 subsystem: api/list-change-dispatcher tags: [web-push, notif-02, list-change, coalescer, tdd, red-green, D-01, D-02, D-03] dependency_graph: requires: [05-02, 05-03, 05-04] provides: [notifyListChange — access-scoped, self-suppressed, coalesced list-change push] affects: - apps/api/src/lib/listChangeDispatcher.ts - apps/api/src/routes/lists.ts - apps/api/tests/lib/listChangeDispatcher.test.ts - apps/api/tests/routes/lists.test.ts tech_stack: added: [] patterns: - real-timer + pollUntil polling for async DB assertions (avoids fake-timer + real-I/O mismatch) - vi.doMock + vi.resetModules per-test pattern (fresh mock instances for each test) - windowMs optional param for testability (coalescer window override in tests) key_files: created: - apps/api/src/lib/listChangeDispatcher.ts - apps/api/tests/lib/listChangeDispatcher.test.ts modified: - apps/api/src/routes/lists.ts - apps/api/tests/routes/lists.test.ts decisions: - "windowMs exposed as optional 3rd arg on notifyListChange for test-time override (avoids fake-timer/real-I/O race)" - "pollUntil() helper (inline, no test-library deps) replaces @testing-library/waitFor for async DB assertion polling" - "vi.doMock + vi.resetModules in beforeEach — required so each test gets a fresh vi.fn() mock instance for dispatchPush" - "DELETE /:id notifyListChange fires after DB delete — sendListChangePush handles missing list gracefully (early return)" - "List create (POST /) does NOT notify — empty list is not a change worth pinging (D-01 spirit)" metrics: duration: 8 completed_date: "2026-06-10" tasks_completed: 2 files_changed: 4 --- # Phase 05 Plan 05: listChangeDispatcher — NOTIF-02 List-Change Push — Summary TDD RED→GREEN: `listChangeDispatcher.ts` (notifyListChange) implemented; hooked into all meaningful list/item mutation points in `routes/lists.ts`; reorder (position) changes excluded; 64 tests GREEN. ## Tasks Executed ### Task 1: listChangeDispatcher — access-scoped, self-suppressed fan-out **Status:** Completed. **Commits:** - RED: `test(05-05): add failing tests for listChangeDispatcher — RED gate` — 97f7026 - GREEN: `feat(05-05): implement listChangeDispatcher — access-scoped, self-suppressed, coalesced push (NOTIF-02)` — 6923104 Created `apps/api/src/lib/listChangeDispatcher.ts` exporting `notifyListChange(listId, actorId, windowMs?)`: **`notifyListChange`** — wraps `coalesceListPush` with a dispatch closure that: 1. Resolves actor `displayName` and list `name` from DB in parallel 2. Builds audience: `{list owner} ∪ {list_shares.userId} MINUS actorId` (D-03) 3. Loads `push_subscriptions` for all audience members 4. Calls `dispatchPush(sub, notification)` per subscription — one failure never aborts the loop 5. D-02 generic copy: `"{Actor} made {N} changes to {ListName}"` — no item text **Threat mitigations:** - T-05-14: audience derived from list access (owner + list_shares only) — never all users - T-05-15: notification body carries actor name + count, no item text (D-02) - T-05-16: actorId filtered before audience union → actor's own subscriptions never dispatched (D-03) **Tests (5/5 GREEN):** - Burst coalescing: 3 rapid calls → 1 `dispatchPush` to non-actor with `count=3`, body contains actor name + "3" - D-03 self-suppression: actor-only list → 0 dispatches - T-05-14 access scoping: unrelated 3rd user (no owner/share) → never dispatched - Empty audience (no other members) → no dispatch, no crash - Empty audience (other has no subscription) → no dispatch, no crash ### Task 2: Hook notifyListChange into list/item mutations (reorder excluded) **Status:** Completed. **Commit:** `feat(05-05): hook notifyListChange into list/item mutations (reorder excluded)` — d2ce4e0 `apps/api/src/routes/lists.ts` updated — `notifyListChange` called (fire-and-forget) after each meaningful mutation: | Route | Mutation | Push? | |-------|----------|-------| | `POST /api/lists/:id/items` | Item added | YES | | `PATCH /api/list-items/:itemId` | checked/text change | YES | | `PATCH /api/list-items/:itemId` | position change (reorder) | **NO** (D-01) | | `DELETE /api/list-items/:itemId` | Item deleted | YES | | `PATCH /api/lists/:id` | List rename/sharing toggle | YES | | `DELETE /api/lists/:id` | List deleted | YES | | `POST /api/lists` | List created | **NO** (empty list, D-01 spirit) | Critical D-01 guard in `PATCH /list-items/:itemId`: ```typescript if (patch.position === undefined) { notifyListChange(item.listId, currentUserId) } ``` **New tests in lists.test.ts (2 tests):** - `PATCH { position }` (reorder) does NOT call `notifyListChange` — spy confirms 0 calls - `PATCH { checked: true }` DOES call `notifyListChange(listId, ownerId)` — spy confirms 1 call with correct args **Final test count:** 59/59 lists.test.ts + 5/5 listChangeDispatcher.test.ts = **64/64 GREEN** ## Verification ``` pnpm --filter @familysync/api exec vitest run tests/lib/listChangeDispatcher.test.ts tests/routes/lists.test.ts Test Files 2 passed (2) Tests 64 passed (64) ``` `pnpm --filter @familysync/api typecheck` — passes (no errors). ## Deviations from Plan ### Auto-fixed Issues **1. [Rule 1 - Bug] Fake timer + real DB I/O race condition in listChangeDispatcher tests** - **Found during:** Task 1 (GREEN phase, first test run) - **Issue:** `vi.useFakeTimers()` + `vi.runAllTimersAsync()` fires the coalescer timer but returns before the subsequent real DB queries (`sendListChangePush`) complete. This caused the "burst coalesces" and "access scoping" tests to fail (0 `dispatchPush` calls observed even though the logic was correct). - **Fix:** 1. Switched test approach to real timers (no `vi.useFakeTimers`) with a tiny `windowMs=10ms` passed to `notifyListChange`. 2. Added optional `windowMs` parameter to `notifyListChange` (defaults to `undefined`, which passes through to `coalesceListPush`'s 45s default) — test-only override. 3. Added inline `pollUntil()` helper (no `@testing-library/waitFor` dependency) that polls a predicate until it passes or a 3s timeout. - **Files modified:** `apps/api/src/lib/listChangeDispatcher.ts`, `apps/api/tests/lib/listChangeDispatcher.test.ts` - **Commit:** 6923104 **2. [Rule 1 - Bug] `vi.mock()` top-level hoisted mock lost after `vi.resetModules()`** - **Found during:** Task 1 (first test run attempt with top-level `vi.mock`) - **Issue:** Top-level `vi.mock('../../src/lib/pushDispatcher.js', ...)` is hoisted before each test file execution, but `vi.resetModules()` in `beforeEach` clears the module registry. When tests dynamically imported `listChangeDispatcher.js`, the fresh load of `pushDispatcher.js` bypassed the mock factory. - **Fix:** Removed top-level `vi.mock`; used `vi.doMock` inside `beforeEach` (after `vi.resetModules`) so each test's dynamic import of `listChangeDispatcher.js` gets a fresh mocked `pushDispatcher.js`. - **Files modified:** `apps/api/tests/lib/listChangeDispatcher.test.ts` - **Commit:** 6923104 ## Known Stubs None. `notifyListChange` is fully wired end-to-end. Push dispatch will fail with a logged error if VAPID keys are malformed (pre-existing infra issue from Plan 05-04, not a stub). ## Threat Flags No new threat surface beyond what the plan's threat model covers. All three threats mitigated: | Threat | Status | |--------|--------| | T-05-14: Info disclosure — push to non-member | Mitigated — audience = owner ∪ list_shares only | | T-05-15: Info disclosure — item text in payload | Mitigated — D-02 generic copy only | | T-05-16: Spoofing — actor notified of own change | Mitigated — D-03 excludeUserId = actorId | ## Self-Check **Files created/verified:** - [x] apps/api/src/lib/listChangeDispatcher.ts — exists (min_lines: 25 ✓, ~110 lines) - [x] apps/api/tests/lib/listChangeDispatcher.test.ts — exists **Key links verified:** - [x] apps/api/src/routes/lists.ts imports and calls `notifyListChange` at 5 mutation sites - [x] apps/api/src/lib/listChangeDispatcher.ts calls `coalesceListPush` from `pushCoalescer.ts` **Commits verified:** - 97f7026: test(05-05): add failing tests for listChangeDispatcher — RED gate - 6923104: feat(05-05): implement listChangeDispatcher — VAPID send + access-scoped, self-suppressed, coalesced push (NOTIF-02) - d2ce4e0: feat(05-05): hook notifyListChange into list/item mutations (reorder excluded) ## TDD Gate Compliance - RED: `test(05-05): add failing tests for listChangeDispatcher — RED gate` — 97f7026 - GREEN: `feat(05-05): implement listChangeDispatcher...` — 6923104 ## Self-Check: PASSED