Fix CloudKit data loss on delete-then-reinsert

[jsutula]

Fixes #418

Root cause

Reinsertion of a record after a delete does not produce a new pending .saveRecord change entry due to missing "undelete" state detection in afterInsert trigger. No new .saveRecord means that only the .deleteRecord is propagated.

Solution

To fulfill the expected behavior of reinsertion resulting in a .saveRecord and assigning fresh timestamps to all fields, two-part solution:

1. Prerequisite change necessary for writing accurate tests: Update MockSyncEngineState to deduplicate changes across types like the real CKSyncEngine.State

From CKSyncEngine.State.add(pendingRecordZoneChanges:) documentation:

Note
The order in which you apply record zone changes is important.
For example:

• If you add .saveRecord(recordA) then .deleteRecord(recordA), the sync engine
  discards the save and sends only the delete change.
• If you add .deleteRecord(recordA) then .saveRecord(recordA), the sync engine
  discards the delete and sends only the save change.

MockSyncEngineState.add(pendingRecordZoneChanges:) deduplicates fully identical changes (type+ID) by virtue of the underlying OrderedSet, but does not deduplicate when the type (save vs delete) is different, as is described above with CKSyncEngine.State. MockSyncEngineState.add(pendingDatabaseChanges:) has a similar issue. The fix: update both to deduplicate based on ID alone which will allow for the cross-type deduplication described above, and add tests.

2. Primary change: Detect reinsertion of soft-deleted record and queue new .saveRecord change

First, update the afterInsert trigger to partially reset the SyncMetadata record when reinsertion is detected (_isDeleted is true). Partial reset involves:


1. Setting _isDeleted back to false

2. Updating userModificationTime
3. Clearing _lastKnownServerRecordAllFields

   • Necessary to guarantee that every field’s timestamp is updated to the current time, even if field value did not change
   • Note that lastKnownServerRecord is intentionally not cleared in order to retain still valid record-level metadata like the change tag.
   • Now that _lastKnownServerRecordAllFields can be cleared, introduce a new fallback in SyncEngine.nextRecordZoneChangeBatch(…) to allow use of lastKnownServerRecord when _lastKnownServerRecordAllFields is unavailable



Second, hook up a new afterUndeleteTrigger to listen for this specific “undelete”/reset occurring. This trigger is responsible for queuing a .saveRecord change via syncEngine.$didUpdate (the same as what happens after a normal insert or update).

Testing

In addition to new unit tests, performed a manual test with 9ea5bdd applied to verify that the data loss issue no longer occurs. Result (compare with before):

Screen.Recording.2026-03-13.at.12.32.44.AM.mov

[jsutula]

I've had a few rounds of refinement to make this a cleaner and more correct fix.
With the latest change to assign fresh timestamps to all fields on reinsertion, this should be fully ready for review now. Description is up to date.

[jsutula]

One note here: clearing _lastKnownServerRecordAllFields may be too blunt an instrument to get the desired result of fresh timestamps for every field. It does guarantee that the next set of changes sent to the server will have fresh timestamps for all fields, but I believe also introduces a window of time (after reinsert but before successful submission of changes) where concurrent modifications from another device could be fetched and applied with no delta-filtering baseline, potentially overwriting locally-reinserted values.

A more direct solution may call for a separate field on SyncMetadata, something like _isReinserted: Bool. Or better yet: roll up _isDeleted and _isReinserted as cases in a new optional enum field PendingStatus. The status being reinserted could then be used to control outbound change inclusion behavior (include changes for all fields, each modified at the row's userModificationTime) and inbound filtering behavior (only take server field value if its modified time is greater than or equal to local row's userModificationTime). I'll wait for feedback before pursuing this in case there may be other options.