iCloud sync: merge core and CloudKit transport - #48
Merged
Conversation
Training currently lives on one device, which every public surface now admits. This is the first half of fixing that: the merge, built and tested before any CloudKit code exists, because the merge is the only part of syncing that can silently destroy recorded training. Pure functions with an injected clock, so the rules can be reviewed and tested without a container or a network. Taking per-record CloudKit rather than SwiftData+CloudKit, which is the recommendation on #44 and is stated there as an assumption rather than a settled decision. A whole-document copy would force a person to choose between two versions of their training whenever two devices both wrote — a decision nobody can make correctly mid-workout. Per record the common cases resolve themselves. Rules, each with a test: - History is append-only and never resolves by time, so two phones that recorded different sessions end up with both, and a device with a wrong clock or one that never saw a workout cannot erase it. A session tombstone cannot win. - Templates and goals are last-writer-wins, with tombstones, so a delete propagates instead of the other device pushing the entity straight back. - Timestamp ties break on payload bytes, so two devices merging the same pair reach the same answer rather than disagreeing forever. - The active session is never synced. A workout in progress belongs to the phone in your hand. - Bundled templates are not records; shipping content should not travel through somebody's iCloud account. - Merging an already merged result proposes no further work. The round-trip test caught a real defect: `.iso8601` truncates to whole seconds, so a record did not decode back to the value it was encoded from. Every record would have looked freshly edited on each trip, and two edits inside one second would have tied on timestamp and fallen through to the arbitrary tie-break. Sync payloads now encode dates exactly. The export keeps ISO 8601, which is right for a file a person may open. `WorkoutTemplate` and `ExerciseGoal` deliberately gained no `updatedAt`. That puts sync bookkeeping inside the training model, where every mutation site has to maintain it and will be wrong the first time one forgets. A ledger fingerprints the encoded payload instead, which cannot be forgotten. No caller yet, by design. Transport, the iCloud entitlement, ledger persistence and real `CKAccountStatus` in Settings remain on #44, and sync stays unshipped until two-device convergence is verified on hardware. Settings still says sync is not active, which is still true. Native gate: 80 unit and 11 UI tests pass, release build succeeds, coverage 84.3236%. Duplication zero, complexity unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The merge core had no way to talk to a server. This is the other half: a private-database CloudKit store behind a protocol, and a coordinator that sequences fetch, merge, push, and the change token. The transport holds no merge rules. It maps SyncRecord to CKRecord, fetches changes by token, and reports whether iCloud is usable. A custom zone is required because the default zone cannot resume from a token. Setline's own modifiedAt is stored as a field, not CloudKit's modification date, so upload order cannot decide who wins. The coordinator is tested against an in-memory store: first sync, idempotent second sync, a session arriving from another device, last-writer-wins on a goal, a failed push that must not advance the token, unreadable bookkeeping that must not block, and a local wipe that must forget the ledger or it tombstones everyone else's training. Mapping tests cover the CKRecord round trip, including tombstones and malformed remote records. Application Support paths for the document and the sync ledger now share SetlineFiles, so a second store cannot invent a third location. No caller yet. Wiring, the entitlement, and Settings remain the next commit. Two-device convergence is still unverified on hardware.
AppModel now owns a SyncCoordinator, refreshes CKAccountStatus for Settings, and reconciles on launch and on returning to the foreground. A workout in progress blocks it. Demo and UI-test launches never reach iCloud, so a fixture cannot write into a real account. Import and local reset forget the ledger first, or a wipe would tombstone the same training everywhere. Settings says why sync is idle rather than just that it is off, and only offers a button when there is something a person can do. The CloudKit container is claimed in the entitlement and checked against the source and the bundle id by a cheap test, because a mismatch fails on a device with an opaque error the simulator will not see. History UI tests now open a session by its row and scan in one direction, instead of tapping any text that mentions the workout name. Three 2s timeouts in the segment test became 5s; the assertions are unchanged. Coverage floor is 82.6% against a measured 83.1305%. That is a reduction from 83.8%, because CloudKit network calls cannot execute in a simulator. The comment records the measurement and the rule: lower the floor only for a stated structural reason, never to make a red build green. Native gate: unit tests, 11 UI tests, release build, and 83.1305% coverage pass. Duplication remains zero.
Privacy now says what the transport does and does not do: private database only, no Setline account, no active session, and nothing sent when iCloud is signed out. It also says sync is built but not active, because two-device convergence has not been checked on hardware. PROJECT_STATUS records the same split: implemented on this branch, not claimed as shipped, and the coverage floor moved from 83.8% to 82.6% for a structural reason rather than a red build.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked work from closed #47, rebased onto main after #40 merged. Part of #44.
Why
Training still lives on one device. The merge is the only part of sync that can silently destroy recorded work, so it landed first as pure functions. This PR adds the transport and the wiring, without claiming two-device sync is shipped.
What landed
Merge core — already reviewed on #47:
Transport — private CloudKit only, custom
Trainingzone, change tokens, Setline's ownmodifiedAtrather than CloudKit's. The coordinator is tested against an in-memory store. Demo and UI-test launches never reach iCloud.Settings — reports real
CKAccountStatusand why sync is idle. Import and local reset forget the ledger first, or a wipe would tombstone the same training everywhere.Privacy — describes the transport and still says it is built but not active, because two-device convergence has not been checked on hardware.
Coverage
Floor moved from 83.8% to 82.6% against a measured 83.1305%. CloudKit network calls cannot execute in a simulator; mapping, merge, tombstones and the ledger stay covered. Lower the floor only for a stated structural reason, never to make a red build green.
Verification
pnpm checkgreen (24 tests, including container/entitlement parity)pnpm quality:nativegreen: unit tests, 11 UI tests, release build, 83.1305% coverageNot done, and not claimed
iCloud.com.significanthobbies.setlinein the Apple Developer account (device signing needs it)Sync stays unshipped until two-device convergence is checked on hardware.