fix(sidebar): write favorite tables once per change and give each its own iCloud record

[datlechin]

This branch now also carries #3210 (keep Local only connections' favorites, saved queries and layouts off iCloud), merged into it as 39936c2. Merging this PR lands both. The #3210 description covers that half.

Found while implementing #3167 (PR #3189). This PR carries three findings from that investigation plus the sync-push fundamentals the folder work and the next sync lane build on.

What was wrong

A record both dirty and tombstoned was pushed as a save and a deletion. SyncChangeTracker.markDirty leaves a pending tombstone in place, and the push appended every tombstone and every dirty record independently. Unstarring a table and starring it again before a sync, or renaming a table or database and renaming it back, put a save and a deletion of the same record into one CKModifyRecordsOperation. What CloudKit does with that pair was not measured, so the CHANGELOG entry names what the app sent rather than a downstream symptom.

A deletion of a record iCloud never had never settled on macOS. PushOutcome.acceptMissingDeletions existed and only the iOS coordinator called it, so on macOS an unknownItem answer to a deletion kept its tombstone and failed every later push until the 30-day prune.

This Mac's own deletion could undo a re-add made during the push. The echo guard covered saved records only. Unstar a favorite, star it again while the deletion is in flight, and the pull that follows applied this Mac's own deletion to the re-added favorite and dropped its pending save.

Table favorites were moved one entry at a time (F3). Every catalog adoption path looped the single-entry mutators, and each call re-encoded the whole favorites set into UserDefaults, marked one sync id and posted .favoriteTablesDidChange, which every window's sidebar answers with a reload. The tracker rewrites its whole tombstone list on each mark, so the cost grows quadratically. Measured in the test host, storage and tracker only, no windows attached:

Favorites moved	Per-entry loop (what main ran)	One commit
50	100 posts, 31 ms	1 post, 2.8 ms
200	400 posts, 265 ms	1 post, 10 ms
1000	2000 posts, 5.3 s	1 post, 50 ms

Every one of those posts also reloads each open window's sidebar, which the table leaves out. The pull had the same shape, one persist and post per pulled favorite. FavoriteDatabasesStorage.rename was two writes and two posts, and removeFavorites(for:) tombstoned one id at a time.

Two favorite tables could share one iCloud record (F5). syncId hashed uuid|database|schema|name joined with a bare |, so ('a|b', 'c', 't') and ('a', 'b|c', 't') on one connection got the same record name. Starring one saved over the other, unstarring one deleted the other's record, and a peer receiving that deletion removed whichever entry a Set iteration reached first. Separately, FavoriteEntry treated a nil and an "" database or schema as different entries while the sync id treated them as one, and the sidebar wrote the table's raw schema while rename and drop looked it up through favoriteSchema, so a table whose driver reported an empty schema lost its star on rename.

SQL typed in a query tab (F4) reaches the catalog service only as .statementsRan, so a rename or drop run there leaves favorites and table settings on the old name.

What changed

Sync push, all record types

• collectPushBatch settles an id that is both dirty and tombstoned from local truth. When the record builds from local storage, the save goes, the deletion is withheld, and the id is recorded in SyncPushBatch.supersededTombstones. Once that save is acknowledged and the id was not edited again during the push, its tombstone is removed first and its dirty mark second, so an interruption between the two leaves a retryable save rather than a deletion. When the record does not build, the deletion goes and the dirty mark is discarded, as unbuildable marks already were. When a store cannot be read, a conflicted deletion is withheld and both marks are kept. markDirty is unchanged: withdrawing the tombstone there loses a deletion whenever the dirty id no longer exists locally, because the push discards unbuildable dirty ids.
• The table and database favorite collectors go through the same path, so their unbuildable dirty marks are discarded too. A Local only connection's database favorites keep their marks, as before.
• performPush calls acceptMissingDeletions, including on a push cut short by an interruption.
• The edit snapshot records generations for tombstoned ids too, and SyncEchoGuard carries the deletions a push completed. A pulled deletion of an id edited since the snapshot is withheld, the same rule the guard already applied to saves.

