From d088dd14e9e77fd829cb2143cf5535aa483abd3e Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sun, 27 Sep 2026 07:42:50 +0200 Subject: [PATCH] fix: stop asking about waiting changes after a group edit reached the server #34 A contact stays marked as changed until the group cards on the server show its membership edits. The engine uploaded those edits as patches of the group cards after the contacts, but never looked at the contact again in that run, so it stayed marked until the next sync. Turning contacts sync off or signing out right after adding a contact to a group therefore asked whether to drop one waiting change, although nothing was waiting. Once the membership patches are accepted, the contacts that have nothing else to upload are now cleared in the same run, behind the same version check as any other clean write. --- .../__tests__/engine/real-planners.test.ts | 32 +++++++++++++++++++ src/device-sync/engine/contacts-sync.ts | 26 ++++++++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/device-sync/__tests__/engine/real-planners.test.ts b/src/device-sync/__tests__/engine/real-planners.test.ts index 2a20e232..fc4e91d5 100644 --- a/src/device-sync/__tests__/engine/real-planners.test.ts +++ b/src/device-sync/__tests__/engine/real-planners.test.ts @@ -11,6 +11,7 @@ import { addDeviceContact, addServerCards, CALENDAR_AUTHORITY, + CONTACTS_AUTHORITY, createHarness, rowWrites, type Harness, @@ -155,6 +156,37 @@ describe('device sync engine with the real planners', () => { expect(h.contacts().every((c) => !c.dirty)).toBe(true); }); + describe('a membership edit on the device', () => { + async function adaJoinsFriends() { + const h = real(); + const [ada] = addServerCards(h, ['Ada Lovelace']); + const group = h.server.addCard('a', { uid: 'g1', kind: 'group', name: { full: 'Friends' }, members: {} }); + await h.run(); + const contact = h.contactNamed('Ada Lovelace')!; + h.device.user.addToGroup(contact.id, Number(h.device.rows('groups')[0]._id)); + return { h, ada, group, contact }; + } + + it('leaves the contact clean once the group card shows it', async () => { + const { h, ada, group, contact } = await adaJoinsFriends(); + + expect((await h.run()).outcome).toBe('ok'); + + const uid = h.server.get('ContactCard', 'a', ada)!.uid as string; + expect(h.server.get('ContactCard', 'a', group)!.members).toEqual({ [uid]: true }); + expect(h.contacts().find((c) => c.id === contact.id)).toMatchObject({ dirty: false }); + }); + + it('leaves nothing waiting for the teardown once it is uploaded', async () => { + const { h, ada, group } = await adaJoinsFriends(); + + expect(await h.teardown(CONTACTS_AUTHORITY)).toEqual({ pending: 0 }); + + const uid = h.server.get('ContactCard', 'a', ada)!.uid as string; + expect(h.server.get('ContactCard', 'a', group)!.members).toEqual({ [uid]: true }); + }); + }); + describe('a device edit of an object the server moved out of every synced collection', () => { /** Ada with two numbers in the synced book; the device deletes the second in place (AOSP). */ async function adaWithDeletedNumber() { diff --git a/src/device-sync/engine/contacts-sync.ts b/src/device-sync/engine/contacts-sync.ts index 9a35eb32..fe917a84 100644 --- a/src/device-sync/engine/contacts-sync.ts +++ b/src/device-sync/engine/contacts-sync.ts @@ -710,7 +710,31 @@ export class ContactsSync extends ItemSync { } pending.actions.push(action as Pending['actions'][number]); } - if (byGroup.size) await this.send(acct, [...byGroup.values()]); + if (!byGroup.size) return; + await this.send(acct, [...byGroup.values()]); + await this.cleanAfterMemberships(acct, rows); + } + + /** + * A contact stays dirty until the group cards show its membership edits. + * Those that now have nothing else to upload are cleaned right away, so + * nothing looks like it still waits (turning sync off would ask about it). + */ + private async cleanAfterMemberships(acct: string, rowIds: number[]): Promise { + await this.refreshGroups(); + const works: Work[] = []; + for (const contact of await this.contactKind.loadByRowIds(rowIds)) { + if (!contact.dirty || contact.deleted || this.isStale(acct, contact.sourceId)) continue; + let plan; + try { + plan = this.contactKind.planUpload(contact, acct); + } catch (error) { + this.plannerFailed(acct, contact.sourceId ?? `row:${contact.rawContactId}`, 'upload', error); + continue; + } + if (plan.kind === 'clean') works.push({ group: plan.ops }); + } + await this.env.writer.write(works); } // ── Deselection ──