fix(sync): re-register clips an applied remote change leaves dirty

CHANGELOG.md

@@ -41,6 +41,9 @@ and release versions follow [Semantic Versioning](https://semver.org/spec/v2.0.0
   deletion, instead of letting an incoming copy of the board, or a clip that
   was still in it, bring it back. The clip's other boards are kept.
 
+- Correct a clip on iCloud right away when an incoming copy of it still lists
+  a board you deleted, instead of waiting for the next full sync.
+
 - Never let a generated title or a background semantic vector computed for
   older text land on a clip whose text has since changed.
 

Packages/GanchoKit/Sources/GanchoKit/SyncLocalStore.swift

@@ -119,7 +119,9 @@ public protocol SyncLocalStore: Sendable {
     /// record so membership rides the clip (the boards extension implements it).
     func boardIDs(forClip clipID: UUID) async throws -> Set<UUID>
     /// Rebuilds a clip's board membership from a synced record, seeding a
-    /// placeholder board for any id whose metadata hasn't synced yet.
+    /// placeholder board for any id whose metadata hasn't synced yet. Ids of
+    /// boards with a pending local deletion are dropped, and dropping one
+    /// flags the clip for re-upload.
     func setBoardMembership(clipID: UUID, boardIDs: Set<UUID>) async throws
 
     // Board metadata sync — the board table's mirror of the clip methods above,

Packages/GanchoKit/Sources/GanchoSync/CKSyncEngineAdapter.swift

@@ -551,20 +551,15 @@ extension CKSyncEngineAdapter {
             pendingRecordZoneChanges: work.changes(clipZoneID: zoneID, boardZoneID: boardZoneID))
     }
 
-    /// Re-adds acknowledged saves whose rows a newer local edit left dirty,
-    /// reading only the save queues those records can be in.
+    /// Re-adds saves whose rows the local store still considers dirty after an
+    /// acknowledgement or an applied fetch. A replaced engine is not a failure:
+    /// the next start re-enqueues every dirty row into its successor.
     private func requeueStillDirty(
-        _ acknowledged: Set<CKRecord.ID>, into engine: CKSyncEngine
+        _ recordIDs: Set<CKRecord.ID>, into engine: CKSyncEngine
     ) async throws {
-        let clips = Set(try await store.pendingUploadIDs())
-        let boards =
-            acknowledged.contains { $0.zoneID == boardZoneID }
-            ? Set(try await store.pendingBoardUploads().map(\.id)) : []
-        guard self.engine === engine else { throw CancellationError() }
-        let dirty = acknowledged.filter { recordID in
-            guard let id = UUID(uuidString: recordID.recordName) else { return false }
-            return recordID.zoneID == boardZoneID ? boards.contains(id) : clips.contains(id)
-        }
+        guard !recordIDs.isEmpty, self.engine === engine else { return }
+        let dirty = try await outbound.stillDirty(recordIDs)
+        guard self.engine === engine else { return }
         engine.state.add(pendingRecordZoneChanges: dirty.map { .saveRecord($0) })
     }
 
@@ -655,7 +650,7 @@ extension CKSyncEngineAdapter: CKSyncEngineDelegate {
         case .fetchedDatabaseChanges(let event):
             await handleFetchedDatabaseChanges(event, syncEngine: syncEngine)
         case .fetchedRecordZoneChanges(let event):
-            await handleFetchedRecordZoneChanges(event)
+            await handleFetchedRecordZoneChanges(event, syncEngine: syncEngine)
         case .sentRecordZoneChanges(let event):
             await handleSentRecordZoneChanges(event, syncEngine: syncEngine)
         case .willFetchChanges, .willSendChanges:
@@ -742,21 +737,36 @@ extension CKSyncEngineAdapter: CKSyncEngineDelegate {
     }
 
     private func handleFetchedRecordZoneChanges(
-        _ event: CKSyncEngine.Event.FetchedRecordZoneChanges
+        _ event: CKSyncEngine.Event.FetchedRecordZoneChanges, syncEngine: CKSyncEngine
     ) async {
         let generation = receiveGeneration
+        let records = event.modifications.map(\.record)
+        var applyError: (any Error)?
         do {
-            try await applyFetched(
-                records: event.modifications.map(\.record),
-                deletions: event.deletions.map(\.recordID))
+            try await applyFetched(records: records, deletions: event.deletions.map(\.recordID))
         } catch {
-            guard generation == receiveGeneration, !(error is CancellationError) else { return }
-            // The engine may persist its opaque fetch token regardless. The
-            // independent poll checkpoint has NOT moved and can replay it.
-            receiveHealth.fail(error)
-            emit(.failed(receiveHealth.failure ?? .unknown))
-            scheduleReceiveRecovery(after: error)
+            applyError = error
+        }
+        guard generation == receiveGeneration, !(applyError is CancellationError) else { return }
+        // An applied clip can come back dirty (its membership rebuild dropped
+        // a board whose local deletion is still pending). No full re-enqueue
+        // follows a push-fed fetch, so register those saves here instead of
+        // leaving them for the next explicit start — also when only part of
+        // the page applied, since its other changes have already committed.
+        let clipZoneName = zoneID.zoneName
+        let fetchedClips = Set(
+            records.map(\.recordID).filter { $0.zoneID.zoneName == clipZoneName })
+        if !isPaused, !outboundFailures.contains(.reset) {
+            do { try await requeueStillDirty(fetchedClips, into: syncEngine) } catch {
+                failOutbound(.pending)
+            }
         }
+        guard let applyError, generation == receiveGeneration else { return }
+        // The engine may persist its opaque fetch token regardless. The
+        // independent poll checkpoint has NOT moved and can replay it.
+        receiveHealth.fail(applyError)
+        emit(.failed(receiveHealth.failure ?? .unknown))
+        scheduleReceiveRecovery(after: applyError)
     }
 
     /// Applies a batch of fetched changes to the local store — shared by the
@@ -876,10 +886,10 @@ extension CKSyncEngineAdapter: CKSyncEngineDelegate {
                 guard let record = failure.error.serverRecord else {
                     throw SyncOutboundFailure.invalidRecord
                 }
-                let localWon = try await outbound.resolveConflict(record)
+                let needsSend = try await outbound.resolveConflict(record)
                 outboundFailures.remove(operation)
                 outboundFailures.remove(.acknowledgement(recordID))
-                if localWon {
+                if needsSend {
                     syncEngine.state.add(pendingRecordZoneChanges: [.saveRecord(recordID)])
                 }
             } catch {

Packages/GanchoKit/Sources/GanchoSync/SyncOutboundWork.swift

@@ -105,7 +105,33 @@ struct SyncOutboundWork: Sendable {
         }
     }
 
-    /// True means a verified local win needs sending again, never a failed apply.
+    /// The given saves whose local rows are still dirty: a newer local edit
+    /// behind an acknowledgement, or an applied server copy whose membership
+    /// rebuild dropped a board with a pending local deletion. Reads only the
+    /// save queues those records can be in. Zones match by name and the result
+    /// carries this device's own zone IDs, the form `prepare` accepts.
+    func stillDirty(_ recordIDs: Set<CKRecord.ID>) async throws -> Set<CKRecord.ID> {
+        let clips =
+            recordIDs.contains { $0.zoneID.zoneName == clipZone.zoneName }
+            ? Set(try await store.pendingUploadIDs()) : []
+        let boards =
+            recordIDs.contains { $0.zoneID.zoneName == boardZone.zoneName }
+            ? Set(try await store.pendingBoardUploads().map(\.id)) : []
+        return Set(
+            recordIDs.compactMap { recordID -> CKRecord.ID? in
+                guard let id = UUID(uuidString: recordID.recordName) else { return nil }
+                switch recordID.zoneID.zoneName {
+                case clipZone.zoneName where clips.contains(id):
+                    return CKRecord.ID(recordName: id.uuidString, zoneID: clipZone)
+                case boardZone.zoneName where boards.contains(id):
+                    return CKRecord.ID(recordName: id.uuidString, zoneID: boardZone)
+                default: return nil
+                }
+            })
+    }
+
+    /// True means the record needs sending again — a verified local win, or a
+    /// server copy that applied but left the row dirty — never a failed apply.
     func resolveConflict(_ record: CKRecord) async throws -> Bool {
         if record.recordType == BoardRecordMapper.recordType {
             guard let board = BoardRecordMapper.decode(record) else {
@@ -118,16 +144,30 @@ struct SyncOutboundWork: Sendable {
         guard let decoded = ClipRecordMapper.decode(record) else {
             throw SyncOutboundFailure.invalidRecord
         }
+        let boardIDs = Set(ClipRecordMapper.boardIDs(from: record))
         let summary = try await store.applyRemoteChanges(
             clips: [
                 .init(
                     item: decoded.item, content: decoded.content,
                     systemFields: ClipRecordMapper.encodeSystemFields(record),
-                    boardIDs: Set(ClipRecordMapper.boardIDs(from: record)))
+                    boardIDs: boardIDs)
             ],
             boards: [], clipDeletions: [], boardDeletions: [])
         guard summary.failed == 0 else { throw SyncOutboundFailure.localWrite }
-        return summary.skippedAsStale > 0
+        if summary.skippedAsStale > 0 { return true }
+        // A plain server win is clean, so this cannot loop. Only a membership
+        // rebuild that dropped a board re-flags the applied row, so the queue
+        // is read just then: a resync full of server wins must not scan it per
+        // record. The apply has committed, so an unreadable state re-sends
+        // instead of reporting a failed apply; preparation skips a clean row.
+        do {
+            guard try await store.boardIDs(forClip: decoded.item.id) != boardIDs else {
+                return false
+            }
+            return try await !stillDirty([record.recordID]).isEmpty
+        } catch {
+            return true
+        }
     }
 }
 

Packages/GanchoKit/Tests/GanchoSyncTests/SyncOutboundWorkTests.swift

@@ -467,4 +467,58 @@ extension SyncOutboundWorkTests {
         try await fixture.work.acknowledge(record)
         #expect(try await !fixture.store.pendingBoardUploads().map(\.id).contains(board.id))
     }
+
+    @Test("A server-wins conflict that drops a pending-deleted board is sent again")
+    func serverWinThatStaysDirty() async throws {
+        let fixture = try Fixture()
+        defer { fixture.clean() }
+        let item = try await fixture.insert()
+        let board = try await fixture.store.createPinboard(name: "synthetic board")
+        try await fixture.store.assign(clipID: item.id, toBoard: board.id)
+        try await fixture.store.deletePinboardForSync(id: board.id)
+        // A newer server copy from another device still names the deleted board.
+        var newer = item
+        newer.updatedAt = Date().addingTimeInterval(60)
+        let server = try #require(
+            ClipRecordMapper.record(
+                for: newer, content: .text("synthetic"), systemFields: nil,
+                zoneID: fixture.work.clipZone, boardIDs: [board.id]))
+
+        #expect(try await fixture.work.resolveConflict(server))
+        #expect(try await fixture.store.boardIDs(forClip: item.id).isEmpty)
+        #expect(try await fixture.store.pendingUploadIDs() == [item.id])
+        let replacement = try #require(
+            try await fixture.work.prepare([server.recordID])[server.recordID])
+        #expect(ClipRecordMapper.boardIDs(from: replacement).isEmpty)
+    }
+
+    @Test("Only fetched clips the apply left dirty are registered for sending")
+    func fetchedClipsLeftDirty() async throws {
+        let fixture = try Fixture()
+        defer { fixture.clean() }
+        let deleted = try await fixture.store.createPinboard(name: "synthetic deleted")
+        try await fixture.store.deletePinboardForSync(id: deleted.id)
+        let kept = try await fixture.store.createPinboard(name: "synthetic kept")
+        try await fixture.store.markBoardUploaded(id: kept.id, systemFields: Data([1]))
+        let stale = ClipItem(preview: "synthetic", contentHash: UUID().uuidString)
+        let clean = ClipItem(preview: "synthetic", contentHash: UUID().uuidString)
+        let staleRecord = try #require(
+            ClipRecordMapper.record(
+                for: stale, content: .text("synthetic"), systemFields: nil,
+                zoneID: fixture.work.clipZone, boardIDs: [deleted.id]))
+        let cleanRecord = try #require(
+            ClipRecordMapper.record(
+                for: clean, content: .text("synthetic"), systemFields: nil,
+                zoneID: fixture.work.clipZone, boardIDs: [kept.id]))
+        let adapter = CKSyncEngineAdapter(
+            store: fixture.store, containerIdentifier: "iCloud.test.gancho",
+            stateStore: .init(load: { nil }, save: { _ in }))
+
+        try await adapter.applyFetched(records: [staleRecord, cleanRecord], deletions: [])
+
+        let keptID = CKRecord.ID(recordName: kept.id.uuidString, zoneID: fixture.work.boardZone)
+        let dirty = try await fixture.work.stillDirty(
+            [staleRecord.recordID, cleanRecord.recordID, keptID])
+        #expect(dirty == [staleRecord.recordID])
+    }
 }

