refactor(sync): share one CloudKit engine and error model between Mac and mobile

[datlechin]

Fixes the red main: the macOS App Tests job fails to build the test target with ambiguous use of 'connection' at TableProTests/Core/Sync/SyncRecordMapperConnectionTests.swift:119 (run 30484954184). This removes the cause rather than qualifying the reference.

Cause

The macOS app and the shared package each declared their own version of six same-named types:

Type	App copy	Package copy
CloudKitSyncEngine	entitlement-gated, optional container	non-optional, no probe
PullResult	internal	public
SyncError	9 cases, .accountUnavailable, .unknown	7 cases, .noAccount, .tokenExpired
SyncStatus	.disabled(DisableReason)	.error(String)
SyncRecordType	8 cases	3 cases
SyncMetadataStorage	static shared, non-isolated	@MainActor

SyncCoordinator imports the package, but every collision silently resolved to the app-local declaration, because a same-module declaration outranks an imported one with no diagnostic. That is how the two engines drifted into carrying different bugs, and why #1990 applied the identical 99-line diff to both files by hand. In a test file that imports both, the same collision becomes an outright ambiguity error.

Change

All six types now live once, in TableProSyncTransport. The app-local copies are deleted, so the shadowing is impossible rather than merely fixed. Net -128 lines across 53 files.

• SyncError keeps the app's 9 cases and richer localized strings, plus .tokenExpired, and now maps CKError.changeTokenExpired.
• The engine keeps the macOS soft dependency: it probes the iCloud entitlement, holds optional container/database, and throws .accountUnavailable instead of trapping. macOS Debug builds genuinely lack that entitlement.
• SyncRecordType owns the record-naming contract, removing four separate copies of it (two mappers plus three hardcoded prefixes in IOSSyncCoordinator).
• iOS gains CloudKit-aware error classification it did not have, so a failed sync names the cause instead of showing a raw error.

Three findings worth review attention

1. Record-name parsing had to be longest-prefix-first. Favorite_ is a proper prefix of both FavoriteFolder_ and FavoriteTable_, so matching in declaration order resolves FavoriteFolder_abc to .favorite with id Folder_abc. That would have silently corrupted favorite-folder and favorite-table sync. SyncRecordTypeTests covers those three names explicitly.
2. SecTaskCopyValueForEntitlement is macOS-only, which is the real reason the shared engine never carried the entitlement probe. It is now #if os(macOS)-guarded; iOS returns true, since its Debug and Release configurations share one entitlements file that always declares icloud-services.
3. The token-expiry catch had to change with no compile error to force it. The unified engine maps CKError.changeTokenExpired to SyncError.tokenExpired inside performPull, but SyncCoordinator caught the raw CKError. Left alone, that catch stops matching and incremental sync stalls forever. SyncCoordinator.isTokenExpired(_:) closes it, and SyncCoordinatorTokenExpiryTests asserts a raw CKError(.changeTokenExpired) does not match, which is the guard against reverting it.

Compatibility

No migration and no forced re-sync. Verified directly:

• Record type raw values and name prefixes are unchanged for every shared case.
• Both metadata stores already wrote identical UserDefaults keys (com.TablePro.sync.dirty.<type> and friends).
• The sync token codec is identical (NSKeyedArchiver, requiringSecureCoding: true, same key).

ConnectionSyncField gating and the production CloudKit schema are untouched, and ProductionSchemaParityTests passes unmodified.

Verification

• Package: 71 tests / 11 suites pass, up from 32 / 6. New coverage for SyncRecordType, SyncError, SyncStatus, and SyncMetadataStorage, which had no direct unit test on either side.
• macOS app builds; test target builds (it does not build on main); sync suites pass.
• iOS target compiles.
• Engine tests moved into the package, where swift test runs them deterministically instead of self-skipping whenever the app test host happens to be entitled.

Two pre-existing problems were verified as not caused by this change: an iOS simulator link failure against FreeTDS/hiredis/libssh2 reproduces identically on unmodified main, and a set of unrelated test failures (etcd, Oracle formatting, plugin capability) reproduces identically at daf79f178, which predates #1990. Both look toolchain-local.

The second commit fixes a pre-existing race, kept separate for review: ConnectionStorageSyncDeleteTests shared temporaryDirectory/tablepro-tests with 11 other test files and one of its cases deleted that directory outright, so it failed under parallel execution. It guards the documented sync delete ordering invariant, so its flakiness mattered.

Not in scope

SyncRecordMapper stays duplicated. The two copies map genuinely different models (TablePro's rich DatabaseConnection versus TableProModels' leaner one) and ConnectionSyncField already reconciles them on the wire. Unifying those models is a larger separate gap.

[chatgpt-codex-connector]

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