fix(connections): read an SSH config saved before the agent socket field existed

.github/macos-test-quarantine.txt

@@ -21,12 +21,11 @@
 #     TableOperationsPluginTests, 12 of these, is gone: every case asserted the deleted fallback and
 #     the thirteenth passed only because an absent driver returns nothing. Its coverage lives in
 #     TableProTests/Core/Database/TableOperationSQLBuilderTests.swift, against a stub driver.
-TableQueryBuilderFilteredQueryTests/filteredQueryExcludesDisabledFilter()
-TableQueryBuilderFilteredQueryTests/filteredQueryWithEnabledFilter()
-ClickHouseDialectTests/testFactoryFallbackWithoutPlugin()
-SQLStatementGeneratorPKRegressionTests/testClickHouseDeleteWithPK()
-DataChangeManagerClickHouseTests/alterTableUpdateCounted()
-DataChangeManagerClickHouseTests/clickhouseUpdateWithoutPrimaryKey()
+#
+#     The three ClickHouse cases are gone the same way: ALTER TABLE ... UPDATE and DELETE WHERE are
+#     written only by ClickHousePlugin.generateStatements, and the app's SQLStatementGenerator emits
+#     the plain form for every engine. Asserted at the layer that owns them in
+#     TableProTests/Core/ClickHouse/ClickHouseDMLStatementTests.swift.
 SQLCompletionProviderTests/testCommaFromScopesColumnsToAllTables()
 SQLCompletionProviderTests/testMySQLProviderTypes()
 SQLCompletionProviderTests/testProviderAcceptsDatabaseType()
@@ -36,13 +35,16 @@ SQLCompletionProviderTests/testProviderAcceptsDatabaseType()
 #     empty registry produces: with nothing registered there is no duplicate to reject.
 ValidateDriverDescriptorTests/rejectsDuplicateAdditionalTypeId()
 ValidateDriverDescriptorTests/rejectsDuplicatePrimaryTypeId()
-PluginCapabilityTests/decodingRemovedRawValueFails()
 
 # --- Stale fixture: the model gained a field the fixture JSON does not carry, so decoding throws
 #     before the assertion is reached.
-DatabaseConnectionExternalAccessTests/decodeJSONWithExplicitValue()
-DatabaseConnectionExternalAccessTests/decodeLegacyJSONDefaultsToReadOnly()
-DataGridSettingsDefaultSortDecoderTests/missingKeyFallsBackToNone()
+#
+#     The three that were here are gone, and they were not all the same. SSHConfiguration required
+#     agentSocketPath despite the property having a default, so any stored config written before
+#     that field existed threw keyNotFound and took the whole connection with it; the decoder now
+#     treats every defaulted key as optional, the way its siblings already did. The row-height one
+#     was the other way round: DataGridRowHeight has been Int-backed since it was introduced, so the
+#     fixture's "rowHeight": "normal" described JSON that never shipped.
 
 # --- Asserts a string the app no longer produces. The behaviour changed on purpose; the expected
 #     value did not follow.

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- A saved connection whose SSH settings were written before the agent socket field existed now loads instead of disappearing.
 - Switch Connection and Open Database now open on a narrow window, and after you remove their toolbar button, instead of doing nothing at all.
 
 ## [0.67.1] - 2026-08-22

TablePro/Models/Connection/SSHTypes.swift

@@ -141,15 +141,20 @@ extension SSHConfiguration {
         case totpMode, totpAlgorithm, totpDigits, totpPeriod
     }
 
