From fe0aaa124745d9e3d5fd6f14818480fdd03ddfd3 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sun, 27 Sep 2026 07:49:14 +0200 Subject: [PATCH] fix: load a book selected while the first sync was cut short #34 A full reconcile that ran out of time resumes after the last object it processed, in the sorted order of every listed id. It listed the collections selected at the time it resumed and marked them all loaded at its end, so a book turned on in between lost the objects that sorted before that point, and was never loaded again. The reconcile now keeps the collections it started with; one selected since is loaded after it like any newly selected collection. --- .../__tests__/engine/selection-state.test.ts | 24 +++++++++++++++++++ src/device-sync/engine/item-sync.ts | 10 +++++--- src/device-sync/engine/sync-state.ts | 6 ++++- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/device-sync/__tests__/engine/selection-state.test.ts b/src/device-sync/__tests__/engine/selection-state.test.ts index 01c683ed..429450a3 100644 --- a/src/device-sync/__tests__/engine/selection-state.test.ts +++ b/src/device-sync/__tests__/engine/selection-state.test.ts @@ -110,6 +110,30 @@ describe('device sync engine: collections loaded or dropped in part', () => { expect(h.contacts().map((c) => c.name)).toEqual(['Ada']); }); + it('downloads every card of a book selected while a first sync was cut short', async () => { + const h = createHarness(); + const extra = h.server.addAddressBook('a', { name: 'Friends' }); + const friends = addServerCards(h, Array.from({ length: 10 }, (_, i) => `F${i}`), 'a', extra); + addServerCards(h, Array.from({ length: 120 }, (_, i) => `P${String(i).padStart(3, '0')}`)); + h.prefs.contactsSelection[`a/${extra}`] = false; + // The first sync (a full reconcile): 'reconcile', 'reconcile:ids', 'reconcile:objects': out of time after chunk 1. + outOfTimeAt(h, 3); + expect((await h.run()).outcome).toBe('cancelled'); + h.checkpoints.onCheckpoint = undefined; + const marker = h.state()!.accounts.a.reconcile!; + expect(friends.filter((id) => id <= (marker.after as string)).length).toBeGreaterThan(0); + + // The user turns Friends on before the next sync. + h.prefs.contactsSelection[`a/${extra}`] = true; + expect((await h.run()).outcome).toBe('ok'); + + const onDevice = new Set(h.contacts().map((c) => c.sourceId)); + expect(friends.filter((id) => !onDevice.has(`a/${id}`))).toEqual([]); + expect(h.contacts()).toHaveLength(130); + expect(h.state()!.accounts.a).toMatchObject({ reconcile: null }); + expect(h.state()!.accounts.a.selected.sort()).toEqual([`a/${h.book}`, `a/${extra}`].sort()); + }); + it('loads the events of a calendar dropped by a stopped run when it is selected again', async () => { const h = real(); const holidays = h.server.addCalendar('a', { name: 'Holidays' }); diff --git a/src/device-sync/engine/item-sync.ts b/src/device-sync/engine/item-sync.ts index 362d5686..af3880b6 100644 --- a/src/device-sync/engine/item-sync.ts +++ b/src/device-sync/engine/item-sync.ts @@ -404,10 +404,11 @@ export abstract class ItemSync { */ protected async fullReconcile(acct: string): Promise { let marker = this.env.store.account(acct).reconcile; + const synced = this.syncedCollections(acct); if (!marker?.from) { await this.env.checkpoints.check('reconcile'); const from = await this.env.jmap.takeState(this.itemType, acct); - const started = { from, phase: 'ids' as const, position: 0, after: null }; + const started = { from, phase: 'ids' as const, position: 0, after: null, collections: synced.map((c) => collectionKey(acct, c)) }; await this.env.writer.write([], this.tail([acct], (next) => { accountOf(next, acct).reconcile = { ...started }; })); @@ -415,7 +416,10 @@ export abstract class ItemSync { } const from = marker.from as string; await this.beforeReconcile(acct); - const collections = this.syncedCollections(acct); + // A resumed reconcile lists the collections it started with (their ids up to `after` are done): + // one selected since then is loaded afterwards, from its first id. + const keys = marker.collections; + const collections = keys ? synced.filter((c) => keys.includes(collectionKey(acct, c))) : synced; const listed = new Set(); for (const collection of collections) { await this.env.checkpoints.check('reconcile:ids'); @@ -441,7 +445,7 @@ export abstract class ItemSync { await this.env.checkpoints.check('reconcile:objects'); const part = todo.slice(i, i + size); position += part.length; - const mark = { from, phase: 'objects' as const, position, after: part[part.length - 1] }; + const mark = { from, phase: 'objects' as const, position, after: part[part.length - 1], ...(keys ? { collections: keys } : {}) }; await this.processChunk(acct, part, [], (next) => { accountOf(next, acct).reconcile = { ...mark }; }); diff --git a/src/device-sync/engine/sync-state.ts b/src/device-sync/engine/sync-state.ts index 99ad9089..896d9d3d 100644 --- a/src/device-sync/engine/sync-state.ts +++ b/src/device-sync/engine/sync-state.ts @@ -18,6 +18,8 @@ export interface ReconcileMarker { position: number; /** The last id processed in that (sorted) order: a resumed reconcile continues after it. */ after: string | null; + /** Keys of the collections the reconcile lists; one selected since is loaded after it. Absent: every synced one. */ + collections?: string[]; } export interface AccountState { @@ -74,12 +76,14 @@ const stringOrNull = (value: unknown): string | null => (typeof value === 'strin function parseReconcile(value: unknown): ReconcileMarker | null { if (!value || typeof value !== 'object') return null; const r = value as Record; - return { + const marker: ReconcileMarker = { from: stringOrNull(r.from), phase: r.phase === 'objects' ? 'objects' : 'ids', position: typeof r.position === 'number' && r.position >= 0 ? r.position : 0, after: stringOrNull(r.after), }; + if (Array.isArray(r.collections)) marker.collections = strings(r.collections); + return marker; } function parseAccount(value: unknown): AccountState {