fix(broker): reconcile deletes into cache + resync before marking outbox done
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).
This commit is contained in:
@@ -410,12 +410,17 @@ export async function runOutboxDrain(): Promise<void> {
|
||||
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
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user