fix(datagrid): stop every second refresh from cancelling its own query (#2021)

[datlechin]

Fixes #2021.

The bug

Clicking Refresh on a table's Data tab fails with a red "Query cancelled" banner on every second click. The grid keeps the old rows underneath.

Root cause

Three defects stacked. The every-second-time pattern is what unpicks them.

1. A finished task drove a real database cancel. PaginationCoordinator.cancelCurrentQuery() decided whether to cancel the database from currentQueryTask != nil || currentRowCountTask != nil. currentQueryTask is cleared on every completion path; currentRowCountTask was never cleared, so after a successful load it held a task that had already finished, and the next refresh sent a cancel to an idle connection. It was aimed at the wrong connection anyway: row counting runs on a separate pooled driver through withMetadataDriver, which the main connection's cancel cannot reach.

2. The libpq cancel flag was sticky and belonged to no particular query. _isCancelled was set unconditionally and consumed only inside the row loop, so a cancel arriving while the connection was idle stayed armed and killed the next query at row 0. A zero-row result never consumed it and left it armed indefinitely.

3. A cancellation was rendered as a failure. The existing guard only caught Swift's CancellationError, and PostgreSQL threw a plain plugin error, so it reached the error banner and was recorded in query history as a failed query.

The alternation follows: a failed query never launches phase-2 row counting, so the leaked handle is absent on the next pass.

Refresh	currentRowCountTask	cancel sent	Result
1	stale, non-nil	yes, arms the flag	"Query cancelled"
2	nil (the failure path starts no row count)	no	succeeds, re-leaks the handle
3	stale, non-nil	yes	"Query cancelled"

Why this is fixed in the driver and not in the boolean

This is a regression of #1655, which fixed the same symptom on 11 June by narrowing that same expression to currentQueryTask != nil. #1999 widened it again on 30 July and the bug came back. Before #1655 the cancel was unconditional and every refresh failed (#1637). Narrowing the expression a third time leaves the real defect in place: a cancel that arrives with nothing running must not be able to reach a later query. Fixed at that level, no future mistake in the caller can poison a query again.

An audit of all 20 driver plugins found the same sticky-latch shape in three of them. MSSQL and MongoDB already reset at query start and were left alone.

Changes

A shared primitive. PluginQueryCancellationGate in TableProPluginKit, a generation guard in the shape of ConnectionAttemptRegistry. cancel() returns nil when nothing is running, so a driver knows not to send a transport-level cancel at all, and a recorded cancel can never match a later query's generation.

PostgreSQL adopts it, covering Redshift and CockroachDB too through LibPQBackedDriver. The PQcancel is now gated on a query actually running, and the streaming path is bracketed so Stop still cancels a long export.

MySQL/MariaDB adopts it. This also closes two leaks the report did not cover: the flag survived every non-row-returning statement (INSERT/UPDATE/DDL) and every zero-row prepared statement, and KILL QUERY was sent to idle connections.

Redis adopts it. A stale flag there could fail a schema refresh, a ping, or a MULTI/EXEC commit. The reset moves from 4 scattered driver entry points to the 3 shared chokepoints every command already funnels through, so it cannot be forgotten again.

App layer. cancelCurrentQuery() delegates to the already-correct cancelInFlightQueryTask(), removing the second, wrong notion of "is a query in flight". Both row-count tasks now clear their own handle behind a generation guard.

Cancellation is no longer an error. New DatabaseCancellationDiagnosis, applied at the banner sink itself so every caller is covered, following AppKit's own contract: presentError: silently ignores NSUserCancelledError, and Apple's error guide says a user cancellation must not show an error dialog. Pressing Stop on PostgreSQL showed a red banner before this change.

Deliberately no SQLSTATE table: PostgreSQL reports 57014 for statement_timeout as well as for a user cancel, and MySQL 1317 covers max_execution_time, so matching on those codes would have silently swallowed genuine timeout errors. Each driver reports a cancellation only when it caused one. Two tests pin that a real timeout still reaches the user.

Testing

• New PluginQueryCancellationGateTests: idle cancel is a no-op, a cancel never matches a later generation, a zero-row query leaves nothing armed, a stale endQuery does not clear the running query.
• New DatabaseCancellationDiagnosisTests, including the two tests that a 57014 timeout and a 1317 interruption are still surfaced.
• MainContentCoordinatorRefreshTests gains cancelWithStaleRowCountHandleDoesNotTouchDriver and repeatedIdleRefreshNeverCancelsDriver. Both were confirmed to fail on the pre-fix code and pass after it.
• All 24 tests across the three affected suites pass, including every pre-existing refresh test.

PluginKit ABI

Additive. scripts/check-pluginkit-abi.sh reports one added public type and nothing removed or changed, so no currentPluginKitVersion bump and no registry re-release. All three drivers are bundled and ship with the app. Needs the abi-additive label.

[chatgpt-codex-connector]

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