Skip to content

fix(lokee): make revert actually reach the database - #261

Merged
huyplb merged 2 commits into
mainfrom
fix/revert-lands-and-history-toolbar
Aug 16, 2026
Merged

fix(lokee): make revert actually reach the database#261
huyplb merged 2 commits into
mainfrom
fix/revert-lands-and-history-toolbar

Conversation

@huyplb

@huyplb huyplb commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Writing an e2e test for the revert flow proved it had never once landed. The
plan looked correct and the driver rejected it:

500 POST /lokee/databases/…/revert  {"error":"near \".\": syntax error"}
CREATE INDEX IDX_CUSTOMERS_EMAIL ON main.customers (email);

Two defects in that one statement.

SQLite cannot take a qualified table in CREATE INDEX

The schema belongs on the index name — CREATE INDEX main.idx ON customers(email)
— while every other dialect qualifies the table, which is why the shared generator
does. SQLite gets a createIndexStatement hook that moves the qualifier across.

The generator used compare's match key as an identifier

CLAUDE.md opens with "The compare key is not an identifier." The generator spread
the source IndexInfo and then overwrote name with the uppercased key, emitting
IDX_CUSTOMERS_EMAIL for idx_customers_email. SQLite folds case so it survived
there; on a case-sensitive target it creates a differently named index and the next
compare reads a rename that never happened.

The cause was a type declaration: IndexDiff.source omitted name, so the
generator could not reach the identifier compare had been passing all along.
Widened it — optional, not required, because @foxschema/sql is published and a
required field would break consumers constructing these objects.

Both are pinned by unit tests, since the e2e suite is not in the CI gate.

The e2e test

apps/e2e/src/tests/schema-revert.test.ts, SQLite-backed so it needs no Docker:

  1. Snapshot a baseline, DROP INDEX, snapshot again → two versions
  2. Set Original = Version 1, open Compare, assert the Migration SQL shows the
    CREATE INDEXand that the file on disk is unchanged, which is the
    "compare before you decide" guarantee
  3. Execute, then read the SQLite file: index back, new version recorded, row count
    still 1

The last step reads the database rather than the UI, which is the only way this
class of bug surfaces.

History toolbar, proved out alongside it

  • The version pickers and the capture credential were two connection-shaped
    controls on two rows, reading as "which of these databases am I looking at?".
    They are one database — recorded and live — so capture moved onto the pickers'
    row and now defaults to the saved connection matching the history database.
  • The graph no longer follows the pickers. Choosing Version 1 as Original used
    to hide every version between the two sides, so the history overview changed as
    a side effect of choosing what to compare. The sidebar checkboxes are the only
    filter now.
  • Execute was dead on the Blueprint tab because the data-loss acknowledgement
    lives on Migration SQL. A risk chip now rides beside the button, and a blocked
    button carries the reader to the decision instead of greying out.

HistoryCompareBar renders in TopToolbar while the fetching lives in
LokeeWeaveView, so it asks for work by bumping a counter in the store. Both
watchers compare against the value seen at mount — the store outlives the
component, and replaying the last request would re-snapshot the database on every
visit. (Caught by a test; an earlier version of that wiring was also an infinite
capture loop, since the callback both reads and sets capturing.)

Repairs to the existing e2e suite

CI does not run e2e, and the shared SchemaBlueprint change had broken it:

  • the summary reads N versions, not Total Versions: N
  • the inspector's sections carry blueprint-* ids, and indexes now render
    where the old panel stored but never showed them

Verification

  • revert e2e 3/3 (reads the SQLite file, not the UI)
  • history e2e 6/6
  • 1613 unit tests, 31 skipped
  • tsc --noEmit clean, eslint . 0 errors

Known gaps

  • The revert has been proven against SQLite only. Against Postgres it reaches
    the database and fails on credentials — the saved connection's password needs
    unlocking, which I can't do.
  • The risk badge reads "Data loss · N lossy" for a purely additive revert. It's
    true that a re-created table comes back empty, but the copy says "I understand
    data will be lost permanently"
    about statements that only CREATE. Worth
    softening separately.

🤖 Generated with Claude Code


Note

High Risk
Changes SQL generation and SQLite DDL for reverts (live schema mutations) plus history capture wiring; incorrect SQL or replayed capture requests could damage databases or record wrong versions.

