From 534cb8239eb33804ba88218d75c82eb8be7c787e Mon Sep 17 00:00:00 2001
From: Linus Rath <139418639+rathlinus@users.noreply.github.com>
Date: Sun, 27 Sep 2026 07:19:16 +0200
Subject: [PATCH] fix: keep the groups of a contact created on the device #34

A contact created on the device and put into a group before its first
sync lost that group: the write after its create took the rows' own
memberships as what the server already had, found them missing there
and deleted the rows, so the group card was never patched.

The create's write now starts from no memberships, as its fallback
already did: the device's memberships count as additions, stay on the
device, keep the contact dirty until the group cards show them, and
upload as patches of the group cards.

Checked with a planner test and end to end through the engine.
---
 .../__tests__/contacts/groups.test.ts         | 24 ++++++++++++++++++-
 .../__tests__/contacts/real-engine.test.ts    | 17 ++++++++++++-
 src/device-sync/contacts/download.ts          |  4 +++-
 src/device-sync/contacts/upload.ts            |  4 +++-
 4 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/src/device-sync/__tests__/contacts/groups.test.ts b/src/device-sync/__tests__/contacts/groups.test.ts
index d9ac7869..786271b9 100644
--- a/src/device-sync/__tests__/contacts/groups.test.ts
+++ b/src/device-sync/__tests__/contacts/groups.test.ts
@@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest';
 import { Data, Groups, MimeType } from '../../android-columns';
 import type { ContactCardWire, OpGroup, UploadPlan } from '../../planner';
 import { appleCard, googleCard } from './fixtures';
-import { Harness, JMAP } from './harness';
+import { ACCOUNT, Harness, JMAP } from './harness';
 
 function actionsOf(plan: UploadPlan<ContactCardWire>) {
   if (plan.kind !== 'upload') throw new Error(`expected an upload, got ${plan.kind}`);
@@ -130,6 +130,28 @@ describe('contacts groups: membership edits', () => {
     });
   });
 
