diff --git a/CHANGELOG.md b/CHANGELOG.md index b9c44be..4501546 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,21 @@ auto-generated per-PR notes; this file is the curated, human-readable history. ## [Unreleased] ### Changed +- **Workbench run lifecycle extracted into a route-scoped `WorkbenchSession`** + (#276 Phase 3a). `run`/`runScript`/`runEntry`/`cancel` orchestration now + lives in `src/ui/workbench/workbench-session.ts`; the run bookkeeping + fields (`runT0`/`runQueryId`/`runTick`) and the in-flight `AbortController` + are private session state (the `RunState` cast and `AppState.abortController` + are gone — issue rule 5: cancellation belongs to its owner). The session is + the sole production writer of the `running` signal; the three run-coupled + reactive effects register through `session.attachShell(...)` with captured + disposers (idempotent on re-render — also fixes a latent double-registration + on the connect → re-render path), and `destroy()` releases effects, the + elapsed ticker, and any in-flight operation (unit-proven; no production + caller until Phase 5's route shells). The architecture guard now carries a + rule list: route sessions (`ui/workbench` ↔ `ui/dashboard`) must not import + each other, and the dashboard route must not import the editor. Behavior + byte-identical. - **Authentication and connection lifecycle extracted into `ConnectionSession`** (#276 Phase 2). OAuth PKCE login/refresh, Basic-auth probing, IdP config resolution, identity, sign-in/out, and the cross-tab dashboard auth handoff diff --git a/build/check-boundaries.mjs b/build/check-boundaries.mjs index 4da81bb..ce14eea 100644 --- a/build/check-boundaries.mjs +++ b/build/check-boundaries.mjs @@ -1,10 +1,13 @@ -// Architecture boundary guard (issue #276 Phase 0). Day-1 rule only: modules -// under src/application/ must not import from src/ui/ or src/editor/ — the -// route/session layer coordinates state and network, it does not reach into -// DOM rendering or the CodeMirror editor adapters. `import type` counts too: -// a type-only import still couples the layers at compile time. Later phases -// (#276 Phases 3-5) extend the rule set (e.g. route sessions must not import -// each other) — add checks here rather than growing a second script. +// Architecture boundary guard (issue #276). Rule list, grown per phase: +// Phase 0 — src/application/ must not import src/ui/ or src/editor/: the +// service layer coordinates state and network, it does not reach into DOM +// rendering or the CodeMirror editor adapters. +// 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. +// `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. // // Hand-rolled regex scan, no AST parser: the codebase has no exotic import // syntax, so scanning for import/export specifiers is enough and keeps this @@ -15,10 +18,16 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); -const appDir = path.join(repoRoot, 'src', 'application'); -const FORBIDDEN = ['src/ui', 'src/editor']; 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). */ +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' }, +]; + function collectFiles(dir) { const out = []; for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { @@ -29,12 +38,6 @@ function collectFiles(dir) { return out; } -const files = fs.existsSync(appDir) ? collectFiles(appDir) : []; -if (files.length === 0) { - console.log('check-boundaries: no files under src/application/ yet'); - process.exit(0); -} - // Matches, in order: static `import ... from '...'` (incl. `import type`), // `export ... from '...'` (incl. `export type`), a bare side-effect // `import '...'`, and dynamic `import('...')`. Each pattern requires only @@ -71,25 +74,38 @@ function resolveRelative(fromFile, spec) { } const violations = []; -for (const file of files) { - const source = fs.readFileSync(file, 'utf8'); - for (const spec of extractSpecifiers(source)) { - if (!spec.startsWith('.')) continue; // bare/package specifiers can't reach src/ui or src/editor - const resolved = resolveRelative(file, spec); - const relResolved = path.relative(repoRoot, resolved).split(path.sep).join('/'); - const inForbidden = FORBIDDEN.some((f) => relResolved === f || relResolved.startsWith(`${f}/`)); - if (inForbidden) { - const relFile = path.relative(repoRoot, file).split(path.sep).join('/'); - violations.push(`${relFile} → ${spec} (resolved: ${relResolved})`); +let checkedFiles = 0; +let activeRules = 0; +for (const rule of RULES) { + const ruleDir = path.join(repoRoot, rule.dir); + const files = fs.existsSync(ruleDir) ? collectFiles(ruleDir) : []; + if (files.length === 0) continue; // directory not born yet — rule activates with it + activeRules += 1; + checkedFiles += files.length; + for (const file of files) { + const source = fs.readFileSync(file, 'utf8'); + for (const spec of extractSpecifiers(source)) { + if (!spec.startsWith('.')) continue; // bare/package specifiers can't reach src dirs + const resolved = resolveRelative(file, spec); + const relResolved = path.relative(repoRoot, resolved).split(path.sep).join('/'); + const hit = rule.forbidden.find((f) => relResolved === f || relResolved.startsWith(`${f}/`)); + if (hit) { + const relFile = path.relative(repoRoot, file).split(path.sep).join('/'); + violations.push(`${relFile} → ${spec} (resolved: ${relResolved}; ${rule.dir} must not import ${hit} — ${rule.why})`); + } } } } if (violations.length) { - console.error('check-boundaries: src/application/ must not import src/ui/ or src/editor/ (issue #276, day-1 rule):'); + console.error('check-boundaries: layer-boundary violations (issue #276):'); for (const line of violations) console.error(` ${line}`); process.exit(1); } -console.log(`check-boundaries: OK (${files.length} file${files.length === 1 ? '' : 's'} under src/application/, no ui/editor imports)`); +if (checkedFiles === 0) { + console.log('check-boundaries: no files under any guarded directory yet'); + process.exit(0); +} +console.log(`check-boundaries: OK (${checkedFiles} file${checkedFiles === 1 ? '' : 's'} across ${activeRules} active rule${activeRules === 1 ? '' : 's'}, no violations)`); process.exit(0); diff --git a/src/state.ts b/src/state.ts index 652630f..af09ddb 100644 --- a/src/state.ts +++ b/src/state.ts @@ -210,7 +210,6 @@ export interface AppState { bannerDismissedFor: Signal; serverVersion: string | null; running: Signal; - abortController: AbortController | null; schemaGraphAbortController: AbortController | null; resultView: Signal<'table' | 'json' | 'panel' | 'filter'>; exporting: Signal; @@ -394,11 +393,10 @@ export function createState(read: StateReader = { loadJSON, loadStr }): AppState // Run state (signals): `running` flips the Run button + results pane via // effects; `resultView` is the active Table/JSON/Chart tab. Via `.value`. running: signal(false), - abortController: null, // In-flight schema-lineage fetch (issue #124's inline drawer graph) — its own - // AbortController, separate from `abortController` (run/script) and the - // export controllers, since a graph fetch isn't gated by `running` and a - // second click/drag must be able to supersede an in-flight one. + // AbortController, separate from the workbench session's own run/script + // controller and the export controllers, since a graph fetch isn't gated by + // `running` and a second click/drag must be able to supersede an in-flight one. schemaGraphAbortController: null, resultView: signal<'table' | 'json' | 'panel' | 'filter'>('table'), // True while a streaming Export (issue #87) is in flight — separate from @@ -564,9 +562,12 @@ export function patchSpecDraft( return { ok: true, invalidTab: null, spec: tab.specParsed! }; } -/** The saved query a tab is linked to (via tab.savedId), or null. */ +/** The saved query a tab is linked to (via tab.savedId), or null. Narrowed to + * exactly the slice it reads (`savedQueries`) — not the full `AppState` — + * so a caller (e.g. `WorkbenchSession`'s own state slice) can pass a Pick + * without a bridging cast. */ export function savedForTab( - state: AppState, tab: Pick | null | undefined, + state: Pick, tab: Pick | null | undefined, ): SavedQueryV2 | null { return (tab && tab.savedId && state.savedQueries.find((q) => q.id === tab.savedId)) || null; } @@ -851,9 +852,11 @@ export function markLibrarySaved(state: AppState): void { } // Push one history entry (most-recent first, capped at 50). Internal — the -// exported recorders below supply the sql/rows/ms. +// exported recorders below supply the sql/rows/ms. Narrowed to `history` +// (the only field it reads/writes) so `recordScriptHistory` below can pass a +// narrower-than-`AppState` slice through unchanged. function pushHistory( - state: AppState, sql: string | null | undefined, rows: number | null, ms: number, + state: Pick, sql: string | null | undefined, rows: number | null, ms: number, save: SaveJSON, now: number, ): void { const s = String(sql || '').trim(); @@ -890,9 +893,12 @@ export function recordHistory( } /** Record a successful multiquery script run as one history entry (the whole - * script text); per-statement row counts aren't meaningful, so rows is null. */ + * script text); per-statement row counts aren't meaningful, so rows is null. + * Narrowed to `Pick` — the only field it reads/writes — + * so a caller (e.g. `WorkbenchSession`'s own state slice) can pass a Pick + * without a bridging cast. */ export function recordScriptHistory( - state: AppState, sql: string, ms: number, save: SaveJSON = saveJSON, now: number = Date.now(), + state: Pick, sql: string, ms: number, save: SaveJSON = saveJSON, now: number = Date.now(), ): void { pushHistory(state, sql, null, Math.round(ms), save, now); } diff --git a/src/ui/app.ts b/src/ui/app.ts index eac7393..b4ebff2 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -19,23 +19,19 @@ import { fieldControls, fieldControlKind, } from '../core/param-pipeline.js'; import type { - ParameterAnalysis, PreparedSource, PreparedBatch, PreparedFieldState, ValidationMode, BoundParamSnapshot, FieldControl, + ParameterAnalysis, PreparedSource, PreparedBatch, PreparedFieldState, ValidationMode, FieldControl, } from '../core/param-pipeline.js'; import { hasOptionalBlocks } from '../core/optional-blocks.js'; import { saveJSON, saveStr } from '../core/storage.js'; -import { sqlString, inferQueryName, shortVersion, supportsExplainPretty, userShortName, withStatementBreak, detectSqlFormat, isSchemaMutatingSql, prepareExportSql, formatBytes, formatRows } from '../core/format.js'; -import { EXPLAIN_VIEWS, parseExplain, detectExplainView, buildExplainQuery } from '../core/explain.js'; +import { sqlString, inferQueryName, shortVersion, userShortName, withStatementBreak, isSchemaMutatingSql, prepareExportSql, formatBytes, formatRows } from '../core/format.js'; import { buildSchemaGraph, expandLineage } from '../core/schema-graph.js'; import { buildCardGraph } from '../core/schema-cards.js'; import type { SchemaCardColumnRow } from '../core/schema-cards.js'; import { toTSV, formatFileMeta, exportFilename, scriptExportName } from '../core/export.js'; import { newResult, parseErrorPos, findExceptionFrame } from '../core/stream.js'; -import { buildResultSource } from '../core/query-source.js'; import { encodeShare } from '../core/share.js'; import { queryName, queryPanel, withQuerySpec } from '../core/saved-query.js'; import { effectiveDashboardRole } from '../core/result-choice.js'; -import { filterExecution } from '../core/filter-execution.js'; -import { readFilterOptions } from '../core/filter-options.js'; import { CORE_SPEC_VALIDATORS, createSpecValidatorRegistry, evaluateSpecText, formatSpecText, hasBlockingSpecErrors, @@ -44,7 +40,6 @@ import type { SpecValidatorEntry, QuerySpecValidationService } from '../core/spe import type { SpecDiagnostic } from '../editor/spec-editor.types.js'; import { assembleReferenceData, buildCompletions } from '../core/completions.js'; import { isQuerylessPanel } from '../core/panel-cfg.js'; -import { isKpiPanel, panelExecution } from '../core/panel-execution.js'; import * as ch from '../net/ch-client.js'; import { createNoopPort } from '../editor/editor-port.js'; import type { EditorPort } from '../editor/editor-port.types.js'; @@ -91,6 +86,7 @@ import type { SchemaGraphFocus, SchemaGraphNode, SchemaGraphEdge } from '../core import type { LineageFocus } from '../net/ch-client.js'; import { createQueryExecutionService } from '../application/query-execution-service.js'; import { createConnectionSession } from '../application/connection-session.js'; +import { createWorkbenchSession } from './workbench/workbench-session.js'; /** Optional globals a plain browser page (or the CM6/Chart/dagre UMD bundles a * `