Fix permanent sync divergence when userModificationTime stamps tie

[notcome]

Summary

Two devices writing the same field with identical microsecond userModificationTime stamps can diverge permanently and silently: one device keeps its own value forever while the server and all other devices converge on a different value, and the stuck device reports fully synced. We hit this reproducibly (8/9 scripted trials) with near-simultaneous same-primary-key inserts from two processes — concurrent writers land nanosecond-identical stamps often, since the clock has microsecond resolution and timers coalesce.

Mechanism

Two behaviors compound:

1. Self-preferring tie-break. The merge guards in CloudKit+StructuredQueries.swift (setValue/setAsset/removeValue) use <=, so on a per-field stamp tie the local/base value overwrites the incoming committed server value. Self-preference is not a total order: both replicas of a tie each conclude "mine wins" and resolve the same tie oppositely.
2. Override without enqueue on the fetch path. When handleFetchedRecordZoneChanges → upsertFromServerRecord preserves local values over an incoming committed record, no upload is enqueued — the .serverRecordChanged conflict path re-enqueues (SyncEngine.swift), but the fetch path does not.

End to end: A and B insert the same key with stamp t. A commits first. B's save conflicts; the tie keeps B's value; the conflict path re-enqueues; B's retry commits — B and the server agree. A then fetches B's commit: the tie keeps A's value, the merge-mutated record (server change tag around A's values) is stored as A's base, and nothing is enqueued. A's row now differs from its base on every future delivery, so the isRowValueModified branch discards every subsequent server value while advancing the base — a silent absorbing state.

The fix

1. Strict < for inbound merges. The three setters gain a requireStrictlyNewer parameter (default false, so outgoing record construction — where an equal-stamp local edit must still apply — is unchanged). The inbound merge passes true: a base field overrules an incoming committed value only when strictly newer. On a tie the committed value wins. CloudKit's CAS chain already totally orders each record's history and replays it identically to every replica, so commit order serves as a deterministic tie-breaker shared by all replicas — no schema change, and safe in a mixed fleet of patched and unpatched clients (a patched device defers on ties; an unpatched peer's conflict path pushes its value; everyone converges on it).
2. Enqueue on override. When an inbound merge on the fetch path actively overrules incoming committed field values (base field strictly newer — e.g. clock skew between writers), a .saveRecord is enqueued for that record, mirroring the conflict path, so the surviving value is re-asserted to the server and replicas that never saw it converge. Two deliberate scope limits, both shaped by the existing test suite: stale replays (incoming older than the stored base) are suppressed, since a replay legitimately loses and must not cause churn; and row-level preservation of pending local edits (isRowValueModified) does not enqueue, since those uploads are already owned by the change triggers.

Tests

Two regression tests in MergeConflictTests using the existing mocks:

• equalTimestampConflictConvergesToServerValue: a fetched record carrying a different value at an equal field stamp must be adopted, leaving row, server, and base converged.
• olderTimestampFetchReassertsLocalWinToServer: a later commit carrying an older field stamp loses the merge, and the surviving local value is enqueued and pushed so the server converges on the LWW winner.

Full suite passes (241 tests). With Sources/ reverted to main and the tests kept, both fail with the expected signatures (device keeps "Mine" over committed "Theirs"; the .saveRecord never enqueued).

One observation for maintainers, made while validating the red case: MockCloudDatabase.modifyRecords stores and returns the same CKRecord instance (MockCloudDatabase.swift:204-205), so a merge that mutates a fetched record in place also mutates mock server storage — under the buggy code the mock "server" appears to adopt the stale device's value when real CloudKit would not. Happy to file that separately.

🤖 Generated with Claude Code

[notcome]

Human message: so Fable was doing some automatic integration testing by hand and hit this issue. At its core it is not hard to understand: if two devices modify at the same time, we will have a tie, and our code is essentially local always wins. We should end up with a ping pong situation, but the reconciliation code on the original winner masks it.

The fix is essentially, if values differ and field timestamp coincide, we always treat server record as winner (Change 1). We additionally add an re-enqueue write on conflict from syncing (Change 2). Change 1 is very understandable to me, but the diff is still bigger than I would hope. And Change 2 is arguably a bit over defensive, but it could rescue existing deployment from this bug – if such bug ever materializes, which would be so unlucky.

I tried my best to understand your codebase but this is a very intricate matter, so please take everything other than the idea a huge grain of salt. I did do the PR under a fresher context, if that helps boost your confidence.

[mbrandonw]

Hi @notcome, thanks for disclosing your AI use to open this PR and providing a shorter analysis of the situation. But just to confirm, you didn't actually encounter this on real devices, right? It is of course trivial to demonstrate the problem with the test suite because time is controlled, and so easy to perform two saves at the same time, but this happening on two real devices is about as likely as winning the lottery. That doesn't mean this shouldn't be investigated, but I do want to make it clear that any time we put into this is almost certainly chasing a problem that will never happen in reality.

Can you explain more how you ran into the problem in an integration test?

And as for this PR, I haven't looked at the implementation, but I was interested to see what the tests were trying to show. Unfortunately the tests aren't really testing a real life situation. The first one is emulating a record being updated locally and sync'd to the server, and then a remote client downloads that record and edits it at the same time and sends it back to the server. The chances of that are even less than two devices editing at the same microsecond. And the second test is also doing something illegitimate in that it is directly editing the timestamp in the record rather than going through the setValue(_:forKey:at:) helper.

I do think a few (legitimate) tests in the suite with some withKnownIssues to document the behavior would be a good thing. And while investigating this I did find a few problems with the MockCloudDatabase that needed to be fixed.