+  it('keeps the groups of a contact created on the device and adds it to their cards', async () => {
+    const h = new Harness();
+    const gid = h.addCard({ uid: 'urn:uuid:g7', kind: 'group', name: { full: 'Team' }, members: { 'urn:uuid:someone': true } });
+    await h.download(gid);
+    const g = await localGroup(h, gid);
+    const rawId = h.device.user.insertContact(ACCOUNT, [
+      { [Data.MIMETYPE]: MimeType.STRUCTURED_NAME, data1: 'New Person', data2: 'New', data3: 'Person' },
+      { [Data.MIMETYPE]: MimeType.GROUP_MEMBERSHIP, [Data.DATA1]: g.groupId },
+    ]);
+    await h.upload(rawId);
+    expect(h.membershipGroups(rawId)).toEqual([`${JMAP}/${gid}`]);
+    expect(h.dirty(rawId)).toBe(true);
+    const uid = (await h.contact(rawId)).shadow!.uid as string;
+    const actions = h.planner.planMembershipUploads([await h.contact(rawId)], await h.groups(), h.ctx);
+    expect(actions).toEqual([{ kind: 'update', id: gid, patch: { [`members/${uid}`]: true } }]);
+    h.send(actions);
+    await h.applyOk(h.planner.planGroupAccepted(g, h.card(gid), h.ctx).ops);
+    await h.upload(rawId);
+    expect(h.dirty(rawId)).toBe(false);
+    expect(h.membershipGroups(rawId)).toEqual([`${JMAP}/${gid}`]);
+  });
+
   it('sends the whole map for the first member of an empty group', async () => {
     const h = new Harness();
     const empty = h.addCard({ uid: 'urn:uuid:g3', kind: 'group', name: { full: 'Empty' } });
diff --git a/src/device-sync/__tests__/contacts/real-engine.test.ts b/src/device-sync/__tests__/contacts/real-engine.test.ts
index df779301..2edfebe5 100644
--- a/src/device-sync/__tests__/contacts/real-engine.test.ts
+++ b/src/device-sync/__tests__/contacts/real-engine.test.ts
@@ -6,7 +6,7 @@ import { describe, expect, it } from 'vitest';
 import { Data, Groups, MimeType } from '../../android-columns';
 import { calendarPlanner } from '../../calendar/planner';
 import { contactsPlanner } from '../../contacts/planner';
-import { addServerCards, CONTACTS_AUTHORITY, createHarness, rowWrites, type Harness } from '../engine/harness';
+import { addServerCards, ANDROID_ACCOUNT, CONTACTS_AUTHORITY, createHarness, rowWrites, type Harness } from '../engine/harness';
 import type { SetTarget } from '../fakes/fake-jmap-server';
 import { JPEG } from './fixtures';
 import { thumbnailOnlyPort } from './harness';
@@ -80,6 +80,21 @@ describe('contacts through the engine', () => {
     expect(await h.teardown(CONTACTS_AUTHORITY)).toEqual({ pending: 0 });
   });
 
+  it('puts a contact created on the device into the group it was added to', async () => {
+    const h = real();
+    const group = h.server.addCard('a', { uid: 'g1', kind: 'group', name: { full: 'Friends' }, members: { other: true }, addressBookIds: { [h.book]: true } });
+    expect((await h.run()).outcome).toBe('ok');
+    const id = h.device.user.insertContact(ANDROID_ACCOUNT, [
+      { [Data.MIMETYPE]: MimeType.STRUCTURED_NAME, [Data.DATA1]: 'New Person' },
+      { [Data.MIMETYPE]: MimeType.GROUP_MEMBERSHIP, [Data.DATA1]: Number(h.device.rows('groups')[0]._id) },
+    ]);
+
+    expect(await h.run()).toMatchObject({ outcome: 'ok', itemErrors: [], stats: { uploaded: { created: 1, updated: 1 } } });
+    const uid = h.server.all('ContactCard', 'a').find((c) => c.kind !== 'group')!.uid as string;
+    expect(h.server.get('ContactCard', 'a', group)?.members).toEqual({ other: true, [uid]: true });
+    expect(memberships(h, id)).toHaveLength(1);
+  });
+
   it('keeps one row for a small photo the provider stores as its thumbnail, and never deletes it on the server', async () => {
     const h = real();
     const base = h.deps.provider;
diff --git a/src/device-sync/contacts/download.ts b/src/device-sync/contacts/download.ts
index 0d8e41b3..1e1575e7 100644
--- a/src/device-sync/contacts/download.ts
+++ b/src/device-sync/contacts/download.ts
@@ -239,6 +239,8 @@ export interface CleanWriteOptions {
   raw?: WriteRow;
   /** The rows' changes were just uploaded (planAccepted): a changed photo is the server's now. */
   accepted?: boolean;
+  /** The memberships the rows held at the last write, when there is no shadow to say: a new contact's are none. */
+  memberBase?: string[];
 }
 
 /** The JMAP account a local contact belongs to. */
@@ -260,7 +262,7 @@ export function cleanWrite(local: LocalContact, card: ContactCardWire, ctx: Ctx,
   const rows = cleanRows(local, remote, keyMap, base, options.accepted ?? false);
   const members = planMemberships({
     rows: local.rows,
-    base: shadowMemberOf(local.shadow),
+    base: shadowMemberOf(local.shadow) ?? options.memberBase ?? null,
     remote: remoteMemberships(server, ctx),
     evidence: rows.evidence,
     restore: true,
diff --git a/src/device-sync/contacts/upload.ts b/src/device-sync/contacts/upload.ts
index 67542358..3c1e7c1c 100644
--- a/src/device-sync/contacts/upload.ts
+++ b/src/device-sync/contacts/upload.ts
@@ -280,7 +280,9 @@ export function planContactAccepted(local: LocalContact, server: ContactCardWire
   }
   const identity = objectRef(accountOf(local, ctx), server.id);
   const raw = { [RawContacts.SOURCE_ID]: identity, [RawContacts.SYNC3]: null, [RawContacts.SYNC4]: null };
-  const write = cleanWrite(local, server, ctx, { assertDirty: null, clearDirty: true, accepted: true, raw });
+  // Just created: the groups it was put in on the device are all still to upload.
+  const memberBase = local.shadow ? undefined : [];
+  const write = cleanWrite(local, server, ctx, { assertDirty: null, clearDirty: true, accepted: true, raw, memberBase });
   const ops = group(local, write.ops.length ? write.ops : [assertContact(local, null)]);
   ops.ref = identity;