fix(compare): fence a run on the question it answered rather than the script it replaced

[datlechin]

Follow-up to #2597 and #2598, both still unreleased. A review of that branch found fourteen things; ten of them held up and are fixed here, four did not and are listed at the end with what the evidence actually showed.

The root cause behind five of them

CompareSyncSession had one scriptRevision answering two different questions: "are these statements still the user's choices" and "is this answer still the question on screen". A comparison advances that revision itself when it publishes, because its new report is what makes the old script stale. So a run could never pass its own fence:

• session.informationalMessage = session.crossEngineNotice after a compare was unreachable. That is the only publisher of the cross-engine warning anywhere in the app, so comparing MySQL against PostgreSQL warned about nothing.
• catch is CancellationError in both run paths was unreachable, because cancelRunningWork() advances the revision before the task observes the cancellation. Pressing Stop left the window silent.
• session.dataPlans = plans wrote back an array captured before a multi-minute row comparison. Ticking one more table during a run either lost the tick or, once the tick's own invalidateScript() broke the fence, threw the whole run's summaries away.

answerRevision now carries the second question. It moves for a new setup, a new key column or a new set of compared columns; it does not move for a tick, a row exclusion or a write-policy toggle, which change only which statements come out of an answer that still stands. owns(claim) (setup + answer + script) fences statements; ownsAnswer(claim) fences a comparison's own results and anything it reports about itself.

A cancel advances both revisions without clearing what is on screen. That distinction matters: apply() cancels the work in flight and then reads the very statements it is about to run, so a cancel that discarded the script would leave every confirmed Apply executing nothing and reporting success. The fence is the counter, not the content.

Cancellation is reported by the canceller instead. Task.cancel() is cooperative and a run inside a driver call may never observe it, so a message published from the cancellation path is a message that may never arrive; and a run that does observe it cannot tell the user's Stop from being superseded by the next run, because both reach it identically. stopRunningWork() publishes synchronously with the press, which is what the connection-side invariant in CLAUDE.md already requires of a cancel. CompareSyncActivity gained .buildingScript so the message can still name what was stopped.

A comparison's summaries are now merged onto the live plans by id, and only onto a plan still asking the question the run answered (same key columns, same column set). The run owns the summary; the user owns everything else on the plan. The structure side follows the same rule through adoptActions(for:): a recompare keeps what the user ticked while it ran, for every object the new report still holds, instead of publishing actions = [:] over it.

Script-build errors keep the full claim rather than the answer claim. A build that failed for a selection the user has since changed would otherwise blame an object they had just excluded.

The rest

buildPlans published outside the fence. It wrote session.sourceSnapshots and session.unreadableTableCount before either caller checked ownership, so a read that finished after the pair moved installed the old pair's snapshots anyway. Those snapshots are what ForeignKeyTopologicalSort orders INSERTs from and what structureStatements generates CREATE TABLE from, so a later Apply could order DDL from the wrong database's foreign key graph. It returns a DataPlanRead now and the caller publishes all of it behind one check.

resetComparison() left the run it invalidated holding runTask. Loading a saved comparison mid-run bumped the setup (correctly fencing the run out) and then let loadDataPlans() overwrite the task handle, so Stop had nothing to cancel while the orphaned run kept reading. The reset cancels first.

MySQL's whole-schema reads ignored their schema: argument. fetchAllIndexes filtered on the session's current database and fetchAllTableMetadata ran a bare SHOW TABLE STATUS, so a caller naming another database was answered about the current one with no error. Both go through routineSchema(schema) now, and the metadata read is SHOW TABLE STATUS FROM <db>.

MySQL's bulk index query escaped ' and not \. With NO_BACKSLASH_ESCAPES off, a database name ending in a backslash escapes the closing quote and the rest of the name parses as SQL. mysqlEscapeStringLiteral already handles both and is what the per-table reads in the same driver use.

BulkMetadata.lookup scanned linearly on every miss. The index, foreign key and table metadata maps are sparse by construction, so every table without an index paid a full scan that lowercased every key: quadratic in the table count, on the read path #2597 exists to make fast. The folded spellings are indexed once per map, and a map whose own keys collide when folded now refuses the fallback rather than picking one arbitrarily.

Six DatabaseDriver requirements had no callers. fetchAllIndexes, fetchAllTableMetadata and the four providesBulk* flags were added to the app's driver protocol and bridged in PluginDriverAdapter, but the compare path reaches PluginDatabaseDriver directly and nothing else called them. Their defaults also accepted a schema and then called fetchTables(), which takes none. Removed; the plugin-side protocol, which is what the compare path uses, is untouched.

