fix: address CodeRabbit review feedback for password sync

Author: datlechin

TablePro/AppDelegate.swift

@@ -52,11 +52,11 @@ class AppDelegate: NSObject, NSApplicationDelegate {
 
     func applicationDidFinishLaunching(_ notification: Notification) {
         NSWindow.allowsAutomaticWindowTabbing = true
-        KeychainHelper.shared.migrateFromLegacyKeychainIfNeeded()
         let syncSettings = AppSettingsStorage.shared.loadSync()
         let passwordSyncExpected = syncSettings.enabled && syncSettings.syncConnections && syncSettings.syncPasswords
         let previousSyncState = UserDefaults.standard.bool(forKey: KeychainHelper.passwordSyncEnabledKey)
         UserDefaults.standard.set(passwordSyncExpected, forKey: KeychainHelper.passwordSyncEnabledKey)
+        KeychainHelper.shared.migrateFromLegacyKeychainIfNeeded()
         if passwordSyncExpected != previousSyncState {
             Task.detached(priority: .background) {
                 KeychainHelper.shared.migratePasswordSyncState(synchronizable: passwordSyncExpected)

TablePro/Core/Storage/KeychainHelper.swift

@@ -263,18 +263,23 @@ final class KeychainHelper {
                 continue
             }
 
-            let deleteQuery: [String: Any] = [
-                kSecClass as String: kSecClassGenericPassword,
-                kSecAttrService as String: service,
-                kSecAttrAccount as String: account,
-                kSecUseDataProtectionKeychain as String: true,
-                kSecAttrSynchronizable as String: !synchronizable
-            ]
-            let deleteStatus = SecItemDelete(deleteQuery as CFDictionary)
-            if deleteStatus != errSecSuccess, deleteStatus != errSecItemNotFound {
-                Self.logger.warning(
-                    "Migrated item '\(account, privacy: .public)' but failed to delete old entry: \(deleteStatus)"
-                )
+            // When opting IN (synchronizable=true), delete the old local-only item safely.
+            // When opting OUT (synchronizable=false), keep the synchronizable item — deleting it
+            // would propagate via iCloud Keychain and remove it from other Macs still opted in.
+            if synchronizable {
+                let deleteQuery: [String: Any] = [
+                    kSecClass as String: kSecClassGenericPassword,
+                    kSecAttrService as String: service,
+                    kSecAttrAccount as String: account,
+                    kSecUseDataProtectionKeychain as String: true,
+                    kSecAttrSynchronizable as String: false
+                ]
+                let deleteStatus = SecItemDelete(deleteQuery as CFDictionary)
+                if deleteStatus != errSecSuccess, deleteStatus != errSecItemNotFound {
+                    Self.logger.warning(
+                        "Migrated item '\(account, privacy: .public)' but failed to delete old entry: \(deleteStatus)"
+                    )
+                }
             }
 
             migratedCount += 1

TablePro/Views/Settings/SyncSettingsView.swift

@@ -19,6 +19,7 @@ struct SyncSettingsView: View {
                 Toggle("iCloud Sync:", isOn: $syncSettings.enabled)
                     .onChange(of: syncSettings.enabled) { _, newValue in
                         persistSettings()
+                        updatePasswordSyncFlag()
                         if newValue {
                             syncCoordinator.enableSync()
                         } else {
@@ -178,9 +179,20 @@ struct SyncSettingsView: View {
     }
 
     private func onPasswordSyncChanged(_ enabled: Bool) {
+        let effective = syncSettings.enabled && syncSettings.syncConnections && enabled
         Task.detached {
-            KeychainHelper.shared.migratePasswordSyncState(synchronizable: enabled)
-            UserDefaults.standard.set(enabled, forKey: KeychainHelper.passwordSyncEnabledKey)
+            KeychainHelper.shared.migratePasswordSyncState(synchronizable: effective)
+            UserDefaults.standard.set(effective, forKey: KeychainHelper.passwordSyncEnabledKey)
+        }
+    }
+
+    private func updatePasswordSyncFlag() {
+        let effective = syncSettings.enabled && syncSettings.syncConnections && syncSettings.syncPasswords
+        let current = UserDefaults.standard.bool(forKey: KeychainHelper.passwordSyncEnabledKey)
+        guard effective != current else { return }
+        Task.detached {
+            KeychainHelper.shared.migratePasswordSyncState(synchronizable: effective)
+            UserDefaults.standard.set(effective, forKey: KeychainHelper.passwordSyncEnabledKey)
         }
     }