fix(connections): make the tag library report its refusals and survive a broken store

CHANGELOG.md

@@ -39,6 +39,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Wrong connection moved when dragging a list that holds a favorite or an active tag filter. (#1311)
 - Crash opening the connection list after two Macs moved two groups inside each other. (#1311)
 - Every saved group lost when one unreadable entry stopped the whole list decoding. (#1311)
+- New tag discarded without a word when the name was already taken.
+- Every saved tag replaced by the preset list when one unreadable entry stopped the library decoding.
+- Tag created in one window missing from another until relaunch.
+- Two Macs re-uploading the whole tag library to each other after a single tag changed.
 - Two Macs re-uploading the whole group list to each other after a single group changed. (#1311)
 - Deleting a group that a broken sync left in a loop also deleting the group it pointed at. (#1311)
 

TablePro/Core/Services/Export/ConnectionExportService.swift

@@ -387,23 +387,23 @@ enum ConnectionExportService {
         }
 
         if let envelopeTags = preview.envelope.tags {
-            let existingTags = TagStorage.shared.loadTags()
             for exportTag in envelopeTags {
-                let alreadyExists = existingTags.contains {
+                /// Re-read per tag rather than once for the envelope: two tags sharing a name in
+                /// one file both passed a snapshot taken before either was added.
+                let alreadyExists = TagStorage.shared.loadTags().contains {
                     $0.name.lowercased() == exportTag.name.lowercased()
                 }
-                if !alreadyExists {
-                    // Match preset tags by name
-                    let preset = ConnectionTag.presets.first {
-                        $0.name.lowercased() == exportTag.name.lowercased()
-                    }
-                    if let preset {
-                        TagStorage.shared.addTag(preset)
-                    } else {
-                        let color = exportTag.color.flatMap { ConnectionColor(rawValue: $0) } ?? .gray
-                        let tag = ConnectionTag(name: exportTag.name, color: color)
-                        TagStorage.shared.addTag(tag)
-                    }
+                guard !alreadyExists else { continue }
+
+                let preset = ConnectionTag.presets.first {
+                    $0.name.lowercased() == exportTag.name.lowercased()
+                }
+                let color = exportTag.color.flatMap { ConnectionColor(rawValue: $0) } ?? .gray
+                let tag = preset ?? ConnectionTag(name: exportTag.name, color: color)
+                do {
+                    try TagStorage.shared.addTag(tag)
+                } catch {
+                    Self.logger.error("Skipped importing tag: \(error.localizedDescription, privacy: .public)")
                 }
             }
         }

TablePro/Core/Storage/TagStorage.swift

@@ -5,23 +5,51 @@
 //  Created by Claude on 20/12/25.
 //
 
+import Combine
 import Foundation
 import os
 import TableProSyncTransport
 
+internal enum TagStorageError: LocalizedError, Equatable {
+    case duplicateName(String)
+    case storeUnreadable
+
+    internal var errorDescription: String? {
+        switch self {
+        case .duplicateName(let name):
+            return String(format: String(localized: "A tag named “%@” already exists."), name)
+        case .storeUnreadable:
+            return String(localized: "The saved tags could not be read. Nothing was changed.")
+        }
+    }
+}
+
 /// Service for persisting the global tag library
 @MainActor
-final class TagStorage {
-    static let shared = TagStorage()
+internal final class TagStorage {
+    internal static let shared = TagStorage()
     nonisolated private static let logger = Logger(subsystem: "com.TablePro", category: "TagStorage")
 
     private let tagsKey = "com.TablePro.tags"
-    private let defaults = AppStorageEnvironment.shared.defaults
+    private let defaults: UserDefaults
+    private let syncTracker: SyncChangeTracker
+    private let appEvents: AppEvents
     private let encoder = JSONEncoder()
     private let decoder = JSONDecoder()
     private var cachedTags: [ConnectionTag]?
+    /// Set when the stored payload could not be understood at all. Every mutation rewrites the
+    /// whole array, so continuing over an unreadable store would replace a user's own tags with
+    /// the preset list this falls back to for display.
+    private var storeIsUnreadable = false
 
-    private init() {
+    internal init(
+        userDefaults: UserDefaults = AppStorageEnvironment.shared.defaults,
+        syncTracker: SyncChangeTracker = .shared,
+        appEvents: AppEvents = .shared
+    ) {
+        self.defaults = userDefaults
+        self.syncTracker = syncTracker
+        self.appEvents = appEvents
         if loadTags().isEmpty {
             saveTags(ConnectionTag.presets)
         }
@@ -30,74 +58,137 @@ final class TagStorage {
     // MARK: - Tag CRUD
 
     /// Load all tags (presets + custom)
-    func loadTags() -> [ConnectionTag] {
+    ///
+    /// A payload that decodes element by element keeps every tag it can read: one entry written by
+    /// a future version, or truncated on disk, used to take the whole library down with it and
+    /// leave the presets standing in its place.
+    internal func loadTags() -> [ConnectionTag] {
         if let cached = cachedTags { return cached }
 
         guard let data = defaults.data(forKey: tagsKey) else {
+            storeIsUnreadable = false
             let tags = ConnectionTag.presets
             cachedTags = tags
             return tags
         }
 
-        do {
-            let tags = try decoder.decode([ConnectionTag].self, from: data)
-            cachedTags = tags
-            return tags
-        } catch {
-            Self.logger.error("Failed to load tags: \(error)")
-            let tags = ConnectionTag.presets
+        if let tags = try? decoder.decode([ConnectionTag].self, from: data) {
+            storeIsUnreadable = false
             cachedTags = tags
             return tags
         }
+
+        guard let salvaged = try? decoder.decode([SalvagedTag].self, from: data) else {
+            Self.logger.error("Tag store could not be read; leaving it untouched")
+            storeIsUnreadable = true
+            return ConnectionTag.presets
+        }
+
+        let tags = salvaged.compactMap(\.tag)
+        Self.logger.error(
+            "Dropped \(salvaged.count - tags.count, privacy: .public) unreadable tag entries"
+        )
+        storeIsUnreadable = false
+        cachedTags = tags
+        return tags
     }
 
-    /// Save all tags
-    func saveTags(_ tags: [ConnectionTag]) {
+    /// Save all tags. A save that failed leaves the store holding the previous set, so a caller
+    /// that goes on to write related state must check the result.
+    @discardableResult
+    internal func saveTags(_ tags: [ConnectionTag]) -> Bool {
+        guard !storeIsUnreadable else {
+            Self.logger.error("Refusing to overwrite an unreadable tag store")
+            return false
+        }
+
         do {
             let data = try encoder.encode(tags)
             defaults.set(data, forKey: tagsKey)
             cachedTags = nil
-            SyncChangeTracker.shared.markDirty(.tag, ids: tags.map { $0.id.uuidString })
+            syncTracker.markDirty(.tag, ids: tags.map { $0.id.uuidString })
+            return true
         } catch {
             Self.logger.error("Failed to save tags: \(error)")
+            return false
         }
     }
 
     /// Add a new custom tag
-    func addTag(_ tag: ConnectionTag) {
+    internal func addTag(_ tag: ConnectionTag) throws {
         var tags = loadTags()
         guard !tags.contains(where: { $0.name.lowercased() == tag.name.lowercased() }) else {
-            return
+            throw TagStorageError.duplicateName(tag.name)
         }
+
         tags.append(tag)
-        saveTags(tags)
+        guard saveTags(tags) else { throw TagStorageError.storeUnreadable }
+        notifyChanged()
+    }
+
+    /// Apply a tag that arrived from another device.
+    ///
+    /// Written as it arrived, and skipped when it matches what is already stored: `saveTags` marks
+    /// every tag dirty and the push uploads every dirty tag, so writing an unchanged record
+    /// re-uploads the whole library to the device it came from, which writes it back.
+    @discardableResult
+    internal func applyRemoteTag(_ tag: ConnectionTag) -> Bool {
+        var tags = loadTags()
+
+        if let index = tags.firstIndex(where: { $0.id == tag.id }) {
+            guard tags[index] != tag else { return false }
+            tags[index] = tag
+        } else {
+            tags.append(tag)
+        }
+
+        return saveTags(tags)
     }
 
     /// Delete a custom tag (presets cannot be deleted)
-    func deleteTag(_ tag: ConnectionTag) {
+    internal func deleteTag(_ tag: ConnectionTag) {
         guard !tag.isPreset else { return }
         var tags = loadTags()
         tags.removeAll { $0.id == tag.id }
-        saveTags(tags)
-        SyncChangeTracker.shared.markDeleted(.tag, id: tag.id.uuidString)
+        guard saveTags(tags) else { return }
+        syncTracker.markDeleted(.tag, id: tag.id.uuidString)
+        notifyChanged()
     }
 
     /// Delete a custom tag and clear it from every connection that referenced it.
     /// Connections are persisted before the tag tombstone fires (sync delete-ordering invariant).
-    func deleteTag(_ tag: ConnectionTag, clearingFrom connectionStorage: ConnectionStorage) {
+    internal func deleteTag(_ tag: ConnectionTag, clearingFrom connectionStorage: ConnectionStorage) {
         guard !tag.isPreset else { return }
         connectionStorage.removeTagId(tag.id)
         deleteTag(tag)
     }
 
     /// Get tag by ID
-    func tag(for id: UUID) -> ConnectionTag? {
+    internal func tag(for id: UUID) -> ConnectionTag? {
         loadTags().first { $0.id == id }
     }
 
     /// Get tags for a list of IDs
-    func tags(for ids: [UUID]) -> [ConnectionTag] {
+    internal func tags(for ids: [UUID]) -> [ConnectionTag] {
         let allTags = loadTags()
         return ids.compactMap { id in allTags.first { $0.id == id } }
     }
+
+    // MARK: - Private
+
+    /// Announced from the mutators rather than from `saveTags`, because a sync pull applies one
+    /// record at a time and raises a single coalesced notification of its own for the batch.
+    private func notifyChanged() {
+        appEvents.connectionUpdated.send(nil)
+    }
+}
+
+/// Decodes one tag and keeps going when it cannot, so a single unreadable entry costs that entry
+/// rather than the whole library.
+private struct SalvagedTag: Decodable {
+    let tag: ConnectionTag?
+
+    init(from decoder: Decoder) throws {
+        tag = try? ConnectionTag(from: decoder)
+    }
 }

TablePro/Core/Sync/SyncCoordinator.swift

@@ -719,14 +719,7 @@ final class SyncCoordinator {
         guard let remoteTag = SyncRecordMapper.toTag(record) else { return false }
         if tombstoneIds.contains(remoteTag.id.uuidString) { return false }
 
-        var tags = services.tagStorage.loadTags()
-        if let index = tags.firstIndex(where: { $0.id == remoteTag.id }) {
-            tags[index] = remoteTag
-        } else {
-            tags.append(remoteTag)
-        }
-        services.tagStorage.saveTags(tags)
-        return true
+        return services.tagStorage.applyRemoteTag(remoteTag)
     }
 
     private func applyRemoteSSHProfile(_ record: CKRecord, tombstoneIds: Set<String>) {

TablePro/Resources/Localizable.xcstrings

@@ -240,6 +240,40 @@
         }
       }
     },
+    "A tag named “%@” already exists." : {
+      "localizations" : {
+        "ko" : {
+          "stringUnit" : {
+            "state" : "translated",
+            "value" : "“%@”라는 이름의 태그가 이미 있습니다."
+          }
+        },
+        "tr" : {
+          "stringUnit" : {
+            "state" : "translated",
+            "value" : "“%@” adlı bir etiket zaten var."
+          }
+        },
+        "vi" : {
+          "stringUnit" : {
+            "state" : "translated",
+            "value" : "Đã có thẻ tên “%@”."
+          }
+        },
+        "zh-Hans" : {
+          "stringUnit" : {
+            "state" : "translated",
+            "value" : "已有名为“%@”的标签。"
+          }
+        },
+        "zh-Hant" : {
+          "stringUnit" : {
+            "state" : "translated",
+            "value" : "已有名為「%@」的標籤。"
+          }
+        }
+      }
+    },
     "Exclude the AUTO_INCREMENT counter" : {
       "extractionState" : "stale",
       "localizations" : {
@@ -1602,6 +1636,40 @@
         }
       }
     },
+    "The saved tags could not be read. Nothing was changed." : {
+      "localizations" : {
+        "ko" : {
+          "stringUnit" : {
+            "state" : "translated",
+            "value" : "저장된 태그를 읽을 수 없습니다. 아무것도 변경되지 않았습니다."
+          }
+        },
+        "tr" : {
+          "stringUnit" : {
+            "state" : "translated",
+            "value" : "Kayıtlı etiketler okunamadı. Hiçbir şey değiştirilmedi."
+          }
+        },
+        "vi" : {
+          "stringUnit" : {
+            "state" : "translated",
+            "value" : "Không đọc được các thẻ đã lưu. Không có gì thay đổi."
+          }
+        },
+        "zh-Hans" : {
+          "stringUnit" : {
+            "state" : "translated",
+            "value" : "无法读取已保存的标签。未做任何更改。"
+          }
+        },
+        "zh-Hant" : {
+          "stringUnit" : {
+            "state" : "translated",
+            "value" : "無法讀取已儲存的標籤。未做任何變更。"
+          }
+        }
+      }
+    },
     "(this Mac)" : {
       "extractionState" : "stale",
       "localizations" : {