fix(structure): save schema changes on the editing tab's own connection (#2015)

[datlechin]

Fixes #2015.

Root cause

TableStructureView+Schema.swift called a 3-argument executeSchemaChanges(tableName:changes:databaseType:) that resolved its target from the app-global DatabaseManager.currentSessionId instead of the editing tab's own connection.id, which was already in scope and used correctly by every other call in the same file.

That global is never re-anchored by window focus. It moves on any fresh connect, on reopening an already-connected connection, on SSH tunnel recovery, and on connect failure, where it falls back to activeSessions.keys.first on an unordered dictionary. With two connections open it routinely points somewhere other than the window the user is working in.

Four consequences, all from that one missing argument:

	Effect
driver(for:)	The ALTER TABLE ran on the other connection's driver, inside its transaction. With a same-named table it silently mutated the wrong database. This is the "connection corruption" in the issue title.
refreshData.send(connectionId)	Broadcast the wrong connection id, so the other window reloaded. This is the reported "view switches back to Schema A".
ExecutionGate.authorize	Safe Mode was evaluated against the wrong connection.
recordQuery	Query history was misattributed.

The tab that saved was meanwhile filtered out of its own refresh by its correct guard changedConnectionId == connection.id, so it never refreshed while a window the user never touched did.

Approach

Adding the missing argument would have made the symptom disappear and left the ambient overload as a trap for the next caller, so the overload is deleted instead. It had exactly one call site. The defect is now a compile error rather than something that can be reintroduced silently.

DBeaver hit this same class from a globally scoped active database and rescoped it per-editor in 6.3.1 (dbeaver/dbeaver#5172, #7473). Apple documents the same caveat on its own NSDocumentController.currentDocument, prescribing resolution from the owning object rather than a global.

Changes

• Deleted the ambient executeSchemaChanges overload. The remaining one takes explicit databaseName:, schemaName:, and connectionId:.
• Deleted dead ambient API carrying the same hazard, all with zero call sites: execute(query:), fetchTables(), fetchColumns(table:), reconnectCurrentSession(), activeDriver, status.
• Renamed currentSessionId to lastActiveSessionId and currentSession to lastActiveSession, documenting that they are for switcher highlighting and for entry points with no window of their own (a new contentless window, a file opened from Finder), never for resolving the target of an operation.
• Pin the database and schema the edited table belongs to before generating any DDL, mirroring the gate the query path already uses. A failed pin aborts the save. The query path only logs and proceeds, which is fine for a SELECT; a misdirected ALTER TABLE is silent and often irreversible.
• Present the discard-changes sheet on the coordinator's own contentWindow instead of NSApp.keyWindow, which is nil when the app is inactive and points at the sheet while one is up. MainContentCoordinator already documents contentWindow as existing for exactly this reason.

Why the schema is pinned too

Switching database on a schema-grouped engine calls resetSchema and drops the driver to the engine's default schema, and those drivers qualify their DDL with whatever schema they are currently on. Without restoring it, a save on sales.orders in MSSQL would have generated ALTER TABLE [dbo].[orders]. The schema is captured before the switch and restored after it.

Tests

There was no test anywhere calling executeSchemaChanges with two active sessions and asserting where the DDL landed. TableProTests/Core/Database/DatabaseManagerSchemaChangeRoutingTests.swift adds six:

• DDL runs on the requested connection while lastActiveSessionId points at a different one. Direct regression test for this issue.
• The database is pinned before any DDL runs.
• A redundant switch is skipped when the session is already on the tab's database.
• A failed pin aborts with nothing executed.
• Engines that need a reconnect to switch database are never switched mid-save.
• A schema-grouped engine keeps the edited table's schema across the pin.

swiftlint lint --strict is clean on every changed file, and these plus CancelledConnectionCleanupTests, DatabaseManagerSessionTests, DatabaseManagerDatabaseSwitchTests, and MultiConnectionTests pass.

No UI automation: reproducing this needs two independently connected live sessions racing in CI, which the existing UI harness does not provide.

Reviewer notes

• One existing assertion was relaxed. genuineFailureSwitchesToRemainingSession asserted lastActiveSessionId == otherId, which only holds if that test owns the entire global session table. Swift Testing runs suites in parallel and several suites inject sessions, so it was already flaky; this suite widened the window and it failed once. It now asserts the real contract: the id moved off the failed session and onto one that still exists. The underlying activeSessions.keys.first nondeterminism in finalizeConnectionFailure is real and left alone as separate scope.
• Left out deliberately: MetadataConnectionPool keys its pooled driver off session.activeDatabase read at call time rather than a tab-pinned value, so the same drift can affect metadata reads. That is a shared abstraction behind every metadata read, and this issue is a write landing on the wrong connection. Worth its own change.

[chatgpt-codex-connector]

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