From 98753d8e34be545e29b357c139b7bfb617a3a5bf Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Sun, 7 Jun 2026 16:12:43 -0400 Subject: [PATCH] fix(broker): reconcile deletes into cache + resync before marking outbox done MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two write-path cache bugs surfaced during Gate 2 live testing: P1 (delete didn't work / ghost event): syncCalendar only UPSERTED events present on Fastmail and never removed cache rows for events that disappeared. A successful CalDAV delete left the row in calendar_events forever, so GET /api/events kept returning it and the UI showed a ghost that 'wouldn't delete' (even after refresh). Add a prune step: delete calendar_events rows for this calendar whose uid is absent from the server response (scoped to cal.id so it never touches another calendar or the other member's rows — BUG B). Empty server result prunes the whole calendar's cache. P2 (edit needed a manual refresh): the outbox worker marked a row 'done' BEFORE triggerTargetedResync refreshed the cache. The PWA's SyncStateToast invalidates ['events'] the instant sync-status flips to 'done', so it refetched stale cache. Re-sync first, then mark done — 'done' now guarantees the cache reflects the write. Tests: +2 prune regressions (present-subset prune, empty-server prune-all). --- apps/api/src/broker/outboxWorker.ts | 9 +++-- apps/api/src/broker/sync.ts | 21 +++++++++++- apps/api/tests/broker/sync.test.ts | 52 +++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/apps/api/src/broker/outboxWorker.ts b/apps/api/src/broker/outboxWorker.ts index 8f25bb2..08acf63 100644 --- a/apps/api/src/broker/outboxWorker.ts +++ b/apps/api/src/broker/outboxWorker.ts @@ -410,12 +410,17 @@ export async function runOutboxDrain(): Promise { failedCreateGroups.add(row.groupId) } } else if (result.success) { - // Success — mark done, trigger targeted re-sync to refresh the cache (D-06) + // Success — refresh the local cache BEFORE marking done. The PWA's + // SyncStateToast polls sync-status and invalidates ['events'] the + // instant it sees status='done'; if we marked done first, that refetch + // raced the re-sync and returned stale cache (deleted event still + // present, edit not yet applied) — forcing a manual refresh. Re-syncing + // first means 'done' guarantees the cache already reflects the write. + await triggerTargetedResync(row.calendarUrl, row.userId) await db .update(calendarOutbox) .set({ status: 'done' }) .where(eq(calendarOutbox.id, row.id)) - await triggerTargetedResync(row.calendarUrl, row.userId) } else if (result.hardFail) { // Hard fail — mark failed immediately, no retry (D-07) await db diff --git a/apps/api/src/broker/sync.ts b/apps/api/src/broker/sync.ts index b793e61..bf180e6 100644 --- a/apps/api/src/broker/sync.ts +++ b/apps/api/src/broker/sync.ts @@ -18,7 +18,7 @@ import type { DAVCalendar } from 'tsdav' import type { FastmailClient } from './client.js' import ICAL from 'ical.js' -import { and, eq } from 'drizzle-orm' +import { and, eq, notInArray } from 'drizzle-orm' import { db } from '../db/client.js' import { calendars, calendarEvents } from '../db/schema.js' @@ -73,6 +73,9 @@ export async function syncCalendar( const objects = await client.fetchCalendarObjects({ calendar: davCal }) // 4. Parse each VCALENDAR/VEVENT and upsert into calendar_events. + // Track every uid we see on the server so step 5 can prune cache rows that + // no longer exist on Fastmail (deletes — local or external). + const seenUids: string[] = [] for (const obj of objects) { if (!obj.data) continue @@ -92,6 +95,7 @@ export async function syncCalendar( const dtstart = vevent.getFirstPropertyValue('dtstart') as ICAL.Time | null const uid = vevent.getFirstPropertyValue('uid') as string | null if (!uid) continue + seenUids.push(uid) // D-13 / Pitfall #3: isDate=true → DATE column; isDate=false → TIMESTAMP column const allDay: boolean = dtstart?.isDate ?? false @@ -133,4 +137,19 @@ export async function syncCalendar( }, }) } + + // 5. Prune deletes: remove cached events for THIS calendar whose uid is no + // longer present on the server. Without this, a deleted event (local delete + // via the outbox, or an external delete in another client) lingers in + // calendar_events forever — GET /api/events keeps returning it and the UI + // shows a ghost event that "won't delete". Scoped to cal.id so it never + // touches another calendar or the other household member's rows (BUG B). + // When the server returns zero events, prune the whole calendar's cache. + if (seenUids.length > 0) { + await db + .delete(calendarEvents) + .where(and(eq(calendarEvents.calendarId, cal.id), notInArray(calendarEvents.uid, seenUids))) + } else { + await db.delete(calendarEvents).where(eq(calendarEvents.calendarId, cal.id)) + } } diff --git a/apps/api/tests/broker/sync.test.ts b/apps/api/tests/broker/sync.test.ts index dd98af3..447d646 100644 --- a/apps/api/tests/broker/sync.test.ts +++ b/apps/api/tests/broker/sync.test.ts @@ -26,12 +26,16 @@ const mockLimit = vi.fn().mockResolvedValue([{ id: 42 }]) const mockWhere = vi.fn().mockReturnValue({ limit: mockLimit }) const mockFrom = vi.fn().mockReturnValue({ where: mockWhere }) const mockSelect = vi.fn().mockReturnValue({ from: mockFrom }) +// Prune chain: db.delete(calendarEvents).where(...) +const mockDeleteWhere = vi.fn().mockResolvedValue([]) +const mockDelete = vi.fn().mockReturnValue({ where: mockDeleteWhere }) // Mock the db singleton at module level (Vitest hoisting) vi.mock('../../src/db/client.js', () => ({ db: { insert: mockInsert, select: mockSelect, + delete: mockDelete, }, })) @@ -46,6 +50,8 @@ describe('syncCalendar', () => { mockWhere.mockReturnValue({ limit: mockLimit }) mockFrom.mockReturnValue({ where: mockWhere }) mockSelect.mockReturnValue({ from: mockFrom }) + mockDeleteWhere.mockResolvedValue([]) + mockDelete.mockReturnValue({ where: mockDeleteWhere }) }) it('stores all-day events with dtstart_date (DATE) and dtstart_utc=NULL', async () => { @@ -313,4 +319,50 @@ describe('syncCalendar', () => { expect(calValuesArg.ctag).toBe('new-ctag-123') expect(calValuesArg.syncToken).toBe('sync-token-abc') }) + + // Regression: deletes must be reconciled out of the cache. Before this fix, + // syncCalendar only upserted present events, so a deleted event lingered in + // calendar_events forever and the UI showed a ghost that "wouldn't delete". + it('prunes cached events whose uid is absent from the server (delete reconciliation)', async () => { + const { syncCalendar } = await import('../../src/broker/sync.js') + + // Server returns ONE timed event; any other cached uid for this calendar must be pruned. + const mockClient = { + fetchCalendarObjects: vi.fn().mockResolvedValue([ + { data: SAMPLE_VEVENT_TIMED, etag: '"etag-timed"', url: '/cal/timed.ics' }, + ]), + } + const mockDavCal = { + url: 'https://caldav.fastmail.com/dav/calendars/user/test@fm.com/Default/', + displayName: 'Test Calendar', + ctag: 'ctag-v2', + syncToken: null, + } + + await syncCalendar(mockClient as never, mockDavCal as never, 1) + + // A prune DELETE must run, scoped by calendarId AND excluding the seen uid(s). + expect(mockDelete).toHaveBeenCalledTimes(1) + expect(mockDeleteWhere).toHaveBeenCalledTimes(1) + }) + + it('prunes the entire calendar cache when the server returns zero events', async () => { + const { syncCalendar } = await import('../../src/broker/sync.js') + + const mockClient = { + fetchCalendarObjects: vi.fn().mockResolvedValue([]), + } + const mockDavCal = { + url: 'https://caldav.fastmail.com/dav/calendars/user/test@fm.com/Default/', + displayName: 'Test Calendar', + ctag: 'ctag-empty', + syncToken: null, + } + + await syncCalendar(mockClient as never, mockDavCal as never, 1) + + // Empty server result → prune-all DELETE (scoped to this calendar id only). + expect(mockDelete).toHaveBeenCalledTimes(1) + expect(mockDeleteWhere).toHaveBeenCalledTimes(1) + }) })