fix(storage): propagate connection persistence failure (data-loss prevention)

CHANGELOG.md

@@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- When saving the connections file fails (disk full, sandbox denied, encoding error), TablePro now aborts the dependent steps instead of continuing as if the save had succeeded. Previously a failed delete could remove the keychain password and queue a CloudKit tombstone for a record that was still on disk; on next sync the connection would be nuked from iCloud as well. Now: delete, add, update, duplicate, batch-delete, sync-pull, group-cleanup, and the plugin secure field migration all check the persistence result and skip the downstream side effects on failure. The connection form surfaces a localized error and keeps the form open instead of dismissing.
 - Result-grid cells on rows marked for deletion keep their dropdown / date / JSON / blob chevron visible at reduced opacity instead of hiding it, so the cell type is still legible while clearly inactive. Click on the dimmed chevron is a no-op; FK arrow navigation is unchanged. Matches the macOS HIG "disabled appearance" guideline.
 - Foreign key navigation from a table with unsaved edits opens the referenced table in a new window tab to preserve the edit buffer. Closing that new tab no longer wipes the original tab's data grid. Previously the new tab's teardown broadcast a connection-scoped event that other coordinators on the same connection received, causing them to release their cell data.
 - Tables sidebar refreshes automatically after a successful SQL import; the refresh notification now fires after the success sheet's dismissal animation, so the main window is key when the observer runs (#1114)

TablePro/Core/Storage/ConnectionStorage.swift

@@ -103,16 +103,24 @@ final class ConnectionStorage {
         }
     }
 
