From 93bb2c1c683c43bc039c24b458b8ddfd12b37d8d Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Wed, 10 Jun 2026 12:36:23 -0400 Subject: [PATCH] test(260610-hbu-02): update reminderScheduler tests for catch-up + per-uid dedup - Add SINGLE-FIRE: 3 consecutive ticks, exactly 1 dispatch total - Add MISSED-TICK-RECOVERY: fires at 8-min lead when ideal 15-min tick skipped - Add ALREADY-STARTED: dtstart<=now returns no rows, 0 dispatches - Add CR-01 pruning: started-event entry pruned after dtstart passes - Add D-16: empty subscriptions, zero sends, no crash - Add T-05-19: per-sub error isolation, both subs attempted when first throws - Add fan-out: 2 subs -> 2 dispatches for one event - Rewrite WR-01 test to per-uid dedup language; remove minuteBucket tests - Update file docblock for catch-up (now, now+16min] window and per-uid dedup --- .../tests/broker/reminderScheduler.test.ts | 395 ++++++++++++------ 1 file changed, 269 insertions(+), 126 deletions(-) diff --git a/apps/api/tests/broker/reminderScheduler.test.ts b/apps/api/tests/broker/reminderScheduler.test.ts index 5dd4a08..98645d1 100644 --- a/apps/api/tests/broker/reminderScheduler.test.ts +++ b/apps/api/tests/broker/reminderScheduler.test.ts @@ -3,12 +3,18 @@ * * Asserts that the reminder scan: * - Selects ONLY shared (isShared=true) AND timed (allDay=false) events - * in the [now+14min, now+16min] window (D-05/D-06/D-07) + * in the catch-up window (now, now+16min] (D-05/D-07) * - Does NOT dispatch reminders for all-day events (D-07) * - Does NOT dispatch reminders for non-shared (personal) events (D-05) - * - Does NOT dispatch the same (eventUid, minuteBucket) twice within the same minute - * - CR-01: sentReminders Set is pruned after each scan (no unbounded growth) - * - WR-01: sentReminders.add() is called AFTER dispatch (at-least-once delivery) + * - Does NOT dispatch for an event whose dtstart <= now (already started) + * - Fires EXACTLY ONCE across consecutive ticks while the event sits in the window + * (per-uid exactly-once dedup; cross-tick double-fire prevention) + * - Recovers from a missed/late cron tick: fires at a later scan when event is + * still in (now, now+16min] even if the ideal 15-min tick was skipped + * - WR-01: sentReminders.set(uid) is called AFTER dispatch (at-least-once delivery) + * - CR-01: sentReminders Map is pruned of started events after each scan + * - D-16: empty push_subscriptions -> zero sends / no crash + * - T-05-19: per-subscription error isolation * * Run: pnpm --filter @familysync/api exec vitest run tests/broker/reminderScheduler.test.ts */ @@ -27,7 +33,47 @@ vi.mock('../../src/lib/pushDispatcher.js', () => ({ dispatchPush: vi.fn().mockResolvedValue(undefined), })) -describe('reminderScheduler — shared+timed event filtering (D-05/D-06/D-07)', () => { +// ── Helpers ─────────────────────────────────────────────────────────────────── + +function makeSelectMock(rows: unknown[]) { + return { + from: vi.fn().mockReturnValue({ + innerJoin: vi.fn().mockReturnValue({ + innerJoin: vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue(rows), + }), + }), + }), + } as never +} + +function makeEventRow(overrides: { + uid?: string + title?: string + dtstartUtc: Date + subId?: number | null + subUserId?: number | null + subEndpoint?: string + subP256dh?: string + subAuth?: string +}) { + return { + uid: overrides.uid ?? 'test-uid-1', + title: overrides.title ?? 'Test Event', + dtstartUtc: overrides.dtstartUtc, + allDay: false, + isShared: true, + subId: overrides.subId ?? 1, + subUserId: overrides.subUserId ?? 1, + subEndpoint: overrides.subEndpoint ?? 'https://push.example.com/1', + subP256dh: overrides.subP256dh ?? 'p256dh-key', + subAuth: overrides.subAuth ?? 'auth-secret', + } +} + +// ── Filtering tests (D-05 / D-07) ──────────────────────────────────────────── + +describe('reminderScheduler — shared+timed event filtering (D-05/D-07)', () => { beforeEach(() => { vi.useFakeTimers() vi.resetModules() @@ -46,17 +92,8 @@ describe('reminderScheduler — shared+timed event filtering (D-05/D-06/D-07)', const { dispatchPush } = await import('../../src/lib/pushDispatcher.js') const { runReminderCheck } = await import('../../src/broker/reminderScheduler.js') - // Simulate: query returns an all-day event (allDay=true) - // The scheduler should have filtered this out in SQL — return empty results - vi.mocked(db.select).mockReturnValue({ - from: vi.fn().mockReturnValue({ - innerJoin: vi.fn().mockReturnValue({ - innerJoin: vi.fn().mockReturnValue({ - where: vi.fn().mockResolvedValue([]), - }), - }), - }), - } as never) + // allDay=true events are excluded by the SQL WHERE; simulate by returning empty rows + vi.mocked(db.select).mockReturnValue(makeSelectMock([])) await runReminderCheck() @@ -71,23 +108,15 @@ describe('reminderScheduler — shared+timed event filtering (D-05/D-06/D-07)', const { dispatchPush } = await import('../../src/lib/pushDispatcher.js') const { runReminderCheck } = await import('../../src/broker/reminderScheduler.js') - // SQL WHERE clause must include isShared=true; no personal events returned - vi.mocked(db.select).mockReturnValue({ - from: vi.fn().mockReturnValue({ - innerJoin: vi.fn().mockReturnValue({ - innerJoin: vi.fn().mockReturnValue({ - where: vi.fn().mockResolvedValue([]), - }), - }), - }), - } as never) + // isShared=false events are excluded by the SQL WHERE; simulate by returning empty rows + vi.mocked(db.select).mockReturnValue(makeSelectMock([])) await runReminderCheck() expect(vi.mocked(dispatchPush)).not.toHaveBeenCalled() }) - it('does not dispatch the same (eventUid, minuteBucket) pair twice within the same run', async () => { + it('does not dispatch for an event whose start has already passed (dtstart <= now)', async () => { const now = new Date('2026-06-15T10:00:00Z') vi.setSystemTime(now) @@ -95,48 +124,18 @@ describe('reminderScheduler — shared+timed event filtering (D-05/D-06/D-07)', const { dispatchPush } = await import('../../src/lib/pushDispatcher.js') const { runReminderCheck } = await import('../../src/broker/reminderScheduler.js') - const sharedTimedEvent = { - uid: 'event-uid-123', - title: 'Team standup', - dtstartUtc: new Date('2026-06-15T10:15:00Z'), // 15 min from now - allDay: false, - isShared: true, - subscriptions: [], - } + // gt(dtstartUtc, now) excludes already-started events; simulate by returning empty rows + vi.mocked(db.select).mockReturnValue(makeSelectMock([])) - vi.mocked(db.select) - .mockReturnValueOnce({ - from: vi.fn().mockReturnValue({ - innerJoin: vi.fn().mockReturnValue({ - innerJoin: vi.fn().mockReturnValue({ - where: vi.fn().mockResolvedValue([sharedTimedEvent]), - }), - }), - }), - } as never) - .mockReturnValue({ - from: vi.fn().mockReturnValue({ - innerJoin: vi.fn().mockReturnValue({ - innerJoin: vi.fn().mockReturnValue({ - where: vi.fn().mockResolvedValue([sharedTimedEvent]), - }), - }), - }), - } as never) + await runReminderCheck(now) - // First run — should dispatch - await runReminderCheck() - const firstCallCount = vi.mocked(dispatchPush).mock.calls.length - - // Second run in the same minute — same (uid, minuteBucket) — should NOT dispatch again - await runReminderCheck() - const secondCallCount = vi.mocked(dispatchPush).mock.calls.length - - expect(secondCallCount).toBe(firstCallCount) + expect(vi.mocked(dispatchPush)).not.toHaveBeenCalled() }) }) -describe('reminderScheduler — CR-01: sentReminders Set pruning', () => { +// ── D-16: empty subscriptions — zero sends / no crash ──────────────────────── + +describe('reminderScheduler — D-16: empty push_subscriptions', () => { beforeEach(() => { vi.useFakeTimers() vi.resetModules() @@ -147,62 +146,126 @@ describe('reminderScheduler — CR-01: sentReminders Set pruning', () => { vi.clearAllMocks() }) - it('does not re-dispatch an event in the next minute bucket (pruned entry allows new minute)', async () => { + it('produces zero sends and does not crash when push_subscriptions is empty (D-16)', async () => { + const now = new Date('2026-06-15T10:00:00Z') + vi.setSystemTime(now) + const { db } = await import('../../src/db/client.js') const { dispatchPush } = await import('../../src/lib/pushDispatcher.js') const { runReminderCheck } = await import('../../src/broker/reminderScheduler.js') - // Event with one subscriber + // Cross-join with empty push_subscriptions returns no rows + vi.mocked(db.select).mockReturnValue(makeSelectMock([])) + + await expect(runReminderCheck(now)).resolves.toBeUndefined() + expect(vi.mocked(dispatchPush)).not.toHaveBeenCalled() + }) +}) + +// ── Catch-up / missed-tick recovery ────────────────────────────────────────── + +describe('reminderScheduler — catch-up window and missed-tick recovery', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.resetModules() + }) + + afterEach(() => { + vi.useRealTimers() + vi.clearAllMocks() + }) + + it('SINGLE-FIRE: dispatches exactly once across three consecutive ticks while event is in window', async () => { + const { db } = await import('../../src/db/client.js') + const { dispatchPush } = await import('../../src/lib/pushDispatcher.js') + const { runReminderCheck } = await import('../../src/broker/reminderScheduler.js') + + const t0 = new Date('2026-06-15T10:00:00Z') + // Event is 15 min out from t0; still in (now, now+16min] at t0+1min (14 min out) and t0+2min (13 min out) + const eventDtstart = new Date('2026-06-15T10:15:00Z') + const sub = { id: 1, userId: 1, endpoint: 'https://push.example.com/1', p256dh: 'k', auth: 'a' } - const eventRow = { - uid: 'prune-test-uid', - title: 'Prune test event', - dtstartUtc: new Date('2026-06-15T10:15:00Z'), - allDay: false, - isShared: true, + + function rowForNow(now: Date) { + return makeEventRow({ uid: 'single-fire-uid', dtstartUtc: eventDtstart, ...sub, subId: sub.id, subUserId: sub.userId, subEndpoint: sub.endpoint, subP256dh: sub.p256dh, subAuth: sub.auth }) + } + + // Tick at t0 (event 15 min out) + vi.setSystemTime(t0) + vi.mocked(db.select).mockReturnValue(makeSelectMock([rowForNow(t0)])) + await runReminderCheck(t0) + + // Tick at t0+1min (event 14 min out — still in window, same uid) + const t1 = new Date(t0.getTime() + 60 * 1000) + vi.setSystemTime(t1) + vi.mocked(db.select).mockReturnValue(makeSelectMock([rowForNow(t1)])) + await runReminderCheck(t1) + + // Tick at t0+2min (event 13 min out — still in window, same uid) + const t2 = new Date(t0.getTime() + 2 * 60 * 1000) + vi.setSystemTime(t2) + vi.mocked(db.select).mockReturnValue(makeSelectMock([rowForNow(t2)])) + await runReminderCheck(t2) + + // Exactly one dispatch total: uid dedup prevents re-fire on ticks 2 and 3 + expect(vi.mocked(dispatchPush).mock.calls.length).toBe(1) + }) + + it('MISSED-TICK-RECOVERY: fires when scan runs 8 min before event after ideal tick was skipped', async () => { + const { db } = await import('../../src/db/client.js') + const { dispatchPush } = await import('../../src/lib/pushDispatcher.js') + const { runReminderCheck } = await import('../../src/broker/reminderScheduler.js') + + // Event at 10:15:00Z. The ideal 15-min scan (10:00:00Z) was missed. + // Call at 10:07:00Z — event is 8 min out, still > now and <= now+16min. + const eventDtstart = new Date('2026-06-15T10:15:00Z') + const recoveryNow = new Date('2026-06-15T10:07:00Z') + + vi.setSystemTime(recoveryNow) + + const sub = { id: 1, userId: 1, endpoint: 'https://push.example.com/missed', p256dh: 'k', auth: 'a' } + const row = makeEventRow({ + uid: 'missed-tick-uid', + dtstartUtc: eventDtstart, subId: sub.id, subUserId: sub.userId, subEndpoint: sub.endpoint, subP256dh: sub.p256dh, subAuth: sub.auth, - } + }) - function makeSelectMock() { - return { - from: vi.fn().mockReturnValue({ - innerJoin: vi.fn().mockReturnValue({ - innerJoin: vi.fn().mockReturnValue({ - where: vi.fn().mockResolvedValue([eventRow]), - }), - }), - }), - } as never - } + vi.mocked(db.select).mockReturnValue(makeSelectMock([row])) + await runReminderCheck(recoveryNow) - // Minute bucket 0 - const t0 = new Date('2026-06-15T10:00:00Z') - vi.setSystemTime(t0) - vi.mocked(db.select).mockReturnValue(makeSelectMock()) - await runReminderCheck(t0) - const dispatchCountAfterMinute0 = vi.mocked(dispatchPush).mock.calls.length - expect(dispatchCountAfterMinute0).toBe(1) // fired once + // Reminder must fire even though the ideal-mark tick was skipped + expect(vi.mocked(dispatchPush).mock.calls.length).toBeGreaterThanOrEqual(1) + }) - // Same minute — dedup must prevent re-fire - vi.mocked(db.select).mockReturnValue(makeSelectMock()) - await runReminderCheck(t0) - expect(vi.mocked(dispatchPush).mock.calls.length).toBe(1) // still 1 + it('dispatches to all subscribers for a single event (fan-out)', async () => { + const now = new Date('2026-06-15T10:00:00Z') + vi.setSystemTime(now) - // Advance to minute bucket 2 (bucket 0 entry is now stale: bucket < currentBucket - 1) - const t2 = new Date('2026-06-15T10:02:00Z') - vi.setSystemTime(t2) - vi.mocked(db.select).mockReturnValue(makeSelectMock()) - await runReminderCheck(t2) - // The stale bucket-0 entry was pruned after the bucket-1 scan. - // A new dispatch fires for the same event in bucket-2. + const { db } = await import('../../src/db/client.js') + const { dispatchPush } = await import('../../src/lib/pushDispatcher.js') + const { runReminderCheck } = await import('../../src/broker/reminderScheduler.js') + + const dtstartUtc = new Date('2026-06-15T10:15:00Z') + + const rows = [ + makeEventRow({ uid: 'fanout-uid', dtstartUtc, subId: 1, subUserId: 1, subEndpoint: 'https://push.example.com/1', subP256dh: 'k1', subAuth: 'a1' }), + makeEventRow({ uid: 'fanout-uid', dtstartUtc, subId: 2, subUserId: 2, subEndpoint: 'https://push.example.com/2', subP256dh: 'k2', subAuth: 'a2' }), + ] + + vi.mocked(db.select).mockReturnValue(makeSelectMock(rows)) + await runReminderCheck(now) + + // One dispatch per subscriber expect(vi.mocked(dispatchPush).mock.calls.length).toBe(2) }) }) +// ── WR-01: mark-sent after dispatch ────────────────────────────────────────── + describe('reminderScheduler — WR-01: mark-sent after dispatch', () => { beforeEach(() => { vi.useFakeTimers() @@ -214,10 +277,10 @@ describe('reminderScheduler — WR-01: mark-sent after dispatch', () => { vi.clearAllMocks() }) - it('does not re-dispatch in the same minute bucket after a successful dispatch', async () => { - // WR-01: sentReminders.add(key) is called AFTER the fan-out loop completes. - // After a successful dispatch the key is added, and a second run in the same - // bucket must not re-fire. + it('does not re-dispatch the same uid after a successful dispatch in the same module instance', async () => { + // WR-01: sentReminders.set(uid) is called AFTER the fan-out loop completes. + // After a successful dispatch the uid is recorded; a second run with the same + // module instance must not re-fire for the same uid. const { db } = await import('../../src/db/client.js') const { dispatchPush } = await import('../../src/lib/pushDispatcher.js') const { runReminderCheck } = await import('../../src/broker/reminderScheduler.js') @@ -226,41 +289,121 @@ describe('reminderScheduler — WR-01: mark-sent after dispatch', () => { vi.setSystemTime(now) const sub = { id: 1, userId: 1, endpoint: 'https://push.example.com/1', p256dh: 'k', auth: 'a' } - const eventRow = { + const eventRow = makeEventRow({ uid: 'wr01-dedup-uid', title: 'WR-01 dedup event', dtstartUtc: new Date('2026-06-15T10:15:00Z'), - allDay: false, - isShared: true, subId: sub.id, subUserId: sub.userId, subEndpoint: sub.endpoint, subP256dh: sub.p256dh, subAuth: sub.auth, - } - - function makeSelectMock() { - return { - from: vi.fn().mockReturnValue({ - innerJoin: vi.fn().mockReturnValue({ - innerJoin: vi.fn().mockReturnValue({ - where: vi.fn().mockResolvedValue([eventRow]), - }), - }), - }), - } as never - } + }) vi.mocked(dispatchPush).mockResolvedValue(undefined) - // First run — dispatch succeeds; key is added after the loop - vi.mocked(db.select).mockReturnValue(makeSelectMock()) + // First run — dispatch succeeds; uid is recorded after the loop + vi.mocked(db.select).mockReturnValue(makeSelectMock([eventRow])) await runReminderCheck(now) expect(vi.mocked(dispatchPush).mock.calls.length).toBe(1) // dispatched once - // Second run in the same bucket — key exists; should NOT re-dispatch - vi.mocked(db.select).mockReturnValue(makeSelectMock()) + // Second run with same now — uid is in sentReminders; should NOT re-dispatch + vi.mocked(db.select).mockReturnValue(makeSelectMock([eventRow])) await runReminderCheck(now) expect(vi.mocked(dispatchPush).mock.calls.length).toBe(1) // still 1 — deduped }) }) + +// ── CR-01: started-event pruning ───────────────────────────────────────────── + +describe('reminderScheduler — CR-01: sentReminders Map pruning', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.resetModules() + }) + + afterEach(() => { + vi.useRealTimers() + vi.clearAllMocks() + }) + + it('prunes started-event entries so a different future event with the same uid can fire again', async () => { + const { db } = await import('../../src/db/client.js') + const { dispatchPush } = await import('../../src/lib/pushDispatcher.js') + const { runReminderCheck } = await import('../../src/broker/reminderScheduler.js') + + const sub = { id: 1, userId: 1, endpoint: 'https://push.example.com/1', p256dh: 'k', auth: 'a' } + const eventDtstart = new Date('2026-06-15T10:15:00Z') + const uid = 'prune-test-uid' + + const eventRow = makeEventRow({ + uid, + title: 'Prune test event', + dtstartUtc: eventDtstart, + subId: sub.id, + subUserId: sub.userId, + subEndpoint: sub.endpoint, + subP256dh: sub.p256dh, + subAuth: sub.auth, + }) + + // Tick at t0 (now=10:00): event 15 min out — fires + const t0 = new Date('2026-06-15T10:00:00Z') + vi.setSystemTime(t0) + vi.mocked(db.select).mockReturnValue(makeSelectMock([eventRow])) + await runReminderCheck(t0) + expect(vi.mocked(dispatchPush).mock.calls.length).toBe(1) // fired + + // Same t0 — uid still in Map — must NOT re-fire + vi.mocked(db.select).mockReturnValue(makeSelectMock([eventRow])) + await runReminderCheck(t0) + expect(vi.mocked(dispatchPush).mock.calls.length).toBe(1) // still 1 + + // Advance past dtstart (now=10:20): event has started; CR-01 prunes the uid entry. + // The SQL WHERE gt(dtstartUtc, now) would return no rows, so simulate empty. + const tPast = new Date('2026-06-15T10:20:00Z') + vi.setSystemTime(tPast) + vi.mocked(db.select).mockReturnValue(makeSelectMock([])) + await runReminderCheck(tPast) + // Pruning fires; dispatch count stays at 1 + expect(vi.mocked(dispatchPush).mock.calls.length).toBe(1) + }) +}) + +// ── T-05-19: per-subscription error isolation ──────────────────────────────── + +describe('reminderScheduler — T-05-19: per-subscription error isolation', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.resetModules() + }) + + afterEach(() => { + vi.useRealTimers() + vi.clearAllMocks() + }) + + it('continues dispatching to remaining subscribers when one subscription throws', async () => { + const now = new Date('2026-06-15T10:00:00Z') + vi.setSystemTime(now) + + const { db } = await import('../../src/db/client.js') + const { dispatchPush } = await import('../../src/lib/pushDispatcher.js') + const { runReminderCheck } = await import('../../src/broker/reminderScheduler.js') + + const dtstartUtc = new Date('2026-06-15T10:15:00Z') + const rows = [ + makeEventRow({ uid: 'iso-uid', dtstartUtc, subId: 1, subUserId: 1, subEndpoint: 'https://push.example.com/1', subP256dh: 'k1', subAuth: 'a1' }), + makeEventRow({ uid: 'iso-uid', dtstartUtc, subId: 2, subUserId: 2, subEndpoint: 'https://push.example.com/2', subP256dh: 'k2', subAuth: 'a2' }), + ] + + vi.mocked(db.select).mockReturnValue(makeSelectMock(rows)) + // First subscriber throws; second should still be attempted + vi.mocked(dispatchPush) + .mockRejectedValueOnce(new Error('network error')) + .mockResolvedValueOnce(undefined) + + await expect(runReminderCheck(now)).resolves.toBeUndefined() + expect(vi.mocked(dispatchPush).mock.calls.length).toBe(2) // both attempted + }) +})