One write per change (F3)

FavoriteTablesStorage has one private commit path: load once, edit the whole set, diff it (removed = old minus new, added = new minus old), persist once, then one batch markDeleted, one batch markDirty and one post, all outside the lock. Toggle, add, remove, removeFavorites(inDatabase:schema:), removeFavorites(for:) and the new retarget(connectionId:_:) all go through it, and TrackedAction is gone. CatalogEditAdoption takes both favorite storages by injection and makes one call per catalog change; moveFavorite is gone. FavoriteDatabasesStorage got the same diff commit, so a rename is one write and one post. A pull collects its table-favorite saves and applies them with the pull's deletions in one commit and one post, before a connection deleted in the same pull is purged, which is the order the per-record apply had. ConnectionLocalState.purge takes the favorite storages by injection so that order is testable.

One iCloud record per favorite (F5)

• syncId(for:) hashes IdentityPath.joined([uuid, database ?? "", schema ?? "", name], separator: "|"). For every name without | or \ that path is the old bare join, so the id is unchanged (golden test d9ddf339...a9d2). When escaping changed the path, the hash takes an escaped| domain prefix. Without it, an escaped path is the old bare join of some other entry: ('a|b', 'c', 't') escapes to exactly the old preimage of ('a\', 'b', 'c|t'). A legacy preimage always starts with a UUID, so a prefixed id can never meet one.
• FavoriteEntry canonicalizes database and schema with the public nilIfEmpty from TableProPluginKit, in its initializer and its decoder. Loading merges entries that differed only by nil versus "" and persists once. removeFavorites(inDatabase:schema:) normalizes its arguments. The sidebar writes the schema through favoriteSchema, the spelling rename and drop read.
• A favorite whose id changed keeps its old id as a legacy alias, computed from the entry rather than stored. The alias is a first-class sync id: added favorites mark it dirty beside the new id, the push saves it as a second record carrying the same fields, and a removal tombstones it too. An older build can therefore see the favorite under the id it computes and delete it by that id, and its deletion reaches this build. An alias shared by two favorites (a pair that used to collide) is not published and not tombstoned while both remain; when one is removed, the alias is marked dirty again and republished with the other's fields, so the record under it never names the removed one.
• A version-gated migrateSyncIdentityIfNeeded() runs from AppDelegate beside migratePluginSecureFieldsIfNeeded(), before SyncCoordinator.start(). It marks the new id and the alias of every re-keyed favorite dirty, then records the version, so an interruption only repeats the marking. It never tombstones anything: a legacy deletion comes back in the same cycle's pull and a Mac on an older build applies it after the new record, losing the favorite for good. An unstar and restar just before upgrading leaves the alias both dirty and tombstoned, and the push settles that as a save.
• A pulled deletion matches the new id first, then the single favorite whose alias it is. Two claimants is ambiguous: logged, nothing removed. A favorite removed through its alias has its new id tombstoned once change tracking resumes, carried out through SyncRemoteDeletionEffects. The echo of this Mac's own two tombstones matches nothing.
• The iOS app neither reads nor computes table favorite ids, so nothing changes there.

F4 is documented, not adopted

Parsing editor DDL to carry favorites and settings is unsafe: a failed statement still posts the event, transactional DDL can roll back without the service knowing, and names resolve against server state the app cannot see, while a wrong adoption deletes settings on every Mac. docs/features/table-operations.mdx now says what the user sees and to use the sidebar's Rename and Delete instead.

Deviations from the plan

• The brief's formula for syncId is kept for every name without | or \; escaped paths add the escaped| domain prefix above. Found in review: without it, a re-keyed favorite's record name can be another favorite's old one.
• The brief called for a persisted map from each favorite to its recorded legacy id. Review showed the map is incomplete (a favorite starred after the upgrade has no entry, so an older build's deletion resolved to the wrong favorite) and that an older build cannot see a favorite this build starred. The alias is now a pure function of the entry, and it is published as a record, so there is no map to keep in step.

Tests and verification

• SyncPushSettlementTests (new, 7 cases): restar before a push sends the save alone and clears the tombstone; a dirty-then-tombstoned existing record is saved; a dirty-and-tombstoned absent record is deleted and its mark discarded; a second unstar during the save keeps the tombstone for the next push; an unreadable store withholds the conflicted deletion; an unknownItem deletion settles; a database renamed and renamed back saves the original and deletes only the detour. Six of the seven fail with the push fixes reverted; the absent-record case passes on main too and guards against settling this in markDirty.
• FavoriteTableSyncIdentityTests (new, 13 cases): a migrated favorite survives the echo of its own push and is saved under both ids; a restar just before the upgrade survives; an older build's deletion by the alias removes the favorite and tombstones the new id, for a migrated favorite and one starred after the upgrade; a favorite starred after the upgrade is saved under both ids; a shared alias is not published, and is republished for the survivor once one is removed; a local removal deletes both records and their echo changes nothing; a pulled alias record is retired on removal; a restar during the deletion survives its echo, plain and re-keyed; a connection deleted in the same pull outranks a favorite save; a pull's saves and deletions post one change.
• FavoriteTablesStorageTests (12 new cases): one post for 200 retargeted favorites, connection scoping, the rename round trip, merge onto an existing entry, no post for a no-op, one post for a container drop, an "" database, canonical entries and decoding, migration marks only re-keyed favorites under both ids and is a no-op the second time, an ambiguous alias deletion, a shared alias outliving one removal.
• SyncRecordMapperFavoriteTableTests (5 new cases): golden id, the three measured colliding pairs, the escaped-path domain, a | and \ round trip, an empty schema decoding as none.
• CatalogEditFavoriteAdoptionTests (new, 3 cases): renaming a database holding 200 favorites posts one table change and one database change; a table rename moves the star; a star written from an empty schema follows the rename.
• Each fix was reverted on its own and its cases re-run: push settlement 6 of 7 fail, identity, canonicalization and batching 10 fail, the three first-review fixes 3 fail, the echo guard and the pre-upgrade restar 2 fail.
• SyncTestEnvironment and ScriptedSyncTransport moved out of SyncCoordinatorEchoTests into TableProTests/Helpers, so the push, identity and echo suites share one fixture. The transport can answer unknownItem for a deletion and echo pushed deletions back through the pull.

Local runs, all through .claude/skills/fix-issue/scripts/verify.sh:

• Build: PASS.
• Tests: after rebasing onto fix(connections): apply remote deletions only for the iCloud sync categories that are on #3179, 125 of 125 across the favorite, push-settlement, identity, echo, pending-deletion, tracker and SQL favorite pull suites. Before the rebase, 339 of 339 across the touched suites and every suite that reaches favorites, sync push or pull, catalog adoption or connection purge (SyncCoordinatorEchoTests, SyncCoordinatorSQLFavoritePullTests, SidebarOutlineScaffoldTests, ConnectionLocalStatePurgeTests, TableScopedSettingsRegistryTests, FavoriteDatabasesStorageTests and the rest).
• SwiftLint --strict on all 20 changed Swift files: 0 violations.
• Docs checks: PASS.

No UI automation: the change is storage and sync plumbing with no new UI, and the UI-facing effect (one sidebar reload per change) is covered by the notification counts.

Security

A security review of the diff found nothing at confidence 8 or above.

Review

Codex ran four passes on the working tree.

1. Review, three P2s, all fixed: a legacy deletion echo could remove a favorite restarred before the upgrade (the push now settles that alias as a save); escaped ids shared a hash namespace with legacy ids (the escaped| domain); a batched pull applied favorite saves after purging a connection deleted in the same pull (restored the old order).
2. Adversarial, five findings. Fixed: deletion echoes bypassed the edit-generation guard and could erase a re-add made during the push (deletion echo guard); the legacy map missed favorites starred after the upgrade, so an older build's deletion resolved to the wrong favorite (aliases are now computed from the entry); settlement cleared the dirty mark before the tombstone, so an interruption between them left a record that would be deleted (tombstone first). Partly addressed: stale payloads under a shared legacy record (fixed by pass 4). Not fixed: reconciling edits made on an older build after a downgrade. Sparkle never installs an older build, and a per-launch reconciliation would have to tombstone every re-keyed favorite whose entry is missing, which an unreadable store would turn into mass deletion.
3. Review, one P2, fixed: an older build could neither see nor delete a favorite this build starred. Aliases are now published as records.
4. Review, one P2, fixed: removing one of two favorites that shared an alias left the alias unpublished or naming the removed one. It is now marked dirty and republished for the survivor.

Not verified: what CloudKit does with a save and a deletion of one record in one operation, and whether it answers unknownItem when deleting a record it never had. The tests model the second through ScriptedSyncTransport, matching what the iOS coordinator already assumes.

Not fixed here

• Favorites on hierarchical engines (Oracle, Snowflake, BigQuery and others) are stored with a nil schema, so a schema rename or drop still misses them. The commit path keeps the existing predicates. Normalizing favorite keys to the resolved scope is a follow-up, and it is a sync-id migration of its own.
• A Mac on an older build still holds two favorites that used to collide as one record, and an unstar there removes whichever one it finds first. Nothing on this side can repair that build's own ids.
• Edits made on an older build after upgrading (a downgrade) are not reconciled on the next upgrade; the migration runs once.
• Other bare-| joins found during the sweep, all local and out of scope: DatabaseEndpoint.id, CompareObjectResult.id, the SourceObjectDiffEngine key, and LinkedFolderWatcher.stableId (re-keying that one orphans everything keyed by the linked connection id).
• Table favorites of a Local only connection are still pushed; the stacked sync-scope lane handles it.
• Remote deletions of database favorites are dropped (SyncPendingDeletions returns early for .favoriteDatabase).
• Column layout deletions are tombstoned as .settings but never pushed, and a pulled settings deletion is ignored.
• The iOS push collection has the same dirty-and-tombstoned shape for connections, groups and tags; their UUID ids rarely reappear, so it was left alone.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated
TablePro	🟢 Ready	View Preview	Sep 30, 2026, 4:23 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

[datlechin]

Merged main (be2f616). #3189 (table folders) landed in the same sync files, so folders now go through this PR's paths instead of their own:

• Push: folders and folder placements are collected through the shared collect/append path, so they get push-time settlement of dirty-and-tombstoned ids, push/hold/drop by owner (SyncBoundary), and owner-held tombstones (TableFolderStorage records the connection as owner). An unreadable folder document keeps every mark instead of discarding it.
• Tracker: feat(sidebar): add folders for tables and views #3189's markDirty tombstone withdrawal is removed; this PR's push-time settlement replaces it. SyncChangeTrackerTests.markDirtyWithdrawsTombstone became markDirtyKeepsTombstoneForThePush.
• Pull: the folder pull reads SyncBoundary.excludedConnectionIds instead of its own connection check.
• Spec change: a folder of a connection this Mac does not hold now goes up, the same owner rule as favorites (SyncBoundaryTests: only Local only, sample and deleted Local only owners are kept off). Until the folder types are verified in Production (feat(sidebar): sync table folders between Macs now that their CloudKit schema is in Production #3226) nothing folder-related is pushed and the marks are kept (new test foldersWaitForTheirSchema).
• deletionOfANeverPushedRecordSettles now uses a group: a tombstone of an undeployed type is held, not sent.

Verified locally: 15 suites, 201 of 201 cases (sync, favorites, folders, boundary, settlement, echo). SwiftLint 0 violations.