Toolbar one-shot flag was burned before the insert. insertSavedComparisonsItemOnce recorded "inserted" ahead of both the guard and insertItem, and the key is global across every Compare window, so a window that did not place the item had no second chance at it.

Swap reset the session twice. swapEndpoints(_:) called session.swapEndpoints(), which resets, and then the funnel that resets again: two setup generations and two remembered-setup writes for one press.

profiles(source:target:mode:) and CompareSyncProfile.storageKey were dead. The list moved to allProfiles() in #2597; only tests still reached the filtered form. Removed, with the three tests that pinned the scoping rewritten to pin the rule that replaced it. CompareOptionsView also kept re-reading UserDefaults on a mode change that no longer filters the list.

Not fixed, and why

• PostgreSQL fetchIndexes returning empty off-schema. Reported as a regression from the new n.nspname predicate. It is not one: fetchForeignKeys, fetchColumns, the trigger reads and the DDL read in the same driver all resolve schema ?? core.currentSchema the same way, and the app's own DatabaseDriver.fetchIndexes(table:) takes no schema at all. The predicate makes the index read match every neighbour rather than diverge from them. The real gap is app-wide and older than this branch.
• Bulk trigger reads passing fallbackTable: nil. Reported as an anchor lost against the per-table path. Every driver that sets providesBulkTriggerFetch (MySQL, PostgreSQL, MSSQL, SQLite, LibSQL, CloudflareD1) populates PluginTriggerInfo.table from its own query, and a whole-schema read has no single table to fall back to, so nil is the only honest value.
• refreshEndpointChrome() called from validateUserInterfaceItem. The altitude complaint is fair but the write is guarded on the rendered pair and generation, so a validation pass does no work unless something actually changed. The hazard it created was the orphaned runTask, fixed above at the reset.
• compareSyncLastSetup shared by every Compare window. Real: the controller map is keyed by connection id, so two windows overwrite each other's remembered setup. Changing the key shape discards what users have already stored, so it wants its own decision rather than riding along here.

Two more things turned up on the way: Stop during an Apply said "Comparison cancelled." for a run that writes as it goes, and a cancelled Apply published CancellationError's raw description as its error text. The message now says what stays applied, matching the close-confirmation alert, and the apply path gets the same silent cancellation arm the two read paths have.

Review

Codex read the working tree cold and found three things, all fixed here and each pinned by a test:

• P1: the first cut of cancelRunningWork() delegated to invalidateAnswer(), which empties statements. Since apply() cancels before reading them, every confirmed Apply would have run zero statements against the target and reported success. testCancellingRunningWorkLeavesTheScriptItIsAboutToApply.
• P2: the structure recompare's new ownsAnswer fence let it publish actions = [:] over inclusions made while it ran, because setAction advances only the script revision. adoptActions(for:) and testARecompareKeepsWhatTheUserIncludedWhileItRan.
• P2: script-build errors were fenced on the answer claim, so a failure could surface against a selection the user had already changed. Back to the full claim.

Verification

• verify.sh build: PASS
• verify.sh test over CompareRunClaimTests, CompareSyncSetupRestoreTests, CompareSyncStartStateTests, CompareMetadataReadPlanTests, TableDefinitionRendererTests, CompareObjectScopeTests, CompareDataPlanGroupingTests, CompareReviewFindingTests: 70 executed, 70 passed, 0 failed. StringCatalogIntegrityTests and CompareCountedStringTests passed in an earlier run over the same tree.
• verify.sh lint over TablePro, Plugins and TableProTests: 0 violations. The one reported doc-symbol miss (AXCell, CLAUDE.md:220) is on main and untouched here.
• 17 new cases in CompareRunClaimTests, covering the fence in both directions, the summary merge, the inclusion carry-over, and each stop message.
• AllPlugins not built locally: the only plugin touched is MySQL, which is bundled and therefore compiled by the app build. PR CI runs the aggregate anyway.

No CHANGELOG entry: every change here lands on work still under [Unreleased], which the fold-it-in rule covers. Nothing user-visible changes shape, so there is nothing to re-screenshot; the two messages that were unreachable before are covered by unit tests.

https://claude.ai/code/session_01J6xU4Zx4DRJ5JaxMP437uT

[chatgpt-codex-connector]

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