diff --git a/apps/web/src/frontend/features/access/components/controls.tsx b/apps/web/src/frontend/features/access/components/controls.tsx index 794f3270..66566d89 100644 --- a/apps/web/src/frontend/features/access/components/controls.tsx +++ b/apps/web/src/frontend/features/access/components/controls.tsx @@ -8,12 +8,18 @@ */ import React from 'react'; import type { PermissionRisk } from '../lib/access'; +import { sectionLabelCls } from '@/shared/components/surfaces'; export const inputCls = 'w-full rounded-md border border-slate-700 bg-slate-950 px-2.5 py-1.5 text-[12px] text-slate-100 outline-none accent-focus'; /** The micro-label above a control. Exported because two panels had their own copy. */ -export const labelCls = 'text-[10px] font-bold uppercase tracking-wide text-slate-500'; +/** + * Re-exported rather than respelled: this string is the same micro-label the + * rest of the app uses, and it drifted into eight variants precisely because + * each feature kept its own copy. + */ +export const labelCls = sectionLabelCls; export const RISK_STYLE: Record = { low: 'text-emerald-300 border-emerald-500/40 bg-emerald-500/10', diff --git a/apps/web/src/frontend/features/lokee-weave/components/VersionChangeChart.tsx b/apps/web/src/frontend/features/lokee-weave/components/VersionChangeChart.tsx index 4d3fe1bf..cfebd93a 100644 --- a/apps/web/src/frontend/features/lokee-weave/components/VersionChangeChart.tsx +++ b/apps/web/src/frontend/features/lokee-weave/components/VersionChangeChart.tsx @@ -8,6 +8,7 @@ */ import React from 'react'; import type { TimelineVersion } from './VersionTimeline'; +import { StatCard } from '@/shared/components/surfaces'; const CHART_H = 180; const BAR_W = 18; @@ -108,16 +109,32 @@ export function VersionBriefing({ const removed = selected.removed ?? 0; return (
- v{selected.number} - - {selected.name || selected.source || 'Snapshot'} - - +{added} - ~{modified} - −{removed} +
+ v{selected.number} + + {selected.name || selected.source || 'Snapshot'} + +
+ {/* Counts as cards, not a run-on of coloured numbers: the sign alone + ("+3 ~2 −0") makes the reader supply the nouns. */} +
+ + + +
); } diff --git a/apps/web/src/frontend/features/sql-editor/components/PeekInsight.tsx b/apps/web/src/frontend/features/sql-editor/components/PeekInsight.tsx index b25fdeed..cfde9059 100644 --- a/apps/web/src/frontend/features/sql-editor/components/PeekInsight.tsx +++ b/apps/web/src/frontend/features/sql-editor/components/PeekInsight.tsx @@ -11,6 +11,7 @@ import { Loader2 } from 'lucide-react'; import { fetchTableInsight, type TableInsightResponse } from '@/shared/api/schemaApi'; import { tableNameParts } from '@/shared/lib/tablePreview'; import { useSqlEditorStore } from '@/app/store/useSqlEditorStore'; +import { StatCard } from '@/shared/components/surfaces'; function tableRef(tableName: string, fallbackSchema?: string): { table: string; schema?: string } { const parts = tableNameParts(tableName); @@ -98,48 +99,34 @@ export const PeekInsight: React.FC<{ className="grid grid-cols-3 gap-2 mb-3" data-testid="data-peek-insight-cards" > -
-

Rows

-

- {data.estimatedRows == null ? '—' : data.estimatedRows.toLocaleString()} -

-

Estimated from catalog

-
-
-

- Null-heavy -

-

- {cards.nullHeavy.length === 0 + + c.name).join(', ')} -

-

- Avg null {pct(cards.avgNull)} -

-
-
-

- High distinct -

-

- {cards.distinctHeavy.length === 0 + : cards.nullHeavy.map((c) => c.name).join(', ') + } + hint={`Avg null ${pct(cards.avgNull)}`} + /> + `${c.name} (${c.nDistinct})`) - .join(', ')} -

-

Top nDistinct columns

-
+ : cards.distinctHeavy.map((c) => `${c.name} (${c.nDistinct})`).join(', ') + } + hint="Top nDistinct columns" + />

