From fc3ce6d1b7aaf011ac53c3272637e74ba4ee6b1b Mon Sep 17 00:00:00 2001 From: huyplb Date: Sun, 25 Oct 2026 00:59:26 -0600 Subject: [PATCH 1/3] fix(sql-editor): Install driver needs a permission, and every button shares one install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #542 put Install driver on the schema browser and on every failed run. The route behind it was open to any signed-in account, and it runs npm (a native build for Db2) on the server for every workspace. - New install-wide permission `admin.drivers` (admins by default; a workspace role cannot lend it). POST /driver/install requires it. Without it the banner and the credential form show the command to run instead of a button. - The server runs one npm at a time, and a second request for a package that is installing shares that run: two installs in one node_modules fail or leave it half-written. - One driver status for every Install button (`shared/lib/driverStatus`): an install from one shows on all, a second click joins it, and a finished one refreshes every dialect on the package (MySQL and MariaDB share mysql2). - The banner appears only for the server's own "Database driver … is not installed" message, not any text saying "is not installed" (SQL Server's Full-Text Search error, a missing Postgres extension), and names the package from it instead of "driver". - The banner and the credential form render the same hints (`DriverInstallHints`): why it does not load, the command, Db2's steps. - Side by side — the layout Compare data needs — offers Install too. Co-Authored-By: Claude Opus 5.5 --- .../components/ConnectionModal.test.tsx | 39 ++++- .../components/ConnectionModal.tsx | 82 +++-------- .../components/MissingDriverBanner.test.tsx | 122 +++++++++++++--- .../components/MissingDriverBanner.tsx | 112 ++++++--------- .../components/ResultsPanel.driver.test.tsx | 51 +++++++ .../sql-editor/components/ResultsPanel.tsx | 13 +- .../shared/components/DriverInstallHints.tsx | 71 +++++++++ .../frontend/shared/lib/driverStatus.test.ts | 117 +++++++++++++++ .../src/frontend/shared/lib/driverStatus.ts | 135 ++++++++++++++++++ .../connection-tools.routes.test.ts | 69 +++++++++ .../connections/connection-tools.routes.ts | 6 +- .../src/internal/driver-install.test.ts | 55 +++++++ .../server/src/internal/driver-install.ts | 36 ++++- packages/shared/src/permissions.test.ts | 12 ++ packages/shared/src/permissions.ts | 8 +- 15 files changed, 771 insertions(+), 157 deletions(-) create mode 100644 apps/web/src/frontend/features/sql-editor/components/ResultsPanel.driver.test.tsx create mode 100644 apps/web/src/frontend/shared/components/DriverInstallHints.tsx create mode 100644 apps/web/src/frontend/shared/lib/driverStatus.test.ts create mode 100644 apps/web/src/frontend/shared/lib/driverStatus.ts create mode 100644 packages/server/src/features/connections/connection-tools.routes.test.ts 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/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/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', ]); From ddfc169d2a49fef1b915e8788319213fd5e2e661 Mon Sep 17 00:00:00 2001 From: huyplb Date: Sun, 25 Oct 2026 00:59:26 -0600 Subject: [PATCH 2/3] fix(sql-editor): the run rail says what the statement strip did MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #542 replaced the strip under the editor with a rail of play buttons and checkboxes, and dropped what the strip said about each statement. - Each row shows its In [n] number — the one the Run tooltip ("In [4] at the caret") and Out [n] use, which nothing showed any more. Clicking it selects the statement. - With Safe mode on the number turns amber for a write Run will confirm, and red with `!` for an UPDATE or DELETE with no WHERE. The tooltip names the cell's kind. Badges are cached by statement text, as the strip did. - A copy button on the row (shown on hover) copies the statement with its variables filled in, secrets masked. The slim Monaco build has no context menu, so it lives on the row, as it did on the strip. - Statements starting on one line (`SELECT 1; SELECT 2;`) or inside a fold no longer draw their controls on top of each other: each row stacks below the one before. A long statement's row stays at the top while its first line is scrolled away. - StatementStrip and its test were left unused; deleted, with SqlEditorPane's dead `reveal` prop. The test-ID catalog no longer lists the strip's IDs. - Samples and comments no longer point at "the strip". Co-Authored-By: Claude Opus 5.5 --- apps/e2e/scripts/run-all.mjs | 1 + apps/e2e/src/generated/test-ids.ts | 45 +- .../e2e/src/tests/sql-editor-run-rail.test.ts | 80 ++++ .../sql-editor/components/SqlEditorPane.tsx | 35 +- .../sql-editor/components/SqlEditorView.tsx | 2 +- .../components/StatementRunGutter.test.tsx | 178 +++++-- .../components/StatementRunGutter.tsx | 223 ++++++--- .../components/StatementStrip.test.tsx | 29 -- .../sql-editor/components/StatementStrip.tsx | 443 ------------------ .../sql-editor/lib/sqlEditorBridge.ts | 2 +- .../sql-editor/lib/sqlEditorSamples.ts | 6 +- .../sql-editor/lib/statementBadges.test.ts | 57 +++ .../sql-editor/lib/statementBadges.ts | 65 +++ .../sql-editor/state/sqlEditorTabLogic.ts | 4 +- .../sql-editor/state/useSqlEditorStore.ts | 8 +- docs/testing/TEST_IDS.md | 19 +- 16 files changed, 564 insertions(+), 633 deletions(-) create mode 100644 apps/e2e/src/tests/sql-editor-run-rail.test.ts delete mode 100644 apps/web/src/frontend/features/sql-editor/components/StatementStrip.test.tsx delete mode 100644 apps/web/src/frontend/features/sql-editor/components/StatementStrip.tsx create mode 100644 apps/web/src/frontend/features/sql-editor/lib/statementBadges.test.ts create mode 100644 apps/web/src/frontend/features/sql-editor/lib/statementBadges.ts 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/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/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 From 4c610305380c0c37a79fe5bd46834e9d12a6ed13 Mon Sep 17 00:00:00 2001 From: huyplb Date: Sun, 25 Oct 2026 00:59:38 -0600 Subject: [PATCH 3/3] docs: the run rail, Install drivers, and Db2 table names as the catalog has them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - DIALECTS.md and the user guide said a Db2 reorg folds a single-case table name to uppercase. The code deliberately does not (`db2ReorgTarget`): only the schema is folded, and the table is quoted as SYSCAT stores it — folding would aim a table created as "orders" at ORDERS. DIALECTS.md is required reading before touching dialect code, so following it would have reintroduced the bug #535 fixed. - The user guide described the statement strip and hovering it to copy a query with values; it now describes the run rail and its copy button. - One transaction: Redis, MongoDB and ClickHouse refuse it rather than rolling back "best-effort". - Install driver needs the Install drivers permission; release notes for both changes. Co-Authored-By: Claude Opus 5.5 --- .cursor/rules/sql-editor-agent-memory.mdc | 2 +- docs/ARCHITECTURE.md | 2 +- docs/releases/UNRELEASED.md | 19 +++++++++++++- docs/user-guide.md | 32 +++++++++++++++-------- packages/sql/src/providers/DIALECTS.md | 10 ++++--- 5 files changed, 48 insertions(+), 17 deletions(-) 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/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/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/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`.