fix(connections): stop a sync acknowledging records a store refused to write

[datlechin]

Reported as a known residual on #2619 and fixed here. Written on top of #2622 and rebased onto main
once that merged, so this is a plain branch off main.

What was wrong

applyPullResult saved the server change token before applying anything, so a record that could
not be written was acknowledged anyway and CloudKit never sent it again. Recovering needed a full
resync, which nothing asks for on its own.

Underneath it, the reason the coordinator could not have known better: a Bool cannot answer this
question. applyRemoteConnection returned false for a tombstoned record, for a record that
changed nothing, and for a store that refused the write, and the caller read all three as "no
change". #2619 and #2622 then widened the conflation by adding an equality guard to the group and
tag stores, so false picked up a fourth meaning.

The record cache had the same problem one line further down. It stored every record in the batch
whether or not the batch applied, and mergeLocalEdits reads that cache as the base a later edit
merges against, so a record that was never written could still stand in as the base for one that
was.

What changed

• RemoteApplyOutcome with three cases: applied, skipped, failed. The connection, group and
  tag arms return it, and GroupStorage.applyRemoteGroup and TagStorage.applyRemoteTag return it
  too, so "identical, nothing to write" stops sharing an answer with "the store refused".
• applyRemoteChanges reports whether every record that can say so was persisted.
• The token and the record cache are committed only on that. A pull that answers false is logged
  and left unacknowledged, so the next one replays the batch.

Replaying is safe by construction: each apply writes by id, and the equality guards mean a record
that already landed writes nothing the second time.

What this does not cover

The settings, SSH profile, table favorite, database favorite and SQL favorite arms return nothing
to say whether they persisted, so they cannot participate and a failure in one of them still
acknowledges the batch. Giving each of them a result is a change per store rather than a change to
the pull, and none of them has the refuse-to-overwrite guard that made this reachable for
connections, groups and tags.

Verification

• verify.sh generate, verify.sh build: PASS
• verify.sh test GroupStorageTests TagStorageTests SyncCoordinatorTokenExpiryTests StringCatalogIntegrityTests: PASS
• swiftlint --strict on every changed file: clean

The three outcomes are tested on both stores: an unchanged record reports skipped and leaves
nothing dirty, a new one reports applied, and one applied over an unreadable store reports
failed. That is where the conflation lived and where a regression would land.

The coordinator's own aggregation is not unit-tested. applyPullResult is private on a type whose
only test suite exercises a static classifier, and constructing a SyncCoordinator needs CloudKit
plumbing that does not exist in the test target; building it to cover !outcomes.contains(.failed)
would be a harness larger than the logic it checks.

https://claude.ai/code/session_01L81TaoPWxkLd15CGw2riPq

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.