[mario-luis-dev]

When a save fails with a reference violation, the sync engine applies the child's ON DELETE action, assuming the parent was deleted on another device. The same error comes back when the parent was never uploaded, for example because its own save failed with quotaExceeded, which nothing retries. The child then gets deleted.

This applies the ON DELETE action only when the server has confirmed the parent before (its last known server record has a change tag). Otherwise it queues the parent and the child again.

To test it, MockCloudDatabase gets a storageQuota: once it's full, saving a new record fails with quotaExceeded, like a full iCloud. addReminderAfterListFailedToSaveWithQuotaExceeded fails on main and passes with this change, and the full suite passes.

[mbrandonw]

Hi @mario-luis-dev, the sorting fix is a good one, but it can be simplified and I have done so here #571. The other problem is trickier, and I don't quite yet understand how CloudKit can actually receive child records before parent. Your tests are not very convincing because they are specifically going outside the normal operations of the sync engine to get a failing test.

We will investigate this more.

[mario-luis-dev]

Thanks @mbrandonw, #571 is a cleaner fix for the sort. I'm happy to take the sorting change out of this PR and rebase what's left on top of yours.

How children reached CloudKit before their parents

In our case it was the sort. Here's what we saw. The setup was SQLiteData 1.12.0 on one simulator, in the development environment, with iCloud turned on for the first time into an empty zone and about 58,000 saves waiting:

• A minute after sync was turned on, sends started coming back with Reference Violation: Parent record <…:games> does not exist on the server for game_achievement_summaries and store_wishlist_items. They were still coming 15 minutes later, and batches of games were still being saved 8 minutes after that.
• Each rejection went through the .referenceViolation branch, which deleted the child as a user delete and queued that deletion. We compared against a backup taken a minute before sync was turned on. All 712 achievement summaries and all 139 wishlist items were gone, plus a playthrough and its sessions logged during the upload. Every parent was still in the local database. The zone started empty, so no other device could have deleted them.
• We ran a probe of the 1.12 comparator over our real table order and those 58,376 saves. It put a child ahead of its parent in 50 of 50 shuffled inputs, and in 0 of 50 once the six tables without foreign keys were removed.

This matches what our users had been reporting for a while: episodes they'd marked as watched coming back unwatched.

The tests

You're right about them. The first two build the failing state by hand: one sends only the child, the other removes the parent's pending save. With #571 in place, I don't have a sequence of ordinary writes and sends that causes the deletion through ordering alone. In the description I also listed a concurrent send (the willResignActive one) as a cause. I haven't verified that CKSyncEngine ever runs two sends at once, so please disregard that one.

Why I'd still guard the deletion

The branch decides "the parent was deleted on the server" from an error that can also mean "the parent isn't on the server yet". In our case that guess was wrong every time. When it's wrong, the result is the worst kind of failure: a user-level delete that cascades, syncs to every device, and shows no error.

Ordering also isn't the only way a parent can be missing. This part comes from reading the code and Apple's docs; I haven't reproduced it:

• Apple's docs say CKSyncEngine "removes changes from the list as it sends them, but retains any that fail due to a recoverable error". Those are the seven transient errors it retries itself (network, rate limit, zone busy and so on).
• handleSentRecordZoneChanges puts a failed save back only for serverRecordChanged, zoneNotFound, unknownItem and batchRequestFailed.
• nextRecordZoneChangeBatch stores a lastKnownServerRecord when it builds the batch. So the sign-in pass that re-queues unknown records skips that parent too.

So suppose a parent's first save fails with any other error. quotaExceeded, when the user's iCloud is full, is the one I'd expect most. The parent then stays off the queue until it's edited again. The next time one of its children is saved, CloudKit returns Reference Violation. If the child's table has one foreign key, the child is deleted while its parent is still in the local database.

If it would help, I can write that as a test that only touches the CloudKit boundary. MockCloudDatabase would fail a given record's save with a given error, which it can't do today, so that's a small addition to the mock. Everything else would go through ordinary writes and sends, without touching the sync engine's state.

The guard is narrow. It applies the ON DELETE action only when the parent's last known server record has a change tag, meaning the server has confirmed the parent at least once. When it has, as in your "remote deleted the list" tests, nothing changes. When it hasn't, both records are queued again, parent first. I'm not attached to this exact shape, so if you'd rather handle it another way, I'm happy to rework it.

[mbrandonw]

Hi @mario-luis-dev, please summarize the output of your AI agent in your own words. That is a lot of text to essentially say that 1) the sort fix in #571 would have fixed your problem, and 2) theoretically there is another problem lurking in the shadows. And I have to take a lot of my own time to read through all of that text and extract out its essence.

If there really is a problem in the library related to failed saves and batches, would it be possible to write a test that emulates reality and shows the problem? And if our mock CloudKit APIs are capable of showing the problem, can they be improved?

[mario-luis-dev]

Sorry about that, fair point. Yes, that's it: #571 fixes what we actually hit, so feel free to close this PR in favor of it. The second issue is only theory for now. I'll try to write a test that shows it using just your mock CloudKit APIs, extending them where needed, and I'll only come back if it really reproduces.

[mario-luis-dev]

Actually, I wrote a test that emulates reality. I added a storage quota to MockCloudDatabase: once it's full, saves fail with quotaExceeded, like they do when the user's iCloud is full. In the test, the user creates a list and a reminder while their iCloud is full, so both saves fail, and nothing retries them. Then they free up space and add a second reminder. That save fails with a reference violation, because the list never made it up, and the library deletes the reminder from the device. The test fails on main and on #571, and passes with the change in this PR.

Also, the first reminder never uploads at all, because nothing re-queues a save that failed with quotaExceeded.

[mbrandonw]

Hi @mario-luis-dev, for quota exceeded errors in particular we have #335 that we will be merging and releasing soon.