Overview
Fixes revert migrations that never applied on SQLite — the driver rejected generated CREATE INDEX SQL (near ".": syntax error). SQLite now qualifies the schema on the index name (main.idx ON customers), not on the table, via a dialect hook. The migration generator also uses each index’s real identifier from IndexDiff.source.name instead of compare’s uppercased match key, which had been emitting wrong names like IDX_CUSTOMERS_EMAIL for idx_customers_email.

Adds a SQLite e2e flow that snapshots history, compares versions, runs revert, and asserts the file on disk (not just the UI). Updates history e2e helpers for the new summary text (N versions) and SchemaBlueprint blueprint-* sections (including indexes).

History UI: capture and refresh move onto HistoryCompareBar with store counter requests so LokeeWeaveView can fetch without callback wiring; capture credential defaults to the connection matching the viewed history DB. The version graph no longer filters when Original/Target pickers change. Revert Execute stays enabled for lossy plans with “Review data loss…” → Migration SQL; a risk chip sits beside the button.

Reviewed by Cursor Bugbot for commit 1187d39. Bugbot is set up for automated code reviews on this repo. Configure here.

Writing an e2e test for the revert flow proved it had never once landed. The
plan looked correct and the driver rejected it:

  500 POST /lokee/databases/…/revert  {"error":"near \".\": syntax error"}
  CREATE INDEX IDX_CUSTOMERS_EMAIL ON main.customers (email);

Two defects in one statement.

**SQLite cannot take a qualified table in CREATE INDEX.** The schema belongs on
the index name — `CREATE INDEX main.idx ON customers(email)` — while every other
dialect qualifies the table, which is why the shared generator does. SQLite gets
a `createIndexStatement` hook that moves the qualifier across.

**The generator used compare's match key as an identifier.** CLAUDE.md opens with
"The compare key is not an identifier", and this spread the source IndexInfo then
overwrote `name` with the uppercased key, emitting IDX_CUSTOMERS_EMAIL for
idx_customers_email. SQLite folds case so it survived there; on a case-sensitive
target it creates a differently named index and the next compare reads a rename
that never happened. The cause was a type: `IndexDiff.source` omitted `name`, so
the generator could not reach the identifier compare had been passing all along.
Widened it — optional, not required, because this package is published and a
required field would break consumers constructing these objects.

Both are pinned by unit tests, since the e2e suite is not in the CI gate.

Alongside the fix, the History toolbar work this proved out:

- The version pickers and the capture credential were two connection-shaped
  controls on two rows, reading as "which of these databases am I looking at?".
  They are one database — recorded and live — so capture moved onto the pickers'
  row and defaults to the saved connection matching the history database.
- The graph no longer follows the pickers. Choosing Version 1 as Original used to
  hide every version between the sides, so the history overview changed as a side
  effect of choosing what to compare. The checkboxes are the only filter now.
- Execute was dead on the Blueprint tab because the data-loss acknowledgement
  lives on Migration SQL. A risk chip now rides beside the button, and a blocked
  button carries the reader to the decision instead of greying out.

HistoryCompareBar renders in TopToolbar while the fetching lives in
LokeeWeaveView, so it asks for work by bumping a counter in the store. Both
watchers compare against the value seen at mount: the store outlives the
component, and replaying the last request would re-snapshot the database on every
visit.

Also repairs the existing e2e suite, which CI does not run and which the shared
SchemaBlueprint change had broken: the summary is `N versions` rather than
`Total Versions: N`, and the inspector's sections carry `blueprint-*` ids — with
indexes now rendering where the old panel stored but never showed them.

Verified: revert e2e 3/3 (reads the SQLite file, not the UI), history e2e 6/6,
1613 unit tests, tsc clean, eslint 0 errors.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_3623c182-4607-486a-982b-da2d5333571a)

Left behind when the capture test moved from clicking a button to bumping the
store counter — the button it used to click now lives in HistoryCompareBar.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_0fc952af-0133-42ca-9d92-b8de4aa90136)

@huyplb
huyplb merged commit d6e75d3 into main Aug 16, 2026
11 checks passed
@huyplb
huyplb deleted the fix/revert-lands-and-history-toolbar branch August 16, 2026 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant