Repository navigation
fix(sync): re-register clips an applied remote change leaves dirty - #209
Merged
Merged
Conversation
- After a server-wins conflict, send the clip again when its membership rebuild dropped a board with a pending local deletion; the conflict reply now reports any row that still needs sending. - After a push-fed fetch, register the fetched clips the apply left dirty with the engine instead of leaving them for the next explicit start. - Move the still-dirty lookup into SyncOutboundWork so acknowledgement, conflict and fetch paths share it, matching zones by name. - Cover both paths with regression tests and document the behavior.
- Register dirty fetched clips even when only part of a push-fed page applied, then report the receive failure as before. - Treat an engine replaced mid-requeue as superseded work rather than a sync failure. - Return still-dirty saves with this device's own zone IDs, the form batch preparation accepts. - Read the upload queue after a server-wins conflict only when the membership rebuild dropped a board, and re-send if that follow-up read fails after the apply committed.
5 tasks done
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
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.
Summary
The pending-board-deletion fix re-flags a clip for upload when an applied remote copy still names a board whose local deletion has not propagated yet. Two adapter paths applied such a copy but never told
CKSyncEngineabout the new save, so the corrected clip waited for the next explicitstart()(panel open or a forced sync). Until then, the server copy kept naming the deleted board.SyncOutboundWork.resolveConflictreturned true only when the local copy won. A server copy that applied and then had a board dropped from its membership came back dirty, but nothing registered it with the engine again.handleFetchedRecordZoneChangesapplied the page and stopped there. The poll path is covered becausestart()and receive recovery re-enqueue all pending work, but a push-fed fetch has no such follow-up.Changes
resolveConflictnow returns "needs sending again" for a verified local win, and also for an applied server copy whose row is still dirty. To avoid scanning the upload queue for every conflict (for example, a resync after an identity reset), it reads the queue only when the post-apply membership differs from the board IDs the record carried. If that follow-up read fails after the apply has committed, the clip is sent again; batch preparation skips a row that turns out to be clean.SyncOutboundWork.stillDirtyand is shared by the acknowledgement, conflict and fetch paths. It matches zones by name and returns this device's own zone IDs, which is the formprepareaccepts.requeueStillDirtytreats an engine that was replaced while it ran as superseded work, not as a sync failure. The next start re-enqueues every dirty row anyway.docs/BOARD_DELETION_SYNC.mdand theSyncLocalStore.setBoardMembershipcontract. Added a Fixed entry to the not-yet-tagged 0.10.0 section of the CHANGELOG and extended the sync line indocs/releases/v0.10.0.md.Follow-ups (not in this PR)
updatedAt. If the board deletion is acknowledged before the clip upload goes out and the stale server copy is then replayed, the equal-timestamp remote wins again. That clears the dirty flag and can seed a placeholder board. While the tombstone is still pending, a replay just re-flags the clip, so this PR is not affected. Fixing it means changing either the tie rule or the re-queue revision, which needs its own change. The existing doc already says cross-zone ordering after acknowledgement is not promised.applyRemoteChanges. The protocol-extension default counts every board upsert asapplied, while the GRDB store counts rejected board metadata asskippedAsStale. The protocol'sapplyRemoteBoardUpsertreturnsVoid, so aligning the two means changing the protocol. Only test doubles use the default, and the difference affects counts in diagnostics only.RemoteApplySummarycould report which clips the membership rebuild re-flagged. The fetch path could then skip its one queue read per page.Testing
New regression tests in
SyncOutboundWorkTests, run against the real GRDB store:serverWinThatStaysDirty: a newer server copy that names a pending-deleted board makesresolveConflictreturn true. The clip stays queued, and the rebuilt record no longer carries the board. Without the fix this returns false.fetchedClipsLeftDirty: afterapplyFetchedwith one clip naming a pending-deleted board and one clean clip, only the stale clip is reported for registration. A clean board in the same request is not reported.conflictstest still pins that a plain server win needs no resend.The engine registration itself (
CKSyncEngine.state.add) cannot be unit-tested, because aCKSyncEnginecannot be built without CloudKit entitlements. The decision logic it uses is the code covered above. Nothing was compiled locally (Linux authoring host), so CI is the compile and test authority.swift-format lint --strictwas run on the touched files, andcheck-version-sync,check-product-truthandcheck-sitepass.Checklist
make formatandmake lintpass (swift-format strict on touched files; full lint in CI)make testpasses (CI)Generated by Claude Code