fix(sidebar): run table maintenance against the database on screen

[datlechin]

What this fixes

Table maintenance from the object browser (OPTIMIZE, ANALYZE, CHECK, REPAIR on MySQL, VACUUM and friends on PostgreSQL) could run against a database other than the one on screen, and report success either way.

Found while investigating #2217, which is about the same confusion between the database you are browsing and the database a tab is bound to, one layer up.

Root cause

executeMaintenance took the session driver and ran the statement on it directly:

guard let driver = DatabaseManager.shared.driver(for: connectionId) else { return }
...
lastResult = try await driver.execute(query: sql)

A maintenance statement names its table and nothing else, so where it lands is decided entirely by the connection's current database. That is not the browse cursor. DatabaseManager.pin moves the shared handle onto a tab's own scope for the length of that tab's query, and its own doc comment says it writes no session state back, so nothing moves it home again: sidebar metadata goes through the pool, not the session driver. Select a tab bound to banshi_test, let it load a page, then right-click a table in the banshi_online list and choose OPTIMIZE, and OPTIMIZE TABLE \role_ability`runs onbanshi_test` while the sheet reports it completed.

Every other statement the user owns already takes a scoped lease: paging, saving row edits, discarding them, running query parameters, executing user SQL, export. This one call site did not, so it was also outside the session-driver gate that keeps two statements off one handle, and registered no running driver, which put it out of Stop's reach.

The fix

Two parts.

The statement takes a scoped lease. withScopedDriver with .protectedWrite, since a half-applied OPTIMIZE or REPAIR cannot be undone by retrying. That also puts it behind the session-driver gate instead of interleaving with a tab's work on one handle, and registers it so the connection is known to be busy.

The object's own database travels with the request. The first version of this used browseScope, on the reasoning that the tree routes maintenance through activateThen(ref), which moves the browse cursor onto the clicked object before the sheet opens. That is true for the tree, but it is trust rather than proof: activate discards the result of switchDatabase, so a USE that fails on a dropped database or a missing privilege still opens the sheet, and the command then runs in the database the cursor never left. And the Database menu bypasses activation entirely: it acts on the sidebar's selection, which the tree publishes without moving the cursor whenever the change is not a single addition, so Cmd-clicking across two databases and then deselecting one leaves a foreign table selected.

So the clicked object's database and schema now ride with the request, from the tree's DatabaseTreeTableRef through ActiveSheet.maintenance into the execution, resolved by DatabaseManager.resolvedScope so a schema is never carried across a database boundary. This is the rule the sidebar's other destructive commands already keep by carrying their ref, and its own comment says why: "which for Truncate and Drop destroys the wrong data".

Known gap. The Database ▸ Table Maintenance menu still falls back to the browsed database, because it acts on WindowSidebarState.selectedTables and TableInfo carries a schema but no database. It now passes the object's schema, which is an improvement for schema-grouped engines, but it cannot name the database. Making the window's object selection carry the database it came from would fix that for every selection-driven command (Truncate, Drop, Export, Structure share it) and is worth doing on its own; it is not folded in here.

Verified

• verify.sh generate, verify.sh build: PASS.
• verify.sh test MaintenanceSheetIdentityTests ScopedDriverPinningTests ScopedDriverRoutingTests ScopedDriverCancellationTests DatabaseManagerTests SidebarContextMenuLogicTests: PASS, 63 cases.
• swiftlint --strict over every file this changes: clean. The one error the run reports is ExecutionAuditLog.swift:40, which is on main already and is not in this diff.

MaintenanceSheetIdentityTests covers the half that reverts: a maintenance request for the same table in two databases, or two schemas, is two different requests. Before this change it was one, which is precisely how the command came to run against whichever database the connection happened to be on.

ScopedDriverPinningTests pins the guarantee the execution now rests on, using a recording driver behind a real PluginDriverAdapter: a scoped statement issues the database switch before the statement; the lease leaves the shared connection on its own database while the browse cursor stays put, which is the exact mechanism the bug rode; and a statement scoped to the browsed database is pinned back onto it after a foreign lease has moved the handle.

No UI automation: the failure needs a live server with two databases holding a same-named table, which the UI suite (bundled SQLite sample, one database) cannot provide. CoordinatorSidebarActionsTests, the only harness that builds a MainContentCoordinator, is #if false in the tree, so the coordinator method itself has no live seam; the tests cover the routing contract it now uses instead.

[chatgpt-codex-connector]

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