diff --git a/apps/api/tests/broker/reminderScheduler.test.ts b/apps/api/tests/broker/reminderScheduler.test.ts index d7b9864..5dd4a08 100644 --- a/apps/api/tests/broker/reminderScheduler.test.ts +++ b/apps/api/tests/broker/reminderScheduler.test.ts @@ -1,5 +1,5 @@ /** - * RED scaffold — reminderScheduler (Plan 05-06 turns this GREEN). + * reminderScheduler tests. * * Asserts that the reminder scan: * - Selects ONLY shared (isShared=true) AND timed (allDay=false) events @@ -7,8 +7,9 @@ * - 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) * - * These tests fail now because reminderScheduler.ts does not yet exist. * Run: pnpm --filter @familysync/api exec vitest run tests/broker/reminderScheduler.test.ts */ @@ -134,3 +135,132 @@ describe('reminderScheduler — shared+timed event filtering (D-05/D-06/D-07)', expect(secondCallCount).toBe(firstCallCount) }) }) + +describe('reminderScheduler — CR-01: sentReminders Set pruning', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.resetModules() + }) + + afterEach(() => { + vi.useRealTimers() + vi.clearAllMocks() + }) + + it('does not re-dispatch an event in the next minute bucket (pruned entry allows new minute)', 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 with one subscriber + 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, + 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 + } + + // 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 + + // 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 + + // 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. + expect(vi.mocked(dispatchPush).mock.calls.length).toBe(2) + }) +}) + +describe('reminderScheduler — WR-01: mark-sent after dispatch', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.resetModules() + }) + + afterEach(() => { + vi.useRealTimers() + 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. + 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 now = new Date('2026-06-15T10:00:00Z') + vi.setSystemTime(now) + + const sub = { id: 1, userId: 1, endpoint: 'https://push.example.com/1', p256dh: 'k', auth: 'a' } + const eventRow = { + 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()) + 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()) + await runReminderCheck(now) + expect(vi.mocked(dispatchPush).mock.calls.length).toBe(1) // still 1 — deduped + }) +}) diff --git a/apps/api/tests/lib/eventChangeDispatcher.test.ts b/apps/api/tests/lib/eventChangeDispatcher.test.ts index 24ec519..b8753e1 100644 --- a/apps/api/tests/lib/eventChangeDispatcher.test.ts +++ b/apps/api/tests/lib/eventChangeDispatcher.test.ts @@ -14,7 +14,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' -// Mock DB +// Mock DB — the factory is hoisted; per-test configuration uses mockReturnValueOnce vi.mock('../../src/db/client.js', () => ({ db: { select: vi.fn(), @@ -27,20 +27,22 @@ vi.mock('../../src/lib/pushDispatcher.js', () => ({ })) // --------------------------------------------------------------------------- -// Helper: build a mock db.select chain. -// dispatchEventChange now calls Promise.all([actorQuery, subsQuery]). -// The first select is for the actor name (users table, returns [{displayName}]), -// the second is for push subscriptions (returns sub rows). +// Helper: configure db.select for two sequential calls used by dispatchEventChange. +// +// dispatchEventChange calls Promise.all([actorQuery, subsQuery]): +// 1st select → actor name query: .from().where().limit() → [{ displayName }] | [] +// 2nd select → subscriptions query: .from().where() → sub rows +// +// We configure the mock BEFORE importing eventChangeDispatcher.js (since each +// test does vi.resetModules + fresh import). // --------------------------------------------------------------------------- -function mockDbSelect( +function setupDbMock( db: { select: ReturnType }, actorDisplayName: string | null, subRows: Array<{ id: number; userId: number; endpoint: string; p256dh: string; auth: string }>, ) { - // Two sequential .select() calls via Promise.all: - // First call → actor name query (select.from.where.limit → [{ displayName }]) - // Second call → subscriptions query (select.from.where → subRows) db.select + // 1st call: actor name query (select.from.where.limit) .mockReturnValueOnce({ from: vi.fn().mockReturnValue({ where: vi.fn().mockReturnValue({ @@ -50,6 +52,7 @@ function mockDbSelect( }), }), }) + // 2nd call: subscriptions query (select.from.where) .mockReturnValueOnce({ from: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue(subRows), @@ -59,7 +62,9 @@ function mockDbSelect( describe('eventChangeDispatcher — trigger conditions (D-04)', () => { beforeEach(() => { - vi.clearAllMocks() + // resetAllMocks clears call history AND resets mockReturnValueOnce queues, + // preventing leaked once-queues from test N polluting test N+1. + vi.resetAllMocks() vi.resetModules() }) @@ -69,7 +74,7 @@ describe('eventChangeDispatcher — trigger conditions (D-04)', () => { const { dispatchEventChange } = await import('../../src/lib/eventChangeDispatcher.js') const otherUserSub = { id: 2, userId: 2, endpoint: 'https://push.example.com/sub/2', p256dh: 'x', auth: 'y' } - mockDbSelect(vi.mocked(db), 'Lucas', [otherUserSub]) + setupDbMock(vi.mocked(db), 'Lucas', [otherUserSub]) await dispatchEventChange( { uid: 'new-event-uid', title: 'Doctor appointment', operation: 'create' }, @@ -85,7 +90,7 @@ describe('eventChangeDispatcher — trigger conditions (D-04)', () => { const { dispatchEventChange } = await import('../../src/lib/eventChangeDispatcher.js') const otherUserSub = { id: 2, userId: 2, endpoint: 'https://push.example.com/sub/2', p256dh: 'x', auth: 'y' } - mockDbSelect(vi.mocked(db), 'Lucas', [otherUserSub]) + setupDbMock(vi.mocked(db), 'Lucas', [otherUserSub]) await dispatchEventChange( { @@ -105,9 +110,9 @@ describe('eventChangeDispatcher — trigger conditions (D-04)', () => { const { dispatchPush } = await import('../../src/lib/pushDispatcher.js') const { dispatchEventChange } = await import('../../src/lib/eventChangeDispatcher.js') - // isMeaningfulChange returns false before DB is queried — no select needed. - // But set up a mock just in case (won't be called). - mockDbSelect(vi.mocked(db), 'Lucas', [ + // isMeaningfulChange returns false before DB is queried — select won't be called. + // Set up mock anyway as a no-op guard. + setupDbMock(vi.mocked(db), 'Lucas', [ { id: 2, userId: 2, endpoint: 'https://push.example.com/sub/2', p256dh: 'x', auth: 'y' }, ]) @@ -129,10 +134,9 @@ describe('eventChangeDispatcher — trigger conditions (D-04)', () => { const { dispatchPush } = await import('../../src/lib/pushDispatcher.js') const { dispatchEventChange } = await import('../../src/lib/eventChangeDispatcher.js') - // DB returns only the actor's own subscription (userId 1). - // The ne() filter in the SQL should exclude it, but here we simulate that - // the application-level filter also catches it. - mockDbSelect(vi.mocked(db), 'Lucas', [ + // DB's ne() filter already excludes the actor; simulate that the DB returns + // only the actor's own sub (userId=1) to test the application-level guard. + setupDbMock(vi.mocked(db), 'Lucas', [ { id: 1, userId: 1, endpoint: 'https://push.example.com/sub/1', p256dh: 'x', auth: 'y' }, ]) @@ -151,7 +155,7 @@ describe('eventChangeDispatcher — trigger conditions (D-04)', () => { const { dispatchEventChange } = await import('../../src/lib/eventChangeDispatcher.js') const otherUserSub = { id: 2, userId: 2, endpoint: 'https://push.example.com/sub/2', p256dh: 'x', auth: 'y' } - mockDbSelect(vi.mocked(db), 'Lucas', [otherUserSub]) + setupDbMock(vi.mocked(db), 'Lucas', [otherUserSub]) await dispatchEventChange( { uid: 'event-uid-4', title: 'Dentist', operation: 'create' }, @@ -169,7 +173,7 @@ describe('eventChangeDispatcher — trigger conditions (D-04)', () => { const { dispatchEventChange } = await import('../../src/lib/eventChangeDispatcher.js') const otherUserSub = { id: 2, userId: 2, endpoint: 'https://push.example.com/sub/2', p256dh: 'x', auth: 'y' } - mockDbSelect(vi.mocked(db), null, [otherUserSub]) // actor row absent → null + setupDbMock(vi.mocked(db), null, [otherUserSub]) // actor row absent → empty array await dispatchEventChange( { uid: 'event-uid-5', title: 'Soccer', operation: 'create' },