diff --git a/.cursor/rules/sql-editor-agent-memory.mdc b/.cursor/rules/sql-editor-agent-memory.mdc index af61e6208..882ed30f5 100644 --- a/.cursor/rules/sql-editor-agent-memory.mdc +++ b/.cursor/rules/sql-editor-agent-memory.mdc @@ -52,7 +52,7 @@ FoxSchema = schema diff/migration + SQL Editor + workspaces. Destinations checkl - **Monarch flag**: module-local ready flag only — never write onto Monaco ESM exports. ## Notebook cells + Data peek + Node `sql` bridge -- One Monaco buffer; cell banding + statement strip `In [n]:` / per-cell ▶; results `Out [n]:`. +- One Monaco buffer; cell banding + the run rail left of the editor (`StatementRunGutter`: `In [n]` number, per-cell ▶, include checkbox); results `Out [n]:`. - Data peek: `DataPeekPanel` from SqlEditorView; FK rust `#9a3412`; `tablePreview.ts` + store APIs. - Node bridge: worker has **no DB handle**; `sql\`...\`` → `cell-query` / `cell-query-result`; bind params only; `sql.raw` sole unescaped path; no cross-call transactions. - SQLite: read-write open, `.run()` for non-SELECT; `resolveFkReferencedColumns` needs real identifier strings. diff --git a/apps/e2e/scripts/run-all.mjs b/apps/e2e/scripts/run-all.mjs index f641d541a..71cfbd652 100644 --- a/apps/e2e/scripts/run-all.mjs +++ b/apps/e2e/scripts/run-all.mjs @@ -123,6 +123,7 @@ const ALWAYS = [ 'src/tests/sql-editor-code-cell-faker.test.ts', 'src/tests/lazy-loading.test.ts', 'src/tests/sql-editor-result-edit.test.ts', + 'src/tests/sql-editor-run-rail.test.ts', 'src/tests/sql-editor-peek-row-form.test.ts', 'src/tests/sql-editor-peek-stacking.test.ts', 'src/tests/sql-editor-compare-multi-target.test.ts', diff --git a/apps/e2e/src/generated/test-ids.ts b/apps/e2e/src/generated/test-ids.ts index de1712bc6..72075c972 100644 --- a/apps/e2e/src/generated/test-ids.ts +++ b/apps/e2e/src/generated/test-ids.ts @@ -1748,6 +1748,12 @@ export const TestIds = { }, }, shared: { + DriverInstallHints: { + /** p */ + driverInstallError: 'driver-install-error', + /** p */ + driverInstallNeedsAdmin: 'driver-install-needs-admin', + }, ErrorBoundary: { /** button · Dismiss */ appErrorBoundaryDismiss: 'app-error-boundary-dismiss', @@ -2185,6 +2191,10 @@ export const TestIds = { sqlInstallDriver: (connectionId: string | number) => `sql-install-driver-${connectionId}`, /** div */ sqlInstallDriverBanner: (connectionId: string | number) => `sql-install-driver-${connectionId}-banner`, + /** button · Install the driver package on the server */ + sqlPaneInstallDriver: (connectionId: string | number) => `sql-pane-install-driver-${connectionId}`, + /** div */ + sqlPaneInstallDriverBanner: (connectionId: string | number) => `sql-pane-install-driver-${connectionId}-banner`, /** div · Drag to resize this table */ sqlPaneResize: 'sql-pane-resize', /** button · Retry this server */ @@ -2529,32 +2539,16 @@ export const TestIds = { sqlVariables: 'sql-variables', }, StatementRunGutter: { + /** button */ + sqlStatementCopy: (i: string | number) => `sql-statement-copy-${i}`, /** div · Statement run checks */ sqlStatementGutter: 'sql-statement-gutter', /** input */ sqlStatementInclude: (i: string | number) => `sql-statement-include-${i}`, /** button */ - sqlStatementRun: (i: string | number) => `sql-statement-run-${i}`, - }, - StatementStrip: { - /** div */ - sqlStatementCell: (i: string | number) => `sql-statement-cell-${i}`, - /** button */ - sqlStatementCopy: 'sql-statement-copy', - /** button */ - sqlStatementCopyRow: (i: string | number) => `sql-statement-copy-row-${i}`, - /** input */ - sqlStatementInclude: (i: string | number) => `sql-statement-include-${i}`, - /** button · In [ ]: */ - sqlStatementReveal: (i: string | number) => `sql-statement-reveal-${i}`, + sqlStatementLabel: (i: string | number) => `sql-statement-label-${i}`, /** button */ sqlStatementRun: (i: string | number) => `sql-statement-run-${i}`, - /** div */ - sqlStatementStrip: 'sql-statement-strip', - /** div · Resize statement list */ - sqlStatementStripResize: 'sql-statement-strip-resize', - /** div */ - sqlStatementValuesPopover: 'sql-statement-values-popover', }, TableBlueprintModal: { /** button · Add column */ @@ -4199,6 +4193,8 @@ export type TestId = | `diff-tree-toggle-selection-${string}` | 'diff-unsupported' | 'dismiss-warnings-btn' + | 'driver-install-error' + | 'driver-install-needs-admin' | 'error-banner' | 'execute-btn' | 'feedback-cancel' @@ -4761,6 +4757,8 @@ export type TestId = | 'sql-open-object-source-btn' | 'sql-page-next' | 'sql-page-prev' + | `sql-pane-install-driver-${string}` + | `sql-pane-install-driver-${string}-banner` | 'sql-pane-refresh' | 'sql-pane-resize' | `sql-pane-retry-${string}` @@ -4891,16 +4889,11 @@ export type TestId = | `sql-sidebar-resize-${string}` | `sql-sidebar-toggle-${string}` | 'sql-sidebar-utilities' - | `sql-statement-cell-${string}` - | 'sql-statement-copy' - | `sql-statement-copy-row-${string}` + | `sql-statement-copy-${string}` | 'sql-statement-gutter' | `sql-statement-include-${string}` - | `sql-statement-reveal-${string}` + | `sql-statement-label-${string}` | `sql-statement-run-${string}` - | 'sql-statement-strip' - | 'sql-statement-strip-resize' - | 'sql-statement-values-popover' | 'sql-tab-add' | `sql-tab-close-${string}` | 'sql-tab-rename' diff --git a/apps/e2e/src/tests/sql-editor-run-rail.test.ts b/apps/e2e/src/tests/sql-editor-run-rail.test.ts new file mode 100644 index 000000000..286ae5034 --- /dev/null +++ b/apps/e2e/src/tests/sql-editor-run-rail.test.ts @@ -0,0 +1,80 @@ +/** + * The run rail left of the SQL editor, in a real browser. + * + * #542 moved each statement's ▶ and run checkbox from the strip under the + * editor into this rail and dropped what the strip said about each statement: + * its In [n] number, Safe mode's warning on a write with no WHERE, selecting a + * statement, and copying it with its values. Two statements on one line also + * drew their controls on top of each other. + */ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import type { Page } from 'playwright'; +import { buildDriver, quitDriver } from '../helpers/driver.js'; +import { saveScreenshot } from '../helpers/screenshot.js'; +import { byTestId } from '../helpers/test-ids.js'; +import { AppPage } from '../pages/AppPage.js'; +import { SqlEditorPage } from '../pages/SqlEditorPage.js'; + +const SQL = 'UPDATE t SET a = 1 WHERE id = 1;\nDELETE FROM t;\nSELECT 1; SELECT 2;\n'; +const mod = process.platform === 'darwin' ? 'Meta' : 'Control'; + +describe('SQL Editor · run rail', () => { + let driver: Page; + let sql: SqlEditorPage; + + beforeAll(async () => { + driver = await buildDriver(); + sql = new SqlEditorPage(driver); + await new AppPage(driver).open(); + await sql.resetPersistedEditorState(); + await driver.reload(); + await driver.waitForSelector(byTestId('toolbar')); + await sql.openView(); + await sql.waitForMonaco(); + await driver.locator('.monaco-editor').first().click(); + await driver.keyboard.press(`${mod}+KeyA`); + await driver.keyboard.insertText(SQL); + await driver.waitForSelector(byTestId('sql-statement-label-3'), { timeout: 15_000 }); + }, 180_000); + + afterAll(async () => { + if (driver) await quitDriver(driver); + }); + + it('numbers each statement and marks the writes Safe mode will confirm', async () => { + const label = (i: number) => driver.locator(byTestId(`sql-statement-label-${i}`)); + expect(await label(0).innerText()).toBe('1'); + expect(await label(0).getAttribute('data-dml')).toBe('UPD'); + expect(await label(1).innerText()).toBe('2!'); + expect(await label(1).getAttribute('data-dml')).toBe('DEL!'); + expect(await label(1).getAttribute('title')).toMatch(/no WHERE — may affect all rows/); + expect(await label(1).evaluate((el) => getComputedStyle(el).color)).not.toBe( + await label(2).evaluate((el) => getComputedStyle(el).color) + ); + expect(await label(2).getAttribute('data-dml')).toBeNull(); + await saveScreenshot(driver, 'run-rail-labels'); + }); + + it('gives two statements on one line a row each, both clickable', async () => { + const box = async (i: number) => (await driver.locator(byTestId(`sql-statement-include-${i}`)).boundingBox())!; + const [a, b] = [await box(2), await box(3)]; + expect(b.y - a.y).toBeGreaterThanOrEqual(14); + await driver.locator(byTestId('sql-statement-include-2')).click(); + expect(await driver.locator(byTestId('sql-statement-include-2')).isChecked()).toBe(true); + expect(await driver.locator(byTestId('sql-statement-include-3')).isChecked()).toBe(false); + await driver.locator(byTestId('sql-statement-include-2')).click(); + }); + + it('selects a statement when its number is clicked, so Run runs that one', async () => { + await driver.locator(byTestId('sql-statement-label-1')).click(); + await expect.poll(() => driver.locator(byTestId('sql-run-btn')).innerText(), { timeout: 5_000 }).toMatch(/Run selection/); + }); + + it('copies a statement from its row', async () => { + await driver.context().grantPermissions(['clipboard-read', 'clipboard-write']); + await driver.locator(byTestId('sql-statement-label-1')).hover(); + await driver.locator(byTestId('sql-statement-copy-1')).click(); + await expect.poll(() => driver.evaluate(() => navigator.clipboard.readText()), { timeout: 5_000 }).toBe('DELETE FROM t;'); + await saveScreenshot(driver, 'run-rail-copied'); + }); +}); diff --git a/apps/web/src/frontend/features/connections/components/ConnectionModal.test.tsx b/apps/web/src/frontend/features/connections/components/ConnectionModal.test.tsx index 0426b3583..7e648ef04 100644 --- a/apps/web/src/frontend/features/connections/components/ConnectionModal.test.tsx +++ b/apps/web/src/frontend/features/connections/components/ConnectionModal.test.tsx @@ -1,15 +1,30 @@ import React from 'react'; -import { fireEvent, render, screen } from '@testing-library/react'; -import { describe, expect, it, vi } from 'vitest'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; vi.mock('@/shared/api/schemaApi', () => ({ checkDriver: vi.fn().mockResolvedValue({ installed: true }), fetchSchemaList: vi.fn().mockResolvedValue([]), installDriver: vi.fn().mockResolvedValue(undefined), + invalidateCache: vi.fn(), })); +import { checkDriver, installDriver } from '@/shared/api/schemaApi'; +import { useAuthStore } from '@/platform/authStore'; +import { clearDriverStatus } from '@/shared/lib/driverStatus'; import { ConnectionModal } from './ConnectionModal'; +const signIn = (role: 'admin' | 'editor') => + useAuthStore.setState({ + user: { id: 'u1', email: 'u1@example.test', onboardingCompleted: true, role, permissions: [] }, + status: 'ready', + }); + +beforeEach(() => { + clearDriverStatus(); + signIn('admin'); +}); + describe('ConnectionModal', () => { it('verifies Azure SQL server certificates by default', () => { const onSave = vi.fn(); @@ -37,4 +52,24 @@ describe('ConnectionModal', () => { }); expect(options.connectionString).not.toContain('TrustServerCertificate=True'); }); + + describe('a missing driver', () => { + const missing = { provider: 'redis', packageName: 'redis', installed: false, installCommand: 'npm install redis' }; + const modal = () => undefined} onSave={() => undefined} />; + + it('offers Install to an admin', async () => { + vi.mocked(checkDriver).mockResolvedValue(missing); + render(modal()); + fireEvent.click(await screen.findByTestId('conn-install-driver-package')); + await waitFor(() => expect(installDriver).toHaveBeenCalledWith('redis')); + }); + + it('offers no Install without the Install drivers permission, and gives the command instead', async () => { + signIn('editor'); + vi.mocked(checkDriver).mockResolvedValue(missing); + render(modal()); + expect((await screen.findByTestId('driver-install-needs-admin')).textContent).toMatch(/npm install redis/); + expect(screen.queryByTestId('conn-install-driver-package')).toBeNull(); + }); + }); }); diff --git a/apps/web/src/frontend/features/connections/components/ConnectionModal.tsx b/apps/web/src/frontend/features/connections/components/ConnectionModal.tsx index 221529e63..ecab29dce 100644 --- a/apps/web/src/frontend/features/connections/components/ConnectionModal.tsx +++ b/apps/web/src/frontend/features/connections/components/ConnectionModal.tsx @@ -15,9 +15,10 @@ import { passwordFieldLabel, type ConnectionAuthMethod, } from '@/shared/lib/provider-settings'; -import type { DriverInfo } from '@/shared/lib/types'; -import { IBM_DB_VERSION } from '@foxschema/ui-shared'; -import { fetchSchemaList, checkDriver as apiCheckDriver, installDriver as apiInstallDriver } from "@/shared/api/schemaApi"; +import { fetchSchemaList } from "@/shared/api/schemaApi"; +import { driverStatusOf, installDriverFor, useDriverStatus } from '@/shared/lib/driverStatus'; +import { DriverInstallHints } from '@/shared/components/DriverInstallHints'; +import { useAuthStore } from '@/platform/authStore'; import { PasswordInput } from '@/shared/components/PasswordInput'; import { Autocomplete } from '@/shared/components/Autocomplete'; import { DatabaseFilePicker } from './DatabaseFilePicker'; @@ -92,8 +93,12 @@ export const ConnectionModal: React.FC = ({ const isFileDialect = isFileDialectOf(selDialect); const [browsing, setBrowsing] = useState(false); - const [driverInfo, setDriverInfo] = useState(null); - const [installing, setInstalling] = useState(false); + // Shared with the SQL editor's banners: an install from either shows on both. + // Checked only while the form is open, as before. + const driver = useDriverStatus(open ? selDialect : ''); + const driverInfo = driver.info; + const installing = driver.installing; + const canInstallDriver = useAuthStore((s) => s.can('admin.drivers')); const [testingState, setTestingState] = useState<{ status: 'idle' | 'testing' | 'success' | 'failed'; @@ -144,18 +149,6 @@ export const ConnectionModal: React.FC = ({ // eslint-disable-next-line react-hooks/exhaustive-deps -- initialOptionsKey is the stable snapshot }, [open, dialect, initialName, initialHasPassword, initialOptionsKey]); - // Check whether the selected provider's driver is installed, whenever the - // modal opens or the provider changes. (Must stay above the early return — - // hooks run unconditionally every render.) - useEffect(() => { - if (!open) return; - let cancelled = false; - setDriverInfo(null); - apiCheckDriver(selDialect) - .then((info) => { if (!cancelled) setDriverInfo(info); }) - .catch(() => { if (!cancelled) setDriverInfo(null); }); - return () => { cancelled = true; }; - }, [open, selDialect]); if (!open) return null; @@ -187,25 +180,9 @@ export const ConnectionModal: React.FC = ({ }; const handleInstall = async () => { - setInstalling(true); setTestingState({ status: 'idle' }); - try { - await apiInstallDriver(selDialect); - const info = await apiCheckDriver(selDialect); - setDriverInfo(info); - if (!info.installed) { - setTestingState({ - status: 'failed', - error: info.error - ? `Driver still not loading: ${info.error}` - : `Install finished but "${info.packageName}" still failed to load. Try: ${info.installCommand ?? `npm install ${info.packageName}`}`, - }); - } - } catch (error: unknown) { - const message = error instanceof Error ? error.message : 'Driver install failed'; - setTestingState({ status: 'failed', error: message }); - } finally { - setInstalling(false); + if (!(await installDriverFor(selDialect))) { + setTestingState({ status: 'failed', error: driverStatusOf(selDialect).installError }); } }; @@ -387,7 +364,7 @@ export const ConnectionModal: React.FC = ({ Driver "{driverInfo.packageName}" is not installed - + } {installing && (
)} - {!installing && driverInfo.error && ( -

- {driverInfo.error.slice(0, 280)} - {driverInfo.installCommand ? ( - <> - {' '} - — run {driverInfo.installCommand} - - ) : null} -

- )} - {!installing && !driverInfo.installed && driverInfo.packageName === 'ibm_db' && ( -

- Db2 needs a native build + IBM clidriver download. Prefer{' '} - foxschema drivers install db2 - , then foxschema stop && foxschema. - In a monorepo:{' '} - - npm install ibm_db@{IBM_DB_VERSION} --foreground-scripts -w @foxschema/db - - . On linux/arm64 use Docker ( - 5nickels/foxschema:latest). -

- )} +
) )} diff --git a/apps/web/src/frontend/features/sql-editor/components/MissingDriverBanner.test.tsx b/apps/web/src/frontend/features/sql-editor/components/MissingDriverBanner.test.tsx index 63df2a561..1016caeb2 100644 --- a/apps/web/src/frontend/features/sql-editor/components/MissingDriverBanner.test.tsx +++ b/apps/web/src/frontend/features/sql-editor/components/MissingDriverBanner.test.tsx @@ -5,6 +5,8 @@ */ import { describe, expect, it, vi, beforeEach } from 'vitest'; import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import type { DriverInfo } from '@/shared/lib/types'; +import type { Permission } from '@foxschema/shared'; const checkDriver = vi.fn(); const installDriver = vi.fn(); @@ -12,38 +14,56 @@ const installDriver = vi.fn(); vi.mock('@/shared/api/schemaApi', () => ({ checkDriver: (...args: unknown[]) => checkDriver(...args), installDriver: (...args: unknown[]) => installDriver(...args), + invalidateCache: () => {}, })); +import { useAuthStore } from '@/platform/authStore'; +import { clearDriverStatus } from '@/shared/lib/driverStatus'; import { MissingDriverBanner } from './MissingDriverBanner'; +const REDIS_MISSING = 'Database driver "redis" is not installed for Redis. Run: npm install redis'; + +function signIn(role: 'admin' | 'editor' | 'viewer', permissions: Permission[] = []) { + useAuthStore.setState({ + user: { id: 'u1', email: 'u1@example.test', onboardingCompleted: true, role, permissions }, + status: 'ready', + }); +} + +/** A server where the redis driver loads once `installed.redis` is true. */ +function redisServer(installed: { redis: boolean }) { + checkDriver.mockImplementation( + async (dialect: string): Promise => ({ + provider: dialect, + packageName: 'redis', + installed: installed.redis, + installCommand: 'npm install redis', + }) + ); +} + beforeEach(() => { + clearDriverStatus(); checkDriver.mockReset(); installDriver.mockReset(); + signIn('admin'); }); describe('MissingDriverBanner', () => { it('offers Install when the selected dialect has no driver, then reloads after it loads', async () => { - checkDriver - .mockResolvedValueOnce({ - provider: 'redis', - packageName: 'redis', - installed: false, - installCommand: 'npm install redis', - }) - .mockResolvedValueOnce({ - provider: 'redis', - packageName: 'redis', - installed: true, - version: '6.3.0', - }); - installDriver.mockResolvedValue({ success: true }); + const installed = { redis: false }; + redisServer(installed); + installDriver.mockImplementation(async () => { + installed.redis = true; + return { success: true }; + }); const onInstalled = vi.fn(); render(); const button = await screen.findByTestId('sql-install-driver'); expect(button.textContent).toMatch(/Install driver/); - expect(screen.getByText(/redis/)).toBeTruthy(); + expect(screen.getByText(/Driver “redis” is not installed/)).toBeTruthy(); fireEvent.click(button); await waitFor(() => expect(installDriver).toHaveBeenCalledWith('redis')); @@ -52,15 +72,73 @@ describe('MissingDriverBanner', () => { }); it('stays hidden when the driver is already installed', async () => { - checkDriver.mockResolvedValue({ - provider: 'postgres', - packageName: 'pg', - installed: true, - }); + checkDriver.mockResolvedValue({ provider: 'postgres', packageName: 'pg', installed: true }); + render(); + await waitFor(() => expect(checkDriver).toHaveBeenCalled()); + expect(screen.queryByTestId('sql-install-driver')).toBeNull(); + }); + + it('shows an install from one banner on every other, and starts it once', async () => { + const installed = { redis: false }; + redisServer(installed); + let finish!: () => void; + installDriver.mockImplementation( + () => + new Promise((resolve) => { + finish = () => { + installed.redis = true; + resolve({ success: true }); + }; + }) + ); render( - + <> + + + ); - await waitFor(() => expect(checkDriver).toHaveBeenCalled()); + fireEvent.click(await screen.findByTestId('sql-install-driver')); + // The other banner shows the install running, and its click joins it. + await waitFor(() => expect((screen.getByTestId('sql-install-driver-conn-2') as HTMLButtonElement).disabled).toBe(true)); + fireEvent.click(screen.getByTestId('sql-install-driver-conn-2')); + finish(); + await waitFor(() => expect(screen.queryByTestId('sql-install-driver-conn-2-banner')).toBeNull()); + expect(screen.queryByTestId('sql-install-driver-banner')).toBeNull(); + expect(installDriver).toHaveBeenCalledTimes(1); + }); + + it('offers no Install without the Install drivers permission, and says who can', async () => { + signIn('editor', ['editor.run', 'editor.dml']); + redisServer({ redis: false }); + render(); + expect(await screen.findByTestId('driver-install-needs-admin')).toBeTruthy(); + expect(screen.getByTestId('driver-install-needs-admin').textContent).toMatch(/npm install redis/); expect(screen.queryByTestId('sql-install-driver')).toBeNull(); }); + + it('names the package from the server’s error before the check answers', () => { + checkDriver.mockReturnValue(new Promise(() => {})); + render(); + expect(screen.getByText(/Driver “redis” is not installed/)).toBeTruthy(); + }); + + it('stays out of a database error that only says "is not installed"', async () => { + checkDriver.mockRejectedValue(new Error('network down')); + render( + + ); + await waitFor(() => expect(checkDriver).toHaveBeenCalled()); + expect(screen.queryByTestId('sql-install-driver-banner')).toBeNull(); + }); + + it('says why an install failed', async () => { + redisServer({ redis: false }); + installDriver.mockRejectedValue(new Error('npm install redis failed (exit 1): EACCES')); + render(); + fireEvent.click(await screen.findByTestId('sql-install-driver')); + expect((await screen.findByTestId('driver-install-error')).textContent).toMatch(/EACCES/); + }); }); diff --git a/apps/web/src/frontend/features/sql-editor/components/MissingDriverBanner.tsx b/apps/web/src/frontend/features/sql-editor/components/MissingDriverBanner.tsx index d5d40dbf3..10a5244f8 100644 --- a/apps/web/src/frontend/features/sql-editor/components/MissingDriverBanner.tsx +++ b/apps/web/src/frontend/features/sql-editor/components/MissingDriverBanner.tsx @@ -2,69 +2,37 @@ * Fox Schema (foxschema) * Copyright 2024-2026 Huy Phan * SPDX-License-Identifier: Apache-2.0 + * + * Install driver, offered where a missing driver stops the SQL editor: the + * schema browser and a failed run. Every banner reads one shared status + * (`shared/lib/driverStatus`), so an install from any of them shows on all, + * and a second click joins the running install instead of starting another. */ -import React, { useEffect, useState } from 'react'; +import React from 'react'; import { AlertTriangle, Download, Loader2 } from 'lucide-react'; -import { checkDriver, installDriver } from '@/shared/api/schemaApi'; -import type { DriverInfo } from '@/shared/lib/types'; +import { useAuthStore } from '@/platform/authStore'; +import { installDriverFor, missingDriverPackage, useDriverStatus } from '@/shared/lib/driverStatus'; +import { DriverInstallHints } from '@/shared/components/DriverInstallHints'; import { SQL_ICON_STROKE } from '@/shared/lib/iconStyle'; -function isMissingDriverError(message: string | undefined): boolean { - return Boolean(message && message.toLowerCase().includes('is not installed')); -} - export const MissingDriverBanner: React.FC<{ dialect: string; - /** Lets the button show before the check returns. */ + /** The error the screen shows. Lets the banner appear before the check answers. */ error?: string; onInstalled?: () => void; testId?: string; }> = ({ dialect, error, onInstalled, testId = 'sql-install-driver' }) => { - const [info, setInfo] = useState(null); - const [installing, setInstalling] = useState(false); - const [fail, setFail] = useState(''); - - useEffect(() => { - let cancelled = false; - setInfo(null); - setFail(''); - checkDriver(dialect) - .then((next) => { - if (!cancelled) setInfo(next); - }) - .catch(() => { - // Unknown dialect or a failed check. The error text is the fallback. - }); - return () => { - cancelled = true; - }; - }, [dialect]); - - const missing = info ? !info.installed : isMissingDriverError(error); - const packageName = info?.packageName ?? 'driver'; + const status = useDriverStatus(dialect); + const canInstall = useAuthStore((s) => s.can('admin.drivers')); + const named = missingDriverPackage(error); + // The check is the answer once it arrives; until then (or if it failed), + // only the server's own missing-driver message counts. + const missing = status.info ? !status.info.installed : named !== null; if (!missing) return null; + const packageName = status.info?.packageName ?? named ?? ''; const install = async () => { - setInstalling(true); - setFail(''); - try { - await installDriver(dialect); - const next = await checkDriver(dialect); - setInfo(next); - if (next.installed) { - onInstalled?.(); - return; - } - setFail( - next.error - ? `Driver still not loading: ${next.error}` - : `Install finished but "${next.packageName}" still failed to load.` - ); - } catch (caught: unknown) { - setFail(caught instanceof Error ? caught.message : 'Driver install failed'); - } finally { - setInstalling(false); - } + if (await installDriverFor(dialect)) onInstalled?.(); }; return ( @@ -75,25 +43,35 @@ export const MissingDriverBanner: React.FC<{
- Driver “{packageName}” is not installed + + {packageName ? `Driver “${packageName}” is not installed` : 'The database driver is not installed'} + - + {canInstall ? ( + + ) : null}
- {fail ?

{fail}

: null} + ); }; diff --git a/apps/web/src/frontend/features/sql-editor/components/ResultsPanel.driver.test.tsx b/apps/web/src/frontend/features/sql-editor/components/ResultsPanel.driver.test.tsx new file mode 100644 index 000000000..05c9c9b08 --- /dev/null +++ b/apps/web/src/frontend/features/sql-editor/components/ResultsPanel.driver.test.tsx @@ -0,0 +1,51 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * A run that failed for a missing driver offers Install in both layouts. + * Side by side is the one Compare data needs, and it had none. + */ +import { render, screen } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('@/shared/api/schemaApi', () => ({ + // Never answers: the banner has only the run's error to go on. + checkDriver: vi.fn(() => new Promise(() => {})), + installDriver: vi.fn(), + invalidateCache: vi.fn(), +})); + +import { useAuthStore } from '@/platform/authStore'; +import { clearDriverStatus } from '@/shared/lib/driverStatus'; +import type { CredentialRun } from '@/features/sql-editor/state/useSqlEditorStore'; +import { ResultsPanel } from './ResultsPanel'; + +const failed: CredentialRun = { + connectionId: 'c1', + name: 'Cache', + dialect: 'redis', + status: 'error', + error: 'Database driver "redis" is not installed for Redis. Run: npm install redis', +}; + +beforeEach(() => { + clearDriverStatus(); + useAuthStore.setState({ + user: { id: 'u1', email: 'u1@example.test', onboardingCompleted: true, role: 'admin', permissions: [] }, + status: 'ready', + }); +}); + +describe('ResultsPanel — a missing driver', () => { + it('offers Install by credential', () => { + render(); + expect(screen.getByTestId('sql-install-driver-c1')).toBeTruthy(); + }); + + it('offers Install side by side', () => { + render(); + expect(screen.getByTestId('sql-pane-install-driver-c1')).toBeTruthy(); + expect(screen.getByTestId('sql-pane-install-driver-c1-banner').textContent).toMatch(/Driver “redis” is not installed/); + }); +}); diff --git a/apps/web/src/frontend/features/sql-editor/components/ResultsPanel.tsx b/apps/web/src/frontend/features/sql-editor/components/ResultsPanel.tsx index 182fb44fd..aceae5fb9 100644 --- a/apps/web/src/frontend/features/sql-editor/components/ResultsPanel.tsx +++ b/apps/web/src/frontend/features/sql-editor/components/ResultsPanel.tsx @@ -144,7 +144,7 @@ type PaneItem = statementSql?: string; } | { key: string; kind: 'running'; label: string; connectionId: string } - | { key: string; kind: 'error'; label: string; error: string; connectionId: string }; + | { key: string; kind: 'error'; label: string; error: string; connectionId: string; dialect: string }; /** Pane role when Compare is on — Source left, Target A/B… on the right. */ function comparePaneRole( @@ -625,6 +625,16 @@ const PaneBody: React.FC<{ {item.error} + {/* Side by side is the layout Compare data needs, so a missing driver + has to be installable here too, not only in By credential. */} +
+ onRefresh?.(item.connectionId)} + /> +
); }; @@ -2012,6 +2022,7 @@ const ResultsPanelView: React.FC = ({ label: credentialLabel(run), error: run.error ?? 'Error', connectionId: run.connectionId, + dialect: run.dialect, }); continue; } diff --git a/apps/web/src/frontend/features/sql-editor/components/SqlEditorPane.tsx b/apps/web/src/frontend/features/sql-editor/components/SqlEditorPane.tsx index 786be2d5a..a9e85b3f1 100644 --- a/apps/web/src/frontend/features/sql-editor/components/SqlEditorPane.tsx +++ b/apps/web/src/frontend/features/sql-editor/components/SqlEditorPane.tsx @@ -38,13 +38,7 @@ import { type SelectColumnPickerAnchor, } from './SelectColumnPicker'; import { StatementRunGutter } from './StatementRunGutter'; - -export interface RevealRequest { - startLine: number; - endLine: number; - /** Bump to re-trigger reveal for the same range. */ - nonce: number; -} +import { statementCopyText } from '../lib/statementBadges'; interface Props { value: string; @@ -64,14 +58,14 @@ interface Props { onSelectionChange?: (hasSelection: boolean) => void; /** Caret moved — offset in the model, so Run can label the right statement. */ onCaretChange?: (offset: number | null) => void; - /** Scroll and select a statement range. */ - reveal?: RevealRequest | null; } /** * Editable Monaco pane for the SQL Editor. Notebook-style cell banding spans * each statement's line range; the glyph margin shows ✓ / ⚠ on the first line. - * Run checkboxes sit in a rail on the left of that line. + * The run rail on the left numbers each statement In [n], with its play button, + * include checkbox and a copy-with-values button; clicking the number selects + * the statement. * Model language is FoxScript (`foxscript`) once Monarch + packs load. */ export const SqlEditorPane: React.FC = ({ @@ -87,7 +81,6 @@ export const SqlEditorPane: React.FC = ({ onRun, onSelectionChange, onCaretChange, - reveal, }) => { const editorRef = useRef(null); const monacoRef = useRef(null); @@ -104,6 +97,7 @@ export const SqlEditorPane: React.FC = ({ const fontSizePref = useUiStore((s) => s.fontSize); const monacoFontSize = MONACO_FONT_PX[fontSizePref] ?? MONACO_FONT_PX.md; const variables = useSqlEditorStore((s) => s.variables); + const safeMode = useSqlEditorStore((s) => s.safeMode); const schemaCache = useSqlEditorStore((s) => s.schemaCache); const [editorLanguage, setEditorLanguage] = useState(() => monacoLanguage(dialect)); const [columnPickerOpen, setColumnPickerOpen] = useState(false); @@ -213,22 +207,22 @@ export const SqlEditorPane: React.FC = ({ refreshFoxscriptSemanticTokens(); }, [schemaCache]); - useEffect(() => { + /** Select a statement's lines (the rail's In [n] number was clicked). */ + const revealStatement = (stmt: SplitStatement) => { const editor = editorRef.current; - if (!editor || !reveal) return; + if (!editor) return; const model = editor.getModel(); - const endColumn = model ? model.getLineMaxColumn(reveal.endLine) : 1; const range = { - startLineNumber: reveal.startLine, + startLineNumber: stmt.startLine, startColumn: 1, - endLineNumber: reveal.endLine, - endColumn, + endLineNumber: stmt.endLine, + endColumn: model ? model.getLineMaxColumn(stmt.endLine) : 1, }; - editor.revealRangeInCenter(range); + editor.revealRangeInCenterIfOutsideViewport(range); editor.setSelection(range); editor.focus(); onSelectionChangeRef.current?.(true); - }, [reveal]); + }; useEffect(() => { return () => { @@ -267,8 +261,11 @@ export const SqlEditorPane: React.FC = ({ editorEpoch={editorEpoch} running={cellRunning} sqlNeedsDestination={sqlNeedsDestination} + safeMode={safeMode} onToggle={(index) => onToggleStatement?.(index)} onRunCell={onRunCell} + onReveal={revealStatement} + copyTextFor={(stmt) => statementCopyText(stmt.text, variables)} />
{ const canEditorDestinations = useAuthStore((s) => s.can('editor.sidebar.destinations')); diff --git a/apps/web/src/frontend/features/sql-editor/components/StatementRunGutter.test.tsx b/apps/web/src/frontend/features/sql-editor/components/StatementRunGutter.test.tsx index ec67c4106..a8f768bf1 100644 --- a/apps/web/src/frontend/features/sql-editor/components/StatementRunGutter.test.tsx +++ b/apps/web/src/frontend/features/sql-editor/components/StatementRunGutter.test.tsx @@ -1,30 +1,56 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * The run rail. What it must still say about each statement now that the + * statement strip is gone — its number, and Safe mode's warning on a write + * with no WHERE — and that every row stays reachable: two statements on one + * line, and a long one whose first line has scrolled away. + */ import { describe, expect, it, vi } from 'vitest'; import React from 'react'; import { fireEvent, render, screen, waitFor } from '@testing-library/react'; -import { splitSqlStatements } from '@/shared/lib/sql-splitter'; -import { StatementRunGutter } from './StatementRunGutter'; +import { splitSqlStatements, statementVerb } from '@/shared/lib/sql-splitter'; +import { rowTops, StatementRunGutter, type GutterEditor } from './StatementRunGutter'; + +// Watched, not replaced: counts how many statements the rail reads. +vi.mock('@/shared/lib/sql-splitter', async (importOriginal) => { + const real = await importOriginal(); + return { ...real, statementVerb: vi.fn(real.statementVerb) }; +}); + +/** An editor whose line `n` starts at `(n - 1) * 20 + pad`, scrolled to `scroll`. */ +function editorDouble({ height = 400, scroll = 0, pad = 0 } = {}): GutterEditor { + return { + getTopForLineNumber: (line: number) => (line - 1) * 20 + pad, + getScrollTop: () => scroll, + getLayoutInfo: () => ({ height }), + onDidScrollChange: () => ({ dispose: () => {} }), + onDidContentSizeChange: () => ({ dispose: () => {} }), + onDidLayoutChange: () => ({ dispose: () => {} }), + }; +} + +function rail(sql: string, props: Partial> = {}) { + return ( + {}} + {...props} + /> + ); +} + +const topOf = (testId: string) => Number.parseFloat((screen.getByTestId(testId).parentElement as HTMLElement).style.top); describe('StatementRunGutter', () => { it('places a run checkbox on each statement and toggles the one that was clicked', () => { const onToggle = vi.fn(); - const statements = splitSqlStatements('SELECT 1;\nSELECT 2;'); - const editor = { - getTopForLineNumber: (line: number) => (line - 1) * 20 + 8, - getScrollTop: () => 0, - getLayoutInfo: () => ({ height: 200 }), - onDidScrollChange: () => ({ dispose: () => {} }), - onDidContentSizeChange: () => ({ dispose: () => {} }), - onDidLayoutChange: () => ({ dispose: () => {} }), - }; - render( - - ); + render(rail('SELECT 1;\nSELECT 2;', { checked: [1], onToggle })); const first = screen.getByTestId('sql-statement-include-0') as HTMLInputElement; const second = screen.getByTestId('sql-statement-include-1') as HTMLInputElement; expect(first.checked).toBe(false); @@ -34,27 +60,103 @@ describe('StatementRunGutter', () => { }); it('hides a statement that is scrolled out of the editor', async () => { - const statements = splitSqlStatements('SELECT 1;\nSELECT 2;'); - const editor = { - getTopForLineNumber: (line: number) => (line - 1) * 20, - getScrollTop: () => 0, - getLayoutInfo: () => ({ height: 10 }), - onDidScrollChange: () => ({ dispose: () => {} }), - onDidContentSizeChange: () => ({ dispose: () => {} }), - onDidLayoutChange: () => ({ dispose: () => {} }), - }; - render( - {}} - /> - ); + render(rail('SELECT 1;\nSELECT 2;', { editorRef: { current: editorDouble({ height: 10 }) } })); expect(screen.getByTestId('sql-statement-include-0')).toBeTruthy(); await waitFor(() => { expect(screen.queryByTestId('sql-statement-include-1')).toBeNull(); }); }); + + it('numbers each statement In [n], the number the Run tooltip and Out [n] refer to', () => { + render(rail('SELECT 1;\nSELECT 2;\nSELECT 3;')); + expect(screen.getByTestId('sql-statement-label-0').textContent).toBe('1'); + expect(screen.getByTestId('sql-statement-label-2').textContent).toBe('3'); + expect(screen.getByTestId('sql-statement-label-1').getAttribute('title')).toMatch(/^In \[2\] · SQL/); + }); + + it('marks, in Safe mode, a write Run will confirm — and a DELETE with no WHERE in red', () => { + render(rail('UPDATE t SET a = 1 WHERE id = 1;\nDELETE FROM t;\nSELECT 1;', { safeMode: true })); + const update = screen.getByTestId('sql-statement-label-0'); + const del = screen.getByTestId('sql-statement-label-1'); + expect(update.getAttribute('data-dml')).toBe('UPD'); + expect(update.getAttribute('title')).toMatch(/Safe mode: UPD asks for confirmation on Run/); + expect(del.getAttribute('data-dml')).toBe('DEL!'); + expect(del.textContent).toBe('2!'); + expect(del.getAttribute('title')).toMatch(/DEL: no WHERE — may affect all rows/); + expect(del.className).toMatch(/text-rose-300/); + expect(screen.getByTestId('sql-statement-label-2').getAttribute('data-dml')).toBeNull(); + }); + + it('marks nothing with Safe mode off, and names a code cell by its kind', () => { + render(rail('DELETE FROM t;\n-- @js\nreturn [];\n-- @end')); + expect(screen.getByTestId('sql-statement-label-0').getAttribute('data-dml')).toBeNull(); + expect(screen.getByTestId('sql-statement-label-1').getAttribute('title')).toMatch(/JavaScript code cell/); + }); + + it('selects the statement when its number is clicked', () => { + const onReveal = vi.fn(); + const statements = splitSqlStatements('SELECT 1;\nSELECT\n 2;'); + render(rail('', { statements, onReveal })); + fireEvent.click(screen.getByTestId('sql-statement-label-1')); + expect(onReveal).toHaveBeenCalledWith(statements[1]); + }); + + it('runs one cell, and holds play while running or when SQL has no Destination', () => { + const onRunCell = vi.fn(); + const sql = 'SELECT 1;\n-- @js\nreturn [];\n-- @end'; + const { rerender } = render(rail(sql, { onRunCell })); + fireEvent.click(screen.getByTestId('sql-statement-run-1')); + expect(onRunCell).toHaveBeenCalledWith(1); + + rerender(rail(sql, { onRunCell, running: true })); + expect((screen.getByTestId('sql-statement-run-0') as HTMLButtonElement).disabled).toBe(true); + + rerender(rail(sql, { onRunCell, sqlNeedsDestination: true })); + expect((screen.getByTestId('sql-statement-run-0') as HTMLButtonElement).disabled).toBe(true); + // A code cell runs without a Destination. + expect((screen.getByTestId('sql-statement-run-1') as HTMLButtonElement).disabled).toBe(false); + }); + + it('keeps two statements on one line apart, so the first stays clickable', async () => { + const onToggle = vi.fn(); + render(rail('SELECT 1; SELECT 2;', { onToggle })); + await waitFor(() => expect(topOf('sql-statement-include-1') - topOf('sql-statement-include-0')).toBeGreaterThanOrEqual(16)); + fireEvent.click(screen.getByTestId('sql-statement-include-0')); + expect(onToggle).toHaveBeenCalledWith(0); + }); + + it('keeps a long statement’s row at the top while its first line is scrolled away', () => { + const statements = splitSqlStatements(`SELECT\n${Array.from({ length: 9 }, (_, i) => ` c${i},`).join('\n')}\n z FROM t;\nSELECT 2;`); + // Lines 1–11 are the first statement; scrolled so line 4 is at the top. + const tops = rowTops(editorDouble({ height: 100, scroll: 60 }), statements); + expect(tops[0]).toBe(2); + // Scrolled past its end, it goes. + expect(Number.isNaN(rowTops(editorDouble({ height: 100, scroll: 230 }), statements)[0])).toBe(true); + }); + + it('copies a statement with its values, and will not copy one with an undefined variable', async () => { + const writeText = vi.fn().mockResolvedValue(undefined); + Object.defineProperty(navigator, 'clipboard', { value: { writeText }, configurable: true }); + const copyTextFor = (stmt: { text: string }) => + stmt.text.includes('missing') + ? { sql: stmt.text, error: 'Undefined variable: missing', hasVars: true } + : { sql: stmt.text.replace('${{id}}', '42'), hasVars: true }; + render(rail('SELECT * FROM t WHERE id = ${{id}};\nSELECT ${{missing}};', { copyTextFor })); + fireEvent.click(screen.getByTestId('sql-statement-copy-0')); + await waitFor(() => expect(writeText).toHaveBeenCalledWith('SELECT * FROM t WHERE id = 42;')); + const blocked = screen.getByTestId('sql-statement-copy-1') as HTMLButtonElement; + expect(blocked.disabled).toBe(true); + expect(blocked.getAttribute('title')).toBe('Undefined variable: missing'); + }); + + it('re-reads only the statement that changed', () => { + const script = (lastWord: string) => + Array.from({ length: 40 }, (_, i) => `select ${i} from t${i};`).join('\n') + `\nupdate t set a = 1 ${lastWord}`; + const props = { safeMode: true, editorRef: { current: editorDouble({ height: 5000 }) } }; + const { rerender } = render(rail(script('wher'), props)); + vi.mocked(statementVerb).mockClear(); + rerender(rail(script('where a = 2;'), props)); + expect(statementVerb).toHaveBeenCalledTimes(1); + expect(screen.getAllByTestId(/^sql-statement-label-/)).toHaveLength(41); + }); }); diff --git a/apps/web/src/frontend/features/sql-editor/components/StatementRunGutter.tsx b/apps/web/src/frontend/features/sql-editor/components/StatementRunGutter.tsx index b8fe32b15..bca075947 100644 --- a/apps/web/src/frontend/features/sql-editor/components/StatementRunGutter.tsx +++ b/apps/web/src/frontend/features/sql-editor/components/StatementRunGutter.tsx @@ -3,18 +3,27 @@ * Copyright 2024-2026 Huy Phan * SPDX-License-Identifier: Apache-2.0 * - * Run checkbox and per-cell play, one row per statement, sitting on the left - * of the editor so the statement list does not take a band of vertical space. + * The run rail: one row per statement on the left of the editor — its In [n] + * number, per-cell play, and the include checkbox — so the statement list no + * longer takes a band of vertical space. + * + * The number carries what the old statement strip showed about each cell: it + * turns amber for a write Safe mode will confirm and red with `!` for one with + * no WHERE, its tooltip names the cell's kind, and a click selects the + * statement. The copy button (shown on hover) copies it with its values. */ -import React, { useEffect, useState } from 'react'; -import { Play } from 'lucide-react'; +import React, { useEffect, useRef, useState } from 'react'; +import { Check, Copy, Play } from 'lucide-react'; import { isCodeCellKind, type SplitStatement } from '@/shared/lib/sql-splitter'; import { SQL_ICON_STROKE } from '@/shared/lib/iconStyle'; +import { statementBadge, statementTitle, type StatementBadge } from '../lib/statementBadges'; const FALLBACK_LINE_PX = 20; const EDITOR_PAD_PX = 8; +/** Height of one row of controls (`h-4`). */ +const ROW_PX = 16; -/** Enough of Monaco to line a checkbox up with a statement's first line. */ +/** Enough of Monaco to line a row up with a statement's first line. */ export interface GutterEditor { getTopForLineNumber: (line: number) => number; getScrollTop: () => number; @@ -24,21 +33,49 @@ export interface GutterEditor { onDidLayoutChange: (listener: () => void) => { dispose: () => void }; } +/** + * Never let two rows overlap. Statements that start on the same line + * (`SELECT 1; SELECT 2;`), or inside a folded region, map to the same y; drawn + * there, the later row covers the earlier one and its controls cannot be + * reached. Each row goes at its line or just below the row before, and the + * In [n] number says which statement it is. + */ +function stack(tops: number[], viewH: number): number[] { + let floor = 0; + return tops.map((top) => { + if (Number.isNaN(top)) return top; + const y = Math.max(top, floor); + if (viewH > 0 && y > viewH - ROW_PX / 2) return Number.NaN; + floor = y + ROW_PX; + return y; + }); +} + function fallbackTops(statements: SplitStatement[]): number[] { - return statements.map((stmt) => (stmt.startLine - 1) * FALLBACK_LINE_PX + EDITOR_PAD_PX); + return stack( + statements.map((stmt) => (stmt.startLine - 1) * FALLBACK_LINE_PX + EDITOR_PAD_PX), + 0 + ); } -function topsFor(editor: GutterEditor, statements: SplitStatement[]): number[] { - const scroll = editor.getScrollTop(); +export function rowTops(editor: GutterEditor, statements: SplitStatement[]): number[] { const viewH = editor.getLayoutInfo?.().height ?? 0; if (!viewH) return fallbackTops(statements); + const scroll = editor.getScrollTop(); const lineH = editor.getTopForLineNumber(2) - editor.getTopForLineNumber(1); const rowH = lineH > 0 ? lineH : FALLBACK_LINE_PX; - return statements.map((stmt) => { - const y = editor.getTopForLineNumber(stmt.startLine) - scroll; - if (y < -rowH || y > viewH) return Number.NaN; - return y + Math.max(0, (rowH - 16) / 2); - }); + const center = Math.max(0, (rowH - ROW_PX) / 2); + return stack( + statements.map((stmt) => { + const top = editor.getTopForLineNumber(stmt.startLine) - scroll; + const bottom = editor.getTopForLineNumber(stmt.endLine) + rowH - scroll; + if (bottom <= 0 || top > viewH) return Number.NaN; + // First line scrolled above the editor while the rest is on screen: keep + // the row at the top, or a long statement cannot be run while reading it. + return Math.max(top, 0) + center; + }), + viewH + ); } interface Props { @@ -49,8 +86,21 @@ interface Props { editorEpoch: number; running?: boolean; sqlNeedsDestination?: boolean; + /** Safe mode marks the writes it will ask to confirm. */ + safeMode?: boolean; onToggle: (index: number) => void; onRunCell?: (index: number) => void; + /** Select a statement in the editor (the number was clicked). */ + onReveal?: (stmt: SplitStatement) => void; + /** The statement with its variables filled in (secrets masked), for the copy button. */ + copyTextFor?: (stmt: SplitStatement) => { sql: string; error?: string; hasVars: boolean }; +} + +function labelTone(badge: StatementBadge): string { + if (badge.dml?.noWhere) return 'text-rose-300 font-bold'; + if (badge.dml) return 'text-amber-300 font-bold'; + if (badge.codeKind) return 'text-violet-300/80'; + return 'text-slate-500'; } export const StatementRunGutter: React.FC = ({ @@ -60,10 +110,18 @@ export const StatementRunGutter: React.FC = ({ editorEpoch, running = false, sqlNeedsDestination = false, + safeMode = false, onToggle, onRunCell, + onReveal, + copyTextFor, }) => { + const [copied, setCopied] = useState(null); const [tops, setTops] = useState(() => fallbackTops(statements)); + // Badges by statement text, so a keystroke re-reads only the statement it + // changed — the strip measured 17 ms per keystroke at 1,700 statements when + // it re-read them all. Keeps only what the last render showed. + const badgeCache = useRef(new Map()); useEffect(() => { const editor = editorRef.current; @@ -76,7 +134,7 @@ export const StatementRunGutter: React.FC = ({ if (frame) return; frame = requestAnimationFrame(() => { frame = 0; - setTops(topsFor(editor, statements)); + setTops(rowTops(editor, statements)); }); }; refresh(); @@ -91,53 +149,106 @@ export const StatementRunGutter: React.FC = ({ }; }, [editorRef, editorEpoch, statements]); + const shown = new Map(); + const badgeOf = (stmt: SplitStatement): StatementBadge => { + const key = `${safeMode ? 1 : 0}|${stmt.kind}|${stmt.text}`; + const found = shown.get(key) ?? badgeCache.current.get(key) ?? statementBadge(stmt, safeMode); + shown.set(key, found); + return found; + }; + + const copy = async (i: number, text: string) => { + try { + await navigator.clipboard.writeText(text); + setCopied(i); + window.setTimeout(() => setCopied((cur) => (cur === i ? null : cur)), 1500); + } catch { + // No clipboard (insecure origin, denied): nothing to show but the icon. + } + }; + + const rows = statements.map((stmt, i) => { + const top = tops[i]; + if (top == null || Number.isNaN(top)) return null; + const badge = badgeOf(stmt); + const title = statementTitle(i, badge); + const isChecked = checked.includes(i); + const playBlocked = sqlNeedsDestination && !isCodeCellKind(stmt.kind); + const copyText = copyTextFor?.(stmt); + return ( +
+ {copyText ? ( + + ) : null} + + + onToggle(i)} + className="h-3.5 w-3.5 shrink-0 cursor-pointer accent-cyan-600" + title={ + checked.length === 0 + ? 'None checked → toolbar Run uses the cell at the caret' + : `Include In [${i + 1}] in toolbar Run` + } + aria-label={`Include In [${i + 1}] in batch Run`} + /> +
+ ); + }); + badgeCache.current = shown; + return (
- {statements.map((stmt, i) => { - const top = tops[i]; - if (top == null || Number.isNaN(top)) return null; - const isChecked = checked.includes(i); - const playBlocked = sqlNeedsDestination && !isCodeCellKind(stmt.kind); - return ( -
- - onToggle(i)} - className="h-3.5 w-3.5 shrink-0 cursor-pointer accent-cyan-600" - title={ - checked.length === 0 - ? 'None checked → toolbar Run uses the cell at the caret' - : 'Include this cell in toolbar Run' - } - aria-label={`Include cell ${i + 1} in batch Run`} - /> -
- ); - })} + {rows}
); }; diff --git a/apps/web/src/frontend/features/sql-editor/components/StatementStrip.test.tsx b/apps/web/src/frontend/features/sql-editor/components/StatementStrip.test.tsx deleted file mode 100644 index 0f3b75b1d..000000000 --- a/apps/web/src/frontend/features/sql-editor/components/StatementStrip.test.tsx +++ /dev/null @@ -1,29 +0,0 @@ -import { describe, expect, it, vi } from 'vitest'; -import React from 'react'; -import { render, screen } from '@testing-library/react'; -import { checkStatement, splitSqlStatements } from '@/shared/lib/sql-splitter'; -import { StatementStrip } from './StatementStrip'; - -// Watched, not replaced: the strip checks each statement it has not seen. -vi.mock('@/shared/lib/sql-splitter', async (importOriginal) => { - const real = await importOriginal(); - return { ...real, checkStatement: vi.fn(real.checkStatement) }; -}); - -const script = (lastWord: string) => - Array.from({ length: 40 }, (_, i) => `select ${i} from t${i};`).join('\n') + `\nupdate t set a = 1 ${lastWord}`; - -const strip = (sql: string) => ( - {}} onReveal={() => {}} /> -); - -describe('StatementStrip', () => { - it('re-checks only the statement that changed', () => { - const { rerender } = render(strip(script('wher'))); - vi.mocked(checkStatement).mockClear(); - rerender(strip(script('where a = 2;'))); - // The edited statement, not all 41. - expect(checkStatement).toHaveBeenCalledTimes(1); - expect(screen.getAllByTestId(/^sql-statement-cell-/)).toHaveLength(41); - }); -}); diff --git a/apps/web/src/frontend/features/sql-editor/components/StatementStrip.tsx b/apps/web/src/frontend/features/sql-editor/components/StatementStrip.tsx deleted file mode 100644 index 2dfeb4523..000000000 --- a/apps/web/src/frontend/features/sql-editor/components/StatementStrip.tsx +++ /dev/null @@ -1,443 +0,0 @@ -import React, { useCallback, useEffect, useRef, useState } from 'react'; -import { createPortal } from 'react-dom'; -import { Check, Copy, Play } from 'lucide-react'; -import { - checkStatement, - dmlLacksWhere, - isCodeCellKind, - isMutatingDmlStatement, - statementVerb, - CODE_CELL_KIND_LABEL, - type CodeCellKind, - type SplitStatement, -} from '@/shared/lib/sql-splitter'; -import { findVariableRefs, substituteVariables } from '@/shared/lib/sql-variables'; -import { useSqlEditorStore } from '@/features/sql-editor/state/useSqlEditorStore'; -import { SQL_ICON_STROKE } from '@/shared/lib/iconStyle'; - -interface Props { - statements: SplitStatement[]; - checked: number[]; - /** True while this tab's execute is in flight — disables per-cell Play. */ - running?: boolean; - /** When true, SQL cells need a Destination (code cells still run). */ - sqlNeedsDestination?: boolean; - onToggle: (index: number) => void; - onReveal: (stmt: SplitStatement) => void; - /** Jupyter-style per-cell run (one statement index). */ - onRunCell?: (index: number) => void; -} - -const STORAGE_KEY = 'foxschema-sql-statement-strip-h'; -const ROW_PX = 34; -const PAD_PX = 14; -const MIN_ROWS = 1; -const MAX_ROWS = 12; -const DEFAULT_ROWS = 3; - -const defaultHeight = () => ROW_PX * DEFAULT_ROWS + PAD_PX; -const minHeight = () => ROW_PX * MIN_ROWS + PAD_PX; -const maxHeight = () => ROW_PX * MAX_ROWS + PAD_PX; - -function loadHeight(): number { - try { - const n = Number(localStorage.getItem(STORAGE_KEY)); - if (Number.isFinite(n) && n >= minHeight()) { - return Math.min(maxHeight(), n); - } - } catch { - /* ignore */ - } - return defaultHeight(); -} - -const preview = (text: string, max = 120, codeKind?: CodeCellKind | null): string => { - let source = text; - if (codeKind) { - // Drop fence markers so the strip shows the cell body, not `-- @node` / `-- @end`. - source = source - .replace(/^\s*--\s*@(?:js|ts|javascript|typescript|node|nodets|node-typescript)\b[^\n]*\n?/i, '') - .replace(/\n?\s*--\s*@end\s*$/i, ''); - } - const compact = source.replace(/\s+/g, ' ').trim(); - return compact.length > max ? compact.slice(0, max) + '…' : compact || text.replace(/\s+/g, ' ').trim(); -}; - -const codeCellBadge = (kind: CodeCellKind) => ({ - label: CODE_CELL_KIND_LABEL[kind].short, - title: `${CODE_CELL_KIND_LABEL[kind].long} code cell (-- @${kind} … -- @end)`, -}); - -const DML_BADGE: Record = { - update: 'UPD', - delete: 'DEL', - merge: 'MRG', -}; - -function resolveSql( - stmtText: string, - variables: ReturnType['variables'] -): { sql: string; error?: string; hasVars: boolean } { - const hasVars = findVariableRefs(stmtText).length > 0; - if (!hasVars) return { sql: stmtText, hasVars: false }; - const expanded = substituteVariables(stmtText, variables, { maskSecrets: true }); - if (!expanded.ok) return { sql: stmtText, error: expanded.error, hasVars: true }; - return { sql: expanded.sql, hasVars: true }; -} - -type PopoverState = { - index: number; - top: number; - left: number; - width: number; - sql: string; - error?: string; - hasVars: boolean; -}; - -/** - * Notebook-style statement strip (Jupyter-inspired): each row is a cell with - * In [n], kind badge, preview, and a per-cell Play — same splitter/run pipeline - * as the toolbar Run. - */ -/** Everything a row shows about its statement, from its text alone. */ -function analyse( - stmt: SplitStatement, - last: boolean, - safeMode: boolean, - variables: ReturnType['variables'] -) { - const status = checkStatement(stmt, { last }); - const codeKind = isCodeCellKind(stmt.kind) ? stmt.kind : null; - const codeBadge = codeKind ? codeCellBadge(codeKind) : null; - const verb = codeKind ? null : statementVerb(stmt.text); - const dmlBadge = safeMode && verb && isMutatingDmlStatement(stmt.text) ? DML_BADGE[verb] : null; - const noWhere = dmlBadge ? dmlLacksWhere(stmt.text) : false; - const resolved = resolveSql(stmt.text, variables); - return { status, codeKind, codeBadge, dmlBadge, noWhere, resolved }; -} - -export const StatementStrip: React.FC = ({ - statements, - checked, - running = false, - sqlNeedsDestination = false, - onToggle, - onReveal, - onRunCell, -}) => { - const [height, setHeight] = useState(loadHeight); - const safeMode = useSqlEditorStore((s) => s.safeMode); - const variables = useSqlEditorStore((s) => s.variables); - // What each row shows about its statement, by statement text. A keystroke - // changes one statement, so the rest are not checked and resolved again: - // re-analysing every statement per keystroke was 17 ms at 1,700 of them. - // Holds only the statements of the last render, and starts over when the - // variables or Safe mode, which every answer reads, change. - const analysisCache = useRef<{ - variables: typeof variables; - safeMode: boolean; - byText: Map>; - } | null>(null); - const previous = - analysisCache.current?.variables === variables && analysisCache.current.safeMode === safeMode - ? analysisCache.current.byText - : new Map>(); - const byText = new Map>(); - const analyses = statements.map((stmt, i) => { - const last = i === statements.length - 1; - const key = `${last ? 1 : 0}|${stmt.kind ?? ''}|${stmt.text}`; - const found = byText.get(key) ?? previous.get(key) ?? analyse(stmt, last, safeMode, variables); - byText.set(key, found); - return found; - }); - analysisCache.current = { variables, safeMode, byText }; - const [popover, setPopover] = useState(null); - const [copiedIndex, setCopiedIndex] = useState(null); - const hideTimer = useRef | null>(null); - - useEffect(() => { - try { - localStorage.setItem(STORAGE_KEY, String(height)); - } catch { - /* ignore */ - } - }, [height]); - - useEffect(() => { - return () => { - if (hideTimer.current) clearTimeout(hideTimer.current); - }; - }, []); - - const clearHide = () => { - if (hideTimer.current) { - clearTimeout(hideTimer.current); - hideTimer.current = null; - } - }; - - const scheduleHide = () => { - clearHide(); - // Long delay so the pointer can move into the overlapping popover / click Copy. - hideTimer.current = setTimeout(() => { - setPopover(null); - }, 500); - }; - - const openPopover = (index: number, el: HTMLElement, stmtText: string) => { - clearHide(); - const rect = el.getBoundingClientRect(); - const resolved = resolveSql(stmtText, variables); - const width = Math.min(520, Math.max(280, rect.width)); - let left = rect.left; - if (left + width > window.innerWidth - 8) { - left = Math.max(8, window.innerWidth - width - 8); - } - // Overlap the row by a few px so there is no gap that dismisses the popover. - let top = rect.bottom - 4; - const approxH = 140; - if (top + approxH > window.innerHeight - 8) { - top = Math.max(8, rect.top - approxH + 4); - } - setPopover({ - index, - top, - left, - width, - sql: resolved.sql, - error: resolved.error, - hasVars: resolved.hasVars, - }); - }; - - const copyText = async (index: number, text: string, hasError?: boolean) => { - if (hasError || !text) return; - try { - await navigator.clipboard.writeText(text); - setCopiedIndex(index); - window.setTimeout(() => setCopiedIndex((cur) => (cur === index ? null : cur)), 1500); - } catch { - /* ignore */ - } - }; - - const startResize = useCallback( - (e: React.MouseEvent) => { - e.preventDefault(); - const startY = e.clientY; - const startH = height; - const onMove = (ev: MouseEvent) => { - const next = Math.min(maxHeight(), Math.max(minHeight(), startH + (ev.clientY - startY))); - setHeight(next); - }; - const onUp = () => { - window.removeEventListener('mousemove', onMove); - window.removeEventListener('mouseup', onUp); - document.body.style.cursor = ''; - document.body.style.userSelect = ''; - }; - document.body.style.cursor = 'row-resize'; - document.body.style.userSelect = 'none'; - window.addEventListener('mousemove', onMove); - window.addEventListener('mouseup', onUp); - }, - [height] - ); - - if (statements.length === 0) return null; - - return ( -
-
- {statements.map((stmt, i) => { - const { status, codeKind, codeBadge, dmlBadge, noWhere, resolved } = analyses[i]!; - const ok = status.level === 'ok'; - const isChecked = checked.includes(i); - const playBlocked = sqlNeedsDestination && !codeKind; - const isCopied = copiedIndex === i; - const accent = codeKind - ? 'border-l-teal-400/80 bg-teal-950/20' - : 'border-l-sky-400/80 bg-sky-950/15'; - return ( -
- - onToggle(i)} - className="w-3.5 h-3.5 accent-cyan-600 cursor-pointer shrink-0 mt-1.5" - title={ - checked.length === 0 - ? 'None checked → toolbar Run uses the first cell' - : 'Include this cell in toolbar Run' - } - aria-label={`Include cell ${i + 1} in batch Run`} - /> - - -
- ); - })} -
-
- - {popover && - createPortal( -
-
- - {popover.hasVars ? 'Query with values' : 'Query'} - - -
- {popover.error ? ( -

{popover.error}

- ) : ( - 
-                {popover.sql}
-
- )} -
, - document.body - )} -
- ); -}; diff --git a/apps/web/src/frontend/features/sql-editor/lib/sqlEditorBridge.ts b/apps/web/src/frontend/features/sql-editor/lib/sqlEditorBridge.ts index 7bb29412e..111107ca3 100644 --- a/apps/web/src/frontend/features/sql-editor/lib/sqlEditorBridge.ts +++ b/apps/web/src/frontend/features/sql-editor/lib/sqlEditorBridge.ts @@ -102,7 +102,7 @@ export function setSqlSelectionGetter(fn: SelectionGetter | null): void { selectionGetter = fn ?? (() => null); } -/** Current editor selection used by Run (selection overrides statement strip). */ +/** Current editor selection used by Run (selection overrides the rail's run checks). */ export function getSelectedSql(): string | null { return selectionGetter(); } diff --git a/apps/web/src/frontend/features/sql-editor/lib/sqlEditorSamples.ts b/apps/web/src/frontend/features/sql-editor/lib/sqlEditorSamples.ts index d426b0865..4b8724c93 100644 --- a/apps/web/src/frontend/features/sql-editor/lib/sqlEditorSamples.ts +++ b/apps/web/src/frontend/features/sql-editor/lib/sqlEditorSamples.ts @@ -425,7 +425,7 @@ return [{ id: 'sample-schema-history-table', title: '★ Sample · Schema history (table v1…vN)', sql: `-- WRITES — Safe mode OFF. SQLite, Postgres, or MySQL. --- Play ONE SQL cell at a time (▶ on the strip), then Compare Schema → Snapshot +-- Play ONE SQL cell at a time (▶ beside its In [n] on the left), then Compare Schema → Snapshot -- target. After v3, open History and click fox_hist_customers. -- @js @@ -684,7 +684,7 @@ return sample.map((r) => ({ id: r.id, name: r.name, inserted })); { id: 'sample-foxscript-notebook', title: '★ Sample · FoxScript notebook (SQL ↔ JS)', - sql: `-- FoxScript = one buffer, many cells. Use the strip ▶ to run one cell, or Run all. + sql: `-- FoxScript = one buffer, many cells. Use the ▶ beside a cell's In [n] to run that cell, or Run all. SELECT 'east' AS region, 10 AS n UNION ALL @@ -987,7 +987,7 @@ return (decoded.people ?? []).map((p) => ({ }, { id: 'sample-js-play-one-cell', - title: '★ Sample · strip ▶ one cell at a time', + title: '★ Sample · ▶ one cell at a time', sql: `-- Tip: click ▶ on In [2] alone — last is null, so the cell must guard. SELECT 100 AS seed; diff --git a/apps/web/src/frontend/features/sql-editor/lib/statementBadges.test.ts b/apps/web/src/frontend/features/sql-editor/lib/statementBadges.test.ts new file mode 100644 index 000000000..1a558d8be --- /dev/null +++ b/apps/web/src/frontend/features/sql-editor/lib/statementBadges.test.ts @@ -0,0 +1,57 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + */ +import { describe, expect, it } from 'vitest'; +import { splitSqlStatements } from '@/shared/lib/sql-splitter'; +import type { SqlVariable } from '@/shared/lib/sql-variables'; +import { statementBadge, statementCopyText, statementTitle } from './statementBadges'; + +const one = (sql: string) => splitSqlStatements(sql)[0]!; + +describe('statementBadge', () => { + it('marks a Safe-mode write, and whether it has a WHERE', () => { + expect(statementBadge(one('UPDATE t SET a = 1 WHERE id = 2;'), true).dml).toEqual({ badge: 'UPD', noWhere: false }); + expect(statementBadge(one('DELETE FROM t;'), true).dml).toEqual({ badge: 'DEL', noWhere: true }); + expect(statementBadge(one('SELECT * FROM t;'), true).dml).toBeNull(); + }); + + it('marks nothing with Safe mode off, nor for a code cell', () => { + expect(statementBadge(one('DELETE FROM t;'), false).dml).toBeNull(); + const cell = statementBadge(one('-- @js\nreturn [];\n-- @end'), true); + expect(cell).toEqual({ codeKind: 'js', dml: null }); + }); +}); + +describe('statementTitle', () => { + it('says which cell, its kind, and the warning', () => { + expect(statementTitle(1, { codeKind: null, dml: { badge: 'DEL', noWhere: true } })).toMatch( + /^In \[2\] · SQL · DEL: no WHERE — may affect all rows\./ + ); + expect(statementTitle(0, { codeKind: 'node', dml: null })).toMatch(/^In \[1\] · Node\.js code cell\./); + }); +}); + +describe('statementCopyText', () => { + const vars: SqlVariable[] = [ + { id: '1', name: 'id', kind: 'scalar', value: 42, updatedAt: 0 }, + { id: '2', name: 'token', kind: 'scalar', value: 'hunter2', secret: true, updatedAt: 0 }, + ]; + + it('fills in variables and masks secrets', () => { + const out = statementCopyText('SELECT * FROM t WHERE id = ${{id}} AND k = ${{token}};', vars); + expect(out.hasVars).toBe(true); + expect(out.sql).toContain('id = 42'); + expect(out.sql).not.toContain('hunter2'); + expect(out.sql).toContain('(secret)'); + }); + + it('returns the text as written when there are no variables', () => { + expect(statementCopyText('SELECT 1;', vars)).toEqual({ sql: 'SELECT 1;', hasVars: false }); + }); + + it('says why when a variable is not defined', () => { + expect(statementCopyText('SELECT ${{missing}};', vars).error).toMatch(/Undefined variable: missing/); + }); +}); diff --git a/apps/web/src/frontend/features/sql-editor/lib/statementBadges.ts b/apps/web/src/frontend/features/sql-editor/lib/statementBadges.ts new file mode 100644 index 000000000..eb39ec7b4 --- /dev/null +++ b/apps/web/src/frontend/features/sql-editor/lib/statementBadges.ts @@ -0,0 +1,65 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * What the run rail says about each statement, and the text "Copy statement + * with values" puts on the clipboard. + * + * Both came from the statement strip. Moving the run checks into a rail + * beside the editor dropped them — and with them the red mark on a DELETE or + * UPDATE that has no WHERE, which is the warning that matters before Run. + */ +import { + CODE_CELL_KIND_LABEL, + dmlLacksWhere, + isCodeCellKind, + isMutatingDmlStatement, + statementVerb, + type CodeCellKind, + type SplitStatement, +} from '@/shared/lib/sql-splitter'; +import { findVariableRefs, substituteVariables, type SqlVariable } from '@/shared/lib/sql-variables'; + +const DML_BADGE: Record = { + update: 'UPD', + delete: 'DEL', + merge: 'MRG', +}; + +export interface StatementBadge { + codeKind: CodeCellKind | null; + /** Safe mode only: a write Run will ask to confirm, and whether it has no WHERE. */ + dml: { badge: 'UPD' | 'DEL' | 'MRG'; noWhere: boolean } | null; +} + +export function statementBadge(stmt: SplitStatement, safeMode: boolean): StatementBadge { + const codeKind = isCodeCellKind(stmt.kind) ? stmt.kind : null; + if (codeKind || !safeMode) return { codeKind, dml: null }; + const verb = statementVerb(stmt.text); + const badge = verb && isMutatingDmlStatement(stmt.text) ? DML_BADGE[verb] : undefined; + return { codeKind, dml: badge ? { badge, noWhere: dmlLacksWhere(stmt.text) } : null }; +} + +/** The rail label's tooltip: which cell, what kind, and any Safe-mode warning. */ +export function statementTitle(index: number, badge: StatementBadge): string { + const kind = badge.codeKind ? `${CODE_CELL_KIND_LABEL[badge.codeKind].long} code cell` : 'SQL'; + const dml = badge.dml + ? badge.dml.noWhere + ? ` · ${badge.dml.badge}: no WHERE — may affect all rows` + : ` · Safe mode: ${badge.dml.badge} asks for confirmation on Run` + : ''; + return `In [${index + 1}] · ${kind}${dml}. Click to select it.`; +} + +/** The statement with its `${{variables}}` filled in, secrets masked. */ +export function statementCopyText( + text: string, + variables: SqlVariable[] +): { sql: string; error?: string; hasVars: boolean } { + const hasVars = findVariableRefs(text).length > 0; + if (!hasVars) return { sql: text, hasVars: false }; + const expanded = substituteVariables(text, variables, { maskSecrets: true }); + if (!expanded.ok) return { sql: text, error: expanded.error, hasVars: true }; + return { sql: expanded.sql, hasVars: true }; +} diff --git a/apps/web/src/frontend/features/sql-editor/state/sqlEditorTabLogic.ts b/apps/web/src/frontend/features/sql-editor/state/sqlEditorTabLogic.ts index fb5e569b4..66e51defd 100644 --- a/apps/web/src/frontend/features/sql-editor/state/sqlEditorTabLogic.ts +++ b/apps/web/src/frontend/features/sql-editor/state/sqlEditorTabLogic.ts @@ -202,7 +202,7 @@ export function indicesToRun( } /** - * All statements inside an editor selection (selection wins over the strip). + * All statements inside an editor selection (selection wins over the rail's run checks). * Falls back to the trimmed selection text when the splitter finds nothing. */ export function statementsFromSelection(selectedSql: string): string[] { @@ -223,7 +223,7 @@ export function canExecuteWithoutDestination(statements: string[]): boolean { } /** - * Statements the Run button would send for the active tab (selection overrides strip). + * Statements the Run button would send for the active tab (selection overrides the rail's run checks). */ export function resolveRunStatements( sql: string, diff --git a/apps/web/src/frontend/features/sql-editor/state/useSqlEditorStore.ts b/apps/web/src/frontend/features/sql-editor/state/useSqlEditorStore.ts index 65a57d472..31cf1415a 100644 --- a/apps/web/src/frontend/features/sql-editor/state/useSqlEditorStore.ts +++ b/apps/web/src/frontend/features/sql-editor/state/useSqlEditorStore.ts @@ -441,7 +441,7 @@ interface SqlEditorState { * Re-run SQL. * - `connectionIds` — refresh only those credentials (keeps other panes). * - `statementIndices` — run only these split indices (per-cell Play; ignores - * strip checks and editor selection). + * rail run checks and editor selection). */ execute: (opts?: { confirmedWrites?: boolean; @@ -1024,7 +1024,7 @@ export const useSqlEditorStore = create()( ? selected.filter((c) => connectionIds.includes(c.id)) : selected; - // Per-cell Play: run exactly those indices (ignore strip checks + selection). + // Per-cell Play: run exactly those indices (ignore run checks + selection). const selectedSql = getSelectedSql(); // Only consulted when nothing is checked and nothing is selected — then // Run means "this statement", not "the first one in the buffer". @@ -1035,7 +1035,7 @@ export const useSqlEditorStore = create()( statementIndices && statementIndices.length > 0 ? statementsToRun(tab.sql, statementIndices) : resolveRunStatements(tab.sql, tab.checkedStatements, selectedSql, caretOffset); - // Align Out [n] with In [n] when running from strip / Play (not selection). + // Align Out [n] with In [n] when running from run checks / Play (not selection). const sourceIndices = fromSelection ? null : indicesToRun( @@ -1121,7 +1121,7 @@ export const useSqlEditorStore = create()( multiTableStatements, connectionIds: connectionIds?.length ? connectionIds : undefined, // Pin what was confirmed. Resuming re-derives from the editor, so - // without this the caret (or strip checks) could move between + // without this the caret (or run checks) could move between // "Run" and "Confirm" and a different statement than the one // listed in the dialog would execute. statementIndices: statementIndices?.length diff --git a/apps/web/src/frontend/shared/components/DriverInstallHints.tsx b/apps/web/src/frontend/shared/components/DriverInstallHints.tsx new file mode 100644 index 000000000..22b2f2d4e --- /dev/null +++ b/apps/web/src/frontend/shared/components/DriverInstallHints.tsx @@ -0,0 +1,71 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * What to tell someone whose driver is missing, wherever they hit it: why it + * does not load, the command to run by hand, Db2's extra steps, and who may + * press Install. The credential form had all of this and the SQL editor's + * banner had none of it; both render this now. + */ +import React from 'react'; +import { IBM_DB_VERSION } from '@foxschema/ui-shared'; +import type { DriverInfo } from '@/shared/lib/types'; + +export const DriverInstallHints: React.FC<{ + info: DriverInfo | null; + /** The missing package, when known from `info` or the error that named it. */ + packageName: string; + installing: boolean; + installError: string; + /** False without the Install drivers permission: say who can, and how. */ + canInstall: boolean; +}> = ({ info, packageName, installing, installError, canInstall }) => { + if (installing) return null; + const command = info?.installCommand ?? (packageName ? `npm install ${packageName}` : ''); + return ( + <> + {installError ? ( +

+ {installError} +

+ ) : null} + {!canInstall ? ( +

+ Installing a driver runs npm on the server, so it needs an admin (the Install drivers + permission). + {command ? ( + <> + {' '} + Ask one, or run {command} where Fox + Schema runs. + + ) : null} +

+ ) : info?.error ? ( +

+ {info.error.slice(0, 280)} + {command ? ( + <> + {' '} + — run {command} + + ) : null} +

+ ) : null} + {packageName === 'ibm_db' ? ( +

+ Db2 needs a native build + IBM clidriver download. Prefer{' '} + foxschema drivers install db2, then{' '} + foxschema stop && foxschema. In a + monorepo:{' '} + + npm install ibm_db@{IBM_DB_VERSION} --foreground-scripts -w @foxschema/db + + . On linux/arm64 use Docker ( + 5nickels/foxschema:latest). +

+ ) : null} + + ); +}; diff --git a/apps/web/src/frontend/shared/lib/driverStatus.test.ts b/apps/web/src/frontend/shared/lib/driverStatus.test.ts new file mode 100644 index 000000000..d201ae95b --- /dev/null +++ b/apps/web/src/frontend/shared/lib/driverStatus.test.ts @@ -0,0 +1,117 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * One driver status for every Install button: a second click joins the + * running install, and a finished one refreshes every dialect on the package. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { DriverInfo } from '@/shared/lib/types'; + +const checkDriver = vi.fn(); +const installDriver = vi.fn(); +const invalidateCache = vi.fn(); + +vi.mock('@/shared/api/schemaApi', () => ({ + checkDriver: (...args: unknown[]) => checkDriver(...args), + installDriver: (...args: unknown[]) => installDriver(...args), + invalidateCache: (...args: unknown[]) => invalidateCache(...args), +})); + +import { + checkDriverStatus, + clearDriverStatus, + driverStatusOf, + installDriverFor, + missingDriverPackage, +} from './driverStatus'; + +/** A server where `installed` lists the packages that load. */ +function server(packages: Record, installed: Set) { + checkDriver.mockImplementation(async (dialect: string): Promise => { + const packageName = packages[dialect]!; + return { provider: dialect, packageName, installed: installed.has(packageName), installCommand: `npm install ${packageName}` }; + }); +} + +beforeEach(() => { + clearDriverStatus(); + checkDriver.mockReset(); + installDriver.mockReset(); + invalidateCache.mockReset(); +}); + +describe('installDriverFor', () => { + it('runs one install for two clicks, and both learn it loaded', async () => { + const installed = new Set(); + server({ redis: 'redis' }, installed); + let finish!: () => void; + installDriver.mockImplementation( + () => + new Promise((resolve) => { + finish = () => { + installed.add('redis'); + resolve(); + }; + }) + ); + await checkDriverStatus('redis'); + const first = installDriverFor('redis'); + const second = installDriverFor('redis'); + expect(second).toBe(first); + expect(driverStatusOf('redis').installing).toBe(true); + finish(); + expect(await Promise.all([first, second])).toEqual([true, true]); + expect(installDriver).toHaveBeenCalledTimes(1); + expect(driverStatusOf('redis')).toMatchObject({ installing: false, installError: '' }); + expect(driverStatusOf('redis').info?.installed).toBe(true); + }); + + it('refreshes every dialect on the package, not only the one installed', async () => { + const installed = new Set(); + server({ mysql: 'mysql2', mariadb: 'mysql2', postgres: 'pg' }, installed); + installDriver.mockImplementation(async () => installed.add('mysql2')); + await Promise.all(['mysql', 'mariadb', 'postgres'].map((d) => checkDriverStatus(d))); + expect(await installDriverFor('mysql')).toBe(true); + expect(invalidateCache).toHaveBeenCalledWith('driver:'); + expect(driverStatusOf('mariadb').info?.installed).toBe(true); + expect(driverStatusOf('postgres').info?.installed).toBe(false); + }); + + it('says why when npm fails, and when the driver still does not load', async () => { + server({ oracle: 'oracledb' }, new Set()); + await checkDriverStatus('oracle'); + installDriver.mockRejectedValueOnce(new Error('npm install oracledb failed (exit 1)')); + expect(await installDriverFor('oracle')).toBe(false); + expect(driverStatusOf('oracle').installError).toBe('npm install oracledb failed (exit 1)'); + + installDriver.mockResolvedValueOnce({ success: true }); + expect(await installDriverFor('oracle')).toBe(false); + expect(driverStatusOf('oracle').installError).toBe( + 'Install finished but "oracledb" still failed to load. Try: npm install oracledb' + ); + }); +}); + +describe('checkDriverStatus', () => { + it('records a failed check as no answer, not as installed', async () => { + checkDriver.mockRejectedValueOnce(new Error('Invalid dialect')); + await checkDriverStatus('nope'); + expect(driverStatusOf('nope')).toMatchObject({ info: null, checkFailed: true }); + }); +}); + +describe('missingDriverPackage', () => { + it('reads the package from the server’s missing-driver message', () => { + expect(missingDriverPackage('Database driver "redis" is not installed for Redis. Run: npm install redis')).toBe('redis'); + }); + + it('ignores other errors that happen to say "is not installed"', () => { + expect( + missingDriverPackage('Full-Text Search is not installed, or a full-text component cannot be loaded.') + ).toBeNull(); + expect(missingDriverPackage('pgstattuple is not installed')).toBeNull(); + expect(missingDriverPackage(undefined)).toBeNull(); + }); +}); diff --git a/apps/web/src/frontend/shared/lib/driverStatus.ts b/apps/web/src/frontend/shared/lib/driverStatus.ts new file mode 100644 index 000000000..c54d35488 --- /dev/null +++ b/apps/web/src/frontend/shared/lib/driverStatus.ts @@ -0,0 +1,135 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * Whether each dialect's driver is installed, shared by every screen that asks. + * + * The credential form, the schema browser and each failed run in the SQL + * editor show the same Install button. Each used to keep its own answer, so an + * install from one left the others saying "not installed" — and offering a + * second npm run into the same node_modules. One store answers them all: an + * install shows as running everywhere, a second click joins it, and a finished + * one refreshes every dialect on that package. + */ +import { useEffect, useSyncExternalStore } from 'react'; +import { checkDriver, installDriver, invalidateCache } from '@/shared/api/schemaApi'; +import type { DriverInfo } from '@/shared/lib/types'; + +export interface DriverStatus { + /** What the server said; null until it answers, or when the check failed. */ + info: DriverInfo | null; + /** The check itself failed (unknown dialect, network), so `info` is not an answer. */ + checkFailed: boolean; + installing: boolean; + /** Why the last install did not leave a loading driver; '' when it did. */ + installError: string; +} + +const EMPTY: DriverStatus = { info: null, checkFailed: false, installing: false, installError: '' }; + +let state: Record = {}; +const listeners = new Set<() => void>(); +/** In-flight installs by package: MySQL and MariaDB both install mysql2. */ +const installs = new Map>(); + +const keyOf = (dialect: string) => dialect.trim().toLowerCase(); + +function patch(dialect: string, next: Partial): void { + const key = keyOf(dialect); + state = { ...state, [key]: { ...(state[key] ?? EMPTY), ...next } }; + listeners.forEach((listener) => listener()); +} + +function subscribe(listener: () => void): () => void { + listeners.add(listener); + return () => { + listeners.delete(listener); + }; +} + +export function driverStatusOf(dialect: string): DriverStatus { + return state[keyOf(dialect)] ?? EMPTY; +} + +/** Ask the server. The API layer caches the answer for 30 s unless `force`. */ +export async function checkDriverStatus(dialect: string, { force = false } = {}): Promise { + if (!dialect.trim()) return; + if (force) invalidateCache(`driver:${dialect}`); + try { + const info = await checkDriver(dialect); + patch(dialect, { info, checkFailed: false }); + } catch { + patch(dialect, { info: null, checkFailed: true }); + } +} + +/** + * Install the dialect's driver. A call while one is running for the same + * package — from another banner, or a second click — shares it. Resolves true + * when the driver loads afterwards; otherwise `installError` says why. + */ +export function installDriverFor(dialect: string): Promise { + const own = keyOf(dialect); + const pkg = driverStatusOf(dialect).info?.packageName ?? own; + const running = installs.get(pkg); + if (running) return running; + const onPackage = () => + Object.keys(state).filter((key) => key === own || state[key]?.info?.packageName === pkg); + const job = (async () => { + patch(dialect, { installing: true, installError: '' }); + for (const key of onPackage()) patch(key, { installing: true }); + try { + await installDriver(dialect); + // Every dialect on this package changed, not only the one asked about. + invalidateCache('driver:'); + await Promise.all(onPackage().map((key) => checkDriverStatus(key))); + const info = driverStatusOf(dialect).info; + if (info?.installed) return true; + const packageName = info?.packageName ?? pkg; + patch(dialect, { + installError: info?.error + ? `Driver still not loading: ${info.error}` + : `Install finished but "${packageName}" still failed to load. Try: ${info?.installCommand ?? `npm install ${packageName}`}`, + }); + return false; + } catch (error: unknown) { + patch(dialect, { installError: error instanceof Error ? error.message : 'Driver install failed' }); + return false; + } finally { + for (const key of onPackage()) patch(key, { installing: false }); + installs.delete(pkg); + } + })(); + installs.set(pkg, job); + return job; +} + +/** + * The package a server error names as missing, or null. + * + * Matches the server's own message (`Database driver "redis" is not installed + * for …`, from DriverDetector and the driver loader) — not any text containing + * "is not installed", which is also SQL Server's Msg 7609 about Full-Text + * Search and Postgres's wording for a missing extension. + */ +export function missingDriverPackage(message: string | undefined): string | null { + const found = message ? /Database driver "([^"]+)" is not installed/.exec(message) : null; + return found ? found[1]! : null; +} + +/** The dialect's driver status, re-checked whenever a screen asks for it. */ +export function useDriverStatus(dialect: string): DriverStatus { + const status = useSyncExternalStore(subscribe, () => driverStatusOf(dialect)); + useEffect(() => { + if (dialect.trim() && !driverStatusOf(dialect).installing) void checkDriverStatus(dialect); + }, [dialect]); + return status; +} + +/** Forget every answer. Tests only: each one starts from nothing checked. */ +export function clearDriverStatus(): void { + state = {}; + installs.clear(); + listeners.forEach((listener) => listener()); +} diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 5ca035acd..98bc76522 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -346,7 +346,7 @@ Rendering on the hot paths (what to keep when editing these components): (`cachedPlan`), so Execute, the Git commit check and the review notes share one build. - The Compare tree rows, the permission matrix rows and the results panel are `React.memo` and are passed stable props; an inline callback or a default `[]` prop - breaks that. The statement strip caches each statement's checks by its text. + breaks that. The run rail caches each statement's Safe-mode badge by its text. ## SQL text: one tokenizer, dialect readings diff --git a/docs/releases/UNRELEASED.md b/docs/releases/UNRELEASED.md index 7364be37c..baacd7302 100644 --- a/docs/releases/UNRELEASED.md +++ b/docs/releases/UNRELEASED.md @@ -17,7 +17,24 @@ so unqualified names resolve to the tables the explorer shows, not the database user's default. Picking a connection in the explorer checks that destination alone. **One transaction** wraps a multi-statement SQL run per destination; a failure rolls it back. Code cells and `-- @set` stay statement -by statement. +by statement. Redis, MongoDB and ClickHouse cannot roll back, so a Run with One +transaction is refused there. + +## SQL Editor: the run rail, and installing a missing driver + +Each statement's ▶ and run checkbox sit on a rail left of the editor, with its +**In [n]** number — the one the Run tooltip and `Out [n]` use. Click the number +to select the statement. With Safe mode on it turns amber for a write Run will +confirm, and red with `!` for an UPDATE or DELETE with no WHERE. Two statements +on one line get a row each, and a long statement's row stays at the top while +you read through it. Hover a row for its copy button, which copies the +statement with its variables filled in (secrets masked). + +When a credential's driver package is missing, the schema browser and a failed +run — in either results layout — offer **Install driver**. Installing runs npm +on the server, so it needs the new **Install drivers** permission (admins by +default, from the account role); without it the banner gives the command to run. +Every Install button shares one status, and the server runs one install at a time. ## Workflow: steer a paused run; connections stay in their workspace diff --git a/docs/testing/TEST_IDS.md b/docs/testing/TEST_IDS.md index 088de0ca2..72c01b659 100644 --- a/docs/testing/TEST_IDS.md +++ b/docs/testing/TEST_IDS.md @@ -8,7 +8,7 @@ or `TestIds` for the same tree with autocomplete. `{name}` marks a part filled i time; "in FilterPicker" marks an ID a shared component draws from the prop written here. How to write a test with them: [WRITING_E2E.md](WRITING_E2E.md). -Every control has a test ID: **999** buttons, text boxes, selects and textareas. +Every control has a test ID: **996** buttons, text boxes, selects and textareas. ## access @@ -937,6 +937,9 @@ Every control has a test ID: **999** buttons, text boxes, selects and textareas. - **controls** · `shared/components/controls.tsx` - `{testId}` · div · passed in by the parent - `{testId}-{value}` · button · passed in by the parent +- **DriverInstallHints** · `shared/components/DriverInstallHints.tsx` + - `driver-install-error` · p + - `driver-install-needs-admin` · p - **ErrorBoundary** · `shared/components/ErrorBoundary.tsx` - `app-error-boundary-dismiss` · button · Dismiss - **FilterPicker** · `shared/components/FilterPicker.tsx` @@ -1176,6 +1179,8 @@ Every control has a test ID: **999** buttons, text boxes, selects and textareas. - `sql-cred-refresh` · button · Refresh this server - `sql-install-driver-{connectionId}` · button in MissingDriverBanner · Install the driver package on the server - `sql-install-driver-{connectionId}-banner` · div in MissingDriverBanner + - `sql-pane-install-driver-{connectionId}` · button in MissingDriverBanner · Install the driver package on the server + - `sql-pane-install-driver-{connectionId}-banner` · div in MissingDriverBanner - `sql-pane-resize` · div · Drag to resize this table - `sql-pane-retry-{connectionId}` · button · Retry this server - `sql-result-{statementIndex}-{action}` · data @@ -1348,19 +1353,11 @@ Every control has a test ID: **999** buttons, text boxes, selects and textareas. - `sql-variable-value-input` · input · value - `sql-variables` · div - **StatementRunGutter** · `features/sql-editor/components/StatementRunGutter.tsx` + - `sql-statement-copy-{i}` · button - `sql-statement-gutter` · div · Statement run checks - `sql-statement-include-{i}` · input + - `sql-statement-label-{i}` · button - `sql-statement-run-{i}` · button -- **StatementStrip** · `features/sql-editor/components/StatementStrip.tsx` - - `sql-statement-cell-{i}` · div - - `sql-statement-copy` · button - - `sql-statement-copy-row-{i}` · button - - `sql-statement-include-{i}` · input - - `sql-statement-reveal-{i}` · button · In [ ]: - - `sql-statement-run-{i}` · button - - `sql-statement-strip` · div - - `sql-statement-strip-resize` · div · Resize statement list - - `sql-statement-values-popover` · div - **TableBlueprintModal** · `features/sql-editor/components/TableBlueprintModal.tsx` - `{testId}` · div · passed in by the parent - `{testId}-toggle` · button · passed in by the parent diff --git a/docs/user-guide.md b/docs/user-guide.md index d215fb698..6df7c3c49 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -306,8 +306,12 @@ compare / migrate). It lives in the same local web UI you open with `foxschema`. against every checked server (handy for comparing data across environments). Picking a connection in the **Schema explorer** checks that one alone, so Run hits the tables on screen; check more Destinations afterwards to fan out. -4. Type SQL in the editor. Multiple statements are fine; use the **statement strip** - under the editor to enable/disable individual statements before Run. Or **select** +4. Type SQL in the editor. Multiple statements are fine. The **run rail** on the + left of the editor gives each statement a row on its first line: its **In [n]** + number (the one the Run tooltip and `Out [n]` use — click it to select the + statement), a ▶ that runs that cell alone, and a checkbox that includes it in + **Run**. With Safe mode on, the number turns amber for a write Run will ask you + to confirm and red with `!` for an UPDATE or DELETE with no WHERE. Or **select** a statement (or any SQL) in the editor — Run becomes **Run selection** and only that text is executed (variables still expand). 5. Click **Run**. Results appear below, grouped by connection (stack or side-by-side). @@ -377,7 +381,8 @@ Tips: second statement is refused. Use the wrench to insert rebuild/reorg/optimize/REINDEX SQL when useful. On **Db2**, that SQL reorganises **every** index on the table (`REORG INDEXES ALL FOR TABLE` through `SYSPROC.ADMIN_CMD`) and quotes catalog - names (folded uppercase unless the table is a mixed-case delimited identifier). + names: the schema folded to uppercase, the table exactly as the catalog stores + it, so a table created as `"orders"` is not aimed at `ORDERS`. The index form opens under the selected index (no jump to a form at the bottom of the section). Index Management, Clone Table, and Query files live in **Utils** — see [Utilities](#utilities). @@ -431,8 +436,8 @@ Tips: ``` Typing `${{` / `${{name.` autocompletes names and table columns. Hover a ref for - its value (or `N×M table`). Hover a statement in the strip (when it uses vars) to - preview **query with values** and **Copy**. Statements in one Run are sequential + its value (or `N×M table`). Hover a statement's row on the run rail and click its + copy button to copy it with its values filled in (secrets stay masked). Statements in one Run are sequential so later SQL can use vars set by earlier `@set`. Multi-destination: `@set` uses the first successful server’s result. Substitution is local (values are pasted into the SQL text before send — not database bind parameters). Missing/empty @@ -562,8 +567,10 @@ Tips: transaction **per destination**. A failure rolls the batch back and leaves later statements unrun (the result row says so). Code cells and `-- @set` stay statement by statement — the checkbox then warns instead of wrapping. - Engines that auto-commit DDL still report each statement; the rollback is - best-effort there. This is separate from Data migrate's Transaction assist. + Engines that auto-commit DDL (MySQL, MariaDB, TiDB, Oracle, CockroachDB) cannot + undo DDL that already ran. Redis, MongoDB and ClickHouse cannot roll back at all, + so a multi-statement Run with One transaction is refused there — run it without. + This is separate from Data migrate's Transaction assist. Writes and DDL are allowed when you confirm them. **SQLite** connections are read-write (the file is opened that way on purpose). **ClickHouse** grid row @@ -692,8 +699,8 @@ On a multi-user install, **Profile → Access control** assigns Fox Schema that is **app** access, not GRANT on a connected database. What you may do to saved connections and schema in the current workspace also follows your **workspace** role there ([Workspaces](#workspaces)). Code cells, Git, -workflow and creating workspaces stay on the account role: owning a workspace -does not grant them. **Admins: One / Several**, **Members can create +workflow, creating workspaces and **Install drivers** stay on the account role: +owning a workspace does not grant them. **Admins: One / Several**, **Members can create workspaces** and **Workspace owners can invite new people** sit on the same panel. **Workspaces** (the tab) lists every workspace; an admin opens one only by joining. @@ -873,8 +880,11 @@ line). Stop it with `foxschema stop`. **Port already in use (Docker).** Change `PORT` in `.env` and restart (`docker compose -f docker-compose.app.yml up -d`). -**"driver not installed" for a database type.** Some drivers are optional/platform- -specific (notably IBM Db2 / `ibm_db`). Prefer: +**"driver not installed" for a database type.** The credential form, the SQL +editor's schema browser and a failed run all offer **Install driver**. Installing +runs npm on the server, so it needs the **Install drivers** permission (admins by +default); without it the banner gives the command to run instead. Some drivers are +optional/platform-specific (notably IBM Db2 / `ibm_db`). Prefer: ```bash foxschema drivers install db2 diff --git a/packages/server/src/features/connections/connection-tools.routes.test.ts b/packages/server/src/features/connections/connection-tools.routes.test.ts new file mode 100644 index 000000000..282c8def3 --- /dev/null +++ b/packages/server/src/features/connections/connection-tools.routes.test.ts @@ -0,0 +1,69 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * Who may install a driver. Checking one is for everyone signed in — the SQL + * editor asks on every failed run — but installing runs npm on the server. + */ +import { describe, expect, it } from 'vitest'; +import type { Permission } from '@foxschema/shared'; +import type { ConnectionModule } from '@foxschema/db'; +import type { AuthedRequest, Middleware } from '../../platform/http/types'; +import { fakeReply } from '../../platform/http/route-doubles'; +import type { ConnectionResolver } from '../../platform/connections/resolve'; +import { createConnectionToolRoutes } from './connection-tools.routes'; + +const routes = createConnectionToolRoutes({ + connectionModule: {} as ConnectionModule, + resolveRef: (async () => { + throw new Error('not used'); + }) as unknown as ConnectionResolver['resolveRef'], +}).flatten(); + +const route = (method: string, path: string) => { + const found = routes.find((r) => r.method === method && r.path === path); + if (!found) throw new Error(`${method} ${path} is not registered`); + return found; +}; + +/** What a route's guards answer `caller`, or 'passed' when they let it through. */ +async function guardsAnswer(middlewares: unknown[], caller: Partial) { + const { reply, sent } = fakeReply(); + for (const middleware of middlewares as Middleware[]) { + let passed = false; + await middleware(caller as AuthedRequest, reply, () => (passed = true)); + if (!passed) return sent; + } + return 'passed' as const; +} + +const caller = (appRole: 'viewer' | 'editor' | 'owner' | 'admin', permissions: Permission[] = []) => ({ + userId: `${appRole}-1`, + appRole, + permissions: new Set(permissions), +}); + +describe('driver routes', () => { + it('refuses an install from an account without admin.drivers, whatever its workspace role', async () => { + const install = route('POST', '/driver/install'); + for (const role of ['viewer', 'editor', 'owner'] as const) { + const sent = await guardsAnswer(install.middlewares, caller(role, ['editor.run', 'editor.ddl', 'workspace.settings'])); + expect(sent, role).not.toBe('passed'); + if (sent === 'passed') continue; + expect(sent.status, role).toBe(403); + expect(sent.body.missing, role).toEqual(['admin.drivers']); + } + }); + + it('lets an admin install, and an account granted admin.drivers', async () => { + const install = route('POST', '/driver/install'); + expect(await guardsAnswer(install.middlewares, caller('admin'))).toBe('passed'); + expect(await guardsAnswer(install.middlewares, caller('editor', ['admin.drivers']))).toBe('passed'); + }); + + it('leaves checking a driver open to everyone signed in', async () => { + const check = route('GET', '/driver/check'); + expect(await guardsAnswer(check.middlewares, caller('viewer'))).toBe('passed'); + }); +}); diff --git a/packages/server/src/features/connections/connection-tools.routes.ts b/packages/server/src/features/connections/connection-tools.routes.ts index ed6302095..21849c8f5 100644 --- a/packages/server/src/features/connections/connection-tools.routes.ts +++ b/packages/server/src/features/connections/connection-tools.routes.ts @@ -12,6 +12,7 @@ import type { AppRequest, AuthedRequest } from '../../platform/http/types'; import { Router } from '../../platform/http/router'; import { scopeOf } from '../../platform/http/scope'; import { sendError, sendThrown } from '../../platform/http/respond'; +import { requirePermissions } from '../../platform/authorization/rbac.guard'; import type { ConnectionRef, ConnectionResolver } from '../../platform/connections/resolve'; export function createConnectionToolRoutes(deps: { @@ -32,7 +33,10 @@ export function createConnectionToolRoutes(deps: { } }); - router.post('/driver/install', async (req: AppRequest, res: FastifyReply) => { + // Installing runs npm (and, for Db2, a native build) on the server, for every + // workspace. Any signed-in account could do it while the only button was in + // the credential form; the SQL editor now offers it on every failed run. + router.post('/driver/install', requirePermissions('admin.drivers'), async (req: AppRequest, res: FastifyReply) => { const { dialect } = (req.body ?? {}) as { dialect?: unknown }; // Without this, a missing dialect reached DriverDetector and came back as a // 500 — the caller's malformed request reported as a server fault. diff --git a/packages/server/src/internal/driver-install.test.ts b/packages/server/src/internal/driver-install.test.ts index caf5fe19f..bd2e3e477 100644 --- a/packages/server/src/internal/driver-install.test.ts +++ b/packages/server/src/internal/driver-install.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest'; import { + createInstallQueue, DRIVER_NPM_INSTALL_ENV, driverInstallHints, ibmDbPackageDir, @@ -77,3 +78,57 @@ describe('isCacheKeyForPackage — match a path segment, not a substring', () => expect(() => purgePackageRequireCache('definitely-not-installed-xyz')).not.toThrow(); }); }); + +describe('createInstallQueue — one npm at a time, one run per package', () => { + /** A run that finishes when the test says so, recording when it started. */ + function gated(log: string[], name: string) { + let finish!: (value: string) => void; + const done = new Promise((resolve) => (finish = resolve)); + const run = async () => { + log.push(`start ${name}`); + const value = await done; + log.push(`end ${name}`); + return value; + }; + return { run, finish }; + } + + it('shares one run between two requests for the same package', async () => { + const queue = createInstallQueue(); + const log: string[] = []; + const redis = gated(log, 'redis'); + const first = queue('redis', redis.run); + const second = queue('redis', async () => { + log.push('second run'); + return 'second'; + }); + expect(second).toBe(first); + redis.finish('installed'); + expect(await second).toBe('installed'); + expect(log).toEqual(['start redis', 'end redis']); + }); + + it('starts another package only after the running one ends', async () => { + const queue = createInstallQueue(); + const log: string[] = []; + const pg = gated(log, 'pg'); + const redis = gated(log, 'redis'); + const a = queue('pg', pg.run); + const b = queue('redis', redis.run); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(log).toEqual(['start pg']); + pg.finish('pg ok'); + await a; + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(log).toEqual(['start pg', 'end pg', 'start redis']); + redis.finish('redis ok'); + expect(await b).toBe('redis ok'); + }); + + it('moves on after a failed run, and runs the package again when asked later', async () => { + const queue = createInstallQueue(); + await expect(queue('pg', async () => Promise.reject(new Error('npm exited 1')))).rejects.toThrow('npm exited 1'); + expect(await queue('redis', async () => 'redis ok')).toBe('redis ok'); + expect(await queue('pg', async () => 'pg ok')).toBe('pg ok'); + }); +}); diff --git a/packages/server/src/internal/driver-install.ts b/packages/server/src/internal/driver-install.ts index f95d5c3a4..c526db95f 100644 --- a/packages/server/src/internal/driver-install.ts +++ b/packages/server/src/internal/driver-install.ts @@ -382,11 +382,45 @@ export function verifyInstalledDriver(packageName: string): VerifyDriverResult { return { ok: true }; } +/** + * One npm at a time, and one run per package. + * + * Every install writes the same node_modules, and two `npm install` processes + * in one tree fail with ENOTEMPTY / EEXIST or leave a package half-written. + * The SQL editor shows an Install button on the schema browser and on every + * failed run, so two clicks — or two people — asking at once is ordinary. A + * second request for a package already installing shares that run; a request + * for another package waits its turn. + */ +export function createInstallQueue(): (key: string, run: () => Promise) => Promise { + const running = new Map>(); + let tail: Promise = Promise.resolve(); + return (key, run) => { + const existing = running.get(key); + if (existing) return existing; + const job = tail.then(() => run()).finally(() => running.delete(key)); + // The queue moves on whether this run succeeded or not. + tail = job.catch(() => undefined); + running.set(key, job); + return job; + }; +} + +const installQueue = createInstallQueue(); + /** * Install + verify. For ibm_db, retries once with a forced reinstall when the * package landed without clidriver (common when npm_config_ignore_scripts is set). + * Queued: see {@link createInstallQueue}. */ -export async function installAndVerifyDriver( +export function installAndVerifyDriver( + packageName: string, + versionPin?: string +): Promise { + return installQueue(packageName, () => runInstallAndVerify(packageName, versionPin)); +} + +async function runInstallAndVerify( packageName: string, versionPin?: string ): Promise { diff --git a/packages/shared/src/permissions.test.ts b/packages/shared/src/permissions.test.ts index 92d5d9802..cb39453fd 100644 --- a/packages/shared/src/permissions.test.ts +++ b/packages/shared/src/permissions.test.ts @@ -171,6 +171,18 @@ describe('effectivePermissions — install keys from the account, workspace keys expect(INSTALL_PERMISSIONS.has('workspace.create')).toBe(true); }); + it('keeps admin.drivers admin-only, and out of reach of a workspace role', () => { + // Installing a driver runs npm on the server for every workspace. + for (const role of ['viewer', 'editor', 'owner'] as const) { + expect(DEFAULT_ROLE_PERMISSIONS[role], role).not.toContain('admin.drivers'); + } + expect(DEFAULT_ROLE_PERMISSIONS.admin).toContain('admin.drivers'); + expect(INSTALL_PERMISSIONS.has('admin.drivers')).toBe(true); + expect(effectivePermissions(DEFAULT_ROLE_PERMISSIONS.viewer, [...DEFAULT_ROLE_PERMISSIONS.owner, 'admin.drivers'])).not.toContain( + 'admin.drivers' + ); + }); + it('describes every install permission in the catalog', () => { for (const p of INSTALL_PERMISSIONS) expect(PERMISSION_META.some((m) => m.id === p)).toBe(true); }); diff --git a/packages/shared/src/permissions.ts b/packages/shared/src/permissions.ts index c11db3dcf..bace7a2aa 100644 --- a/packages/shared/src/permissions.ts +++ b/packages/shared/src/permissions.ts @@ -80,6 +80,7 @@ export const PERMISSIONS = [ // Administration 'admin.users', 'admin.roles', + 'admin.drivers', ] as const; export type Permission = (typeof PERMISSIONS)[number]; @@ -143,6 +144,7 @@ export const PERMISSION_META: PermissionMeta[] = [ { id: 'workspace.create', group: 'Workspace', label: 'Create workspaces', description: 'Create workspaces besides your own. Install-wide: comes from the account role, not a workspace role.' }, { id: 'admin.users', group: 'Admin', label: 'Manage users', description: 'List FoxSchema logins, assign app roles, and activate or deactivate accounts. Not the same as database users on a connected server.' }, { id: 'admin.roles', group: 'Admin', label: 'Configure roles', description: 'Edit which FoxSchema permissions each app role receives, including Grant privileges for database GRANT/REVOKE.' }, + { id: 'admin.drivers', group: 'Admin', label: 'Install drivers', description: 'Install a database driver package on the server (npm install, and a native build for Db2). It downloads and runs code on the machine Fox Schema runs on, for every workspace. Install-wide: comes from the account role, not a workspace role.' }, ]; const ALL = [...PERMISSIONS]; @@ -275,8 +277,9 @@ export function isWorkspaceRole(value: unknown): value is WorkspaceRole { /** * Permissions that belong to the install rather than to a workspace, so they * come from the account role. `editor.advanced` runs code on the server: - * owning a workspace must never grant it. Workflows and Git repositories are - * install-wide, and so is managing accounts and creating workspaces. + * owning a workspace must never grant it, nor `admin.drivers`, which installs + * packages on it. Workflows and Git repositories are install-wide, and so is + * managing accounts and creating workspaces. */ export const INSTALL_PERMISSIONS: ReadonlySet = new Set([ 'editor.advanced', @@ -288,6 +291,7 @@ export const INSTALL_PERMISSIONS: ReadonlySet = new Set( 'git.manage', 'admin.users', 'admin.roles', + 'admin.drivers', 'workspace.create', ]); diff --git a/packages/sql/src/providers/DIALECTS.md b/packages/sql/src/providers/DIALECTS.md index 89c18d630..00b74b447 100644 --- a/packages/sql/src/providers/DIALECTS.md +++ b/packages/sql/src/providers/DIALECTS.md @@ -121,9 +121,13 @@ Each dialect lives in `providers//.sql-dialect.ts`. (`REORG INDEX name` → SQL0270N reason 89). Defrag emits `CALL SYSPROC.ADMIN_CMD('REORG INDEXES ALL FOR TABLE …')` then `RUNSTATS` (`providers/db2/db2.index-fragmentation.ts`). Quote names as `SYSCAT` stores - them: fold the schema (and a single-case table) to uppercase before quoting; - keep mixed case on a delimited table name. Quoting the typed case - (`"demo_a"."ORDERS"`) is SQL2211N for a table the probe just listed. + them: fold the **schema** to uppercase (it is what the user typed in the + connection, and schema browse looks it up with `toUpperCase()`), but quote the + **table exactly as given** — every caller passes the name schema browse read + from `SYSCAT.TABLES`. Do not fold the table, single-case or not: a table + created as `"orders"` would be aimed at `ORDERS` — SQL2211N if there is none, + someone else's table if there is (`db2ReorgTarget`). Quoting the typed schema + case (`"demo_a"."ORDERS"`) is SQL2211N for a table the probe just listed. - `modifyColumnStatements` — `SET DATA TYPE` can't carry nullability; emit a **separate** `SET NOT NULL` / `DROP NOT NULL`. - `dropIndexStatement` / `dropTriggerStatement` schema-prefixed, no `ON table`.