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

[datlechin]

Ten entries out of the quarantine file, 42 to 32. One of them was a real bug the quarantine was hiding.

The bug

SSHConfiguration.init(from:) decoded agentSocketPath with try container.decode, while every sibling field that has a default used decodeIfPresent ?? default. The property has a default too:

var agentSocketPath: String = ""
...
agentSocketPath = try container.decode(String.self, forKey: .agentSocketPath)

So an SSH config written before that field existed throws keyNotFound, and a connection that fails to decode is a connection the user no longer has. Every defaulted key now decodes as optional, which is what the rest of the initializer already did.

DatabaseConnectionExternalAccessTests was catching this and sat under the label "stale fixture: the model gained a field the fixture JSON does not carry". The fixture was right and the code was wrong.

The others, and why the label mattered

The same three-case group turned out to hold three different causes. Taking the group label at face value would have produced migration paths for data that never existed.

Case	Verdict
agentSocketPath missing	Real decoder bug, fixed above
"rowHeight": "normal"	DataGridRowHeight has been Int-backed since 301c9fb28 introduced it, so that string never shipped. Fixture invented it
"sshTunnelMode": { "kind": ... }	The wire key has always been mode; git log -S'case kind' finds nothing. Fixture invented it
"sslConfig": { "mode": ... }	Three of its four keys missing. The encoder writes all four

Three invented values in one hand-written fixture, which is a class of rot rather than three mistakes. Both external-access cases now build their JSON by encoding a real DatabaseConnection and editing the one key under test, so the fixture cannot describe a document the encoder would not produce.

Tests moved to the layer that owns the behaviour

ALTER TABLE ... UPDATE and ALTER TABLE ... DELETE WHERE are written in exactly one place, ClickHousePlugin.swift:386. The app's SQLStatementGenerator emits the plain form for every engine. Three cases asserted the ClickHouse shape through DataChangeManager with no driver connected, which that layer cannot produce and never could.

generateStatements on ClickHousePluginDriver is internal, and the driver constructs from a config without dialling anywhere, so the assertion is testable at its owner. ClickHouseDMLStatementTests covers update, delete, and two the old cases did not: that an insert stays a plain INSERT, and that a delete absent from deletedRowIndices produces nothing.

That needed four plugin files in the test target rather than one. ClickHousePlugin.swift alone fails to compile there because ClickHousePluginDriver is spread across three extension files.

The rest

• TableQueryBuilderFilteredQueryTests built TableQueryBuilder(databaseType: .mysql) with no dialect, and the dialect is what carries the SQL syntax, so there was no WHERE to produce. TableQueryBuilderNoSQLTests in the same file asserts exactly that for MongoDB. The sibling count suite already supplied one.
• ClickHouseDialectTests/testFactoryFallbackWithoutPlugin asserted an empty fallback "when plugin not loaded". The ClickHouse driver is bundled and the test host is the app, so the fallback is unreachable and the factory returns the real dialect.
• PluginCapabilityTests/decodingRemovedRawValueFails asserted raw value 3 was undecodable. PluginCapability grew a fourth case and 3 is now documentInspector. It tests a value outside the range instead, plus a round-trip over every declared case.

Verification

build                                        PASS
ClickHouseDMLStatementTests + PluginCapability   9 of 9
DatabaseConnectionExternalAccessTests + DefaultSort  7 of 7

The quarantine file records where each removed case went, so the next person is not looking for a deleted suite.

Still quarantined

32, with the largest block being 12 change-tracking cases. Those need a decision this PR does not make: reloadVersion increments on clearChanges, discardChanges and configureForTable, and deliberately not on recordCellChange, because a reload there would discard the user's uncommitted edits. The tests assert the opposite.

[chatgpt-codex-connector]

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