From 08afc492b89cddf0cd87a7bba797e7e2fe5cee26 Mon Sep 17 00:00:00 2001 From: Kaan Karaca Date: Thu, 13 Aug 2026 19:14:56 +0300 Subject: [PATCH] test(calendar): prove non-Google sources are safe on an old client (#1396) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Device A on a newer build connects CalDAV and syncs `provider='caldav'` rows. Device B is still on a Google-only build and pulls them down. The item handlers were already defensive in the other direction — an absent `provider` falls back to `'google'` — but this direction had never been verified, and it is the one that can lose data: if the old client decided an unrecognised row was corrupt and cleaned it up, that delete would sync back and remotely destroy the connection the user just made. Finding: it does not. `main/sync/calendar-cross-version-compat.test.ts` runs the handlers — which is exactly what a Google-only build does — against `ics`, `caldav` and `microsoft` rows and asserts they are: - stored verbatim, never rewritten to `google` - not archived, not deleted, not "repaired" on a second pull - pushed back out with the provider intact - accepted as parents for external events (the missing-parent guard keys on the row existing, not on the provider being syncable) - subject to the same delete-clock rules as google — a stale tombstone is skipped, a causally-newer one applied, identically The payload round trip is checked too: `provider` is `z.string().optional()` rather than an enum, so an old client's zod does not strip a value it has never seen — a stripped field would come back as the `?? 'google'` fallback and silently re-home the calendar. Consequence: no sync type negotiation gate is needed. The PR #754 pattern was the fallback if the old client had misbehaved; it does not, so new-provider rows fan out unconditionally and an old device carries them inert until it updates. Findings written up under `apps/docs/src/architecture/sync-handlers.md`. --- .../calendar-cross-version-compat.test.ts | 310 ++++++++++++++++++ apps/docs/src/architecture/sync-handlers.md | 44 +++ 2 files changed, 354 insertions(+) create mode 100644 apps/desktop/src/main/sync/calendar-cross-version-compat.test.ts diff --git a/apps/desktop/src/main/sync/calendar-cross-version-compat.test.ts b/apps/desktop/src/main/sync/calendar-cross-version-compat.test.ts new file mode 100644 index 000000000..9050dd2e5 --- /dev/null +++ b/apps/desktop/src/main/sync/calendar-cross-version-compat.test.ts @@ -0,0 +1,310 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { eq } from 'drizzle-orm' +import { calendarBindings } from '@memry/db-schema/schema/calendar-bindings' +import { calendarExternalEvents } from '@memry/db-schema/schema/calendar-external-events' +import { calendarSources } from '@memry/db-schema/schema/calendar-sources' +import { + CalendarBindingSyncPayloadSchema, + CalendarSourceSyncPayloadSchema +} from '@memry/contracts/sync-payloads' +import { + asClientDb, + createTestDataDb, + type TestDatabaseResult, + type TestDb +} from '@tests/utils/test-db' +import { listCalendarSources } from '../calendar/repositories/calendar-sources-repository' +import { calendarBindingHandler } from './item-handlers/calendar-binding-handler' +import { calendarSourceHandler } from './item-handlers/calendar-source-handler' +import { calendarExternalEventHandler } from './item-handlers/calendar-external-event-handler' +import type { ApplyContext, DrizzleDb } from './item-handlers/types' + +vi.mock('../lib/logger', () => ({ + createLogger: () => ({ + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn() + }) +})) + +/** + * Cross-version compatibility for multi-provider calendar (#1396). + * + * The scenario: device A runs a build that can connect ICS / CalDAV / Outlook + * and creates sources with those provider ids. Device B is still on a build + * that only knows Google, and pulls those rows down. + * + * The item handlers were already defensive in the *other* direction — an + * absent `provider` falls back to `'google'` — but this direction had never + * been verified. The failure that would matter is not "B cannot use the + * calendar"; it is B deciding the row is corrupt and cleaning it up, which + * would remotely delete the connection the user just made on A. + * + * These tests exercise the old client's semantics directly: the handlers are + * provider-blind, so running them against `ics` / `caldav` / `microsoft` rows + * is exactly what a Google-only build does with them. + */ + +const UNKNOWN_PROVIDERS = ['ics', 'caldav', 'microsoft'] as const + +function makeCtx(testDb: TestDatabaseResult): ApplyContext { + return { + db: testDb.db as unknown as DrizzleDb, + emit: vi.fn() + } +} + +function sourcePayload(provider: string): Record { + return { + provider, + kind: 'calendar', + accountId: 'someone@example.com', + remoteId: `${provider}-remote-1`, + title: `${provider} calendar`, + timezone: 'UTC', + color: '#2563eb', + isPrimary: true, + isSelected: true, + isMemryManaged: false, + syncCursor: 'cursor-1', + syncStatus: 'ok', + lastSyncedAt: '2026-04-12T08:00:00.000Z', + metadata: { url: 'https://example.com/feed.ics' }, + archivedAt: null, + createdAt: '2026-04-12T08:00:00.000Z', + modifiedAt: '2026-04-12T08:00:00.000Z' + } +} + +describe('cross-version compat: non-Google calendar sources over sync (#1396)', () => { + let testDb: TestDatabaseResult + let db: TestDb + let ctx: ApplyContext + + beforeEach(() => { + testDb = createTestDataDb() + db = testDb.db + ctx = makeCtx(testDb) + }) + + afterEach(() => { + testDb.close() + }) + + describe('an old client accepts a provider it has never heard of', () => { + it.each(UNKNOWN_PROVIDERS)('stores a %s source verbatim, not coerced to google', (provider) => { + const result = calendarSourceHandler.applyUpsert( + ctx, + `${provider}-calendar:remote-1`, + sourcePayload(provider), + { 'device-a': 1 } + ) + + expect(result).toBe('applied') + const row = db + .select() + .from(calendarSources) + .where(eq(calendarSources.id, `${provider}-calendar:remote-1`)) + .get() + expect(row?.provider).toBe(provider) + // The `?? 'google'` fallback exists for payloads that omit the field. + // A payload that carries one must never be rewritten. + expect(row?.provider).not.toBe('google') + }) + + it.each(UNKNOWN_PROVIDERS)( + 'leaves a %s source unarchived and undeleted after it lands', + (provider) => { + const id = `${provider}-calendar:remote-1` + calendarSourceHandler.applyUpsert(ctx, id, sourcePayload(provider), { 'device-a': 1 }) + + const row = db.select().from(calendarSources).where(eq(calendarSources.id, id)).get() + + expect(row).toBeDefined() + expect(row?.archivedAt).toBeNull() + // Still visible to every read path, which filters on `archivedAt IS NULL`. + expect(listCalendarSources(asClientDb(db), { provider }).map((s) => s.id)).toEqual([id]) + } + ) + + it('does not "repair" an unknown provider on a second pull', () => { + const id = 'caldav-calendar:remote-1' + calendarSourceHandler.applyUpsert(ctx, id, sourcePayload('caldav'), { 'device-a': 1 }) + // Device A edits the title; the row arrives again with the same provider. + calendarSourceHandler.applyUpsert( + ctx, + id, + { ...sourcePayload('caldav'), title: 'Renamed', modifiedAt: '2026-04-13T08:00:00.000Z' }, + { 'device-a': 2 } + ) + + const row = db.select().from(calendarSources).where(eq(calendarSources.id, id)).get() + expect(row?.provider).toBe('caldav') + expect(row?.title).toBe('Renamed') + expect(row?.archivedAt).toBeNull() + }) + + it('never writes an unknown-provider source back out with a rewritten provider', () => { + const id = 'ics-calendar:remote-1' + calendarSourceHandler.applyUpsert(ctx, id, sourcePayload('ics'), { 'device-a': 1 }) + + const pushed = calendarSourceHandler.buildPushPayload(db as unknown as DrizzleDb, id) + + expect(pushed).not.toBeNull() + expect(JSON.parse(pushed as string)).toMatchObject({ provider: 'ics' }) + }) + }) + + describe('bindings and external events follow the same rule', () => { + it.each(UNKNOWN_PROVIDERS)('stores a %s binding verbatim', (provider) => { + const result = calendarBindingHandler.applyUpsert( + ctx, + `calendar_binding:${provider}:event:event-1`, + { + sourceType: 'event', + sourceId: 'event-1', + provider, + remoteCalendarId: 'remote-cal', + remoteEventId: 'remote-event', + ownershipMode: 'memry_managed', + writebackMode: 'broad', + createdAt: '2026-04-12T08:00:00.000Z', + modifiedAt: '2026-04-12T08:00:00.000Z' + }, + { 'device-a': 1 } + ) + + expect(result).toBe('applied') + const row = db + .select() + .from(calendarBindings) + .where(eq(calendarBindings.id, `calendar_binding:${provider}:event:event-1`)) + .get() + expect(row?.provider).toBe(provider) + expect(row?.archivedAt).toBeNull() + }) + + it('accepts an external event whose parent source is an unknown provider', () => { + const sourceId = 'caldav-calendar:remote-1' + calendarSourceHandler.applyUpsert(ctx, sourceId, sourcePayload('caldav'), { 'device-a': 1 }) + + const result = calendarExternalEventHandler.applyUpsert( + ctx, + 'calendar_external_event:caldav:remote-event-1', + { + sourceId, + remoteEventId: 'remote-event-1', + title: 'Standup', + startAt: '2026-04-14T09:00:00.000Z', + endAt: '2026-04-14T09:15:00.000Z', + isAllDay: false, + timezone: 'UTC', + status: 'confirmed', + createdAt: '2026-04-12T08:00:00.000Z', + modifiedAt: '2026-04-12T08:00:00.000Z' + }, + { 'device-a': 1 } + ) + + expect(result).toBe('applied') + // The parent-source guard (#837) is keyed on the row existing, not on + // the provider being one this build can sync. + expect(db.select().from(calendarExternalEvents).all()).toHaveLength(1) + }) + }) + + describe('the payload round trip keeps the provider', () => { + it.each(UNKNOWN_PROVIDERS)('survives schema parse for %s sources', (provider) => { + const parsed = CalendarSourceSyncPayloadSchema.parse(sourcePayload(provider)) + + // `provider` is `z.string().optional()`, not an enum — an old client's + // zod must not strip a value it does not recognise, because a stripped + // field would come back as the `?? 'google'` fallback and silently + // re-home the calendar. + expect(parsed.provider).toBe(provider) + }) + + it.each(UNKNOWN_PROVIDERS)('survives schema parse for %s bindings', (provider) => { + const parsed = CalendarBindingSyncPayloadSchema.parse({ + sourceType: 'event', + sourceId: 'event-1', + provider, + remoteCalendarId: 'remote-cal', + remoteEventId: 'remote-event' + }) + + expect(parsed.provider).toBe(provider) + }) + + it('survives the full local → push payload → remote apply loop', () => { + const id = 'microsoft-calendar:remote-1' + calendarSourceHandler.applyUpsert(ctx, id, sourcePayload('microsoft'), { 'device-a': 1 }) + + // What device A ships… + const pushed = calendarSourceHandler.buildPushPayload(db as unknown as DrizzleDb, id) + const decoded = CalendarSourceSyncPayloadSchema.parse(JSON.parse(pushed as string)) + + // …applied on a second device, exactly as the pull path does it. + const other = createTestDataDb() + try { + const otherCtx = makeCtx(other) + calendarSourceHandler.applyUpsert(otherCtx, id, decoded, { 'device-a': 1 }) + const row = other.db.select().from(calendarSources).where(eq(calendarSources.id, id)).get() + expect(row?.provider).toBe('microsoft') + expect(row?.metadata).toEqual({ url: 'https://example.com/feed.ics' }) + expect(row?.syncCursor).toBe('cursor-1') + } finally { + other.close() + } + }) + }) + + describe('delete-clock behaviour is provider-independent', () => { + it.each(UNKNOWN_PROVIDERS)( + 'applies a remote delete for %s exactly as it would for google', + (provider) => { + const id = `${provider}-calendar:remote-1` + calendarSourceHandler.applyUpsert(ctx, id, sourcePayload(provider), { 'device-a': 1 }) + + expect(calendarSourceHandler.applyDelete(ctx, id, { 'device-a': 2 })).toBe('applied') + expect( + db.select().from(calendarSources).where(eq(calendarSources.id, id)).get() + ).toBeUndefined() + } + ) + + it.each(UNKNOWN_PROVIDERS)( + 'skips a stale remote delete for %s when the local copy has unseen changes', + (provider) => { + const id = `${provider}-calendar:remote-1` + calendarSourceHandler.applyUpsert(ctx, id, sourcePayload(provider), { 'device-a': 5 }) + + // A tombstone from a device that had not seen the local edits. + expect(calendarSourceHandler.applyDelete(ctx, id, { 'device-b': 1 })).toBe('skipped') + expect( + db.select().from(calendarSources).where(eq(calendarSources.id, id)).get()?.provider + ).toBe(provider) + } + ) + + it('applies a google delete and an unknown-provider delete identically', () => { + calendarSourceHandler.applyUpsert(ctx, 'google-calendar:g', sourcePayload('google'), { + 'device-a': 1 + }) + calendarSourceHandler.applyUpsert(ctx, 'caldav-calendar:c', sourcePayload('caldav'), { + 'device-a': 1 + }) + + const googleResult = calendarSourceHandler.applyDelete(ctx, 'google-calendar:g', { + 'device-a': 2 + }) + const caldavResult = calendarSourceHandler.applyDelete(ctx, 'caldav-calendar:c', { + 'device-a': 2 + }) + + expect(caldavResult).toBe(googleResult) + expect(db.select().from(calendarSources).all()).toHaveLength(0) + }) + }) +}) diff --git a/apps/docs/src/architecture/sync-handlers.md b/apps/docs/src/architecture/sync-handlers.md index 6dd41cea8..61aeb518c 100644 --- a/apps/docs/src/architecture/sync-handlers.md +++ b/apps/docs/src/architecture/sync-handlers.md @@ -324,3 +324,47 @@ const result = mergeFields({ ``` When a record predates Phase 8 and lacks `fieldClocks`, `initAllFieldClocks(docClock, fields)` initializes them on first merge. + +## Unknown Calendar Providers Are Not Corruption + +`calendar_sources`, `calendar_bindings` and `calendar_external_events` all carry +a free-form `provider` string. The multi-provider calendar work (#1390) means a +device on a newer build can create rows with `provider` set to `ics`, `caldav` +or `microsoft` and sync them to a device that only knows `google`. + +The item handlers were already defensive in one direction — a payload that +omits `provider` falls back to `'google'`, for rows written before the column +carried meaning. The other direction had never been verified, and it is the +one that can lose user data: if an old client decided an unrecognised row was +corrupt and cleaned it up, the delete would sync back and remotely destroy the +connection the user had just made on the new device. + +**Finding: it does not.** Verified in +`main/sync/calendar-cross-version-compat.test.ts`: + +- The handlers are provider-blind. `applyUpsert` writes `provider` verbatim and + never rewrites a value it does not recognise; `?? 'google'` only fires when + the key is genuinely absent. +- Nothing archives, deletes, or "repairs" a source because its provider is + unknown. The only two deletes on `calendar_sources` are an explicit + user-initiated disconnect — scoped by `provider` — and applying a remote + tombstone. There is no orphan sweep or credential-reconcile pass over these + tables. +- `buildPushPayload` round-trips `provider` unchanged, so an old client + re-pushing a row it cannot use does not re-home the calendar. +- Delete-clock resolution is provider-independent: a stale tombstone is skipped + and a causally-newer one is applied, identically for `google` and for a + provider the build has never heard of. +- The missing-parent guard on `calendar_external_events` keys on the parent row + existing, not on the provider being one this build can sync, so external + events under an unknown-provider source apply normally. + +**Consequence: no sync type negotiation gate is needed** for these rows. The +PR #754 pattern would have been the fallback if the old client had misbehaved; +it does not, so new-provider rows fan out to every device unconditionally and +an old device simply carries them read-only. + +What an old client _cannot_ do is sync such a source: no provider is registered +for that id, so nothing polls it, `hasLocalAuth` reads false, and the settings +row reports it as connected-without-credentials. That is inert, not +destructive, and it resolves itself the moment that device updates.