docs(quick-260610-hbu): plan/summary/verification + STATE row (reminder resilience, Verified)
This commit is contained in:
Lucas Berger
+146
@@ -0,0 +1,146 @@
|
||||
---
|
||||
phase: quick-260610-hbu
|
||||
plan: 01
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
files_modified:
|
||||
- apps/api/src/broker/reminderScheduler.ts
|
||||
- apps/api/tests/broker/reminderScheduler.test.ts
|
||||
autonomous: true
|
||||
requirements: [PUSH-REMIND-RESILIENCE]
|
||||
must_haves:
|
||||
truths:
|
||||
- "A shared timed event whose start is in (now, now+16min] triggers a reminder push on the next scan, even if the cron tick at the ideal 15-min mark was missed/late (catch-up)."
|
||||
- "A given shared timed event fires EXACTLY ONCE across all scans while it sits in the catch-up window — no cross-tick double-fire."
|
||||
- "An event whose dtstart has already passed (dtstart <= now) does NOT trigger a reminder."
|
||||
- "The notification body reflects the actual lead time (e.g. 'Starts in 8 min'), guarded to a minimum of 1."
|
||||
- "Existing Phase-5 behaviours stay green: isShared-only (D-05), allDay excluded (D-07), always-visible notification (D-11), empty push_subscriptions -> zero sends/no crash (D-16), per-event and per-sub error isolation (T-05-17/18/19)."
|
||||
artifacts:
|
||||
- path: "apps/api/src/broker/reminderScheduler.ts"
|
||||
provides: "Resilient catch-up reminder scan with per-uid exactly-once dedup, in-memory (D-12)."
|
||||
contains: "runReminderCheck"
|
||||
- path: "apps/api/tests/broker/reminderScheduler.test.ts"
|
||||
provides: "Updated tests covering single-fire across consecutive ticks, missed-tick recovery, already-started exclusion, plus retained Phase-5 behaviours."
|
||||
contains: "missed"
|
||||
key_links:
|
||||
- from: "apps/api/src/broker/reminderScheduler.ts"
|
||||
to: "calendarEvents.dtstartUtc"
|
||||
via: "WHERE gt(now) AND lte(now+16min)"
|
||||
pattern: "gt\\(.*dtstartUtc"
|
||||
- from: "apps/api/src/broker/reminderScheduler.ts"
|
||||
to: "dispatchPush"
|
||||
via: "fan-out per subscription, then mark uid sent"
|
||||
pattern: "dispatchPush"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Make the Phase 5 reminder scheduler resilient to missed/late node-cron ticks (UAT Test 1, 2026-06-10). Replace the fixed `[now+14min, now+16min]` single-tick window with a catch-up scan `(now, now+16min]`, and replace per-`(uid, minuteBucket)` dedup with per-`uid` exactly-once dedup so a recovered reminder fires once, not 2-3 times.
|
||||
|
||||
Purpose: A scheduled 15-min reminder did NOT fire on a real iOS device because node-cron missed the tick during the 2-min window and the scan had no catch-up — the window slid past and the reminder was dropped permanently. The dispatch pipeline is PROVEN working (manual injected-time scan -> Apple 201 -> notification arrived). This is purely a scheduling-resilience defect.
|
||||
|
||||
Output: Updated `reminderScheduler.ts` (in-memory only, no schema/deps/Redis per D-12) and updated tests, both passing typecheck + vitest.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@$HOME/.claude/gsd-core/workflows/execute-plan.md
|
||||
@$HOME/.claude/gsd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@./CLAUDE.md
|
||||
@apps/api/src/broker/reminderScheduler.ts
|
||||
@apps/api/tests/broker/reminderScheduler.test.ts
|
||||
@apps/api/src/lib/pushDispatcher.ts
|
||||
@.planning/phases/05-web-push-notifications/05-UAT.md
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Make the reminder scan catch-up resilient with per-uid exactly-once dedup</name>
|
||||
<files>apps/api/src/broker/reminderScheduler.ts</files>
|
||||
<behavior>
|
||||
Target behaviours (proven by Task 2 tests):
|
||||
- Window: scan shared timed events whose `dtstartUtc` is in `(now, now+16min]` — strictly greater than `now` (future, excludes already-started) and at most 16 min out. A missed/late tick is recovered on the next run because the lower bound is `now`, not `now+14min`.
|
||||
- Dedup: keyed on `uid` ALONE. The same event fires EXACTLY ONCE no matter how many consecutive ticks it sits in the window.
|
||||
- Mark-after-dispatch (WR-01 preserved): mark the uid as sent only AFTER all dispatch attempts for that event complete, so a throwing dispatch does not pre-suppress retry (at-least-once delivery property).
|
||||
- Bounded memory / re-arm: track per-uid the event's `dtstart` (ms). Prune entries whose dtstart is in the past (`dtstart <= now`) — bounds growth and avoids permanently suppressing a future re-created uid.
|
||||
- Body: lead-accurate. Compute `minutes = Math.max(1, Math.round((dtstart - now) / 60000))` and set body to `` `Starts in ${minutes} min` ``.
|
||||
</behavior>
|
||||
<action>
|
||||
Edit apps/api/src/broker/reminderScheduler.ts in place. Keep the injectable `now = new Date()` signature on `runReminderCheck` (tests rely on it) and keep `startReminderScheduler` unchanged.
|
||||
|
||||
Query / window changes:
|
||||
- Replace `windowStart = now+14min` / `gte(dtstartUtc, windowStart)` with a strict lower bound on `now`: use Drizzle `gt(calendarEvents.dtstartUtc, now)`. Import `gt` from drizzle-orm and drop the now-unused `gte` import.
|
||||
- Keep `windowEnd = now + 16*60*1000` and `lte(calendarEvents.dtstartUtc, windowEnd)`.
|
||||
- Leave the rest of the query identical: innerJoin calendars, `eq(calendars.isShared, true)` (D-05 in QUERY, T-05-17), `eq(calendarEvents.allDay, false)` (D-07), cross-join ALL pushSubscriptions via `sql\`1=1\``, the same selected columns. Pass the raw `now` Date to `gt` (Drizzle/mysql2 binds Date params for datetime columns the same way the existing `lte` does).
|
||||
|
||||
Dedup changes:
|
||||
- Replace `const sentReminders = new Set<string>()` with `const sentReminders = new Map<string, number>()` mapping `uid -> dtstartMs`. Update the module header comment block: key is now the bare event `uid` (not `uid:minuteBucket`); value is the event's dtstart in ms used for pruning; "acceptable data loss on process restart" (D-12) still applies. Remove the `minuteBucket` variable entirely.
|
||||
- In the dispatch loop: `if (sentReminders.has(uid)) continue` (no minuteBucket). After the fan-out loop, `sentReminders.set(uid, event.dtstartUtc.getTime())` (mark-after-dispatch — WR-01).
|
||||
- Build the notification body from the actual lead: `const minutes = Math.max(1, Math.round((event.dtstartUtc.getTime() - now.getTime()) / 60000))` then `body: \`Starts in ${minutes} min\``. Keep title fallback (`event.title ?? uid`), `tag: \`reminder-${uid}\``, and `navigate` exactly as-is.
|
||||
|
||||
Pruning changes:
|
||||
- Replace the CR-01 minuteBucket-based prune block with: iterate `sentReminders` entries and `sentReminders.delete(uid)` for any entry whose stored dtstartMs is `<= now.getTime()` (event has started). Update the CR-01 comment to describe started-event pruning.
|
||||
|
||||
Comments / decisions to preserve and update: D-05, D-07, D-11, D-12, D-16, T-05-17, T-05-18, T-05-19, WR-01, CR-01 must remain referenced in the header/inline comments with their meanings updated for per-uid dedup and the catch-up window. Add a short inline CAVEAT comment near the dedup: if an event's dtstart is RESCHEDULED earlier after a reminder already fired, it will not re-fire — acceptable for v1.
|
||||
|
||||
Do NOT: add schema/migration changes, new dependencies, Redis, or DB persistence. Do NOT change pushDispatcher or its call signature. Do NOT place fenced code blocks anywhere — this is directive prose.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>pnpm --filter @familysync/api typecheck</automated>
|
||||
</verify>
|
||||
<done>reminderScheduler.ts compiles under strict TS; query uses `gt(dtstartUtc, now)` + `lte(dtstartUtc, now+16min)`; dedup is a `Map<string, number>` keyed by bare uid; body is lead-accurate; pruning drops started events; injectable `now` and `startReminderScheduler` unchanged; no new imports beyond `gt` (and `gte` removed).</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: Update tests for catch-up + single-fire + missed-tick recovery</name>
|
||||
<files>apps/api/tests/broker/reminderScheduler.test.ts</files>
|
||||
<behavior>
|
||||
New/updated assertions:
|
||||
- Single-fire across consecutive ticks: an event in the catch-up window fires EXACTLY ONCE when `runReminderCheck` is invoked on multiple consecutive minutes while the event still sits in `(now, now+16min]` (regression for the secondary cross-tick double-fire bug).
|
||||
- Missed-tick recovery (core Test 1 regression): skip the scan at the ideal 15-min mark; call `runReminderCheck` a few minutes later with the event now ~8 min out (still `> now`, `<= now+16min`) -> the reminder STILL fires.
|
||||
- Already-started exclusion: an event with `dtstart <= now` produces NO dispatch.
|
||||
- Retained behaviours stay green: isShared-only (D-05), allDay excluded (D-07), empty pushSubscriptions -> zero sends / no crash (D-16), per-sub error isolation, fan-out to all subs, WR-01 mark-after-dispatch.
|
||||
</behavior>
|
||||
<action>
|
||||
Edit apps/api/tests/broker/reminderScheduler.test.ts. Keep the existing vitest setup: `vi.useFakeTimers()` + `vi.resetModules()` in beforeEach, the `db.select` mock and `dispatchPush` mock, and the chained mock shape `from().innerJoin().innerJoin().where()` whose `.where()` resolves to the row array. The query shape is unchanged by Task 1, so the existing `makeSelectMock()` helper pattern remains valid — reuse it.
|
||||
|
||||
Because `sentReminders` is module-level state, each independent test must get a fresh module via the existing `vi.resetModules()` + dynamic `await import(...)` pattern already used. Keep importing `runReminderCheck`, `db`, `dispatchPush` inside each test after `resetModules`.
|
||||
|
||||
Rewrite/replace the dedup tests for per-uid semantics:
|
||||
- Replace the "(eventUid, minuteBucket) twice within the same run" test and the CR-01 "re-dispatch in next minute bucket" test (which asserted the OLD per-minuteBucket re-fire) with a SINGLE-FIRE-ACROSS-TICKS test: set an event row with `dtstartUtc` = now+15min and one subscriber; call `runReminderCheck(t0)`, then `runReminderCheck(t0 + 1 min)`, then `runReminderCheck(t0 + 2 min)` — each time re-mocking `db.select` (event still in window: 15, 14, 13 min out). Assert `dispatchPush` was called exactly ONCE total across all three runs.
|
||||
- Add a MISSED-TICK-RECOVERY test: event `dtstartUtc` fixed (e.g. `2026-06-15T10:15:00Z`). Do NOT run at the ideal mark. Call `runReminderCheck` with `now = 2026-06-15T10:07:00Z` (event 8 min out, still > now and <= now+16min). Assert `dispatchPush` fired (>= 1 call). This is the Test 1 regression.
|
||||
- Add an ALREADY-STARTED test: with the real query this is excluded by `gt(dtstartUtc, now)` in SQL, so the mocked `.where()` returns `[]` (mirror the existing all-day/non-shared "SQL filtered it out -> empty rows" pattern) and assert `dispatchPush` NOT called. Name it clearly (e.g. "does not dispatch for an event whose start has already passed (dtstart <= now)").
|
||||
- Update the WR-01 test description and any "minute bucket" wording to per-uid dedup language, but keep its assertion (second run in the same scan context does not re-dispatch).
|
||||
- Keep the all-day (D-07), non-shared (D-05) empty-result tests as-is. Update the file's top docblock to describe the catch-up `(now, now+16min]` window and per-uid exactly-once dedup instead of the old `[now+14,now+16]` / minuteBucket description.
|
||||
|
||||
Do NOT place fenced code blocks in this action prose. Do NOT introduce real DB or real push; keep everything mocked.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>pnpm --filter @familysync/api test</automated>
|
||||
</verify>
|
||||
<done>`pnpm --filter @familysync/api test` (vitest) passes green. The suite includes: single-fire-across-three-consecutive-ticks (exactly 1 dispatch), missed-tick-recovery (fires at 8-min lead), already-started exclusion (0 dispatches), plus retained D-05/D-07/D-16/WR-01/per-sub-isolation coverage.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
- `pnpm --filter @familysync/api typecheck` exits 0 (no TS errors in reminderScheduler.ts).
|
||||
- `pnpm --filter @familysync/api test` exits 0 with all reminderScheduler tests green, including the three new behaviours.
|
||||
- Manual grep sanity: `grep -n "gt(" apps/api/src/broker/reminderScheduler.ts` shows the `gt(...dtstartUtc, now)` lower bound; `grep -n "minuteBucket" apps/api/src/broker/reminderScheduler.ts` returns nothing (old per-bucket logic fully removed).
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- A shared timed event in `(now, now+16min]` fires a reminder on the next scan after a missed/late tick (catch-up) — Test 1 closed.
|
||||
- Each such event fires EXACTLY ONCE across consecutive ticks (cross-tick double-fire bug fixed).
|
||||
- Events with `dtstart <= now` never fire.
|
||||
- Notification body reflects actual lead time, guarded to >= 1 min.
|
||||
- All prior Phase-5 decisions/threat mitigations preserved (D-05/D-07/D-11/D-12/D-16, T-05-17/18/19, WR-01, CR-01).
|
||||
- In-memory only: no schema change, no new deps, no Redis/DB persistence. Only `reminderScheduler.ts` and its test touched.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/quick/260610-hbu-make-phase-5-reminder-scheduler-resilien/260610-hbu-SUMMARY.md` when done.
|
||||
</output>
|
||||
+145
@@ -0,0 +1,145 @@
|
||||
---
|
||||
phase: quick-260610-hbu
|
||||
plan: "01"
|
||||
subsystem: api/broker
|
||||
tags: [push-notifications, reminder-scheduler, resilience, dedup]
|
||||
dependency_graph:
|
||||
requires: []
|
||||
provides: [resilient-reminder-scan, per-uid-exactly-once-dedup]
|
||||
affects: [apps/api/src/broker/reminderScheduler.ts]
|
||||
tech_stack:
|
||||
added: []
|
||||
patterns: [catch-up-window, per-uid-dedup, mark-after-dispatch]
|
||||
key_files:
|
||||
created: []
|
||||
modified:
|
||||
- apps/api/src/broker/reminderScheduler.ts
|
||||
- apps/api/tests/broker/reminderScheduler.test.ts
|
||||
decisions:
|
||||
- "Window changed from [now+14min, now+16min] to (now, now+16min] for missed-tick catch-up"
|
||||
- "Dedup key changed from uid:minuteBucket (Set) to bare uid (Map<uid, dtstartMs>) for cross-tick exactly-once"
|
||||
- "Body changed from hardcoded 'Starts in 15 min' to lead-accurate 'Starts in N min'"
|
||||
- "CR-01 pruning changed from minuteBucket-age to started-event (dtstartMs <= now)"
|
||||
metrics:
|
||||
duration: "~15min"
|
||||
completed: "2026-06-10"
|
||||
tasks_completed: 2
|
||||
files_modified: 2
|
||||
---
|
||||
|
||||
# Phase quick-260610-hbu Plan 01: Reminder Scheduler Resilience Summary
|
||||
|
||||
**One-liner:** Catch-up scan window `(now, now+16min]` with per-uid `Map` dedup replaces fixed `[now+14, now+16]` + minuteBucket `Set`, closing the missed-tick reminder drop (UAT Test 1).
|
||||
|
||||
## Tasks Completed
|
||||
|
||||
| Task | Name | Commit | Files |
|
||||
|------|------|--------|-------|
|
||||
| 1 | Catch-up window + per-uid dedup in reminderScheduler.ts | 3fdb242 | apps/api/src/broker/reminderScheduler.ts |
|
||||
| 2 | Update tests for catch-up + single-fire + missed-tick | 93bb2c1 | apps/api/tests/broker/reminderScheduler.test.ts |
|
||||
|
||||
## Verification Output
|
||||
|
||||
### pnpm --filter @familysync/api typecheck
|
||||
|
||||
```
|
||||
$ tsc --noEmit
|
||||
(exit 0 — no output)
|
||||
```
|
||||
|
||||
### pnpm --filter @familysync/api test (reminderScheduler only)
|
||||
|
||||
```
|
||||
RUN v4.1.8 /home/luc/Projects/familysync/apps/api
|
||||
|
||||
Test Files 1 passed (1)
|
||||
Tests 10 passed (10)
|
||||
Start at 12:34:43
|
||||
Duration 1.01s (transform 181ms, setup 647ms, import 74ms, tests 114ms, environment 0ms)
|
||||
```
|
||||
|
||||
### pnpm --filter @familysync/api test (full suite)
|
||||
|
||||
```
|
||||
Test Files 4 failed | 19 passed (23)
|
||||
Tests 68 failed | 166 passed (234)
|
||||
Duration 22.54s
|
||||
```
|
||||
|
||||
The 4 failing test files and 68 failing tests are all pre-existing `ECONNREFUSED 127.0.0.1:3306` DB-integration failures (MariaDB not running locally). None are related to reminderScheduler. The reminderScheduler file is tested exclusively through mocked DB calls and is not among the failures.
|
||||
|
||||
### Grep sanity
|
||||
|
||||
```
|
||||
grep -n "gt(" apps/api/src/broker/reminderScheduler.ts
|
||||
103: gt(calendarEvents.dtstartUtc, now),
|
||||
|
||||
grep -n "minuteBucket" apps/api/src/broker/reminderScheduler.ts
|
||||
42: // Key: event uid (bare string — no minuteBucket suffix).
|
||||
142: // Per-uid exactly-once dedup (D-12): keyed on bare uid, no minuteBucket.
|
||||
(no variable named minuteBucket remains)
|
||||
```
|
||||
|
||||
## What Changed
|
||||
|
||||
### Task 1 — reminderScheduler.ts
|
||||
|
||||
**Query window:** `gte(dtstartUtc, windowStart)` where `windowStart = now+14min` replaced with `gt(dtstartUtc, now)`. Upper bound `lte(dtstartUtc, windowEnd)` where `windowEnd = now+16min` unchanged. Import `gte` dropped; `gt` added.
|
||||
|
||||
**Dedup store:** `Set<string>` keyed `uid:minuteBucket` replaced with `Map<string, number>` keyed `uid` (value = dtstartMs). The `minuteBucket` variable is removed entirely.
|
||||
|
||||
**Dispatch loop check:** `sentReminders.has(uid:minuteBucket)` replaced with `sentReminders.has(uid)`. WR-01 mark-after-dispatch preserved: `sentReminders.set(uid, dtstartMs)` after fan-out loop.
|
||||
|
||||
**Notification body:** Hardcoded `'Starts in 15 min'` replaced with `\`Starts in ${minutes} min\`` where `minutes = Math.max(1, Math.round((dtstartMs - now) / 60000))`.
|
||||
|
||||
**CR-01 pruning:** Stale minuteBucket iteration replaced with started-event pruning: delete entries whose stored `dtstartMs <= now.getTime()`.
|
||||
|
||||
### Task 2 — reminderScheduler.test.ts
|
||||
|
||||
New tests added:
|
||||
- **SINGLE-FIRE across 3 consecutive ticks:** 3 calls to `runReminderCheck` with event still in window; asserts `dispatchPush` called exactly once.
|
||||
- **MISSED-TICK-RECOVERY:** No scan at the ideal 15-min mark; call at 8 min before start; asserts dispatch fires.
|
||||
- **ALREADY-STARTED:** dtstart <= now excluded by SQL `gt`; mocked as empty rows; 0 dispatches.
|
||||
- **D-16:** Empty subscriptions row set; zero sends, no crash.
|
||||
- **T-05-19:** Per-subscription error isolation; first sub throws, second still dispatched.
|
||||
- **Fan-out:** 2 subscriber rows for one event uid; 2 dispatches.
|
||||
|
||||
Old minuteBucket tests removed:
|
||||
- `(eventUid, minuteBucket) twice within the same run` — replaced by per-uid SINGLE-FIRE test.
|
||||
- `CR-01: re-dispatch in next minute bucket` — replaced by CR-01 started-event pruning test.
|
||||
|
||||
Retained:
|
||||
- D-07 all-day exclusion test.
|
||||
- D-05 non-shared exclusion test.
|
||||
- WR-01 mark-after-dispatch test (updated to per-uid dedup language).
|
||||
|
||||
## Decisions Made
|
||||
|
||||
1. **Window: open lower bound on `now`** — `gt(now)` instead of `gte(now+14min)` gives the catch-up property: any scan while the event is still in the future finds it, regardless of when the cron last fired.
|
||||
|
||||
2. **Dedup: uid-only Map** — Removing the minuteBucket suffix from the key means a recovered (late) scan on the same event does not see a new key and does not double-fire. The Map value (dtstartMs) is used only for CR-01 pruning, not for keying.
|
||||
|
||||
3. **CR-01 pruning on started-event, not stale-bucket** — With per-uid dedup there is no bucket concept. Pruning on `dtstartMs <= now` achieves the same bounded-growth goal and is semantically cleaner: an event that has started will never re-enter `(now, now+16min]`.
|
||||
|
||||
4. **Body is lead-accurate** — `Math.max(1, ...)` guards against a 0-min display if dispatch runs extremely close to dtstart.
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
None — plan executed exactly as written.
|
||||
|
||||
## Known Stubs
|
||||
|
||||
None.
|
||||
|
||||
## Threat Flags
|
||||
|
||||
None — no new network endpoints, auth paths, or schema changes introduced.
|
||||
|
||||
## Self-Check
|
||||
|
||||
- [x] `apps/api/src/broker/reminderScheduler.ts` exists and compiles (typecheck exit 0)
|
||||
- [x] `apps/api/tests/broker/reminderScheduler.test.ts` exists and passes (10/10)
|
||||
- [x] Commit 3fdb242 exists
|
||||
- [x] Commit 93bb2c1 exists
|
||||
|
||||
## Self-Check: PASSED
|
||||
+98
@@ -0,0 +1,98 @@
|
||||
---
|
||||
phase: quick-260610-hbu
|
||||
verified: 2026-06-10T12:39:30Z
|
||||
status: passed
|
||||
score: 5/5 must-haves verified
|
||||
overrides_applied: 0
|
||||
---
|
||||
|
||||
# Quick Task 260610-hbu: Reminder Scheduler Resilience Verification Report
|
||||
|
||||
**Task Goal:** Make the Phase 5 reminder scheduler resilient to missed/late cron ticks — catch-up scan + per-uid exactly-once dedup
|
||||
**Verified:** 2026-06-10T12:39:30Z
|
||||
**Status:** passed
|
||||
**Re-verification:** No — initial verification
|
||||
|
||||
---
|
||||
|
||||
## Goal Achievement
|
||||
|
||||
### Observable Truths
|
||||
|
||||
| # | Truth | Status | Evidence |
|
||||
|---|-------|--------|---------|
|
||||
| 1 | A shared timed event in (now, now+16min] fires on the next scan even after a missed/late tick (catch-up). | VERIFIED | `gt(calendarEvents.dtstartUtc, now)` at line 103; lower bound is `now`, not `now+14min`. Any scan while event is still future will find it. MISSED-TICK-RECOVERY test fires at 8-min lead. |
|
||||
| 2 | A given event fires EXACTLY ONCE across all scans while in the catch-up window — no cross-tick double-fire. | VERIFIED | `sentReminders` is `Map<string, number>` keyed on bare uid (line 47); `if (sentReminders.has(uid)) continue` at line 145. SINGLE-FIRE test asserts `dispatchPush` called exactly once across 3 consecutive ticks. |
|
||||
| 3 | An event whose dtstart has already passed (dtstart <= now) does NOT trigger a reminder. | VERIFIED | `gt(calendarEvents.dtstartUtc, now)` at line 103 excludes already-started events at the DB level. "already-started" test mocks empty rows, asserts 0 dispatches. |
|
||||
| 4 | Notification body reflects actual lead time (e.g. "Starts in 8 min"), guarded to minimum 1. | VERIFIED | Line 150: `const minutes = Math.max(1, Math.round((event.dtstartUtc.getTime() - now.getTime()) / 60000))` then `body: \`Starts in ${minutes} min\`` at line 156. Not hardcoded. |
|
||||
| 5 | Existing Phase-5 behaviours stay green: isShared-only (D-05), allDay excluded (D-07), always-visible notification (D-11), empty push_subscriptions -> zero sends/no crash (D-16), per-event and per-sub error isolation (T-05-18/T-05-19). | VERIFIED | All present in source (lines 101-102 for D-05/D-07; line 98 for D-11 via dispatchPush; lines 141-186 for T-05-18/19 try/catch isolation). Tests for D-05, D-07, D-16, T-05-19 all pass (10/10). |
|
||||
|
||||
**Score:** 5/5 truths verified
|
||||
|
||||
---
|
||||
|
||||
### Required Artifacts
|
||||
|
||||
| Artifact | Expected | Status | Details |
|
||||
|----------|----------|--------|---------|
|
||||
| `apps/api/src/broker/reminderScheduler.ts` | Resilient catch-up scan with per-uid exactly-once dedup, `runReminderCheck` exported | VERIFIED | 213 lines; contains `runReminderCheck`, `startReminderScheduler`; `Map<string, number>` dedup; `gt` lower bound; lead-accurate body |
|
||||
| `apps/api/tests/broker/reminderScheduler.test.ts` | Tests covering single-fire, missed-tick recovery, already-started exclusion, plus retained Phase-5 behaviours | VERIFIED | 409 lines; contains "SINGLE-FIRE", "MISSED-TICK-RECOVERY", "already-started", D-05/D-07/D-16/WR-01/CR-01/T-05-19 tests |
|
||||
|
||||
---
|
||||
|
||||
### Key Link Verification
|
||||
|
||||
| From | To | Via | Status | Details |
|
||||
|------|----|-----|--------|---------|
|
||||
| `reminderScheduler.ts` | `calendarEvents.dtstartUtc` | `WHERE gt(now) AND lte(now+16min)` | VERIFIED | Line 103: `gt(calendarEvents.dtstartUtc, now)`. Line 104: `lte(calendarEvents.dtstartUtc, windowEnd)`. |
|
||||
| `reminderScheduler.ts` | `dispatchPush` | fan-out per subscription, then mark uid sent | VERIFIED | Lines 163-174: per-sub loop calling `await dispatchPush(sub, notification)`. Line 179: `sentReminders.set(uid, ...)` after the fan-out loop (WR-01 preserved). |
|
||||
|
||||
---
|
||||
|
||||
### Behavioral Spot-Checks
|
||||
|
||||
| Behavior | Command | Result | Status |
|
||||
|----------|---------|--------|--------|
|
||||
| TypeScript compilation (strict) | `pnpm --filter @familysync/api typecheck` | exit 0, no output | PASS |
|
||||
| All 10 reminderScheduler tests pass | `pnpm --filter @familysync/api exec vitest run tests/broker/reminderScheduler.test.ts` | 1 file passed, 10/10 tests passed | PASS |
|
||||
| `gt(dtstartUtc, now)` lower bound present | `grep -n "gt(" reminderScheduler.ts` | line 103: `gt(calendarEvents.dtstartUtc, now)` | PASS |
|
||||
| `gte` import/usage removed | `grep -n "gte" reminderScheduler.ts` | no matches | PASS |
|
||||
| `minuteBucket` variable fully removed | `grep -n "minuteBucket" reminderScheduler.ts` | only in comments, no variable | PASS |
|
||||
| Dedup store is `Map<string, number>` keyed by uid | `grep -n "Map" reminderScheduler.ts` | line 47: `new Map<string, number>()` | PASS |
|
||||
| Lead-accurate body with Math.max guard | `grep -n "Math.max" reminderScheduler.ts` | line 150: `Math.max(1, Math.round(...))` | PASS |
|
||||
| WR-01 mark-after-dispatch preserved | `grep -n "sentReminders.set" reminderScheduler.ts` | line 179: after fan-out loop | PASS |
|
||||
| CR-01 started-event pruning | `grep -n "dtstartMs" reminderScheduler.ts` | lines 192-194: prune on `dtstartMs <= now.getTime()` | PASS |
|
||||
| Required test names present | grep for SINGLE-FIRE, MISSED, already-started in test file | all found at lines 178, 214, 119 | PASS |
|
||||
|
||||
---
|
||||
|
||||
### Anti-Patterns Found
|
||||
|
||||
None. No TBD/FIXME/XXX markers in modified files. No hardcoded empty data. No stubs.
|
||||
|
||||
The two `minuteBucket` occurrences at lines 42 and 142 of `reminderScheduler.ts` are comment text only (no variable), correctly documenting the old scheme for historical context — not a stub indicator.
|
||||
|
||||
---
|
||||
|
||||
### Human Verification Required
|
||||
|
||||
None. All must-haves are verifiable programmatically and confirmed by running commands.
|
||||
|
||||
---
|
||||
|
||||
## Gaps Summary
|
||||
|
||||
No gaps. All 5 must-have truths are VERIFIED against the actual codebase:
|
||||
|
||||
- Catch-up window implemented: `gt(dtstartUtc, now)` lower bound (not `gte(now+14min)`) confirmed in source and tested.
|
||||
- Per-uid exactly-once dedup: `Map<string, number>` keyed on bare uid confirmed; `minuteBucket` variable fully absent.
|
||||
- Already-started exclusion: strict `gt` lower bound confirmed; test coverage present.
|
||||
- Lead-accurate body: `Math.max(1, Math.round(...))` formula confirmed in source.
|
||||
- Phase-5 decisions preserved: D-05/D-07/D-11/D-12/D-16/T-05-18/T-05-19/WR-01/CR-01 all present in source and tested.
|
||||
- Typecheck: exit 0.
|
||||
- Test suite: 10/10 passing, including the three required new tests (SINGLE-FIRE, MISSED-TICK-RECOVERY, already-started exclusion).
|
||||
|
||||
---
|
||||
|
||||
_Verified: 2026-06-10T12:39:30Z_
|
||||
_Verifier: Claude (gsd-verifier)_
|
||||
Reference in New Issue
Block a user