diff --git a/apps/e2e/src/pages/LokeeHistoryPage.ts b/apps/e2e/src/pages/LokeeHistoryPage.ts index 1af4980d..4131e32b 100644 --- a/apps/e2e/src/pages/LokeeHistoryPage.ts +++ b/apps/e2e/src/pages/LokeeHistoryPage.ts @@ -55,7 +55,7 @@ export class LokeeHistoryPage { (expected) => { const el = document.querySelector('[data-testid="lokee-summary"]'); const text = el?.textContent ?? ''; - return new RegExp(`Total Versions:\\s*${expected}\\b`, 'i').test(text); + return new RegExp(`\\b${expected}\\s+versions?\\b`, 'i').test(text); }, n, { timeout: timeoutMs } @@ -121,10 +121,21 @@ export class LokeeHistoryPage { } /** Sections render only once loaded, so callers must await the inspector first. */ - async inspectorHasSection(section: 'growth' | 'source' | 'columns' | 'indexes'): Promise { + async inspectorHasSection(section: 'growth' | 'source' | 'history'): Promise { return this.page.locator(`[data-testid="lokee-inspector-${section}"]`).isVisible(); } + /** + * Columns / indexes / keys / triggers are `SchemaBlueprint` now — the same + * component Compare Schema renders — so they carry `blueprint-*` ids wherever + * they appear rather than an inspector-specific one. + */ + async blueprintHasSection( + section: 'summary' | 'columns' | 'primary-key' | 'indexes' | 'foreign-keys' | 'triggers' + ): Promise { + return this.page.locator(`[data-testid="blueprint-${section}"]`).isVisible(); + } + async inspectorText(): Promise { return (await this.page.locator('[data-testid="lokee-object-inspector"]').innerText()) ?? ''; } @@ -145,7 +156,7 @@ export class LokeeHistoryPage { async versionCount(): Promise { const text = await this.summaryText(); - const match = text.match(/Total Versions:\s*(\d+)/i); + const match = text.match(/(\d+)\s+versions?\b/i); return match ? Number(match[1]) : 0; } @@ -158,4 +169,74 @@ export class LokeeHistoryPage { await box.waitFor({ state: 'visible', timeout: 10_000 }); if (!(await box.isChecked())) await box.check(); } + + // ── Compare versions → revert ──────────────────────────────────────────── + + /** Pick the Original side by its visible label (e.g. "Version 1"). */ + async selectOriginalVersion(label: string): Promise { + const select = this.page.locator('[data-testid="lokee-original-version"]'); + await select.waitFor({ state: 'visible', timeout: 20_000 }); + const option = select.locator('option', { hasText: label }); + await option.waitFor({ state: 'attached', timeout: 20_000 }); + const value = await option.getAttribute('value'); + if (!value) throw new Error(`No Original version option matching ${label}`); + await select.selectOption(value); + } + + async openCompareModal(): Promise { + await clickWhen(this.page, '[data-testid="lokee-compare-versions-btn"]'); + await this.page.waitForSelector('[data-testid="lokee-version-compare"][data-state="ready"]', { + timeout: 30_000, + }); + } + + async compareTab(tab: 'DIFF' | 'DDL_DIFF' | 'SQL'): Promise { + await clickWhen(this.page, `[data-testid="lokee-cmp-tab-${tab}"]`); + } + + async migrationSqlText(): Promise { + await this.compareTab('SQL'); + const pane = this.page.locator('[data-testid="lokee-cmp-ddl"]'); + await pane.waitFor({ state: 'visible', timeout: 20_000 }); + return pane.innerText(); + } + + /** + * Apply the revert. A lossy plan parks the button on "Review data loss…", + * which navigates to Migration SQL rather than running — acknowledge there, + * then press it again. A safe plan runs on the first press. + */ + async executeRevert(): Promise { + const run = this.page.locator('[data-testid="lokee-cmp-run-revert"]'); + await run.waitFor({ state: 'visible', timeout: 20_000 }); + await this.page.waitForFunction( + () => { + const el = document.querySelector('[data-testid="lokee-cmp-run-revert"]'); + return el instanceof HTMLButtonElement && !el.disabled; + }, + { timeout: 30_000 } + ); + if ((await run.innerText()).includes('Review data loss')) { + await run.click(); + const ack = this.page.locator('[data-testid="lokee-cmp-confirm-lossy"]'); + await ack.waitFor({ state: 'visible', timeout: 10_000 }); + await ack.check(); + } + await run.click(); + } + + /** Visible toast text, so a failed revert reports the driver's reason. */ + async toastText(): Promise { + const toasts = this.page.locator('[data-testid="app-toast"]'); + const count = await toasts.count(); + const parts: string[] = []; + for (let i = 0; i < count; i++) { + parts.push((await toasts.nth(i).innerText().catch(() => '')) ?? ''); + } + return parts.join(' | '); + } + + async compareModalOpen(): Promise { + return this.page.locator('[data-testid="lokee-version-compare"]').isVisible(); + } } diff --git a/apps/e2e/src/tests/schema-history.test.ts b/apps/e2e/src/tests/schema-history.test.ts index a82a9bb9..6282bae1 100644 --- a/apps/e2e/src/tests/schema-history.test.ts +++ b/apps/e2e/src/tests/schema-history.test.ts @@ -110,9 +110,12 @@ describe.skipIf(!ready)('Schema Sync · History (SQLite)', () => { await history.clickObjectNamed('customers'); const text = await history.inspectorText(); expect(text).toMatch(/email/i); - expect(await driver.locator('[data-testid="lokee-inspector-columns"]').isVisible()).toBe(true); - expect(await driver.locator('[data-testid="lokee-inspector-indexes"]').count()).toBe(0); - expect(await driver.locator('[data-testid="lokee-inspector-triggers"]').isVisible()).toBe(true); + // The blueprint is Compare Schema's own component now, so the sections + // carry `blueprint-*` ids — and indexes, which the old bespoke inspector + // stored but never surfaced, are among them. + expect(await history.blueprintHasSection('columns')).toBe(true); + expect(await history.blueprintHasSection('indexes')).toBe(true); + expect(await history.blueprintHasSection('triggers')).toBe(true); expect(text).toMatch(/trg_customers_audit/i); expect(await driver.locator('[data-testid="lokee-inspector-script-diff"]').isVisible()).toBe(true); }); diff --git a/apps/e2e/src/tests/schema-revert.test.ts b/apps/e2e/src/tests/schema-revert.test.ts new file mode 100644 index 00000000..80e105be --- /dev/null +++ b/apps/e2e/src/tests/schema-revert.test.ts @@ -0,0 +1,169 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * Schema Sync → History → Compare versions → revert, against a local SQLite file. + * + * This is the one flow whose correctness cannot be argued from unit tests: the + * revert plan is built from stored objects, but whether it *lands* depends on + * the generator, the driver and the live schema agreeing. Everything up to the + * button was already covered; this drives the button and then reads the file on + * disk to prove the schema actually moved. + * + * Requires `npm run dev` and `sqlite3` on PATH. Skips when sqlite3 is missing. + */ +import { describe, it, beforeAll, afterAll, expect } from 'vitest'; +import { execFileSync, execSync } from 'node:child_process'; +import { mkdirSync, rmSync, existsSync } from 'node:fs'; +import { join } from 'node:path'; +import type { Page } from 'playwright'; +import { buildDriver, quitDriver } from '../helpers/driver.js'; +import { AppPage } from '../pages/AppPage.js'; +import { SqlEditorPage } from '../pages/SqlEditorPage.js'; +import { LokeeHistoryPage } from '../pages/LokeeHistoryPage.js'; + +const RUN = Date.now().toString(36); +const DIR = `/tmp/foxschema-e2e-schema-revert-${RUN}`; +const DB = join(DIR, 'revert.db'); +const NAME = `E2E Revert ${RUN}`; + +function hasSqlite3(): boolean { + try { + execSync('which sqlite3', { stdio: 'ignore' }); + return true; + } catch { + return false; + } +} + +function sqlite(input: string): string { + return execFileSync('sqlite3', [DB], { input, encoding: 'utf8' }); +} + +function schemaText(): string { + return execFileSync('sqlite3', [DB, '.schema'], { encoding: 'utf8' }); +} + +const ready = hasSqlite3(); + +describe.skipIf(!ready)('Schema Sync · History revert (SQLite)', () => { + let driver: Page; + let app: AppPage; + let sql: SqlEditorPage; + let history: LokeeHistoryPage; + + beforeAll(async () => { + rmSync(DIR, { recursive: true, force: true }); + mkdirSync(DIR, { recursive: true }); + sqlite(` +CREATE TABLE customers ( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL, + email TEXT +); +CREATE INDEX idx_customers_email ON customers(email); +INSERT INTO customers (id, name, email) VALUES (1, 'Ada', 'ada@example.com'); +`); + expect(existsSync(DB)).toBe(true); + + driver = await buildDriver(); + app = new AppPage(driver); + sql = new SqlEditorPage(driver); + history = new LokeeHistoryPage(driver); + + await app.open(); + await sql.resetPersistedEditorState(); + await driver.reload(); + await driver.waitForSelector('[data-testid="toolbar"]'); + + await sql.addSqliteCredential(NAME, DB); + await driver.locator('[data-testid="view-sync-btn"]').click(); + await driver.waitForSelector('[data-testid="sync-pane-switcher"]'); + await history.selectSavedTargetByName(NAME); + }, 120_000); + + afterAll(async () => { + if (driver) await quitDriver(driver); + rmSync(DIR, { recursive: true, force: true }); + }); + + it('records the baseline, then a version with the index dropped', async () => { + await history.snapshotTarget(); + + // The change to undo. Dropping an index is the cleanest case to assert on: + // reverting it is a pure CREATE, so the plan is safe rather than lossy and + // the file tells us plainly whether it landed. + sqlite('DROP INDEX idx_customers_email;\n'); + expect(schemaText()).not.toMatch(/idx_customers_email/i); + + await history.snapshotTarget(); + await history.openHistoryPane(); + await history.selectHistoryDatabaseContaining(RUN); + await history.waitForGraph(); + await expect.poll(async () => history.versionCount(), { timeout: 30_000 }).toBe(2); + }, 120_000); + + it('shows the reverse DDL before anything is applied', async () => { + await history.selectOriginalVersion('Version 1'); + await history.openCompareModal(); + + // The tree is the shared SchemaDiffTree; the blueprint is SchemaBlueprint. + expect(await driver.locator('[data-testid="lokee-cmp-summary"]').isVisible()).toBe(true); + expect(await driver.locator('[data-testid="schema-blueprint"]').isVisible()).toBe(true); + + const migration = await history.migrationSqlText(); + expect(migration, migration).toMatch(/CREATE INDEX/i); + expect(migration, migration).toMatch(/idx_customers_email/i); + + // Nothing has run yet — the point of comparing before deciding. + expect(schemaText()).not.toMatch(/idx_customers_email/i); + }, 120_000); + + it('applies the revert and records it as a new version', async () => { + // Record what the button actually sent. A silent no-op and a rejected + // request look identical from the file system, and toasts expire before a + // 30s poll finishes — so capture the response rather than infer it. + const calls: string[] = []; + driver.on('response', (res) => { + const url = res.url(); + if (url.includes('/lokee/') && url.includes('/revert') && res.request().method() === 'POST') { + calls.push(`${res.status()} ${url}`); + void res + .text() + .then((body) => calls.push(`body: ${body.slice(0, 300)}`)) + .catch(() => undefined); + } + }); + + await history.executeRevert(); + + // The file is the source of truth: the index is back. If it is not, the + // toast carries the driver's reason — without it this failure is just + // "nothing happened", which is the least actionable thing a test can say. + try { + await expect + .poll(() => schemaText(), { timeout: 30_000 }) + .toMatch(/idx_customers_email/i); + } catch (error) { + throw new Error( + `Revert did not reach the database.\nPOSTs: ${calls.join(' :: ') || '(none)'}\nToast: ${await history.toastText()}\nSchema: ${schemaText()}`, + { cause: error } + ); + } + + // A revert is itself a migration, so history gains a version for it rather + // than rewriting the one it reverted to. + await driver.waitForSelector('[data-testid="lokee-version-compare"]', { + state: 'detached', + timeout: 30_000, + }); + await expect + .poll(async () => history.versionCount(), { timeout: 30_000 }) + .toBeGreaterThanOrEqual(3); + + // The row survived: reverting an index must not rebuild the table. + const rows = sqlite('SELECT count(*) FROM customers;\n').trim(); + expect(rows).toBe('1'); + }, 180_000); +}); diff --git a/apps/web/src/frontend/components/lokee-weave/HistoryCompareBar.tsx b/apps/web/src/frontend/components/lokee-weave/HistoryCompareBar.tsx index 85fb5e59..d06832b4 100644 --- a/apps/web/src/frontend/components/lokee-weave/HistoryCompareBar.tsx +++ b/apps/web/src/frontend/components/lokee-weave/HistoryCompareBar.tsx @@ -7,8 +7,10 @@ * are versions of one captured database instead of two live connections. */ import React, { useMemo } from 'react'; -import { ArrowLeftRight, ArrowRight } from 'lucide-react'; +import { ArrowLeftRight, ArrowRight, Camera, Loader2, RefreshCw } from 'lucide-react'; import { useLokeeHistoryStore } from '../../store/lokeeHistoryStore'; +import { useSyncStore } from '../../store/useSyncStore'; +import { SQL_ICON_STROKE } from '../sql-editor/sqlIconStyle'; import { historyVersionLabel, lokeeDatabaseLabel, @@ -26,6 +28,12 @@ export function HistoryCompareBar(): React.ReactElement { const setOriginalVersionId = useLokeeHistoryStore((s) => s.setOriginalVersionId); const setTargetVersionId = useLokeeHistoryStore((s) => s.setTargetVersionId); const swapSides = useLokeeHistoryStore((s) => s.swapSides); + const connections = useSyncStore((s) => s.connections); + const captureConnectionId = useLokeeHistoryStore((s) => s.captureConnectionId); + const setCaptureConnectionId = useLokeeHistoryStore((s) => s.setCaptureConnectionId); + const capturing = useLokeeHistoryStore((s) => s.capturing); + const requestCapture = useLokeeHistoryStore((s) => s.requestCapture); + const requestRefresh = useLokeeHistoryStore((s) => s.requestRefresh); const newestFirst = useMemo(() => sortVersionsNewestFirst(versions), [versions]); const resolved = useMemo( @@ -37,8 +45,8 @@ export function HistoryCompareBar(): React.ReactElement { Boolean(resolved.original && resolved.target && resolved.original.id === resolved.target.id); return ( -
-
+
+
Original @@ -99,7 +107,7 @@ export function HistoryCompareBar(): React.ReactElement {
-
+
Target @@ -126,6 +134,58 @@ export function HistoryCompareBar(): React.ReactElement { ))}
+ + {/* Capture lives on this row too. It used to sit on a second bar with its + own credential picker, which read as a *third* connection control next + to the two above it — three pickers for two ideas. */} +
+
+
+ Capture +
+ +
+
+ + +
+
); } diff --git a/apps/web/src/frontend/components/lokee-weave/LokeeWeavePage.tsx b/apps/web/src/frontend/components/lokee-weave/LokeeWeavePage.tsx index 371267cf..3ee66e11 100644 --- a/apps/web/src/frontend/components/lokee-weave/LokeeWeavePage.tsx +++ b/apps/web/src/frontend/components/lokee-weave/LokeeWeavePage.tsx @@ -58,8 +58,6 @@ export interface LokeeWeavePageProps { ) => Promise; /** Shorter header when shown inside Schema Sync. */ embedded?: boolean; - /** Original + Target version ids from the Compare-style History bar. */ - compareVersionIds?: readonly string[]; } const FILTERABLE_TYPES: LokeeObjectType[] = [ @@ -187,15 +185,14 @@ export const LokeeWeavePage: React.FC = ({ onSelectObject, onSaveVersionMeta, embedded = false, - compareVersionIds, }) => { - const [filters, setFilters] = useState(() => { - const base = freshFilters(); - if (compareVersionIds && compareVersionIds.length > 0) { - return { ...base, versionIds: new Set(compareVersionIds) }; - } - return base; - }); + // Deliberately independent of the Original/Target pickers. Driving this + // filter from them hid every version between the two sides — pick "Version 1" + // as Original and Version 2 silently vanished from the graph — so the history + // overview changed under the reader as a side effect of choosing what to + // compare. The pickers choose the diff; the graph shows the history; the + // checkboxes below are the only thing that filters it. + const [filters, setFilters] = useState(freshFilters); const [locked, setLocked] = useState(true); const [selectedVersionId, setSelectedVersionId] = useState(null); const [editName, setEditName] = useState(''); @@ -207,12 +204,6 @@ export const LokeeWeavePage: React.FC = ({ [dto.versions, selectedVersionId] ); - const compareKey = (compareVersionIds ?? []).join('|'); - useEffect(() => { - if (!compareKey) return; - setFilters((f) => ({ ...f, versionIds: new Set(compareKey.split('|')) })); - }, [compareKey]); - useEffect(() => { if (!selectedVersion) { setEditName(''); diff --git a/apps/web/src/frontend/components/lokee-weave/LokeeWeaveView.test.tsx b/apps/web/src/frontend/components/lokee-weave/LokeeWeaveView.test.tsx index 8a5d5013..e0a5dc34 100644 --- a/apps/web/src/frontend/components/lokee-weave/LokeeWeaveView.test.tsx +++ b/apps/web/src/frontend/components/lokee-weave/LokeeWeaveView.test.tsx @@ -9,7 +9,7 @@ */ import React from 'react'; import { describe, expect, it, vi, beforeEach } from 'vitest'; -import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { act, render, screen, waitFor } from '@testing-library/react'; import type { VersionGraphDTO } from './graphTypes'; const listLokeeDatabases = vi.fn(); @@ -108,7 +108,8 @@ describe('LokeeWeaveView', () => { await waitFor(() => expect(screen.getByTestId('graph')).toBeTruthy()); // The subtitle must name the database, not the saved connection. expect(screen.getByTestId('graph').textContent).toContain('[postgres] localhost/foxdb.public'); - expect(screen.getByTestId('lokee-weave-chrome')).toBeTruthy(); + // The chrome row is gone — Refresh and Capture moved into HistoryCompareBar. + expect(screen.queryByTestId('lokee-weave-chrome')).toBeNull(); }); it('shows an empty state rather than an empty canvas', async () => { @@ -188,7 +189,10 @@ describe('LokeeWeaveView', () => { expect(useLokeeHistoryStore.getState().versions.map((v) => v.id)).toEqual(['v2', 'v1']); }); - it('captures a schema from the chosen credential', async () => { + it('captures when the toolbar bar asks for it', async () => { + // The credential picker and Capture button moved to HistoryCompareBar, which + // renders in TopToolbar. It asks by bumping the store counter, so that is + // the seam to test here rather than a button this component no longer owns. listLokeeDatabases.mockResolvedValueOnce([]).mockResolvedValue([DB]); loadVersionGraph.mockResolvedValue({ ...DTO, truncatedObjects: false }); captureSchema.mockResolvedValue({ @@ -202,12 +206,12 @@ describe('LokeeWeaveView', () => { }); render(); + await waitFor(() => expect(listLokeeDatabases).toHaveBeenCalled()); - await waitFor(() => expect(screen.getByTestId('lokee-capture-btn')).toBeTruthy()); - fireEvent.change(screen.getByTestId('lokee-capture-connection'), { - target: { value: 'c1' }, + act(() => { + useLokeeHistoryStore.getState().setCaptureConnectionId('c1'); + useLokeeHistoryStore.getState().requestCapture(); }); - fireEvent.click(screen.getByTestId('lokee-capture-btn')); await waitFor(() => expect(captureSchema).toHaveBeenCalledWith( @@ -218,4 +222,17 @@ describe('LokeeWeaveView', () => { expect(toast).toHaveBeenCalledWith(expect.objectContaining({ title: 'Captured v1' })) ); }); + + it('does not capture or refetch on mount — the counters start at zero', async () => { + // A naive effect on the request counters would fire once per mount, so a + // visit to History would silently snapshot the database. + listLokeeDatabases.mockResolvedValue([DB]); + loadVersionGraph.mockResolvedValue({ ...DTO, truncatedObjects: false }); + + render(); + await waitFor(() => expect(screen.getByTestId('graph')).toBeTruthy()); + + expect(captureSchema).not.toHaveBeenCalled(); + expect(loadVersionGraph).toHaveBeenCalledTimes(1); + }); }); diff --git a/apps/web/src/frontend/components/lokee-weave/LokeeWeaveView.tsx b/apps/web/src/frontend/components/lokee-weave/LokeeWeaveView.tsx index 4fe07a03..e4728763 100644 --- a/apps/web/src/frontend/components/lokee-weave/LokeeWeaveView.tsx +++ b/apps/web/src/frontend/components/lokee-weave/LokeeWeaveView.tsx @@ -9,7 +9,7 @@ * load, or be empty is handled here, so the graph itself remains trivially * testable with a fixture and has no idea a network exists. */ -import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { ArrowRight, Camera, GitBranch, Loader2, RefreshCw, TriangleAlert } from 'lucide-react'; import { LokeeWeavePage } from './LokeeWeavePage'; import { VersionCompareModal } from './VersionCompareModal'; @@ -73,12 +73,18 @@ export function LokeeWeaveView({ const setStoreDatabases = useLokeeHistoryStore((s) => s.setDatabases); const setStoreVersions = useLokeeHistoryStore((s) => s.setVersions); const originalVersionId = useLokeeHistoryStore((s) => s.originalVersionId); + const captureConnectionId = useLokeeHistoryStore((s) => s.captureConnectionId); + const setCaptureConnectionId = useLokeeHistoryStore((s) => s.setCaptureConnectionId); + const capturing = useLokeeHistoryStore((s) => s.capturing); + const setCapturing = useLokeeHistoryStore((s) => s.setCapturing); + const captureRequest = useLokeeHistoryStore((s) => s.captureRequest); + const refreshRequest = useLokeeHistoryStore((s) => s.refreshRequest); const targetVersionId = useLokeeHistoryStore((s) => s.targetVersionId); const [databases, setDatabases] = useState([]); - const [captureConnectionId, setCaptureConnectionId] = useState(''); + const [dto, setDto] = useState(EMPTY_DTO); const [loading, setLoading] = useState(true); - const [capturing, setCapturing] = useState(false); + const [error, setError] = useState(null); const [selectedObject, setSelectedObject] = useState(null); // Bumped to re-run the effect; a plain refetch() would race the in-flight one. @@ -135,11 +141,39 @@ export function LokeeWeaveView({ return [...new Set(ids)]; }, [dto.versions, originalVersionId, targetVersionId]); + /** + * Default the capture credential to the saved connection that *is* the history + * database being viewed. Two connection-shaped controls sitting side by side + * with unrelated values is what made the bar read as "which of these two + * databases am I looking at?" — when they are the same database, one recorded + * and one live. Compare's Target is the fallback, as before. + */ + const activeDatabase = useMemo( + () => databases.find((d) => d.id === activeId), + [databases, activeId] + ); + const matchingCredentialId = useMemo(() => { + if (!activeDatabase) return undefined; + const same = (a?: string | null, b?: string | null) => + (a ?? '').toLowerCase() === (b ?? '').toLowerCase(); + return connections.find( + (c) => + same(c.dialect, activeDatabase.dialect) && + same(c.host, activeDatabase.host) && + same(c.database, activeDatabase.database) + )?.id; + }, [connections, activeDatabase]); + useEffect(() => { - if (!captureConnectionId && selectedTargetConnectionId) { - setCaptureConnectionId(selectedTargetConnectionId); - } - }, [captureConnectionId, selectedTargetConnectionId]); + if (captureConnectionId) return; + const next = matchingCredentialId ?? selectedTargetConnectionId; + if (next) setCaptureConnectionId(next); + }, [ + captureConnectionId, + matchingCredentialId, + selectedTargetConnectionId, + setCaptureConnectionId, + ]); useEffect(() => { let cancelled = false; @@ -192,6 +226,16 @@ export function LokeeWeaveView({ const refresh = useCallback(() => setReloadToken((n) => n + 1), []); + // HistoryCompareBar renders in TopToolbar, so it asks for work by bumping a + // counter rather than holding a callback. 0 is the initial value — acting on + // it would refetch on every mount. + const seenRefreshRequest = useRef(refreshRequest); + useEffect(() => { + if (refreshRequest === seenRefreshRequest.current) return; + seenRefreshRequest.current = refreshRequest; + refresh(); + }, [refreshRequest, refresh]); + const saveVersionMeta = useCallback( async (versionId: string, patch: { name: string; description: string }) => { if (!activeId) return; @@ -266,74 +310,33 @@ export function LokeeWeaveView({ } finally { setCapturing(false); } - }, [captureConnectionId, capturing, connections, refresh, bumpLokeeEpoch, setStoreDatabaseId]); + }, [ + captureConnectionId, + capturing, + connections, + refresh, + bumpLokeeEpoch, + setStoreDatabaseId, + setCapturing, + ]); + + // Fire on the counter alone, through a ref. Depending on `runCapture` here + // is an infinite loop: it reads `capturing`, and it also *sets* it, so every + // toggle gives the callback a new identity, re-runs this effect while the + // counter is still non-zero, and captures again. Caught by a hanging test. + // + // The baseline is whatever the counter held at mount, not zero: the store + // outlives this component, so leaving History after a capture and coming + // back would otherwise re-fire that request and snapshot the database again. + const runCaptureRef = useRef(runCapture); + runCaptureRef.current = runCapture; + const seenCaptureRequest = useRef(captureRequest); + useEffect(() => { + if (captureRequest === seenCaptureRequest.current) return; + seenCaptureRequest.current = captureRequest; + void runCaptureRef.current(); + }, [captureRequest]); - const chrome = ( -
- {!embedded && ( - - )} - -
- - -
-
- ); @@ -366,8 +369,7 @@ export function LokeeWeaveView({ if (loading) { return (
- {chrome} -
+
Loading schema history…
@@ -378,8 +380,7 @@ export function LokeeWeaveView({ if (error) { return (
- {chrome} -
+
Could not load schema history
{error}
@@ -399,8 +400,7 @@ export function LokeeWeaveView({ if (!activeId || dto.versions.length === 0) { return (
- {chrome} -
+
No schema history yet
@@ -416,7 +416,6 @@ export function LokeeWeaveView({ return (
- {chrome} {compareBar} {dto.truncatedObjects && (
@@ -430,7 +429,6 @@ export function LokeeWeaveView({ dto={dto} subtitle={subtitle} embedded={embedded} - compareVersionIds={compareVersionIds} onSelectObject={handleSelectObject} onSaveVersionMeta={saveVersionMeta} /> diff --git a/apps/web/src/frontend/components/lokee-weave/VersionCompareModal.tsx b/apps/web/src/frontend/components/lokee-weave/VersionCompareModal.tsx index 25e63e16..739a96e4 100644 --- a/apps/web/src/frontend/components/lokee-weave/VersionCompareModal.tsx +++ b/apps/web/src/frontend/components/lokee-weave/VersionCompareModal.tsx @@ -188,6 +188,27 @@ export function VersionCompareModal({ if (data) void loadPlan(); }, [data, loadPlan]); + /** + * Why Execute cannot run yet — the empty string means it can. + * + * The button lives in the toolbar and the data-loss acknowledgement lives on + * the Migration SQL tab, so a reader on the Blueprint tab saw a greyed-out + * button with no stated reason. Comparing before deciding is the whole point + * of this dialog; the decision has to be reachable from wherever you are. + */ + const blockedReason = useMemo(() => { + if (!captureConnectionId) return 'Choose a credential in the bar above to run this.'; + if (!plan) return planning ? 'Still planning…' : 'No plan yet.'; + if (plan.alreadyAtTarget) return 'The live schema already matches Original.'; + if (plan.statements.length === 0) return 'Nothing to apply.'; + if (plan.reversal.risk === 'blocked') { + return 'Blocked — this cannot be applied without losing data the schema cannot restore.'; + } + if (plan.reversal.risk === 'lossy' && !confirmLossy) return 'ACK_LOSSY'; + return ''; + }, [captureConnectionId, plan, planning, confirmLossy]); + const needsLossyAck = blockedReason === 'ACK_LOSSY'; + const runRevert = useCallback(async () => { if (!captureConnectionId || !plan) return; setRunning(true); @@ -340,29 +361,50 @@ export function VersionCompareModal({ testIdPrefix="lokee-cmp" size="compact" /> - +
+ {/* Risk travels with the button, not just with the tab + that happens to show the statements. */} + {plan && plan.statements.length > 0 && ( + + {riskStyle(plan.reversal.risk).label} + {plan.reversal.lossyCount > 0 ? ` · ${plan.reversal.lossyCount}` : ''} + + )} + +
diff --git a/apps/web/src/frontend/store/lokeeHistoryStore.ts b/apps/web/src/frontend/store/lokeeHistoryStore.ts index 7fdb8852..b0dc750b 100644 --- a/apps/web/src/frontend/store/lokeeHistoryStore.ts +++ b/apps/web/src/frontend/store/lokeeHistoryStore.ts @@ -25,6 +25,21 @@ interface LokeeHistoryState { setDatabases: (rows: LokeeDatabase[]) => void; setVersions: (rows: HistoryVersionOption[]) => void; swapSides: () => void; + + /** + * Capture belongs to the same toolbar row as the pickers, but the fetching + * and the graph live in LokeeWeaveView. Rather than passing callbacks + * through the store, the bar bumps a counter and the view watches it — the + * store stays plain data, which is what keeps it testable. + */ + captureConnectionId: string; + setCaptureConnectionId: (id: string) => void; + capturing: boolean; + setCapturing: (busy: boolean) => void; + captureRequest: number; + requestCapture: () => void; + refreshRequest: number; + requestRefresh: () => void; } export const useLokeeHistoryStore = create((set, get) => ({ @@ -41,6 +56,14 @@ export const useLokeeHistoryStore = create((set, get) => ({ setTargetVersionId: (targetVersionId) => set({ targetVersionId }), setDatabases: (databases) => set({ databases }), setVersions: (versions) => set({ versions }), + captureConnectionId: '', + setCaptureConnectionId: (captureConnectionId) => set({ captureConnectionId }), + capturing: false, + setCapturing: (capturing) => set({ capturing }), + captureRequest: 0, + requestCapture: () => set({ captureRequest: get().captureRequest + 1 }), + refreshRequest: 0, + requestRefresh: () => set({ refreshRequest: get().refreshRequest + 1 }), swapSides: () => { const next = swapHistoryCompare(get().versions, { originalVersionId: get().originalVersionId, diff --git a/packages/sql/src/interfaces/diff.types.interface.ts b/packages/sql/src/interfaces/diff.types.interface.ts index 28e2b2fa..8950ab4b 100644 --- a/packages/sql/src/interfaces/diff.types.interface.ts +++ b/packages/sql/src/interfaces/diff.types.interface.ts @@ -10,10 +10,19 @@ export interface ColumnDiff { } export interface IndexDiff { + /** Uppercased compare-key match name — NOT a real identifier, see source.name. */ name: string; status: 'ADDED' | 'REMOVED' | 'MODIFIED' | 'UNCHANGED'; - source?: { columns: string[]; unique: boolean; constraint?: boolean }; - target?: { columns: string[]; unique: boolean; constraint?: boolean }; + /** + * `name` here is the index's own identifier in its native casing, which is + * what DDL must use. Compare has always passed the whole IndexInfo through; + * only this declaration hid the field, so generators reached for the + * uppercased key instead and emitted IDX_CUSTOMERS_EMAIL for + * idx_customers_email. Optional rather than required because this package is + * published — every producer inside the repo sets it. + */ + source?: { name?: string; columns: string[]; unique: boolean; constraint?: boolean }; + target?: { name?: string; columns: string[]; unique: boolean; constraint?: boolean }; /** * True when this ADDED/REMOVED pair is only an index rename: same columns + * uniqueness as an unmatched index on the other side. Does not mark the table diff --git a/packages/sql/src/modules/sql-generator.module.test.ts b/packages/sql/src/modules/sql-generator.module.test.ts index c087afed..d3be2dc0 100644 --- a/packages/sql/src/modules/sql-generator.module.test.ts +++ b/packages/sql/src/modules/sql-generator.module.test.ts @@ -1109,3 +1109,50 @@ describe('SqlGeneratorModule foreign key hardening', () => { ); }); }); + +describe('CREATE INDEX identifier', () => { + it('uses the index its own name, not compare\'s uppercased match key', () => { + // CLAUDE.md: "The compare key is not an identifier." The generator spread + // the source IndexInfo then overwrote `name` with the key, so a revert + // emitted IDX_CUSTOMERS_EMAIL for idx_customers_email — a different index + // on any dialect that does not fold case, and a phantom rename next compare. + const gen = new SqlGeneratorModule(); + const steps = gen.generateMigrationPlan( + [ + { + tableName: 'CUSTOMERS', + objectType: 'TABLE', + status: 'MODIFIED', + columnDiffs: [], + foreignKeyDiffs: [], + indexDiffs: [ + { + name: 'IDX_CUSTOMERS_EMAIL', + status: 'ADDED', + source: { name: 'idx_customers_email', columns: ['email'], unique: false }, + }, + ], + sourceTable: { + name: 'customers', + objectType: 'TABLE', + columns: [], + indices: [], + foreignKeys: [], + }, + targetTable: { + name: 'customers', + objectType: 'TABLE', + columns: [], + indices: [], + foreignKeys: [], + }, + }, + ], + 'postgres' + ); + const sql = steps.flatMap((s) => s.statements).join('\n'); + expect(sql).toMatch(/idx_customers_email/); + expect(sql).not.toMatch(/IDX_CUSTOMERS_EMAIL/); + }); +}); + diff --git a/packages/sql/src/modules/sql-generator.module.ts b/packages/sql/src/modules/sql-generator.module.ts index 3cb2c9a2..8c28779b 100644 --- a/packages/sql/src/modules/sql-generator.module.ts +++ b/packages/sql/src/modules/sql-generator.module.ts @@ -679,7 +679,15 @@ export class SqlGeneratorModule { if (!srcIdx) continue; // Bare index name (its schema follows the qualified table) — a qualified // index name is a syntax error in Postgres/MySQL/SQL Server. - statements.push(this.createIndexSql({ ...srcIdx, name: this.bareName(idx.name) }, tableName, dialect)); + // + // `idx.name` is compare's uppercased match key, not an identifier, so + // it would create IDX_CUSTOMERS_EMAIL where the source has + // idx_customers_email — a different index on any dialect that does not + // fold case, and a phantom rename on the next compare. The source's own + // name is the identifier; the key is only the fallback for a source + // that somehow carries none. + const indexName = this.bareName(srcIdx.name || idx.name); + statements.push(this.createIndexSql({ ...srcIdx, name: indexName }, tableName, dialect)); } // Add new / recreate modified FK constraints (after all column changes are done). diff --git a/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.test.ts b/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.test.ts new file mode 100644 index 00000000..4f75138e --- /dev/null +++ b/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.test.ts @@ -0,0 +1,41 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + */ +import { describe, expect, it } from 'vitest'; +import { sqliteSqlDialect } from './sqlite.sql-dialect.js'; + +describe('sqlite CREATE INDEX', () => { + it('puts the schema on the index, never on the table', () => { + // `CREATE INDEX idx ON main.customers(email)` is `near ".": syntax error` + // in SQLite — the qualifier belongs on the index name. Found by driving a + // revert end to end; the plan looked right and the driver rejected it. + const sql = sqliteSqlDialect.createIndexStatement!( + { name: 'idx_customers_email', columns: ['email'], unique: false }, + 'main.customers' + ); + expect(sql).toBe( + 'CREATE INDEX IF NOT EXISTS main.idx_customers_email ON customers (email);' + ); + expect(sql).not.toMatch(/ON\s+main\./); + }); + + it('handles an unqualified table and a unique index', () => { + expect( + sqliteSqlDialect.createIndexStatement!( + { name: 'idx_u', columns: ['a', 'b'], unique: true }, + 'customers' + ) + ).toBe('CREATE UNIQUE INDEX IF NOT EXISTS idx_u ON customers (a, b);'); + }); + + it('carries a partial-index filter through', () => { + expect( + sqliteSqlDialect.createIndexStatement!( + { name: 'idx_active', columns: ['id'], unique: false, filter: 'active = 1' }, + 'main.t' + ) + ).toBe('CREATE INDEX IF NOT EXISTS main.idx_active ON t (id) WHERE active = 1;'); + }); +}); diff --git a/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.ts b/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.ts index 487fb2be..061f38a2 100644 --- a/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.ts +++ b/packages/sql/src/providers/sqlLite/sqlite.sql-dialect.ts @@ -1,4 +1,5 @@ import type { SqlDialect, ColumnSpec } from '../../modules/sql-dialect.interface.js'; +import type { IndexInfo } from '../../interfaces/schema.interface.js'; import { makeDialectTypeFns, plain, sized, decimalAs } from '../../modules/type-mapping.js'; const types = makeDialectTypeFns({ @@ -97,6 +98,23 @@ export const sqliteSqlDialect: SqlDialect = { return `DROP INDEX IF EXISTS ${indexName};`; }, + /** + * SQLite takes the schema on the *index* name, never on the table: + * `CREATE INDEX main.idx ON customers(email)` is valid, while + * `CREATE INDEX idx ON main.customers(email)` is `near ".": syntax error`. + * Every other dialect qualifies the table, so the shared generator does too — + * this hook moves the qualifier across. + */ + createIndexStatement(index: IndexInfo, qualifiedTable: string): string { + const dot = qualifiedTable.lastIndexOf('.'); + const schema = dot < 0 ? '' : qualifiedTable.slice(0, dot); + const table = dot < 0 ? qualifiedTable : qualifiedTable.slice(dot + 1); + const name = schema ? `${schema}.${index.name}` : index.name; + const unique = index.unique ? ' UNIQUE' : ''; + const where = index.filter?.trim() ? ` WHERE ${index.filter.trim()}` : ''; + return `CREATE${unique} INDEX IF NOT EXISTS ${name} ON ${table} (${index.columns.join(', ')})${where};`; + }, + dropTriggerStatement(triggerName: string, _qualifiedTable: string): string { return `DROP TRIGGER IF EXISTS ${triggerName};`; },