docs/BOARD_DELETION_SYNC.md

@@ -11,8 +11,12 @@ Board metadata is skipped, and membership rebuilds exclude only the deleted
 board IDs. Other memberships, including placeholders for genuinely unknown
 boards, continue to apply. When a rebuild drops a deleted board ID, the clip is
 re-queued for upload with its received revision, so the server copy stops
-naming the deleted board. A fetched page counts rejected board metadata as
-`skippedAsStale`; the clip itself can still apply normally.
+naming the deleted board. The CloudKit adapter registers that upload with the
+engine as soon as the clip is applied — after a push-fed fetch (including a
+page that only partly applied) and after a server-wins conflict reply — rather
+than leaving it for the next explicit start; a clip that applied cleanly is not
+re-sent. A fetched page counts rejected board metadata as `skippedAsStale`; the
+clip itself can still apply normally.
 
 The public sync protocol and server-wins policy for ordinary board metadata are
 unchanged. No schema or permanent deletion history is added. After the adapter
@@ -22,3 +26,5 @@ or retroactively remove boards resurrected before the correction.
 
 `BoardTombstoneTests` covers metadata, membership-only arrivals, combined pages,
 preserved unrelated memberships, and the acknowledgement boundary.
+`SyncOutboundWorkTests` covers the re-send after a server-wins conflict and
+which fetched clips are registered again.

docs/releases/v0.10.0.md

@@ -32,7 +32,8 @@ away. An app-wide review also tightens privacy, sync and keyboard handling.
   while sync is off no longer return, images and semantic vectors are removed
   with their clip, and the `gancho` CLI refuses to save or copy secrets
   unless you ask it to. A board you delete stays deleted while sync is still
-  sending the deletion, and credit cards are detected inside mixed text.
+  sending the deletion, clips that still list it are corrected on iCloud right
+  away, and credit cards are detected inside mixed text.
 - **Safer storage, backups and licensing.** A failed backup export never
   damages the previous backup, a malformed Keychain key fails safely instead of
   being replaced, store recovery sets aside the whole database generation