diff --git a/CHANGELOG.md b/CHANGELOG.md index be7aac0..c2ca121 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,23 @@ auto-generated per-PR notes; this file is the curated, human-readable history. ## [Unreleased] ### Changed +- **The app.ts → services refactor is complete** (#276, Phase 5). The + temporary `App` delegates from Phases 2–4 are deleted — consumers read + `app.conn`/`app.catalog`/`app.params`/`app.queryDoc`/`app.prefs` directly + through narrow interfaces (`app.saveVarRecent` survives as the one + documented exception); the workbench DOM + effects moved into + `ui/workbench/workbench-shell.ts` behind a narrow deps bag, making + `renderApp` a thin composition call; the dashboard shell is typed against + a narrow `DashboardApp`; the per-tab ClickHouse `session_id` helpers got + their final home (`application/ch-session-params.ts`, one shared + implementation); the boundary guard now also forbids either shell from + importing `src/ui/app.ts`. **Behavior fix**: signing out now tears the + session down — an in-flight run is aborted and server-`KILL`ed, exports + and lineage fetches cancel, and the schema/reference caches invalidate + before the login screen renders (previously stale work could land after + sign-out and caches survived account switches). + `docs/ARCHITECTURE.md` rewritten to describe the actual modular-monolith + shape (it still documented the pre-refactor god object as the design). - **Phase 4 of the app.ts → services refactor complete** (#276): five more focused modules under `src/application/`, all constructible without `App`/`AppState`/DOM, behavior byte-identical. `WorkbenchParameterSession` diff --git a/build/check-boundaries.mjs b/build/check-boundaries.mjs index ce14eea..6f6a360 100644 --- a/build/check-boundaries.mjs +++ b/build/check-boundaries.mjs @@ -5,6 +5,10 @@ // Phase 3 — the route sessions must not import each other's implementation // modules (ui/workbench ↔ ui/dashboard), and the dashboard route must not // depend on the editor ports at all. +// Phase 5 — neither route shell (ui/workbench/**, ui/dashboard.ts + +// ui/dashboard/**) may import src/ui/app.ts: both receive everything they +// need injected (a narrow deps bag / the App type only) — reaching back +// into app.ts itself would recreate the coupling this phase removes. // `import type` counts too: a type-only import still couples the layers at // compile time. Extend RULES below in later phases rather than growing a // second script. @@ -21,17 +25,37 @@ const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..' const SOURCE_EXT = /\.(ts|tsx|js|mjs)$/; /** Each rule: every source file under `dir` must not import anything that - * resolves under any of `forbidden` (directories, repo-relative). */ + * resolves under any of `forbidden` (directories OR single files, + * repo-relative — `dir` itself may also name a single file, e.g. + * `src/ui/dashboard.ts`, since that route shell has no dedicated directory + * of its own the way `src/ui/workbench/**`/`src/ui/dashboard/**` do). */ const RULES = [ { dir: 'src/application', forbidden: ['src/ui', 'src/editor'], why: 'issue #276 day-1 rule' }, - { dir: 'src/ui/workbench', forbidden: ['src/ui/dashboard'], why: 'issue #276 Phase 3: route sessions must not import each other' }, - { dir: 'src/ui/dashboard', forbidden: ['src/ui/workbench', 'src/editor'], why: 'issue #276 Phase 3: route sessions must not import each other; dashboard has no editor' }, + { + dir: 'src/ui/workbench', + forbidden: ['src/ui/dashboard', 'src/ui/app.ts'], + why: 'issue #276 Phase 3/5: route sessions must not import each other, and the shell must not reach back into app.ts (everything it needs is injected)', + }, + { + dir: 'src/ui/dashboard', + forbidden: ['src/ui/workbench', 'src/editor', 'src/ui/app.ts'], + why: 'issue #276 Phase 3/5: route sessions must not import each other, dashboard has no editor, and the shell must not reach back into app.ts', + }, + { + // The dashboard route's own shell file (no dedicated directory, unlike + // its `dashboard-session.ts` runtime under `src/ui/dashboard/`) — same + // Phase 5 rule as the workbench shell above. + dir: 'src/ui/dashboard.ts', + forbidden: ['src/ui/app.ts'], + why: 'issue #276 Phase 5: the dashboard shell must not reach back into app.ts (everything it needs is injected)', + }, ]; -function collectFiles(dir) { +function collectFiles(target) { + if (fs.statSync(target).isFile()) return SOURCE_EXT.test(target) ? [target] : []; const out = []; - for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { - const full = path.join(dir, entry.name); + for (const entry of fs.readdirSync(target, { withFileTypes: true })) { + const full = path.join(target, entry.name); if (entry.isDirectory()) out.push(...collectFiles(full)); else if (SOURCE_EXT.test(entry.name)) out.push(full); } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index f1a8dee..ab8bed0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1,56 +1,123 @@ # Architecture +A modular ES-module SPA that builds to one self-contained HTML file served from +ClickHouse. No framework; state reactivity is `@preact/signals-core` +(ADR-0001), strict TypeScript throughout (ADR-0002). This document reflects +the post-#276 shape: a **modular monolith** — explicit application services and +route-scoped sessions behind a small composition root. + ## Layers ``` -core/ pure logic — strings/numbers/JWT/PKCE/SQL-tokenize/stream-parse -net/ integration — OAuth (config+flow) and the ClickHouse® HTTP client -ui/ presentation — hyperscript (h), icons, and render modules -state model — the state object and pure operations over it -main bootstrap — OAuth callback handling + initial render +core/ pure logic (no DOM, no globals, no imports from other layers) +net/ integration: OAuth + the ClickHouse HTTP client (fetch injected via ctx) +application/ route-agnostic services & sessions (no App, no DOM, no ui/editor imports) +ui/workbench/ the workbench route: session (run lifecycle) + shell (DOM + effects) +ui/dashboard/ the dashboard route: session (tile/filter runtime); ui/dashboard.ts is its shell +ui/* render modules (hyperscript), editor ports live in editor/ +ui/app.ts composition/bootstrap: constructs everything, wires routes +state.ts the shared signal-backed model + pure ops +main.ts page bootstrap: OAuth callback, share links, route dispatch ``` -The dependency direction is strictly downward: `ui` → `net`/`state`/`core`, -`net` → `core`, `core` → nothing. There are no cycles, so esbuild bundles -`main.js` into one IIFE. +Dependency direction is strictly downward. Enforced mechanically by +`build/check-boundaries.mjs` (runs in `pretest` as `check:arch`): -## The injected-seam pattern +- `src/application/**` never imports `src/ui/**` or `src/editor/**` (type-only + imports count). +- `src/ui/workbench/**` and `src/ui/dashboard/**` never import each other, + never import the editor (dashboard), and never import `src/ui/app.ts` — + shells receive everything injected. -Every side effect is passed in, never imported directly by logic: +Two known, deliberate exceptions predate #276 and are out of its scope: +`core/saved-io.ts` imports a type from `editor/spec-editor.types.js`, and +`editor/codemirror-adapter.ts` imports `ui/dom.js` + `ui/dnd-mime.ts`. -- `loadOAuthConfig(fetchFn, basePath)` and all of `net/` take `fetchFn`. -- `ch-client` functions take a `ctx = { fetch, origin, getToken, refresh, - onSignedOut }`. -- `createApp(env)` injects `document`, `window`, `location`, `fetch`, `crypto`, - `sessionStorage`. -- `generatePKCE(cryptoObj)`, `storage` helpers `(…, store)`, `timeAgo(ts, now)` - all accept their dependency. +## The services (`src/application/`) -This is why the suite needs no network/DOM mocking libraries — plain stubs -suffice, and coverage is genuine. +Each is a `create*(deps)` factory taking a narrow dependency bag — never the +`App` object or the full `AppState` (narrow `Pick`-shaped state slices are +structurally satisfied by `AppState`). Side effects are always injected +(fetch via the ClickHouse `ctx`, clocks, `uid`, storage, timers), so every +service is tested with plain stubs at the per-file coverage gate. + +| Module | Owns | +|---|---| +| `query-execution-service` (`app.exec`) | the shared request/stream/normalize read core + the script transport loop (retry classification, stop-on-first-failure, per-attempt `query_id`); stateless `kill(queryId)` — cancellation is caller-owned (`AbortController`s live with the owning session) | +| `connection-session` (`app.conn`) | auth + connection lifecycle: OAuth PKCE login/refresh, Basic probing, IdP config, identity, token storage, sign-out, the cross-tab dashboard auth handoff, and **the single live `chCtx` object** (mutated in place — `authConfirmed` by `net/ch-client`, `origin` by sign-in/handoff — never reconstructed) | +| `schema-catalog-service` (`app.catalog`) | server version, schema tree, lazy columns, SQL reference/completions, entity-doc cache, `invalidate()` | +| `workbench-parameter-session` (`app.params`) | `{name:Type}` analysis/prepare/gate policy, input-vs-execute hardening, enum inference, recent values; reads the live shared `AppState` slices through accessors | +| `export-service` (`app.exports`) | direct + script export behind an injectable `ExportSink` (`pickFile`/`pickDirectory`); hold-back exception inspection, `.partial` semantics, its own cancellation state | +| `query-document-session` (`app.queryDoc`) | Spec evaluation/diagnostics/dirty flags over `QueryTab`s, editor-mode policy | +| `saved-query-service` (`app.saved`) | create/commit saved queries (validate-before-persist), history recording, share-URL building — typed results; the shell renders messages | +| `schema-graph-session` (`app.graph`) | lineage load/expand/node-detail lifecycle with stale-request guards; abort state is session-private | +| `app-preferences` (`app.prefs`) | typed preference persistence (`save(name, value)` + `toggleTheme()`) | +| `ch-session-params` | pure helpers minting/attaching the per-tab ClickHouse HTTP `session_id` (TEMPORARY/SET stickiness), shared by the workbench hooks and export wiring | + +## Route sessions and shells -## The controller (`ui/app.js`) +- `ui/workbench/workbench-session.ts` (`app.workbench`) owns the running + operation: `run`/`runScript`/`runEntry`/`cancel`, the private run + bookkeeping (`runT0`/`runQueryId`/ticker) and the in-flight + `AbortController`. It is the **sole production writer of the `running` + signal**. DOM stays out via injected hooks; the three run-coupled reactive + effects register through `attachShell(...)` with captured disposers + (idempotent on re-render). +- `ui/workbench/workbench-shell.ts` (`mountWorkbenchShell(deps)`) builds the + workbench DOM (header, sidebar, splitters, tabs, toolbar, var strip, + results) and registers every other effect. `ui/app.ts`'s `renderApp` is a + thin call into it. +- `ui/dashboard/dashboard-session.ts` owns the dashboard runtime: the 6-way + tile pool, wave generations (reserved at wave creation), per-slot + cancellation, filter waves and merging, `destroy()`. Its input is an + explicit `DashboardRuntimeInput` built by the shell from the favorites + list — a stored dashboard document can replace that source without touching + the session. `ui/dashboard.ts` is its shell (own header; no sidebar), typed + against a narrow `DashboardApp`, not `App`. -`createApp(env)` returns the `app` object that every render module receives: +Lifecycle ownership: **cancellation state always lives with the session that +owns the operation** (issue #276 rule 5) — never in the transport service. +`destroy()`/`invalidate()` are wired where a session really ends today: +`signOut` tears down the workbench session, cancels graph/export work, and +invalidates the catalog before the login screen renders. There is no +route-remount mechanism — the dashboard is its own browser tab whose closure +JS never observes — so no teardown theater beyond that. -- `app.state` — the state model -- `app.dom` — live references to mounted regions, repopulated by `renderApp` -- `app.actions` — callbacks (run, newTab, share, toggleSaved, loadColumns, …) -- `app.chCtx` — the ClickHouse fetch context -- persistence: `app.savePref(name, value)` (raw prefs), `app.saveJSON` (lists) +## The `App` object -Render modules import nothing from `app.js` (avoiding a cycle); they take `app` -as a parameter. +`createApp(env)` still returns one `app` object, but it is now a composition +surface, not a controller: the session/service members above, the shared +`state`, the live `dom` region dictionary (reset wholesale per mount), the +injected env seams (Chart, Dagre, editor ports, pickers, clocks), the +`actions` registry (the DOM↔session event boundary render modules call +through), and a handful of genuinely shell-owned composites (`showLogin`, +`signOut`, `toggleTheme`, `openDashboard`, `renderApp`/`renderDashboard`). +Render modules keep taking `app` as a parameter and import nothing from +`app.ts`; most declare their own narrow `Pick`-shaped interfaces +(`LoginApp`, `ShortcutsApp`, `ResultsApp`, `DashboardApp`, …). One deliberate +delegate survives with a documented reason: `app.saveVarRecent` (a mutable +test seam the parameter session reads live). + +## Injected-seam pattern + +Unchanged from the beginning and now applied uniformly: every side effect is +passed in, never imported — `createApp(env)` injects +`document/window/location/fetch/crypto/sessionStorage`, `ch-client` functions +take a `ctx = {fetch, origin, getToken, refresh, authHeader, onSignedOut}`, +and every `create*Service(deps)` receives its transport/clock/uid/storage +explicitly. The suite needs no network/DOM mocking libraries — plain stubs +suffice, and coverage is genuine. ## Query execution -`runQuery` (in `net/ch-client.js`) streams ClickHouse's -`JSONStringsEachRowWithProgress` format: each newline-delimited object is folded -into the result via the pure `applyStreamLine`. TSV/JSON output formats are -fetched as raw text. A single automatic token refresh fires on 401/403 or a -`token_verification_exception` body. +`runQuery` in `net/ch-client.ts` streams `JSONStringsEachRowWithProgress`, +folded via the pure `applyStreamLine`; a single automatic token refresh on +401/403/`token_verification_exception` (before `authConfirmed` flips, an auth +failure signs out; after, it is a query error). ## Build -`build/build.mjs` runs esbuild (bundle + minify, IIFE) and inlines the script + -`styles.css` into `build/template.html`, producing `dist/sql.html`. +`build/build.mjs` runs esbuild (bundle + minify, IIFE), inlines the script and +`styles.css` into `build/template.html` → a single `dist/sql.html`. Four +bundled runtime dependencies (CodeMirror 6, Chart.js, dagre, +`@preact/signals-core`); the served page makes zero third-party requests. diff --git a/src/application/app-preferences.ts b/src/application/app-preferences.ts index 708e5a0..a802cd9 100644 --- a/src/application/app-preferences.ts +++ b/src/application/app-preferences.ts @@ -9,7 +9,8 @@ // each preference. Every write site except `toggleTheme` already mutates its // own state field itself (splitters.ts sets `ctx.state.sidebarPx` before // calling `ctx.save(...)`; dashboard.ts sets `state.dashLayout`/`dashCols` -// before `app.savePref(...)`; app.ts's `setResultRowLimit` sets +// before calling `app.prefs.save(...)` directly — #276 Phase 5 deleted the +// flat `App.savePref` delegate; app.ts's `setResultRowLimit` sets // `state.resultRowLimit` first) — so `save(name, value)` is a pure typed // persist call, no state slice needed. `toggleTheme` is the one exception // (issue ruling): the state flip AND the persist happen together here: the @@ -43,12 +44,12 @@ export interface AppPreferencesDeps { export interface AppPreferences { /** Generic persist-only setter — the exact `(name, value)` shape app.ts's - * public `savePref` has always exposed (dashboard.ts/saved-history.ts/ - * splitters.ts call it through that unchanged delegate). This IS the - * service's write API: per-key typed setters were considered and dropped - * (review) — every real call site already holds a validated - * `{name, value}` pair, so a per-key surface would ship with zero - * callers (CLAUDE.md rule 5: no speculative primitives). */ + * former `App.savePref` delegate used to expose (#276 Phase 5 deleted it; + * dashboard.ts/saved-history.ts/splitters.ts's callers call `app.prefs.save` + * directly now). This IS the service's write API: per-key typed setters + * were considered and dropped (review) — every real call site already + * holds a validated `{name, value}` pair, so a per-key surface would ship + * with zero callers (CLAUDE.md rule 5: no speculative primitives). */ save(name: PreferenceKey, value: unknown): void; /** Flips `state.theme` light↔dark AND persists it in one call (issue * ruling — the one preference whose state mutation moves here, not just diff --git a/src/application/ch-session-params.ts b/src/application/ch-session-params.ts new file mode 100644 index 0000000..ff63729 --- /dev/null +++ b/src/application/ch-session-params.ts @@ -0,0 +1,68 @@ +// The ClickHouse HTTP `session_id` policy (#276 Phase 5 final home) — +// `sessionParams`/`needsSession`/`sessionParamsFor` moved out of app.ts +// wholesale (byte-identical logic) into their own constructible-without-App +// module so both app.ts's workbench-hook wiring and the ExportService dep +// wiring share ONE implementation instead of two independently-maintained +// copies of the same three functions. +// +// A ClickHouse HTTP session ties a tab's requests together so session state — +// temporary tables, SET settings — survives across the separate HTTP requests +// of a multiquery script (and across successive runs in the tab). ClickHouse's +// HTTP interface runs one statement per request and is otherwise stateless, so +// without this a `CREATE TEMPORARY TABLE …; INSERT …; SELECT …` script can't +// see its own temp table. The id is per-tab (lazily minted) so tabs don't share +// state and never collide on the per-session lock (only one query runs at a +// time, guarded by `running`). No `session_timeout` override is needed: +// ClickHouse resets the idle timer when each query is *released* (end of the +// request, not the start) and cancels it while a query runs, so the default +// (60s) never lapses between a script's back-to-back statements. +// +// Only TEMPORARY tables and session `SET`s need a session; permanent DDL/DML +// and SELECTs are global. So we attach a session_id ONLY when the SQL needs +// one — or when the tab already opened one (sticky, so a temp table / SET +// from an earlier run stays visible to later runs in that tab). Ordinary +// scripts run session-LESS, which avoids the session lock / replica-affinity +// reset that intermittently surfaces as a "Network error". (Schema / +// reference loads are always session-less — they fan out in parallel and +// would deadlock on the lock.) + +import { leadingKeyword } from '../core/sql-split.js'; + +/** The one field this module reads/writes on a tab — real callers always pass + * a `state.ts` `QueryTab` (whose own `chSession?: string` satisfies this + * structurally), but the policy itself needs nothing else from a tab. */ +export interface ChSessionCarrier { + chSession?: string; +} + +export interface ChSessionParamsDeps { + /** A unique id for the `session_id` (mints `uid('sess-')`). Same seam + * app.ts's own `uid` already is (crypto.randomUUID, with a non-secure- + * context fallback) — injected here rather than re-derived. */ + uid: (prefix: string) => string; +} + +export interface ChSessionParams { + sessionParams(tab: ChSessionCarrier): { session_id: string }; + needsSession(sqls: string[]): boolean; + sessionParamsFor(tab: ChSessionCarrier, sqls: string[]): Record; +} + +/** Build a `ChSessionParams` bound to `deps`. Trivial constructor — no + * validation, no defaulting. */ +export function createChSessionParams(deps: ChSessionParamsDeps): ChSessionParams { + function sessionParams(tab: ChSessionCarrier): { session_id: string } { + tab.chSession = tab.chSession || deps.uid('sess-'); + return { session_id: tab.chSession }; + } + + function needsSession(sqls: string[]): boolean { + return sqls.some((s) => /\bTEMPORARY\b/i.test(s) || leadingKeyword(s) === 'SET'); + } + + function sessionParamsFor(tab: ChSessionCarrier, sqls: string[]): Record { + return tab.chSession != null || needsSession(sqls) ? sessionParams(tab) : {}; + } + + return { sessionParams, needsSession, sessionParamsFor }; +} diff --git a/src/application/schema-catalog-service.ts b/src/application/schema-catalog-service.ts index 511a461..f4d58e9 100644 --- a/src/application/schema-catalog-service.ts +++ b/src/application/schema-catalog-service.ts @@ -139,9 +139,9 @@ export function createSchemaCatalogService(deps: SchemaCatalogDeps): SchemaCatal // connection (the keystroke rule, #25): keywords/functions drive both // version-correct highlighting and the autocomplete list; completion then // runs client-side. `refData`/`completions` are private closure state — - // `app.catalog.refData`/`app.catalog.completions` (and app.ts's mirrored - // `app.refData`/`app.completions`) read them via the getters above/below, - // always the CURRENT value. + // `app.catalog.refData`/`app.catalog.completions` (#276 Phase 5 deleted the + // flat `App.refData`/`App.completions` delegates app.ts used to mirror them + // onto) read them via the getters above/below, always the CURRENT value. let refData: AssembledReference = assembleReferenceData(null); // built-in fallback until loaded let completions: CompletionItem[] = buildCompletions(refData, state.schema.value as SchemaDb[] | null); diff --git a/src/application/workbench-parameter-session.ts b/src/application/workbench-parameter-session.ts index cf5220d..65e6742 100644 --- a/src/application/workbench-parameter-session.ts +++ b/src/application/workbench-parameter-session.ts @@ -24,9 +24,10 @@ // // `hardenedVars` (#170 review: names whose value hardened to invalid) is // owned here as a private `Set`, exposed as a plain (never -// reassigned, only mutated in place) property — `app.hardenedVars` in app.ts -// is assigned this SAME instance (not a copy), so `app.hardenedVars.has(...)` -// (app.test.ts) keeps working unchanged. +// reassigned, only mutated in place) property — read directly as +// `app.params.hardenedVars` (#276 Phase 5 deleted the flat `App.hardenedVars` +// alias; `app.test.ts` reads this session's own instance through `app.params` +// now). // // `saveVarRecent`'s two faces: `saveVarRecent()` (this session's own, real, // `saveJSON`-calling implementation — what `app.saveVarRecent`'s one-line @@ -110,8 +111,8 @@ export interface WorkbenchParameterSessionDeps { export interface WorkbenchParameterSession { /** Names of `{name:Type}` variables whose value has hardened to invalid * (#170 review) — a single `Set` instance, mutated in place (never - * reassigned), so app.ts's `app.hardenedVars = session.hardenedVars` - * aliases the SAME object other modules/tests read/mutate. */ + * reassigned). No flat `App` delegate (#276 Phase 5 deleted it) — other + * modules/tests read/mutate this SAME object as `app.params.hardenedVars`. */ readonly hardenedVars: Set; tabAnalysis(sql: string): ParameterAnalysis; prepareAnalyzedBatch(analysis: ParameterAnalysis, wallNowMs: number, validationMode?: ValidationMode): PreparedBatch; diff --git a/src/editor/codemirror-adapter.ts b/src/editor/codemirror-adapter.ts index 3f50ff8..ce6a4fe 100644 --- a/src/editor/codemirror-adapter.ts +++ b/src/editor/codemirror-adapter.ts @@ -41,9 +41,9 @@ import type { EditorPort, EditorSelection } from './editor-port.types.js'; // ── Local typed contracts ─────────────────────────────────────────────────── // core/completions.js's own `AssembledReference`/`CompletionContext` shapes -// are imported directly (below) — this adapter's `app.refData` and the -// completion-context plumbing match them exactly. `app.completions` keeps its -// OWN wider local `CompletionItem` (kind: string, not the closed +// are imported directly (below) — this adapter's `app.catalog.refData` and +// the completion-context plumbing match them exactly. `app.catalog.completions` +// keeps its OWN wider local `CompletionItem` (kind: string, not the closed // `CompletionKind` union): this adapter renders an unrecognized `kind` as a // bare CSS chip rather than rejecting it (see `completionSourceFor`'s // `type: it.kind` below), so its own candidate contract is deliberately more @@ -71,7 +71,7 @@ interface InfoItem { fullType?: string; } -/** The complete completion-candidate shape `app.completions` holds (built by +/** The complete completion-candidate shape `app.catalog.completions` holds (built by * core/completions.js's `buildCompletions`, which in practice never emits * anything outside `CompletionKind` — but this adapter's own contract stays * open on `kind`, see above). `completionSourceFor` reads every field; @@ -97,16 +97,19 @@ export interface SqlCompletionContext { } /** `langExtensionFor`'s own narrow app slice — just the reference data (or - * its absence — no connection yet). Declared `unknown` (not - * `AssembledReference` directly) so the real, injected `app` controller's - * own placeholder type (`App.refData` in `ui/app.types.ts`, kept loose - * there since nothing else reads its actual shape) is assignable here too — - * `createCodeMirrorEditor` must stay assignable to `env.types.ts`'s - * `Editor?: (app: App) => EditorPort` seam. This adapter is the one real - * consumer of the precise shape, so it narrows with a local `as` at each - * read site instead. */ + * its absence — no connection yet), grouped under `catalog` (#276 Phase 5 — + * `app.catalog` is `SchemaCatalogService`; `refData`/`completions`/ + * `entityDoc` no longer exist as flat `App` delegates). Declared `unknown` + * (not `AssembledReference` directly) so the real, injected `app` + * controller's own `SchemaCatalogService.refData` (always a real object, + * never `null`/`undefined` in production — but this adapter's own tests + * still exercise the "no reference data at all" defensive branch) is + * assignable here too — `createCodeMirrorEditor` must stay assignable to + * `env.types.ts`'s `Editor?: (app: App) => EditorPort` seam. This adapter is + * the one real consumer of the precise shape, so it narrows with a local + * `as` at each read site instead. */ interface DialectApp { - refData?: unknown; + catalog?: { refData?: unknown }; } /** `hoverSourceFor`/`infoFor`'s own narrow app slice: the reference data plus @@ -114,7 +117,7 @@ interface DialectApp { * `CodeMirrorEditorApp` the port itself needs (tests call these two * directly with exactly this shape). */ interface ReferenceApp extends DialectApp { - entityDoc?: (name: string) => Promise; + catalog?: { refData?: unknown; entityDoc?: (name: string) => Promise }; } /** The subset of a DOM drag event `handleDrop` reads — real `DragEvent`s (the @@ -136,10 +139,11 @@ export interface DropEvent { export interface CodeMirrorEditorApp extends ReferenceApp { state: AppState; dom: { sqlEditorView?: EditorView }; - /** `unknown` for the same reason as `DialectApp.refData` above (matches - * `App.completions`'s own loose `Json` placeholder); narrowed with a - * local `as` in `completionSourceFor`. */ - completions?: unknown; + /** `completions` stays `unknown` for the same reason as `DialectApp`'s + * `catalog.refData` above (matches `SchemaCatalogService.completions`'s + * real, always-populated shape); narrowed with a local `as` in + * `completionSourceFor`. */ + catalog?: { refData?: unknown; completions?: unknown; entityDoc?: (name: string) => Promise }; actions: { loadColumns(db: string, table: string): Promise }; } @@ -171,9 +175,9 @@ const LITERAL_NODE = /String|Comment|QuotedIdentifier/; * deliberately doesn't pair (it would fight the #134 `{name:Type}` variables). */ export function langExtensionFor(app: DialectApp): Extension[] { - // `as`: `app.refData` is `unknown` here (see `DialectApp`'s doc comment) — - // this adapter is the one real consumer of its actual shape. - const ref = app.refData as AssembledReference | null | undefined; + // `as`: `app.catalog.refData` is `unknown` here (see `DialectApp`'s doc + // comment) — this adapter is the one real consumer of its actual shape. + const ref = app.catalog?.refData as AssembledReference | null | undefined; const dialect = SQLDialect.define({ keywords: (ref ? ref.keywords : []).join(' ').toLowerCase(), builtin: Object.keys(ref ? ref.functions : {}).join(' ').toLowerCase(), @@ -239,9 +243,9 @@ export function inputGuards(view: EditorView, from: number, to: number, text: st /** * Completion source: CM6's UI over the pure core ranking (#26 parity v0). * `filter: false` keeps `rankCompletions`' order (CM6 would fuzzy-rescore and - * dedup otherwise). Candidates come from `app.completions` at call time, so + * dedup otherwise). Candidates come from `app.catalog.completions` at call time, so * schema/reference updates need no reconfigure. Never queries — `info` resolves - * through app.entityDoc's lazy cache, and only for the row the user rests on. + * through app.catalog.entityDoc's lazy cache, and only for the row the user rests on. */ export function completionSourceFor(app: CodeMirrorEditorApp): (ctx: SqlCompletionContext) => CompletionResult | null { return (ctx) => { @@ -260,14 +264,14 @@ export function completionSourceFor(app: CodeMirrorEditorApp): (ctx: SqlCompleti // lines before the caret, so a FROM above the caret is in view; a FROM below // it degrades gracefully to the global pool (no scope). c.scope = fromScopeAt(doc, ctx.pos, toks); - // `as`: `app.completions` is `unknown` here (see `CodeMirrorEditorApp`'s - // doc comment). Real values are always core/completions.ts's own - // `buildCompletions` output (`CompletionKind`-typed); this adapter's own - // `CompletionItem` deliberately keeps `kind: string` open for its - // pass-through rendering (see the top-of-file note), and - // `rankCompletions` only ever compares `kind` against its own known - // literals, so the cast is behaviorally safe either way. - const items = rankCompletions((app.completions as CompletionItem[] | undefined || []) as CoreCompletionItem[], c); + // `as`: `app.catalog.completions` is `unknown` here (see + // `CodeMirrorEditorApp`'s doc comment). Real values are always + // core/completions.ts's own `buildCompletions` output (`CompletionKind`- + // typed); this adapter's own `CompletionItem` deliberately keeps + // `kind: string` open for its pass-through rendering (see the top-of-file + // note), and `rankCompletions` only ever compares `kind` against its own + // known literals, so the cast is behaviorally safe either way. + const items = rankCompletions((app.catalog?.completions as CompletionItem[] | undefined || []) as CoreCompletionItem[], c); if (!items.length) return null; return { from: c.from, @@ -309,26 +313,27 @@ export function applyFor(it: ApplyItem): Completion['apply'] { type InfoFn = () => Node | null | Promise; // The active row's description: static keyword docs immediately, function -// docs lazily via app.entityDoc (cached, one query per name ever — #27). +// docs lazily via app.catalog.entityDoc (cached, one query per name ever — #27). // CM6 shows it as a side tooltip (the old dropdown used a footer). An `info` // FUNCTION must yield a DOM node (a bare string is only legal when `info` // itself is the string) — CM6's addInfoPane appendChild()s the result. export function infoFor(app: ReferenceApp, it: InfoItem): InfoFn | undefined { const doc = (text: string | null | undefined): Node | null => (text ? h('div', null, text) : null); if (it.kind === 'keyword') { - // `as`: `app.refData` is `unknown` (see `DialectApp`'s doc comment); read - // fresh on every call (not hoisted) — CM6 may invoke this `info` callback - // well after refData has since changed (#25 load lands mid-session). + // `as`: `app.catalog.refData` is `unknown` (see `DialectApp`'s doc + // comment); read fresh on every call (not hoisted) — CM6 may invoke this + // `info` callback well after refData has since changed (#25 load lands + // mid-session). return () => { - const refData = app.refData as AssembledReference | null | undefined; + const refData = app.catalog?.refData as AssembledReference | null | undefined; return doc(refData && refData.keywordDocs[it.label.toUpperCase()]); }; } if (it.kind === 'fn' || it.kind === 'agg' || it.kind === 'cast') { - if (!app.entityDoc) return undefined; + if (!app.catalog?.entityDoc) return undefined; // `!`: just checked above; a deferred closure doesn't retain that // narrowing across the function boundary, but the guard already ran. - return () => Promise.resolve(app.entityDoc!(it.label)).then(doc); + return () => Promise.resolve(app.catalog!.entityDoc!(it.label)).then(doc); } // A column whose `detail` is a compacted type summary (#177) exposes the full // declared type here — CM6's detail column has no native title fallback. @@ -357,8 +362,8 @@ const lookupFn = (functions: Record, word: stri */ export function hoverSourceFor(app: ReferenceApp): (view: EditorView, pos: number) => Tooltip | null { return (view, pos) => { - // `as`: `app.refData` is `unknown` here (see `DialectApp`'s doc comment). - const refData = app.refData as AssembledReference | null | undefined; + // `as`: `app.catalog.refData` is `unknown` here (see `DialectApp`'s doc comment). + const refData = app.catalog?.refData as AssembledReference | null | undefined; if (!refData) return null; if (LITERAL_NODE.test(syntaxTree(view.state).resolveInner(pos, 0).name)) return null; // Identifiers can't span lines — scan the line, not the whole doc. @@ -378,8 +383,8 @@ export function hoverSourceFor(app: ReferenceApp): (view: EditorView, pos: numbe fn.ret ? h('span', { class: 'hover-ret' }, ' → ' + fn.ret) : null)); const doc = h('div', { class: 'hover-doc' }, fn.desc || ''); dom.appendChild(doc); - if (!fn.desc && app.entityDoc) { - Promise.resolve(app.entityDoc(w.word)).then((d) => { if (d) doc.textContent = d; }); + if (!fn.desc && app.catalog?.entityDoc) { + Promise.resolve(app.catalog.entityDoc(w.word)).then((d) => { if (d) doc.textContent = d; }); } } else { dom.appendChild(h('div', { class: 'hover-doc' }, kwDoc)); @@ -446,7 +451,7 @@ const COLUMN_LOAD_DELAY_MS = 300; * FROM-driven lazy column loading (#84): parse the statement around the caret, * find its FROM/JOIN tables whose columns aren't loaded yet, and fetch them via * the app's existing `loadColumns` (which writes the `'loading'` sentinel to - * dedupe, caches per connection, and rebuilds `app.completions`). Uses the whole + * dedupe, caches per connection, and rebuilds `app.catalog.completions`). Uses the whole * document (not the keystroke-path line slice) so a FROM below the caret still * prefetches. Resolves to whether it fetched anything — the caller refreshes an * open dropdown only after re-checking the view is still live (destroy race). diff --git a/src/main.ts b/src/main.ts index 9a8f47c..b4f5db9 100644 --- a/src/main.ts +++ b/src/main.ts @@ -19,7 +19,7 @@ import { isQuerylessPanel } from './core/panel-cfg.js'; import { setTabSpecDraft, SAVED_VIEWS } from './state.js'; import type { State } from './ui/app.types.js'; import type { BootstrapEnv } from './env.types.js'; -import type { ResolvedIdpConfig } from './net/oauth-config.js'; +import type { ConnectionSession } from './application/connection-session.js'; import type { SpecEditorApp } from './editor/spec-editor.js'; import type { ShortcutKeydownEvent } from './ui/shortcuts.js'; @@ -28,15 +28,13 @@ import type { ShortcutKeydownEvent } from './ui/shortcuts.js'; * own `createApp()` call below) satisfies this directly, and so does * tests/unit/main.test.ts's long-standing minimal `fakeApp()` fixture — no * cast needed on either side (same convention as ui/shortcuts.ts's - * ShortcutsApp). */ + * ShortcutsApp). Identity/auth reads go through `conn` (#276 Phase 5 — + * the flat `App` delegates were deleted; `loadConfig` is now `resolveConfig`, + * its real name on `ConnectionSession`). */ export interface BootstrapApp { state: Pick; - isSignedIn(): boolean; - loadConfig(): Promise; - setTokens(id: string, refresh?: string): void; - receiveAuthHandoff(handoffEnv: { opener?: Window | null }): Promise; - ensureFreshToken(): Promise; - ensureConfig(): Promise; + conn: Pick; renderDashboard(): void; renderApp(): void; /** The real `App.showLogin` is `(msg?: string) => void` — every other real @@ -72,7 +70,7 @@ export async function bootstrap(app: BootstrapApp, env: BootstrapEnv): Promise<{ callbackError = 'OAuth state mismatch — please try again.'; } else { try { - const cfg = await app.loadConfig(); + const cfg = await app.conn.resolveConfig(); const tokens = await exchangeCodeForTokens(env.fetch, cfg, { code, // `verifier` is written to sessionStorage just before the redirect; @@ -84,7 +82,7 @@ export async function bootstrap(app: BootstrapApp, env: BootstrapEnv): Promise<{ }); const bearer = bearerFromTokens(tokens, cfg.bearer); if (!bearer) throw new Error('Token response missing bearer token'); - app.setTokens(bearer, tokens.refresh_token); + app.conn.setTokens(bearer, tokens.refresh_token); } catch (e) { callbackError = 'OAuth token exchange failed: ' + ((e instanceof Error && e.message) || e); } @@ -155,26 +153,26 @@ export async function bootstrap(app: BootstrapApp, env: BootstrapEnv): Promise<{ // one-time credential handoff from the opener before deciding what to render. // A cold/bookmarked visit has no opener → falls through to the login screen, // which after sign-in returns to /sql/dashboard and renders the dashboard. - if (dash && !app.isSignedIn()) { - await app.receiveAuthHandoff(env); + if (dash && !app.conn.isSignedIn()) { + await app.conn.receiveAuthHandoff(env); // The opener may hand over an *expired* id_token whose refresh token is still // good (an idle opener refreshes only lazily). Attempt a refresh before // giving up — otherwise a valid handoff would still bounce to a full re-login. - if (!app.isSignedIn()) await app.ensureFreshToken(); + if (!app.conn.isSignedIn()) await app.conn.ensureFreshToken(); } - if (app.isSignedIn()) { + if (app.conn.isSignedIn()) { // Signed in either via a valid OAuth token or a restored basic session. ss.removeItem('oauth_shared'); // consumed // Resolve config first so the header shows the real CH identity (the // ch_auth=basic username, not the raw email claim) on first paint. // (ensureConfig is a no-op in basic mode.) - await app.ensureConfig(); + await app.conn.ensureConfig(); if (dash) app.renderDashboard(); else app.renderApp(); } else { app.showLogin(callbackError); } - return { callbackError, signedIn: app.isSignedIn() }; + return { callbackError, signedIn: app.conn.isSignedIn() }; } // Set once by tests/setup.js to keep the browser-only autostart block below diff --git a/src/ui/app.ts b/src/ui/app.ts index 5827069..3b9091a 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -9,15 +9,15 @@ import { Icon } from './icons.js'; import { createState, activeTab, savedForTab, tabPanel, - normalizeRowLimit, MOBILE_BREAKPOINT_PX, + normalizeRowLimit, } from '../state.js'; import type { QueryTab, AppState, SpecValidationService } from '../state.js'; import type { SavedQueryV2 } from '../generated/json-schema.types.js'; -import { splitStatements, leadingKeyword } from '../core/sql-split.js'; +import { splitStatements } from '../core/sql-split.js'; import { analysisView, fieldControls, fieldControlKind } from '../core/param-pipeline.js'; import { hasOptionalBlocks } from '../core/optional-blocks.js'; import { saveJSON, saveStr } from '../core/storage.js'; -import { sqlString, inferQueryName, shortVersion, userShortName, withStatementBreak, formatBytes, formatRows } from '../core/format.js'; +import { sqlString, inferQueryName, shortVersion, withStatementBreak, formatBytes } from '../core/format.js'; import { toTSV } from '../core/export.js'; import { newResult, parseErrorPos } from '../core/stream.js'; import { effectiveDashboardRole } from '../core/result-choice.js'; @@ -33,14 +33,13 @@ import { createNoopPort } from '../editor/editor-port.js'; import type { EditorPort } from '../editor/editor-port.types.js'; import { createNoopSpecEditor } from '../editor/spec-editor.js'; import { createSpecCompletionSources } from '../editor/spec-completion-adapter.js'; -import { SCHEMA_GRAPH_MIME } from './dnd-mime.js'; import { renderTabs, selectTab, newTab, closeTab, loadIntoNewTab } from './tabs.js'; import type { QueryOrName } from './tabs.js'; -import { effect, batch } from '@preact/signals-core'; -import { renderSchema } from './schema.js'; +import { batch } from '@preact/signals-core'; import { renderResults } from './results.js'; import type { Result, QueryResult, ScriptResult, ScriptEntry } from './results.js'; import { renderDashboard } from './dashboard.js'; +import { toggleThemeDom } from './theme-toggle.js'; import { openSchemaView } from './explain-graph.js'; import type { SchemaLineageNode, DetachedGraphApp } from './explain-graph.js'; import { openDetailPane } from './schema-detail.js'; @@ -58,12 +57,9 @@ import type { ComboField } from './combobox.js'; import { recentOptions } from '../core/recent-values.js'; import { paramComparisonColumns } from '../core/param-comparison.js'; import type { SchemaDb } from '../core/from-scope.js'; -import type { AssembledReference, CompletionItem } from '../core/completions.js'; -import { libraryControls, renderLibraryTitle } from './file-menu.js'; import { renderLogin } from './login.js'; import { openShortcuts } from './shortcuts.js'; import { startDrag } from './splitters.js'; -import type { DragCtx, DragRect, DragStartEvent, SplitterAxis } from './splitters.js'; import { flashToast } from './toast.js'; import type { App, ActionsRegistry, SchemaFocus } from './app.types.js'; import type { CreateAppEnv } from '../env.types.js'; @@ -71,14 +67,15 @@ import { createQueryExecutionService } from '../application/query-execution-serv import { createConnectionSession } from '../application/connection-session.js'; import { createSchemaCatalogService } from '../application/schema-catalog-service.js'; import { createWorkbenchParameterSession } from '../application/workbench-parameter-session.js'; +import { createChSessionParams } from '../application/ch-session-params.js'; import { createExportService } from '../application/export-service.js'; import type { ExportSink, FileHandleLike, DirectoryHandleLike } from '../application/export-service.js'; import { createSchemaGraphSession, SchemaGraphAuthRequiredError } from '../application/schema-graph-session.js'; import { createAppPreferences } from '../application/app-preferences.js'; -import type { PreferenceKey } from '../application/app-preferences.js'; import { createWorkbenchSession } from './workbench/workbench-session.js'; import { createQueryDocumentSession } from '../application/query-document-session.js'; import { createSavedQueryService } from '../application/saved-query-service.js'; +import { mountWorkbenchShell } from './workbench/workbench-shell.js'; /** Optional globals a plain browser page (or the CM6/Chart/dagre UMD bundles a * `