-    /// Save all connections
-    func saveConnections(_ connections: [DatabaseConnection]) {
+    /// Save all connections. Returns `true` if persisted, `false` if encoding or
+    /// the atomic write failed. Callers that mutate dependent state (sync tracker,
+    /// keychain entries) MUST check the return value and abort on `false`.
+    /// Continuing on a failed save can nuke a user's password while leaving the
+    /// connection record on disk, then have the next sync delete the record from
+    /// iCloud too.
+    @discardableResult
+    func saveConnections(_ connections: [DatabaseConnection]) -> Bool {
         let storedConnections = connections.map { StoredConnection(from: $0) }
 
         do {
             let data = try encoder.encode(storedConnections)
             try data.write(to: fileURL, options: .atomic)
             cachedConnections = nil
+            return true
         } catch {
             Self.logger.error("Failed to save connections: \(error)")
+            return false
         }
     }
 
@@ -125,7 +133,10 @@ final class ConnectionStorage {
     func addConnection(_ connection: DatabaseConnection, password: String? = nil) {
         var connections = loadConnections()
         connections.append(connection)
-        saveConnections(connections)
+        guard saveConnections(connections) else {
+            Self.logger.error("Aborted addConnection: persistence failed for \(connection.id, privacy: .public)")
+            return
+        }
         if !connection.localOnly && !connection.isSample {
             syncTracker.markDirty(.connection, id: connection.id.uuidString)
         }
@@ -140,7 +151,10 @@ final class ConnectionStorage {
         var connections = loadConnections()
         if let index = connections.firstIndex(where: { $0.id == connection.id }) {
             connections[index] = connection
-            saveConnections(connections)
+            guard saveConnections(connections) else {
+                Self.logger.error("Aborted updateConnection: persistence failed for \(connection.id, privacy: .public)")
+                return
+            }
             if !connection.localOnly && !connection.isSample {
                 syncTracker.markDirty(.connection, id: connection.id.uuidString)
             }
@@ -159,7 +173,10 @@ final class ConnectionStorage {
     func deleteConnection(_ connection: DatabaseConnection) {
         var connections = loadConnections()
         connections.removeAll { $0.id == connection.id }
-        saveConnections(connections)
+        guard saveConnections(connections) else {
+            Self.logger.error("Aborted deleteConnection: persistence failed for \(connection.id, privacy: .public)")
+            return
+        }
         if !connection.localOnly && !connection.isSample {
             syncTracker.markDeleted(.connection, id: connection.id.uuidString)
         }
@@ -181,7 +198,10 @@ final class ConnectionStorage {
         let idsToDelete = Set(connectionsToDelete.map(\.id))
         var all = loadConnections()
         all.removeAll { idsToDelete.contains($0.id) }
-        saveConnections(all)
+        guard saveConnections(all) else {
+            Self.logger.error("Aborted deleteConnections: persistence failed for \(idsToDelete.count, privacy: .public) connection(s)")
+            return
+        }
         for conn in connectionsToDelete where !conn.localOnly && !conn.isSample {
             syncTracker.markDeleted(.connection, id: conn.id.uuidString)
         }
@@ -233,7 +253,10 @@ final class ConnectionStorage {
         // Save the duplicate connection
         var connections = loadConnections()
         connections.append(duplicate)
-        saveConnections(connections)
+        guard saveConnections(connections) else {
+            Self.logger.error("Aborted duplicateConnection: persistence failed for \(duplicate.id, privacy: .public)")
+            return duplicate
+        }
         if !duplicate.localOnly {
             syncTracker.markDirty(.connection, id: duplicate.id.uuidString)
         }
@@ -403,7 +426,9 @@ final class ConnectionStorage {
         }
 
         if changed {
-            saveConnections(connections)
+            if !saveConnections(connections) {
+                Self.logger.error("Failed to persist plugin secure field migration; will retry on next launch")
+            }
         }
     }
 }

TablePro/Core/Storage/GroupStorage.swift

@@ -112,7 +112,9 @@ final class GroupStorage {
             }
         }
         if changed {
-            storage.saveConnections(connections)
+            if !storage.saveConnections(connections) {
+                Self.logger.error("Failed to clear groupId references after group deletion")
+            }
         }
     }
 

TablePro/Core/Sync/SyncCoordinator.swift

@@ -446,7 +446,9 @@ final class SyncCoordinator {
         if !connectionIdsToDelete.isEmpty {
             var connections = ConnectionStorage.shared.loadConnections()
             connections.removeAll { connectionIdsToDelete.contains($0.id) }
-            ConnectionStorage.shared.saveConnections(connections)
+            if !ConnectionStorage.shared.saveConnections(connections) {
+                Self.logger.error("Failed to apply remote connection deletions: persistence error")
+            }
         }
         if !groupIdsToDelete.isEmpty {
             var groups = GroupStorage.shared.loadGroups()
@@ -504,7 +506,10 @@ final class SyncCoordinator {
         } else {
             connections.append(remoteConnection)
         }
-        ConnectionStorage.shared.saveConnections(connections)
+        guard ConnectionStorage.shared.saveConnections(connections) else {
+            Self.logger.error("Failed to apply remote connection update: persistence error for \(remoteConnection.id, privacy: .public)")
+            return false
+        }
         return true
     }
 

TablePro/ViewModels/WelcomeViewModel.swift

@@ -391,7 +391,11 @@ final class WelcomeViewModel {
         for i in connections.indices where ids.contains(connections[i].id) {
             connections[i].groupId = groupId
         }
-        storage.saveConnections(connections)
+        guard storage.saveConnections(connections) else {
+            connections = storage.loadConnections()
+            rebuildTree()
+            return
+        }
         rebuildTree()
     }
 
@@ -400,7 +404,11 @@ final class WelcomeViewModel {
         for i in connections.indices where ids.contains(connections[i].id) {
             connections[i].groupId = nil
         }
-        storage.saveConnections(connections)
+        guard storage.saveConnections(connections) else {
+            connections = storage.loadConnections()
+            rebuildTree()
+            return
+        }
         rebuildTree()
     }
 
@@ -526,7 +534,11 @@ final class WelcomeViewModel {
             }
         }
 
-        storage.saveConnections(connections)
+        guard storage.saveConnections(connections) else {
+            connections = storage.loadConnections()
+            rebuildTree()
+            return
+        }
         if !dirtyIds.isEmpty {
             services.syncTracker.markDirty(.connection, ids: dirtyIds)
         }
@@ -561,7 +573,11 @@ final class WelcomeViewModel {
             order += 1
         }
 
-        storage.saveConnections(connections)
+        guard storage.saveConnections(connections) else {
+            connections = storage.loadConnections()
+            rebuildTree()
+            return
+        }
         if !dirtyIds.isEmpty {
             services.syncTracker.markDirty(.connection, ids: dirtyIds)
         }

TablePro/Views/ConnectionForm/ConnectionFormCoordinator.swift

@@ -309,7 +309,10 @@ final class ConnectionFormCoordinator {
         var savedConnections = storage.loadConnections()
         if isNew {
             savedConnections.append(connectionToSave)
-            storage.saveConnections(savedConnections)
+            guard storage.saveConnections(savedConnections) else {
+                saveError = String(localized: "Could not save the connection. Check disk space and permissions, then try again.")
+                return
+            }
             if !connectionToSave.localOnly {
                 services.syncTracker.markDirty(.connection, id: connectionToSave.id.uuidString)
             }
@@ -324,7 +327,10 @@ final class ConnectionFormCoordinator {
                 return
             }
             savedConnections[index] = connectionToSave
-            storage.saveConnections(savedConnections)
+            guard storage.saveConnections(savedConnections) else {
+                saveError = String(localized: "Could not save the connection. Check disk space and permissions, then try again.")
+                return
+            }
             if !connectionToSave.localOnly {
                 services.syncTracker.markDirty(.connection, id: connectionToSave.id.uuidString)
             }