fix(sync): tombstone a deleted connection's SQL favorites and folders

[datlechin]

Deleting a connection removed its SQL favorites and folders from the Mac but never marked them deleted for sync, so they stayed in iCloud and came back on the next device to sync or on a fresh install.

What was wrong

Every other delete path in SQLFavoriteManager tombstones what it removed:

func deleteFavorites(ids: [UUID]) async {
    let result = await storage.deleteFavorites(ids: ids)
    if result {
        for id in ids { syncTracker.markDeleted(.favorite, id: id.uuidString) }
        ...
    }
}

removeFavoritesAndFolders(for:) could not, and the reason is in the signature: it is the one delete keyed on something other than the records themselves, and deleteFavoritesAndFolders(connectionId:) returned Bool. By the time it came back the rows were gone, so there was nothing left to ask which ids to tombstone. It posted a UI notification and stopped there.

.favorite and .favoriteFolder are both .synced in SyncScope, and deleteFavorites already drives the same machinery for .favorite, so these records really were reaching CloudKit and really were staying there.

The fix

deleteFavoritesAndFolders reports what it deleted instead of whether it deleted. The two SELECTs run inside the same BEGIN IMMEDIATE transaction as the DELETEs, so nothing can be inserted between the read and the delete and be removed without a tombstone.

markDeleted runs after the storage delete has committed, per the sync ordering rule in CLAUDE.md: it posts a change notification that can start a sync, and a sync that reads a store still holding the record re-uploads what was just deleted.

One thing worth a reviewer's eye

SyncCoordinator.applyRemoteChanges suppresses the change tracker for its duration, but its call to removeFavoritesAndFolders sits inside a detached Task, so it runs after the defer has already cleared isSuppressed. Applying a remote connection deletion therefore writes local tombstones and pushes them back.

I left that as is. CloudKit deletes are idempotent, so the redundant push is harmless, and it is the correct outcome in the case that actually matters: favorites this device created and had not yet pushed when the connection was deleted elsewhere. Restructuring the suppression window to cover an async call belongs in its own change.

Verification

• build PASS
• test SQLFavoriteDeletionSyncTests SQLFavoriteStorageTests SQLFavoriteEditValidationTests SyncChangeTrackerTests PASS
• lint TablePro TableProTests reports three violations, all pre-existing on main and all fixed by style: satisfy the swiftlint strict gate in the operation reporting code #2293. Nothing in the files this PR touches.
• Skill(security-review) on the diff: no findings. The two new statements are literals with bound parameters, SQLITE_TRANSIENT matches the existing helper, and tombstones carry UUIDs only.

SQLFavoriteDeletionSyncTests builds a manager on an isolated SyncMetadataStorage and asserts three things: a connection delete tombstones both its favorites and its folders, another connection's records are neither deleted nor tombstoned, and a delete that removed nothing leaves no tombstone behind, since a stray tombstone would delete a record another device still has.

[chatgpt-codex-connector]

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