fix: never undo server changes to a card moved out of synced books #34

When another client moved a contact or an event out of every address
book or calendar the device syncs while the device had an unsent edit
of it, the download only noted it for dropping after the upload. Its
shadow stayed at the old version while the stored state moved past the
change, so the upload passed `ifInState` and resent whole maps built
from the old shadow: a number or an occurrence added on the server
meanwhile was lost. A full reconcile that found such an object did the
same.

The download now merges the new version into the dirty rows like any
other change, so the upload builds on it, and the rows still go once
it is accepted. A contact's collections are read from its shadow:
SYNC1 lists only the selected books and is empty for a card that left
them all, so such a contact was never dropped after its upload. The
calendar planner cannot merge an event that is in no synced calendar
yet; until it can, such an event waits on the device and is reported
instead of overwriting the server.

Checked with the real planners through `/changes` and through a full
reconcile.

src/device-sync/engine/contacts-sync.ts

@@ -151,7 +151,8 @@ export class ContactsSync extends ItemSync {
         deleted: c.deleted,
         isNew: !c.sourceId,
         poison: c.poison,
-        collections: c.collections,
+        // The card's books as last seen: SYNC1 lists only the selected ones, none for a card that left them all.
+        collections: c.shadow ? this.cardKeys(accountOfRef(c.sourceId), c.shadow) : c.collections,
       }),
       fingerprint: (c) =>
         poisonFingerprint({

src/device-sync/engine/item-sync.ts

@@ -408,14 +408,16 @@ export abstract class ItemSync {
       if (flags.dirty || flags.deleted) verify.push(id);
       else gone.add(id);
     }
+    const outside: string[] = [];
     if (verify.length) {
-      // Destroyed, or moved out of the synced collections? Only a destroy takes dirty rows.
+      // Destroyed, or moved out of the synced collections? Only a destroy takes dirty rows; the
+      // others are downloaded like any change, so a dirty item merges the server's version.
       const { list, notFound } = await this.env.jmap.get<ServerObject>(this.itemType, acct, verify, ['id']);
       for (const id of notFound) gone.add(id);
-      for (const object of list) this.outsideOf(acct).add(refOf(acct, object.id));
+      for (const object of list) outside.push(object.id);
     }
     const selected = collections.map((c) => collectionKey(acct, c));
-    await this.processIds(acct, [], [...gone], (next) => {
+    await this.processIds(acct, outside, [...gone], (next) => {
       const account = accountOf(next, acct);
       account.itemsState = from;
       account.reconcile = null;
@@ -555,7 +557,14 @@ export abstract class ItemSync {
     written: Map<Kind, Set<string>> | null,
   ): Work | null {
     if (!kind || !this.inSelection(acct, object)) {
-      return found ? this.removeWork(acct, object.id, found, 'outside') : null;
+      if (!found) return null;
+      const meta = found.kind.meta(found.local);
+      if (found.kind === kind && meta.dirty && !meta.deleted && !meta.isNew) {
+        // Uploaded first, dropped afterwards: merged like any dirty item, so the upload builds on the server's version.
+        this.outsideOf(acct).add(refOf(acct, object.id));
+        return this.downloadWork(kind, acct, object, found.local, written);
+      }
+      return this.removeWork(acct, object.id, found, 'outside');
     }
     if (found && found.kind !== kind) {
       const meta = found.kind.meta(found.local);
@@ -1263,6 +1272,8 @@ export abstract class ItemSync {
     const counted = () => {
       if (created) this.env.report.stats.uploaded.created++;
       else this.env.report.stats.uploaded.updated++;
+      // In no synced collection any more (moved away on the server): the rows go after the upload.
+      if (!this.inSelection(acct, server)) this.outsideOf(acct).add(refOf(acct, server.id));
       this.onAccepted(kind, acct, server);
     };
     const failed = (reason: string, message: string) => {