diff --git a/apps/web/src/frontend/shared/components/surfaces.test.tsx b/apps/web/src/frontend/shared/components/surfaces.test.tsx new file mode 100644 index 00000000..f9b414fd --- /dev/null +++ b/apps/web/src/frontend/shared/components/surfaces.test.tsx @@ -0,0 +1,82 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * What the shared surfaces guarantee to the screens built out of them. + * + * These are presentation primitives, so the useful assertions are the ones a + * redesign could break silently: that a tone still reaches the value, that the + * provenance line is optional rather than rendered empty, and that every + * caller's label comes out spelled the same way — the drift that motivated + * extracting them in the first place. + */ +import { describe, expect, it } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import { SectionLabel, StatCard, Panel, sectionLabelCls } from './surfaces'; + +describe('SectionLabel', () => { + it('is one spelling, so two callers cannot drift apart', () => { + const { container } = render( + <> + Rows + Null-heavy + + ); + const classes = [...container.querySelectorAll('p')].map((p) => p.className); + expect(new Set(classes).size).toBe(1); + expect(classes[0]).toBe(sectionLabelCls); + }); + + it('adds caller classes without dropping its own', () => { + render(Scope); + const el = screen.getByText('Scope'); + expect(el.className).toContain('mb-1'); + expect(el.className).toContain('uppercase'); + }); +}); + +describe('StatCard', () => { + it('carries the tone to the value, not the label', () => { + render(); + expect(screen.getByText('email').className).toContain('text-amber-200'); + expect(screen.getByText('Null-heavy').className).not.toContain('text-amber-200'); + }); + + it('defaults to the neutral tone', () => { + render(); + expect(screen.getByText('2.4M').className).toContain('text-slate-100'); + }); + + it('renders no hint line at all when there is no provenance to give', () => { + // Not an empty

: a blank line under the number reads as a value that + // failed to load, which is the opposite of "this figure needs no caveat". + const { container } = render(); + expect(container.querySelectorAll('p')).toHaveLength(2); + }); + + it('keeps the hint when one is given', () => { + render(); + expect(screen.getByText('Estimated from catalog')).toBeTruthy(); + }); + + it('accepts a zero value rather than treating it as absent', () => { + // `−0 Removed` is a real answer on a snapshot briefing, and a falsy check + // here would blank it. + render(); + expect(screen.getByTestId('zero').textContent).toContain('0'); + }); +}); + +describe('Panel', () => { + it('can drop its padding for callers that own their own spacing', () => { + const { rerender } = render(x); + expect(screen.getByTestId('p').className).toContain('px-2.5'); + rerender( + + x + + ); + expect(screen.getByTestId('p').className).not.toContain('px-2.5'); + }); +}); diff --git a/apps/web/src/frontend/shared/components/surfaces.tsx b/apps/web/src/frontend/shared/components/surfaces.tsx new file mode 100644 index 00000000..d99a41bd --- /dev/null +++ b/apps/web/src/frontend/shared/components/surfaces.tsx @@ -0,0 +1,80 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * The three surfaces every screen in this app is built out of. + * + * They were extracted by counting, not by taste. The frontend held 85 distinct + * card/panel class strings, and the small uppercase label above a group came in + * eight spellings — `text-[10px]` and `text-[11px]`, `tracking-wide` and + * `tracking-wider`, `text-slate-400` and `text-slate-500` — across some seventy + * uses. None of that variation meant anything; it is what a screen looks like + * when each one is written on its own. + * + * `labelCls` in the access feature already said this once, but it lives inside + * a feature, so nothing else could reach it without crossing a boundary the + * architecture test forbids. Shared is where a primitive every feature needs + * belongs. + */ +import React from 'react'; + +/** The uppercase micro-label that titles a group. One spelling, everywhere. */ +export const sectionLabelCls = 'text-[10px] font-bold uppercase tracking-wide text-slate-500'; + +/** The app's card treatment: one border, one ground, one radius. */ +export const panelCls = 'rounded-lg border border-slate-800 bg-slate-950/50'; + +export const SectionLabel: React.FC<{ + children: React.ReactNode; + className?: string; +}> = ({ children, className }) => ( +

{children}

