test(05-review): add CR-01/WR-01 reminder pruning tests and IN-01 actor-name tests

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
+  })
+})

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.
+// Helper: configure db.select for two sequential calls used by dispatchEventChange.
-// dispatchEventChange now calls Promise.all([actorQuery, subsQuery]).
+//
-// The first select is for the actor name (users table, returns [{displayName}]),
+// dispatchEventChange calls Promise.all([actorQuery, subsQuery]):
-// the second is for push subscriptions (returns sub rows).
+//   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<typeof vi.fn> },
   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.
+    // isMeaningfulChange returns false before DB is queried — select won't be called.
-    // But set up a mock just in case (won't be called).
+    // Set up mock anyway as a no-op guard.
-    mockDbSelect(vi.mocked(db), 'Lucas', [
+    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).
+    // DB's ne() filter already excludes the actor; simulate that the DB returns
-    // The ne() filter in the SQL should exclude it, but here we simulate that
+    // only the actor's own sub (userId=1) to test the application-level guard.
-    // the application-level filter also catches it.
+    setupDbMock(vi.mocked(db), 'Lucas', [
-    mockDbSelect(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' },