From f07c85d0c9beb1f0b010391fd0bd18e4bf6e8783 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Tue, 9 Jun 2026 21:00:22 -0400 Subject: [PATCH] docs(05-03): complete pushCoalescer TDD plan --- .planning/ROADMAP.md | 6 +- .planning/STATE.md | 10 +- .../05-03-SUMMARY.md | 112 ++++++++++++++++++ 3 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 .planning/phases/05-web-push-notifications/05-03-SUMMARY.md diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 9e20916..65a1e64 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -194,7 +194,7 @@ Plans: **Wave 2** *(blocked on Wave 1)* - [x] 05-02-PLAN.md — TDD: pushDispatcher (VAPID send + dual-format payload + 410/404 prune) (D-11) -- [ ] 05-03-PLAN.md — TDD: pushCoalescer (per-list/actor debounce, generic copy, self-suppress) (D-01/D-02/D-03) +- [x] 05-03-PLAN.md — TDD: pushCoalescer (per-list/actor debounce, generic copy, self-suppress) (D-01/D-02/D-03) **Wave 3** *(blocked on Wave 2)* @@ -246,7 +246,7 @@ Note: Phase 4 depends only on Phase 1 and can begin as soon as Phase 1 is comple | 2. Calendar Display | 5/5 | Complete | 2026-06-05 | | 3. Event Write-Back + PWA Install | 12/12 | Complete | 2026-06-07 | | 4. Shared Lists + Live Sync | 6/6 | Complete | 2026-06-09 | -| 5. Web Push Notifications | 2/8 | In Progress| | +| 5. Web Push Notifications | 3/8 | In Progress| | | 6. UX Polish | 0/? | Not started | - | ## Backlog @@ -255,7 +255,7 @@ Note: Phase 4 depends only on Phase 1 and can begin as soon as Phase 1 is comple **Goal:** [Captured for future planning] Abstract the calendar backend behind a provider interface so Fastmail/CalDAV is one implementation among potentially many. Shipping with a single provider is fine, but the broker, sync, and event-expansion layers should be structured so additional providers (e.g. other CalDAV hosts, Google Calendar, generic ICS feeds) can be added without rework. Captures the "provider" seam as an explicit architectural concern. **Requirements:** TBD -**Plans:** 2/8 plans executed +**Plans:** 3/8 plans executed Plans: diff --git a/.planning/STATE.md b/.planning/STATE.md index 50fe683..cf6a03b 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -4,13 +4,13 @@ milestone: v1.0 milestone_name: milestone status: executing stopped_at: Completed 05-02-PLAN.md -last_updated: "2026-06-10T00:55:56.184Z" +last_updated: "2026-06-10T01:00:12.755Z" last_activity: 2026-06-10 -- Phase 05 execution started progress: total_phases: 15 completed_phases: 4 total_plans: 36 - completed_plans: 30 + completed_plans: 31 percent: 27 --- @@ -26,7 +26,7 @@ See: .planning/PROJECT.md (updated 2026-06-07) ## Current Position Phase: 05 (web-push-notifications) — EXECUTING -Plan: 3 of 8 +Plan: 4 of 8 Status: Ready to execute Last activity: 2026-06-10 -- Phase 05 execution started @@ -67,6 +67,7 @@ Progress: [██████████] 100% | Phase 04 P07 | 6 | 2 tasks | 4 files | | Phase 05 P01 | 20 | 4 tasks | 15 files | | Phase 05 P02 | 5 | 1 tasks | 1 files | +| Phase 05 P03 | 5 | - tasks | - files | ## Accumulated Context @@ -104,6 +105,7 @@ Recent decisions affecting current work: - [Phase 04-07]: D-04-07-guard: isShared owner-only guard placed after access check, before updateValues construction; mirrors DELETE handler idiom (if !access.isOwner → 403) - [Phase ?]: VAPID config is env-injected at runtime via docker-compose.yml environment block; no key baked into image (Phase 5 D-transposability) - [Phase ?]: dispatchPush uses sub.id (not a separate dbRowId argument) — 2-arg signature matches existing test +- [Phase ?]: coalesceListPush dispatch signature is (listId, actorId, count) — test scaffold canonical; richer payload deferred to Plan 05-05 caller ### Roadmap Evolution @@ -145,6 +147,6 @@ Recent decisions affecting current work: ## Session Continuity -Last session: 2026-06-10T00:55:56.178Z +Last session: 2026-06-10T01:00:09.552Z Stopped at: Completed 05-02-PLAN.md Resume file: None diff --git a/.planning/phases/05-web-push-notifications/05-03-SUMMARY.md b/.planning/phases/05-web-push-notifications/05-03-SUMMARY.md new file mode 100644 index 0000000..52bbe5c --- /dev/null +++ b/.planning/phases/05-web-push-notifications/05-03-SUMMARY.md @@ -0,0 +1,112 @@ +--- +phase: 05-web-push-notifications +plan: 03 +subsystem: api/push-coalescer +tags: [web-push, coalescer, debounce, tdd, red-green, D-01, D-03] +dependency_graph: + requires: [05-01, 05-02] + provides: [coalesceListPush — per-(list,actor) sliding debounce] + affects: [apps/api/src/lib/pushCoalescer.ts] +tech_stack: + added: [] + patterns: [module-level Map singleton (listEmitter.ts idiom), sliding debounce setTimeout, injected dispatch for testability] +key_files: + created: + - apps/api/src/lib/pushCoalescer.ts + modified: + - apps/api/tests/lib/pushCoalescer.test.ts +decisions: + - "dispatch signature is (listId, actorId, count) — matches existing RED scaffold; richer payload shape deferred to caller (Plan 05-05)" + - "key is ${listId}:${actorId} — per-(list,actor) matches D-01 intent; allows two members editing same list to coalesce independently" + - "sliding debounce (each call resets timer) — per plan spec; leading debounce not used" + - "dispatch return value is a Promise; errors caught and logged inside fire() so caller loop never breaks" +metrics: + duration: 5 + completed_date: "2026-06-10" + tasks_completed: 2 + files_changed: 2 +--- + +# Phase 05 Plan 03: pushCoalescer — per-(list,actor) debounce — Summary + +TDD RED→GREEN: `pushCoalescer.ts` implemented; per-(list,actor) sliding debounce collapses list-change bursts into a single dispatch call carrying (listId, actorId, count). + +## Tasks Executed + +### Task 1: RED — fix lint warning, add actorId assertion + +**Status:** Completed. Commit: `7af827a` + +The existing RED scaffold in `apps/api/tests/lib/pushCoalescer.test.ts` (from Plan 05-01) had a lint warning: `calledActorId` was destructured in test 1 but never asserted. Added `expect(calledActorId).toBe(actorId)` to make the self-suppression assertion explicit in the burst-coalescing test as well (not only in the dedicated D-03 test). + +Tests still fail after this change (RED preserved): `Cannot find module '.../pushCoalescer.js'`. + +### Task 2: GREEN — implement pushCoalescer.ts + +**Status:** Completed. Commit: `c1758de` + +Created `apps/api/src/lib/pushCoalescer.ts`: + +**`coalesceListPush(listId, actorId, dispatch, windowMs=45000)`** +- Module-level `Map` keyed by `${listId}:${actorId}` +- First call in a burst: inserts entry with count=1, starts `setTimeout(windowMs)` +- Subsequent calls within window: `clearTimeout`, increments count, resets timer (sliding debounce) +- On timer fire: deletes map entry, calls `dispatch(listId, actorId, count)` — self-deleting entries keep the map bounded (T-05-08) +- dispatch errors caught and logged with `[pushCoalescer]` prefix; never throws to caller + +## Verification + +``` +pnpm --filter @familysync/api exec vitest run tests/lib/pushCoalescer.test.ts + + Test Files 1 passed (1) + Tests 3 passed (3) +``` + +`pnpm --filter @familysync/api typecheck` — passes. + +## TDD Gate Compliance + +- RED: `test(05-03): add actorId assertion in burst test — fix unused var lint warning` — 7af827a +- GREEN: `feat(05-03): implement pushCoalescer — per-(list,actor) sliding debounce (D-01/D-03)` — c1758de + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Lint warning — unused `calledActorId` in burst test** +- **Found during:** Task 1 (RED) +- **Issue:** `calledActorId` was destructured in test 1 but the assertion was missing, producing an unused-variable lint warning. +- **Fix:** Added `expect(calledActorId).toBe(actorId)` — the burst test now also asserts self-suppression, not just the dedicated D-03 test. +- **Files modified:** apps/api/tests/lib/pushCoalescer.test.ts +- **Commit:** 7af827a + +### Dispatch signature simplification + +The plan's `` section describes `dispatch(payload, actorId)` where payload is a rich object `{title, body, tag, navigate}`. The existing RED scaffold (committed in Plan 05-01) uses `dispatch(listId, actorId, count)` — a simpler 3-argument form that defers notification copy construction to the caller. + +The test is canonical; the implementation matches the test. The richer payload construction (D-02 generic copy: `"${actorName} updated ${listName}"`, `"${N} change(s)"`) is owned by the caller in Plan 05-05, which has the actorName/listName context from the DB row and passes a closure over `dispatchPush`. + +## Known Stubs + +None. The coalescer is complete and testable. Plan 05-05 wires it into the list-change fan-out with actual notification copy. + +## Threat Flags + +No new threat surface. `pushCoalescer.ts` is a pure in-memory utility module — no network endpoints, no auth paths, no file access. + +T-05-06 (generic copy — no item text): mitigated by design — the coalescer passes only count, not item text; copy construction in Plan 05-05 will follow D-02. +T-05-07 (self-notification): mitigated — `actorId` threaded to dispatch so caller can apply `WHERE userId != actorId`. +T-05-08 (unbounded map): accepted — entries self-delete on timer fire; two-person household keeps keys bounded. + +## Self-Check + +**Files verified:** +- [x] apps/api/src/lib/pushCoalescer.ts — exists +- [x] apps/api/tests/lib/pushCoalescer.test.ts — modified + +**Commits verified:** +- 7af827a: test(05-03): add actorId assertion in burst test — fix unused var lint warning (RED gate) +- c1758de: feat(05-03): implement pushCoalescer — per-(list,actor) sliding debounce (D-01/D-03) (GREEN gate) + +## Self-Check: PASSED