+); + +export const Panel: React.FC<{ + children: React.ReactNode; + /** Extra classes; padding is included by default so callers rarely need it. */ + className?: string; + padded?: boolean; + testId?: string; +}> = ({ children, className, padded = true, testId }) => ( +
+ {children} +
+); + +/** + * How much of a thing there is, and what that number came from. + * + * Label, value, and a line of provenance under it — the shape Peek Insight and + * the snapshot briefing had each grown separately. The hint is not decoration: + * a row count read from a catalog estimate and one read by counting are + * different claims, and the card is where that gets said. + */ +export type StatTone = 'default' | 'positive' | 'warning' | 'danger' | 'info'; + +const TONE_CLS: Record = { + default: 'text-slate-100', + positive: 'text-emerald-300', + warning: 'text-amber-200', + danger: 'text-rose-300', + info: 'text-sky-200', +}; + +export const StatCard: React.FC<{ + label: React.ReactNode; + value: React.ReactNode; + hint?: React.ReactNode; + tone?: StatTone; + testId?: string; +}> = ({ label, value, hint, tone = 'default', testId }) => ( + + {label} +

{value}

+ {hint != null &&

{hint}

} + +); diff --git a/docs/plans/excel-and-sheets-import.md b/docs/plans/excel-and-sheets-import.md new file mode 100644 index 00000000..8b22949f --- /dev/null +++ b/docs/plans/excel-and-sheets-import.md @@ -0,0 +1,196 @@ +# Excel and Google Sheets import — plan + +Status: proposal. Nothing implemented. + +Written after the streaming work in #204 (CSV) and #207 (NDJSON), and it assumes +those readers are the shape new formats plug into. + +## These are two different problems + +They get asked for together, so they look like one feature. They are not. + +| | Excel (.xlsx) | Google Sheets | +| --- | --- | --- | +| what it is | a file format | a remote API | +| hard part | parsing without loading it all | **authentication** | +| needs a dependency | yes (ZIP + XML) | an HTTP client, or nothing | +| data ceiling | 1,048,576 rows × 16,384 cols *per sheet* | **10 million cells per spreadsheet, total** | +| offline | yes | no | +| fits the existing upload flow | yes, unchanged | no — there is no file | + +Excel is a natural extension of what exists. Google Sheets is an integration +feature wearing a file-import costume, and most of its work is OAuth, not +parsing. + +**Sequence them separately.** Doing Excel first also makes Sheets cheaper, +because "export to xlsx and import that" becomes a real answer for many users. + +--- + +## Part 1 — Excel (.xlsx) + +### What the format actually is + +A ZIP archive of XML parts: + +``` +xl/workbook.xml sheet names + ids +xl/worksheets/sheet1.xml cells, by row +xl/sharedStrings.xml the string table +xl/styles.xml number formats — this is how dates are stored +``` + +Three consequences that decide the design: + +**1. `sharedStrings.xml` is the memory problem, not the sheet.** In a typical +export nearly every text cell is an *index* into one shared string table. A +sheet can be streamed row by row; the string table it points into cannot, +because a row near the end may reference index 3. For a large workbook this +single part can be hundreds of MB, and it lands in memory whatever else is +streamed. Plan for it — it is the ceiling. + +*(Inline strings — `t="inlineStr"` — avoid this, but exporters rarely emit them.)* + +**2. Dates are numbers.** `45000` is a date only because `styles.xml` says its +format code is a date format. Get this wrong and every date column imports as +an integer. Worse, there are two epochs (1900 on Windows, 1904 on old Mac +files) and the well-known 1900 leap-year bug. This is where naive importers +lose data silently, and it will need explicit tests. + +**3. Formulas have two values.** A cell carries the formula *and* its cached +result. Import the cached value; a file saved without cached values (rare, but +it happens) has to degrade to null rather than to the formula text. + +Also decide up front, because each is a visible product question: + +- **which sheet?** A workbook has many. Import the first, prompt, or import all + as separate tables? +- **where does the header start?** Real spreadsheets have title rows, blank + rows, and merged banner cells above the actual header. +- **merged cells** — value belongs to the top-left cell; the rest are empty. +- **.xls (pre-2007)** is a completely different binary format. Recommend + declaring it out of scope and saying so in the UI, rather than half-supporting it. + +### Options + +**A. `exceljs` streaming reader** — `stream.xlsx.WorkbookReader` yields rows +without materialising the workbook. Mature, handles styles/dates/shared +strings. Cost: a real dependency tree, and CI runs `npm audit` plus a +backdoor scan on every build. + +**B. DuckDB `read_xlsx`** — *already a dependency* (`@duckdb/node-api` 1.5.4, +registered as a dialect). Zero new supply-chain surface, and DuckDB would do +the type inference and batching itself. **Unverified**: I could not test it +here because the native binding is broken in this checkout (see below), and +the `excel` extension may need to be downloadable at runtime, which is a +problem for offline/desktop installs. + +**C. Hand-roll** — needs a ZIP reader (Node has zlib but no archive reader) +plus streaming XML. Not worth it; this is the one place a library earns its +keep. + +**Recommendation: validate B, fall back to A.** B is strictly better if it +works, because it costs nothing new. That validation is a half-day spike, not +a guess, and it must cover: extension availability offline, date handling, +and memory on a large file. + +> **Blocker found while checking:** `@duckdb/node-bindings-darwin-arm64` is +> missing `libduckdb.dylib` in this checkout, so `require('@duckdb/node-bindings')` +> throws `ERR_DLOPEN_FAILED`. That breaks the **DuckDB dialect at runtime**, +> independently of Excel. Worth fixing or confirming it is local-only before +> building anything on DuckDB. + +### Work, in order + +1. **Spike DuckDB** (half day): can it read a real .xlsx offline, with correct + dates, without loading the whole file? Decide B vs A on the result. +2. **Reader** behind the same seam as `CsvStreamReader` — emit rows in + batches, feed the existing batched writer. +3. **Sheet + header selection** in the import UI. +4. **Date/number/boolean coercion**, with tests for both epochs, the 1900 leap + bug, and format-code detection. +5. **Capacity**: extend `importCapacity()` — xlsx compresses ~10:1, so a + 40 MB upload can be a 400 MB sheet. The current byte-based estimate will + be wrong for it, and the message should say so. + +--- + +## Part 2 — Google Sheets + +### The scale conversation is different + +Google caps a spreadsheet at **10 million cells**. A maxed-out sheet is a few +hundred MB of JSON, not 1 GB, and the API is the bottleneck long before the +parser is. Nothing here needs the streaming work. + +### Two paths, an order of magnitude apart in cost + +**Path A — export URL (recommended first).** A published sheet exposes a CSV +endpoint: + +``` +https://docs.google.com/spreadsheets/d//gviz/tq?tqx=out:csv&sheet= +``` + +The user pastes a link; the server fetches it and runs it through the **CSV +path that already exists**. No OAuth, no tokens, no consent screen, and it +works today with a URL field and a fetch. + +Limits worth stating plainly in the UI: the sheet must be shared +("anyone with the link" or published), and it is a snapshot, not a live +connection. + +**Path B — Sheets API v4 with OAuth.** Needed only for private sheets or +scheduled refresh: + +- register an OAuth client, ship a client id, host a redirect +- consent screen; Google **verification** if distributed publicly, which is a + process with a review, not a checkbox +- refresh-token storage — the secrets vault already exists for this +- quotas: 300 req/min/project, 60/min/user; paging by A1 range +- desktop vs web need different OAuth flows (loopback vs hosted redirect) + +That is a feature with an ongoing compliance surface, not an afternoon. + +**Recommendation: ship Path A, and only build Path B if users actually ask for +private or scheduled access.** Path A covers "I have a sheet, get it into my +database" — which is the request most people mean. + +### SSRF, if Path A is built + +Fetching a user-supplied URL server-side is a classic SSRF vector, and this +server holds decrypted database credentials. Any implementation must: + +- allow **only** `https://docs.google.com/…` — an allowlist, not a denylist +- refuse redirects off that host +- cap response size using `importCapacity()` +- set a timeout, and never surface the raw response body on error + +This is the part to get right; the parsing is free. + +--- + +## What to settle first + +Both formats inherit two open questions from the streaming work, and each new +format multiplies the cost of getting them wrong: + +1. **Type inference sample size.** Inference currently reads a whole column. + Streaming must sample — and a column that looks INTEGER for 1,000 rows but + holds text at row 900,000 will be typed wrong. +2. **Headerless column naming.** Columns are named from the widest row, which + is only known at the end. + +Answer those, wire CSV and NDJSON through the streaming path, and Excel plugs +into a proven seam instead of a hypothetical one. + +## Suggested order + +| # | Step | Why here | +| --- | --- | --- | +| 1 | Settle inference sampling + headerless naming | Everything downstream inherits it | +| 2 | Wire CSV/NDJSON streaming into the write path | Proves the seam end to end | +| 3 | Google Sheets via export URL | Cheapest real win; reuses the CSV path | +| 4 | DuckDB xlsx spike → pick B or A | Decides the dependency question with evidence | +| 5 | Excel reader on the streaming seam | The actual format work | +| 6 | Sheets OAuth | Only if private/scheduled access is genuinely wanted | diff --git a/docs/plans/restructure.md b/docs/plans/restructure.md new file mode 100644 index 00000000..7d67bca1 --- /dev/null +++ b/docs/plans/restructure.md @@ -0,0 +1,318 @@ +# Restructure plan — splitting the app as it grows + +Status: steps 1 and 2 in progress — see Sequencing. Corrections from implementation are +marked inline and dated. + +## What the numbers say + +Re-measured on `main` at **0.2.34** (the 0.2.16 figures this plan was written against are +superseded; the note under them about directory boundaries was wrong — see Proposal 1): + +| Area | Lines | Note | +| --- | --- | --- | +| `packages/core/src/providers` | 7,096 (60 files) | **Mixed**, not Node-only: dialects+settings are pure, adapters+providers are not | +| `packages/core/src/modules` | 10,044 (37 files) | Mostly pure: splitter, dialects, generator, compare | +| `packages/core/src/cores` | 1,026 (8 files) | Mixed: connection-string pure, factory/pool/detector Node | +| `packages/core/src/interfaces` | 338 (5 files) | Types — all pure | +| `apps/web/src/backend` | 10,702 (73 files) | `routes.ts` alone was **838** | +| `apps/web/src/frontend` | 36,766 (140 files) | | +| `apps/cli/src` | 5,854 (76 files) | | + +The decisive measurement was not size but **reachability**: the transitive import closure +of `browser.ts` is 55 files / 9,222 lines and pulls in **zero** `node:` built-ins, while +`index.ts` reaches 89 files / 14,977 lines and pulls in `node:fs`, `node:path`, +`node:module`. That is what made membership decidable per file. + +**Core has zero runtime dependencies.** Drivers are loaded dynamically and declared +nowhere. So "core is heavy" is not about install weight — it is about *scope*: one +package holds both pure string logic and the entire database driver layer, behind two +entry points (`browser.ts`, 103 export lines; `index.ts`). + +A symptom found while measuring, now fixed: `apps/web` aliased `@foxschema/core` to +`browser.ts` in `vite.config.ts` but to `index.ts` in `tsconfig.json`, so the typechecker +and the bundler disagreed about what the name meant. Nothing was broken by it at the time +— every frontend import happened to exist in both — but it is why `npx vitest run` from +`apps/web` left `ConnectionFactory` undefined. + +## The contradiction to resolve first + +Two requirements were stated together: + +1. core should "handle the backend role — play with the database, execute and query" +2. core should be "lightweight so everyone can import it" + +**These cannot both hold in one package.** (1) needs Node, drivers, sockets, pooling. +(2) needs browser-safe, zero-dep, tree-shakeable. The current `browser.ts` / `index.ts` +dual entry is a workaround for exactly this tension, and it is already leaking: + +- the frontend cannot import `@foxschema/core` directly — it goes through a Vite alias + to `browser.ts` plus mirror files in `apps/web/src/frontend/lib/` +- FoxFlow needs a hand-written `types/foxschema-core.d.ts` stub because `tsc` would + otherwise type-check core's source under stricter flags +- under `tsx`, a bare-specifier import of core resolves to an **empty namespace** + +Every one of those is a symptom of one package trying to be two. + +## Proposal 1 — split core along the seam that already exists + +`browser.ts` is already an exact manifest of "the pure half". Promote it to a package. + +``` +@foxschema/sql pure, browser-safe, zero deps (73 files) + splitter · sql-template · dialects · type-mapping + compare · sql-generator · code-cell-exec · interfaces + per-dialect: *.sql-dialect.ts · *.settings.ts · *.connection.ts + +@foxschema/db Node only, depends on @foxschema/sql (37 files) + connection-factory · driver-detector · pool-cache + connection.module · migration.module + per-dialect: *.adapter.ts · *.provider.ts +``` + +**Correction (implemented 2026-08-05).** The line above that read +"`providers/*` → db, `modules/*` → sql" was wrong, and it is the one thing that +would have made this look mechanical when it is not. The seam does not follow +directory boundaries — **every dialect directory contributes to both packages**. +`providers/db2/` alone splits 4 files to `sql` and 5 to `db`. The real cut is +*dialect knowledge vs driver execution*, which is a better boundary but requires +per-file assignment, not `git mv` of two directories. + +Why this split and not another: + +- it is **mechanical in the sense that matters** — the transitive closure of + `browser.ts` (55 files, 9,222 lines) imports zero Node built-ins, measured, so + membership is decidable per file rather than argued +- it satisfies both stated requirements without conflict: `@foxschema/sql` is the + "everyone can import" package; `@foxschema/db` is the "backend role" package +- it deletes the mirror-file convention in `apps/web/src/frontend/lib/` — the frontend + imports `@foxschema/sql` for real +- publishing becomes honest: `@foxschema/sql` can go to npm immediately (no native + deps); `@foxschema/db` needs a real release story + +**Correction (verified 2026-08-05 — this reverses the previous correction).** A draft +claimed FoxFlow "consumes the driver half, to execute and query", and concluded that +`@foxschema/db` was the package with an external consumer and therefore on the critical +path for npm publishing. That is wrong. Every symbol FoxFlow's stub declares was checked +against both packages: + +| FoxFlow uses | lives in | +| --- | --- | +| `SqlDialect`, `CanonicalType`, `CanonicalBase`, `RenderedType` | `sql` | +| `resolveDialect`, `DIALECT_MAP` | `sql` | +| `PROVIDER_SETTINGS`, `getProviderSettings` | `sql` | +| `buildConnectionString`, `DEFAULT_PORTS` | `sql` | +| `CompareModule`, `SqlGeneratorModule` | `sql` | + +All 12 are in `@foxschema/sql`; none are db-owned. FoxFlow reuses **dialect knowledge**, +not the driver layer — it brings its own `pg`/`mysql2`. So: + +- the package with the external consumer is `@foxschema/sql`, which is zero-dep and + **publishable to npm today** — no native deps, no peer deps, no release story needed +- the hard packaging problem (native drivers as `optionalDependencies`) is **not** on the + critical path; `@foxschema/db` can stay an internal workspace package indefinitely +- `apps/cli` is the one consumer that genuinely needs the driver half — it runs migrations + +The lesson worth keeping: both drafts asserted FoxFlow's seam from the shape of its +directory name (`packages/pipes/db`) instead of reading what it imports. + +Cost, actual: the rename touched 74 import sites across `apps/web`, `apps/cli`, and the +packages themselves, plus 2 tsconfigs, 2 bundler aliases, 2 app manifests — and two +**operational** references that a pure import-rewrite would have missed: +`.github/workflows/version-bump.yml` (bumped `packages/core/package.json`) and +`scripts/sync-public-packages.sh` (mirrored `packages/core` to a public repo). Half a day, +not a day. FoxFlow depends via a `file:` path, so its symlink breaks the moment the +directory is renamed — that repo needs a coordinated change, not a follow-up. + +## Proposal 2 — feature modules behind a transport-agnostic service layer + +The real obstacle to adding GraphQL is not the absence of a GraphQL library. It is that +`routes.ts` is 834 lines where HTTP parsing, permission checks, and business logic are +interleaved, so there is no layer for a second transport to sit on. + +``` +apps/web/src/backend/ + features/ + compare/ service.ts types.ts (schema diff, migration plan) + editor/ service.ts types.ts (execute, code cells, beam) + assignment/ service.ts types.ts (new) + admin/ service.ts types.ts (users, roles) + transport/ + rest/ one router per feature, thin + graphql/ resolvers over the same services (future) +``` + +Rule: **a service never sees `req`/`res`.** It takes a typed input plus an +`ActorContext` (`{ userId, role, can(permission) }`) and returns typed output or throws +a typed error. Transports translate. + +Two consequences worth stating plainly: + +- REST and GraphQL become **two thin adapters over one implementation**, so a permission + fix lands in both at once. Today's per-statement RBAC check would otherwise have to be + re-implemented in the GraphQL resolver — and that is exactly how the gaps in #147, + #152 and #154 appeared. +- Each feature is independently testable without HTTP, which is most of what makes the + current backend tests awkward. + +## Proposal 3 — split the frontend by feature, not by file type + +`TableBlueprintModal.tsx` at 2,975 lines and `useSqlEditorStore.ts` at 2,130 are the +maintenance cost. The store in particular is one object holding tabs, results, variables, +bookmarks, secrets, schema cache, data peek, and beam. + +``` +frontend/features/ + compare/ components/ store/ + editor/ components/ store/ (split the store per concern) + admin/ components/ store/ +frontend/shared/ grid · monaco · ui primitives +``` + +Do this **last**. It is the largest change and the least urgent — a big React file is +annoying, a leaky package boundary is structural. + +## Sequencing + +Ordered by (value ÷ risk), each step independently shippable and revertible: + +| # | Step | Status | Why here | +| --- | --- | --- | --- | +| 1 | Split `@foxschema/sql` out of core | **done** (2026-08-05) | Unblocks everything else; also fixed the vite/tsconfig alias divergence | +| 2 | Extract `features/*/service.ts`, leave REST as-is | **started** — `actor.ts`, `connections/resolve.ts`, `compare/` | Pure refactor, no API change, makes #3 trivial | +| 3 | Add the GraphQL transport | next | Now genuinely additive — resolvers over existing services | +| 4 | Split the editor store by concern | later | Highest churn area; do it once the boundaries above are stable | +| 5 | Split the frontend by feature | later | Cosmetic relative to 1–3 | + +## Guardrails to add alongside + +Three of the bugs found this week were boundary failures that a rule would have caught: + +- ~~**`import/no-restricted-paths`** — forbid `@foxschema/sql` importing anything Node.~~ + **Done differently.** `packages/sql/src/purity.test.ts` asserts it instead: no `node:` + imports, no dependency back on `db`, no runtime deps. A test beat a lint rule here + because the root `eslint.config.js` deliberately runs security rules only, so adding + `import/*` would have meant adopting a plugin the config had reasons to avoid. Still + worth a rule for `frontend/**` importing `backend/**`, which nothing checks yet — though + the frontend half is now enforced by omission: `@foxschema/db` is not aliased in + `vite.config.ts`, so importing the driver layer fails the build. +- **A shared `hasOwn` helper.** The `beamDialects['toString']` bug and the earlier + `setBinding` duplicate-binding bug were the same defect twice. A helper plus + `no-prototype-builtins` is cheaper than fixing it a third time. +- **Make the gates non-bypassable.** #158 merged with two TS errors and two failing + tests. Whatever the structure, that is the failure that costs the most. Note the + ESLint security job is currently **red on `main`** for an unrelated reason (a disable + comment naming a `react-hooks` rule the root config never registers), which is how a + gate stops being a gate — people learn to ignore it. + +## Assignment = scheduled workflows (cron) under RBAC + +Clarified after the first draft. This changes where the feature sits and raises one +question that must be settled before any code. + +### The hard part is not cron. It is *whose permissions a job runs with*. + +Every check in the system today resolves against a **live request**: `authed.appRole`, +`authed.permissions`, `resolveRef(userId, …)`. A job that fires at 03:00 has no request +and no session. So the actor has to come from somewhere, and the choice is consequential: + +| Option | Behaviour | Problem | +| --- | --- | --- | +| **A. Freeze at create time** — store the creator's permissions with the job | Fast, no lookup | A user demoted from owner to viewer, or **disabled, or deleted**, keeps executing owner-level SQL forever. Privilege escalation through time. | +| **B. Re-resolve at fire time** from the creator's *current* role | Demotion takes effect on the next run | Needs a defined behaviour when the creator is gone: fail the run, or disable the job | +| **C. Dedicated service principal** per job, assigned by an admin | Clean audit story, independent of staff churn | Another identity to manage; overkill until there are many jobs | + +**Recommend B**, with the job recording `created_by` and re-resolving on every fire: + +- creator disabled or deleted → job is **suspended**, not silently skipped, and surfaces + in the job list with a reason +- creator demoted → the run fails the same per-statement gate a live request would hit, + and the failure is recorded on the run +- this makes the enable/disable + permanent-delete work discussed earlier a hard + dependency: deleting a user must decide the fate of their jobs + +Option A is the one to avoid. It is the default people reach for, and it quietly turns +"remove someone's access" into a lie. + +### Credentials are a real constraint, not a detail + +`resolveRef` merges a **session password** for connections saved without one. A job at +03:00 has no session. So either: + +- assignments are restricted to connections saved **with** a stored password, and the UI + says so at create time (recommended — explicit and safe), or +- a per-assignment credential is captured at create time and encrypted alongside it, + which widens what the secrets vault must protect + +Silently failing at 03:00 because a password was never stored is the outcome to design +out. + +### Where it lands in the structure + +- `features/assignment/service.ts` — CRUD, validation, next-run preview +- a **separate scheduler process**, not a `setInterval` in the API. The API restarts on + self-update (`scheduleUiRelaunch`) and can run multiple instances; both drop timers or + double-fire. A scheduler needs a durable claim (row-level lease) so two instances + cannot run the same job twice. +- runs go through the **same** `features/editor/service.ts` path as a live request, so + the per-statement dml/ddl/grant gate applies unchanged. A second execution path is + exactly where the gaps in #147/#152/#154 came from. + +New permissions: `assignment.view`, `assignment.create`, `assignment.run`, +`assignment.manage` (others' jobs). A job can never exceed its creator's grants — the +invariant to test first. + +### Overlap with FoxFlow — decide deliberately + +FoxFlow already **is** a workflow engine with cron triggers, reusing `@foxschema/core`: +cron trigger schema with timezone and `executionType`, `cron-parser` next-run previews, +catch-up and overlap policy, a scheduler app, run/checkpoint state. + +Building a second scheduler in FoxSchema duplicates that. Three honest options: + +**Decided: the products stay separate.** FoxSchema does not depend on FoxFlow at +runtime, and FoxFlow keeps consuming `@foxschema/db` rather than rewriting a driver +layer. Assignment is therefore built in FoxSchema, narrow by design. + +The boundary, written down so it can be defended: + +> A FoxSchema **assignment** is *one saved script, one schedule, one connection*. +> It has no branching, no fan-out, no inter-step data passing, and no retry graph. +> The moment a requirement needs any of those, it is a FoxFlow workflow, not an +> assignment. + +Concretely in scope: cron expression + timezone, a saved script (or a generated +migration), one target connection, on/off, run history with per-statement results, +overlap policy (skip if still running). + +Concretely out of scope: multiple steps, conditional edges, passing rows between steps, +external triggers (webhook/pubsub), fan-out across connections. Server Beam's two-endpoint +`sql.on` already covers the one cross-server case that matters, inside a single script. + +The relationship runs one way: **FoxFlow depends on FoxSchema's db package; FoxSchema +never depends on FoxFlow.** + +## Open questions + +1. ~~**Package naming.**~~ **Settled 2026-08-05:** `@foxschema/sql` + `@foxschema/db`, + and `@foxschema/core` retired. Chosen over keeping `core` for one half, which would + have left an existing import silently meaning something new. +2. **What is "assignment"?** Named as a target but does not exist yet. Its data model + decides whether it is a peer feature or part of admin. +3. ~~Does the CLI take `@foxschema/db` or talk to the API?~~ **Settled, but not as + written above.** `apps/cli` imports the driver layer directly (it runs migrations), so + it takes `@foxschema/db` — as a workspace package, which needs no publishing. FoxFlow + turned out to need `@foxschema/sql` only (see the correction in Proposal 1), and that + package is zero-dep and publishable today. **The native-driver packaging story is + therefore off the critical path**, reversing the previous conclusion. +4. **GraphQL scope.** Read-only projection over compare/editor results, or full mutation + parity with REST? The first is a weekend; the second doubles the RBAC surface. +5. ~~**FoxFlow's `file:` dependency.**~~ **Repointed 2026-08-05** to `packages/sql` + (11 files + the `.d.ts` stub renamed); FoxFlow's gates are green. The `file:` link and + the stub still exist, but they no longer have to: `@foxschema/sql` is now + **publish-ready** (`npm run publish:sql`), and a consumer typechecks against the real + shipped declarations under `moduleResolution: nodenext` with `skipLibCheck: false` — + verified against a packed tarball in a clean project. Publishing lets FoxFlow drop the + `file:` dep, the 44-line stub, both `paths` entries, and the "clone it beside this + repo" error message in one change. +6. **The public `foxschema-core` mirror repo.** No longer a sync target. Archive it, or + repoint it at one of the new packages. diff --git a/scripts/mirror-assets/compare-multi-target-scrolled.png b/scripts/mirror-assets/compare-multi-target-scrolled.png new file mode 100644 index 00000000..597b2094 Binary files /dev/null and b/scripts/mirror-assets/compare-multi-target-scrolled.png differ diff --git a/scripts/mirror-assets/compare-multi-target-sync-scroll.png b/scripts/mirror-assets/compare-multi-target-sync-scroll.png new file mode 100644 index 00000000..107abd1e Binary files /dev/null and b/scripts/mirror-assets/compare-multi-target-sync-scroll.png differ diff --git a/scripts/mirror-assets/sync-public-packages.png b/scripts/mirror-assets/sync-public-packages.png new file mode 100644 index 00000000..b3573ab4 Binary files /dev/null and b/scripts/mirror-assets/sync-public-packages.png differ