+    /// Every property here declares a default, so every key decodes as optional. A required decode
+    /// on a field that has a default cannot round-trip a payload written before that field existed:
+    /// it throws `keyNotFound` and takes the whole connection with it, because a connection that
+    /// fails to decode is a connection the user no longer has. `agentSocketPath` was the one still
+    /// required, which is why a stored SSH config from before it existed could not be read back.
     init(from decoder: Decoder) throws {
         let container = try decoder.container(keyedBy: CodingKeys.self)
-        enabled = try container.decode(Bool.self, forKey: .enabled)
-        host = try container.decode(String.self, forKey: .host)
+        enabled = try container.decodeIfPresent(Bool.self, forKey: .enabled) ?? false
+        host = try container.decodeIfPresent(String.self, forKey: .host) ?? ""
         port = try container.decodeIfPresent(Int.self, forKey: .port)
-        username = try container.decode(String.self, forKey: .username)
+        username = try container.decodeIfPresent(String.self, forKey: .username) ?? ""
         authMethod = (try? container.decodeIfPresent(SSHAuthMethod.self, forKey: .authMethod)) ?? .password
-        privateKeyPath = try container.decode(String.self, forKey: .privateKeyPath)
-        agentSocketPath = try container.decode(String.self, forKey: .agentSocketPath)
+        privateKeyPath = try container.decodeIfPresent(String.self, forKey: .privateKeyPath) ?? ""
+        agentSocketPath = try container.decodeIfPresent(String.self, forKey: .agentSocketPath) ?? ""
         jumpHosts = try container.decodeIfPresent([SSHJumpHost].self, forKey: .jumpHosts) ?? []
         totpMode = try container.decodeIfPresent(TOTPMode.self, forKey: .totpMode) ?? .none
         totpAlgorithm = try container.decodeIfPresent(TOTPAlgorithm.self, forKey: .totpAlgorithm) ?? .sha1

TableProTests/Core/ChangeTracking/DataChangeManagerClickHouseTests.swift

@@ -14,35 +14,6 @@ import Testing
 @MainActor
 @Suite("DataChangeManager ClickHouse UPDATE Validation")
 struct DataChangeManagerClickHouseTests {
-    @Test("ClickHouse ALTER TABLE UPDATE is counted as an update statement")
-    func alterTableUpdateCounted() async throws {
-        let manager = DataChangeManager()
-        manager.configureForTable(
-            tableName: "users",
-            columns: ["id", "name"],
-            primaryKeyColumns: ["id"],
-            databaseType: .clickhouse
-        )
-
-        manager.recordCellChange(
-            rowIndex: 0,
-            columnIndex: 1,
-            columnName: "name",
-            oldValue: "Alice",
-            newValue: "Bob",
-            originalRow: ["1", "Alice"]
-        )
-
-        #expect(manager.hasChanges)
-
-        let statements = try manager.generateSQL()
-        #expect(!statements.isEmpty)
-
-        // ClickHouse generates ALTER TABLE ... UPDATE instead of UPDATE
-        let hasAlterTableUpdate = statements.contains { $0.sql.hasPrefix("ALTER TABLE") }
-        #expect(hasAlterTableUpdate)
-    }
-
     @Test("ClickHouse ALTER TABLE UPDATE passes validation without throwing")
     func alterTableUpdatePassesValidation() async {
         let manager = DataChangeManager()
@@ -94,31 +65,4 @@ struct DataChangeManagerClickHouseTests {
         #expect(hasStandardUpdate)
     }
 
-    @Test("ClickHouse UPDATE without primary key uses all columns in WHERE clause")
-    func clickhouseUpdateWithoutPrimaryKey() async throws {
-        let manager = DataChangeManager()
-        manager.configureForTable(
-            tableName: "logs",
-            columns: ["timestamp", "message"],
-            primaryKeyColumns: [],
-            databaseType: .clickhouse
-        )
-
-        manager.recordCellChange(
-            rowIndex: 0,
-            columnIndex: 1,
-            columnName: "message",
-            oldValue: "old log",
-            newValue: "new log",
-            originalRow: ["2024-01-01", "old log"]
-        )
-
-        let statements = try manager.generateSQL()
-        #expect(!statements.isEmpty)
-
-        let alterStatement = statements.first { $0.sql.hasPrefix("ALTER TABLE") }
-        #expect(alterStatement != nil)
-        #expect(alterStatement?.sql.contains("UPDATE") == true)
-        #expect(alterStatement?.sql.contains("WHERE") == true)
-    }
 }

TableProTests/Core/ChangeTracking/SQLStatementGeneratorPKRegressionTests.swift

@@ -127,27 +127,6 @@ struct SQLStatementGeneratorPKRegressionTests {
 
     // MARK: - ClickHouse DELETE with PK
 
-    @Test("ClickHouse delete with PK uses ALTER TABLE DELETE")
-    func testClickHouseDeleteWithPK() throws {
-        let generator = try makeGenerator(databaseType: .clickhouse)
-        let changes = [makeDeleteChange(rowIndex: 0, originalRow: ["1", "John", "john@test.com"])]
-
-        let statements = generator.generateStatements(
-            from: changes,
-            insertedRowData: [:],
-            deletedRowIndices: [0],
-            insertedRowIndices: []
-        )
-
-        #expect(statements.count == 1)
-        let stmt = statements[0]
-        #expect(stmt.sql.contains("ALTER TABLE"))
-        #expect(stmt.sql.contains("DELETE WHERE"))
-        #expect(stmt.sql.contains("`id`"))
-        #expect(!stmt.sql.contains("`name`"))
-        #expect(!stmt.sql.contains("`email`"))
-    }
-
     // MARK: - UPDATE with PK
 
     @Test("PostgreSQL update with PK uses PK-only WHERE")

TableProTests/Core/ClickHouse/ClickHouseDMLStatementTests.swift

@@ -0,0 +1,124 @@
+//
+//  ClickHouseDMLStatementTests.swift
+//  TableProTests
+//
+
+import Foundation
+@testable import TablePro
+import TableProPluginKit
+import Testing
+
+/// ClickHouse mutates through `ALTER TABLE … UPDATE` and `ALTER TABLE … DELETE WHERE` rather than
+/// the plain `UPDATE` and `DELETE FROM` every other engine takes.
+///
+/// That shape belongs to the driver: `ClickHousePluginDriver.generateStatements` is the only thing in
+/// repository that writes it, and the app's `SQLStatementGenerator` always emits the plain form.
+/// Three cases used to assert the ClickHouse shape through `DataChangeManager` with no driver
+/// connected, which the app layer cannot produce and never could, so they sat in the quarantine
+/// file. Asserted here against the driver, they hold.
+@Suite("ClickHouse DML statements")
+struct ClickHouseDMLStatementTests {
+    private let table = "users"
+    private let columns = ["id", "name"]
+
+    /// The driver, not the plugin: `generateStatements` belongs to `ClickHousePluginDriver`. It is
+    /// pure SQL shaping with no connection behind it, so a config that never dials anywhere is
+    /// enough to exercise it.
+    private var driver: ClickHousePluginDriver {
+        ClickHousePluginDriver(config: DriverConnectionConfig(
+            host: "localhost",
+            port: 8_123,
+            username: "default",
+            password: "",
+            database: "default",
+            ssl: SSLConfiguration(),
+            additionalFields: [:]
+        ))
+    }
+
+    private func change(
+        _ type: PluginRowChange.ChangeType,
+        cells: [(columnIndex: Int, columnName: String, oldValue: PluginCellValue, newValue: PluginCellValue)] = [],
+        originalRow: [PluginCellValue]? = nil
+    ) -> PluginRowChange {
+        PluginRowChange(rowIndex: 0, type: type, cellChanges: cells, originalRow: originalRow)
+    }
+
+    @Test("An update becomes ALTER TABLE ... UPDATE keyed on the original row")
+    func updateUsesAlterTable() {
+        let statements = driver.generateStatements(
+            table: table,
+            columns: columns,
+            primaryKeyColumns: ["id"],
+            changes: [change(
+                .update,
+                cells: [(columnIndex: 1, columnName: "name", oldValue: .text("Alice"), newValue: .text("Bob"))],
+                originalRow: [.text("1"), .text("Alice")]
+            )],
+            insertedRowData: [:],
+            deletedRowIndices: [],
+            insertedRowIndices: []
+        )
+
+        #expect(statements?.count == 1)
+        let sql = statements?.first?.statement ?? ""
+        #expect(sql.hasPrefix("ALTER TABLE"))
+        #expect(sql.contains("UPDATE"))
+        #expect(sql.contains("WHERE"))
+        #expect(sql.contains("`users`"))
+    }
+
+    @Test("A delete becomes ALTER TABLE ... DELETE WHERE")
+    func deleteUsesAlterTable() {
+        let statements = driver.generateStatements(
+            table: table,
+            columns: columns,
+            primaryKeyColumns: ["id"],
+            changes: [change(.delete, originalRow: [.text("1"), .text("Alice")])],
+            insertedRowData: [:],
+            deletedRowIndices: [0],
+            insertedRowIndices: []
+        )
+
+        #expect(statements?.count == 1)
+        let sql = statements?.first?.statement ?? ""
+        #expect(sql.hasPrefix("ALTER TABLE"))
+        #expect(sql.contains("DELETE WHERE"))
+        #expect(sql.contains("`users`"))
+    }
+
+    /// Inserts are ordinary. Only the two mutating statements need the ALTER form, and asserting it
+    /// here keeps a later change from applying it where it does not belong.
+    @Test("An insert stays a plain INSERT")
+    func insertStaysPlain() {
+        let statements = driver.generateStatements(
+            table: table,
+            columns: columns,
+            primaryKeyColumns: ["id"],
+            changes: [change(.insert)],
+            insertedRowData: [0: [.text("7"), .text("Carol")]],
+            deletedRowIndices: [],
+            insertedRowIndices: [0]
+        )
+
+        let sql = statements?.first?.statement ?? ""
+        #expect(sql.hasPrefix("INSERT INTO"))
+        #expect(!sql.contains("ALTER TABLE"))
+    }
+
+    /// A row the caller did not mark deleted is skipped, so a stale change cannot delete anything.
+    @Test("A delete not listed in deletedRowIndices produces nothing")
+    func unlistedDeleteIsSkipped() {
+        let statements = driver.generateStatements(
+            table: table,
+            columns: columns,
+            primaryKeyColumns: ["id"],
+            changes: [change(.delete, originalRow: [.text("1"), .text("Alice")])],
+            insertedRowData: [:],
+            deletedRowIndices: [],
+            insertedRowIndices: []
+        )
+
+        #expect(statements == nil)
+    }
+}

TableProTests/Core/ClickHouse/ClickHouseDialectTests.swift

@@ -29,11 +29,16 @@ struct ClickHouseDialectTests {
         #expect(adapter.dataTypes.contains("UInt32"))
     }
 
-    @Test("Factory returns empty dialect when plugin not loaded")
+    /// The ClickHouse driver is one of the plugins bundled inside the app, and the test host is the
+    /// app, so the factory resolves the real dialect here. This asserted an empty fallback on the
+    /// premise that no plugin was loaded, which stopped being true and is why it sat in the
+    /// quarantine file: the fallback is unreachable from a host that ships the driver.
+    @Test("The factory resolves ClickHouse's dialect from the bundled plugin")
     @MainActor
-    func testFactoryFallbackWithoutPlugin() {
+    func testFactoryResolvesBundledDialect() {
         let dialect = SQLDialectFactory.createDialect(for: .clickhouse)
-        // Without plugin loaded, factory returns empty fallback
-        #expect(dialect.keywords.isEmpty)
+
+        #expect(!dialect.keywords.isEmpty)
+        #expect(dialect.keywords.contains("OPTIMIZE"))
     }
 }

TableProTests/Core/Database/DatabaseConnectionExternalAccessTests.swift

@@ -17,57 +17,37 @@ struct DatabaseConnectionExternalAccessTests {
         #expect(connection.externalAccess == .readOnly)
     }
 
+    /// Both cases build their JSON by encoding a real connection and editing one key, rather than
+    /// hand-writing a document. The hand-written fixture they replace had drifted three ways at
+    /// once: it named the SSH tunnel's discriminator `kind` when the wire key has always been
+    /// `mode`, gave `sslConfig` only one of its four keys, and omitted `agentSocketPath` entirely.
+    /// None of those shapes was ever written by the app, so the test failed on its own scaffolding
+    /// instead of on the property it exists to check. Encoding first means the fixture cannot
+    /// describe a document the encoder would not produce.
+    private func encodedConnection(_ connection: DatabaseConnection) throws -> [String: Any] {
+        let data = try JSONEncoder().encode(connection)
+        return try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any])
+    }
+
+    private func decodeConnection(from object: [String: Any]) throws -> DatabaseConnection {
+        let data = try JSONSerialization.data(withJSONObject: object)
+        return try JSONDecoder().decode(DatabaseConnection.self, from: data)
+    }
+
     @Test("Decoding legacy JSON without externalAccess defaults to readOnly")
     func decodeLegacyJSONDefaultsToReadOnly() throws {
-        let json = """
-        {
-            "id": "11111111-2222-3333-4444-555555555555",
-            "name": "Legacy",
-            "host": "localhost",
-            "port": 3306,
-            "database": "test",
-            "username": "root",
-            "type": "MySQL",
-            "sshConfig": { "enabled": false, "host": "", "port": 22, "username": "", "authMethod": "password", "privateKeyPath": "" },
-            "sslConfig": { "mode": "preferred" },
-            "color": "None",
-            "sshTunnelMode": { "kind": "disabled" },
-            "safeModeLevel": "silent",
-            "additionalFields": {},
-            "sortOrder": 0,
-            "localOnly": false
-        }
-        """
-        let data = Data(json.utf8)
-        let connection = try JSONDecoder().decode(DatabaseConnection.self, from: data)
-        #expect(connection.externalAccess == .readOnly)
+        var object = try encodedConnection(DatabaseConnection(name: "Legacy"))
+        object.removeValue(forKey: "externalAccess")
+
+        #expect(try decodeConnection(from: object).externalAccess == .readOnly)
     }
 
     @Test("Decoding JSON with explicit externalAccess preserves value")
     func decodeJSONWithExplicitValue() throws {
-        let json = """
-        {
-            "id": "11111111-2222-3333-4444-555555555555",
-            "name": "Test",
-            "host": "localhost",
-            "port": 3306,
-            "database": "",
-            "username": "",
-            "type": "MySQL",
-            "sshConfig": { "enabled": false, "host": "", "port": 22, "username": "", "authMethod": "password", "privateKeyPath": "" },
-            "sslConfig": { "mode": "preferred" },
-            "color": "None",
-            "sshTunnelMode": { "kind": "disabled" },
-            "safeModeLevel": "silent",
-            "externalAccess": "blocked",
-            "additionalFields": {},
-            "sortOrder": 0,
-            "localOnly": false
-        }
-        """
-        let data = Data(json.utf8)
-        let connection = try JSONDecoder().decode(DatabaseConnection.self, from: data)
-        #expect(connection.externalAccess == .blocked)
+        var object = try encodedConnection(DatabaseConnection(name: "Test"))
+        object["externalAccess"] = ExternalAccessLevel.blocked.rawValue
+
+        #expect(try decodeConnection(from: object).externalAccess == .blocked)
     }
 
     @Test("Encoding round-trips externalAccess")