diff --git a/.changeset/prefetch-run-gate.md b/.changeset/prefetch-run-gate.md new file mode 100644 index 00000000..95c2c523 --- /dev/null +++ b/.changeset/prefetch-run-gate.md @@ -0,0 +1,6 @@ +--- +'@noormdev/cli': patch +'@noormdev/sdk': patch +--- + +Decide which files to skip with one or two tracking queries per batch instead of one or two per file. Skipped files no longer write execution rows, and a build where nothing changed writes no operation record. The SDK's `run.file(path, { preview: true })` now renders without executing. The TUI's re-run confirmation now finds earlier runs. diff --git a/docs/design/prefetch-run-gate.md b/docs/design/prefetch-run-gate.md new file mode 100644 index 00000000..814d2e18 --- /dev/null +++ b/docs/design/prefetch-run-gate.md @@ -0,0 +1,43 @@ +# Prefetch run gate + + +## Problem + +`executeFiles` decides skip-or-run one file at a time. Each file pays a `needsRun` SELECT and an `updateFileExecution` UPDATE, skip or not, so a 500-file build against a remote database spends about 1,000 round trips before any real work. The change executor has the same shape: `needsRunFile` runs a boundary SELECT and a history SELECT per file. + + +## Concepts + +| Term | Meaning | +|------|---------| +| Gate | The pass before any tracking write: hash every file, fetch tracking state once, decide every file in memory. | +| Run set | Files the gate decided must run. Only these get a pending row. | +| Skip set | Files the gate decided are unchanged. They produce a `FileResult` and a `file:skip` event, and no database write. | +| Decision function | A pure function from (latest record, current checksum, force) to `NeedsRunResult`. The per-file lookup and the gate share it. | + + +## Business rules + +1. A skipped file is never recorded. No pending row, no skipped row, no UPDATE. +2. A build or run whose run set is empty writes no operation row. +3. Templates are rendered, then hashed, then decided. The rendered SQL is kept and executed, so each template renders once. +4. The skip rules stay what they are today: force, new, failed, pending, a skip that is not `unchanged`, stale parent, changed checksum, and lookup error all run. Rows already in existing databases (`skipped/unchanged`, `pending`) still decide correctly. +5. Execution stays sequential, in discovery order. +6. A failure cascade still marks the run-set files that were never reached as `skipped`. These rows are run-set files that never executed, not unchanged-skips, and `needsRun` already reads them as must-run. +7. On the change path the operation row is still written when every file skips. It records the change's combined checksum, which the change-level `needsRun` compares against. + + +## Approach + +The prefetch is one SELECT per batch: the newest execution row per filepath for the config, chosen by a `MAX(id) ... GROUP BY filepath` subquery joined back to the executions and change tables. This form runs unchanged on Postgres, MySQL, MSSQL and SQLite, and has no IN list, so it never reaches MSSQL's 2,100-parameter limit. The result includes filepaths outside the batch, and the runner ignores those in memory. + +The change path fetches the opposite-direction boundary once per change rather than once per file. It then fetches the newest qualifying row per filepath for that change in one query. + +`runFile` goes through `executeFiles` like the other entry points. This removes the legacy insert-per-file path (`executeSingleFile`, `recordExecution`), which recorded skips. + + +## Rejected approaches + +- **`ROW_NUMBER() OVER (PARTITION BY filepath)`.** Needs SQLite 3.25+ and MySQL 8, and the subquery form is equally fast at this scale. +- **`WHERE filepath IN (...)` and reduce in JS.** Returns the whole history and is capped by MSSQL's parameter limit. +- **Raw-hash gate for templates.** A template's output depends on config and secrets, so the raw bytes cannot prove the output is unchanged. diff --git a/docs/dev/change.md b/docs/dev/change.md index 48a5940d..41ff3279 100644 --- a/docs/dev/change.md +++ b/docs/dev/change.md @@ -70,10 +70,26 @@ Changes use checksum-based detection at the change level. Each condition has an | `force` | `--force` flag provided | Run | | — | Successfully applied, unchanged | Skip | -Unlike the runner which tracks individual files, changes track the combined checksum of all files in `change/` or `revert/`. - The `stale` status is set by teardown operations that wipe the database. When you run `db:teardown`, all applied changes are marked as stale so they'll re-run on the next `ff` operation. +This check uses the combined checksum of all files in `change/` or `revert/`. + +When the change runs, each file gets its own decision, so a retry after a fix re-runs only the files that need it. `ChangeHistory.latestFileExecutions(name, direction)` reads the newest qualifying execution row for every file of the change in two SELECTs, whatever the file count. `decideNeedsRunFile` then judges each file: + +| Reason | Condition | Action | +|--------|-----------|--------| +| `force` | `--force` flag provided | Run | +| `new` | No qualifying row, or the lookup failed | Run | +| `failed` | Last attempt failed | Run | +| `changed` | File checksum differs | Run | +| — | Last success has a matching checksum | Skip, `'already applied'` | + +A row qualifies only while its success still stands. Rows from a `reverted` or `stale` operation do not count, and neither do rows older than the newest operation in the opposite direction, so apply, revert, apply re-runs every file. + +A filepath listed more than once through `.txt` manifests runs at its first position and skips as `'already applied'` at the rest. + +A skipped file pushes its `skipped` result and makes no database call. Pending rows are written for the files that will run. The operation row is written even when every file skips, because it records the change checksum. + ## Basic Usage @@ -275,6 +291,8 @@ Change execution is recorded in the same tables as the runner. Names below are t | `status` | `'pending'`, `'success'`, `'failed'`, `'skipped'` | | `duration_ms` | Execution time (integer, not float) | +`skipped` marks a file a failure left unreached. See [Change Detection](#change-detection) for files that get no row. + **Note:** The `duration_ms` column is an integer. `performance.now()` returns floats, so all writes use `Math.round(durationMs)`. This is important for PostgreSQL compatibility. @@ -496,9 +514,10 @@ const marked = await tracker.markAllAsStale() ``` The base `Tracker` class (from `core/runner`) provides: -- `needsRun()` / `needsRunByName()` - Change detection by filepath or name +- `latestExecutions()` - Newest execution row per filepath, for the batch skip decision +- `needsRunByName()` - Change detection by change name - `createOperation()` - Create operation records with direction -- `createFileRecords()` - Create pending file records upfront +- `createFileRecords()` - Create pending records for the files about to run - `updateFileExecution()` - Update individual file status - `skipRemainingFiles()` - Mark remaining files as skipped - `finalizeOperation()` - Complete operation with final status diff --git a/docs/dev/datamodel.md b/docs/dev/datamodel.md index 3d689ee9..5984ffe4 100644 --- a/docs/dev/datamodel.md +++ b/docs/dev/datamodel.md @@ -731,16 +731,16 @@ Used in `__noorm_executions__` and file results. ### Skip Reasons -Free-form text, not an enum. The values emitted today: +Free-form text, not an enum. A skip row marks only a file a failure left unreached. The values written today: | Reason | Meaning | |--------|---------| -| unchanged | File checksum matches previous run | -| already-run | File was already executed successfully | -| already applied | The change as a whole was already applied | +| `Skipped: failure in {file}` | A runner batch aborted at a named file; remaining files skipped | | change failed | Parent change failed with no single culprit file | | `{file} failed: {error}` | Parent change failed at a named file; remaining files skipped | +A file skipped because a prior run covers it gets no row. Rows written by older versions may still hold `unchanged` (runner) or `already applied` (change) for that case. + ### Lock diff --git a/docs/dev/runner.md b/docs/dev/runner.md index ac6584a6..36d66048 100644 --- a/docs/dev/runner.md +++ b/docs/dev/runner.md @@ -10,7 +10,7 @@ You have SQL files that need to run against a database. But running them manuall - What happens if execution fails halfway through? - How do you preview what will run before committing? -noorm's runner solves this with checksum-based change detection and execution tracking. Files that haven't changed are skipped. Failed files are automatically retried. Everything is logged to the database for auditability. +noorm's runner solves this with checksum-based change detection and execution tracking. Files that haven't changed are skipped. Failed files are automatically retried. Every execution is logged to the database for auditability. ## How It Works @@ -24,14 +24,15 @@ The runner provides four execution modes: | **Dir** | Execute all files in a directory | Directory path | | **Files** | Execute specific files selectively | Array of file paths | -When you run a file: +When you run a batch (a single file is a batch of one): -1. Compute SHA-256 checksum of file contents -2. Check tracking database for previous execution -3. Skip if unchanged, run if new/changed/failed -4. Render template if `.sql.tmpl` file -5. Execute SQL against database -6. Record result in tracking tables +1. Render every `.sql.tmpl` file, then compute the SHA-256 checksum of the SQL that will execute +2. Read the newest execution row per file from the tracking database, in one query for the whole batch +3. Skip if unchanged, run if new/changed/failed. A skipped file writes no tracking row +4. If no file is left to run, return here without writing an operation row +5. Create the operation and a `pending` row for each file about to run +6. Execute SQL against database, in discovery order +7. Record result in tracking tables ## Change Detection @@ -143,9 +144,9 @@ Every execution is recorded in two tables. Names below are the MySQL/SQLite pref |-------|-------------| | `change_id` | FK to parent operation | | `filepath` | File that was executed | -| `checksum` | SHA-256 of file contents | -| `status` | `'success'`, `'failed'`, `'skipped'` | -| `skip_reason` | `'unchanged'` if skipped | +| `checksum` | SHA-256 of the executed SQL (the rendered output, for a template) | +| `status` | `'pending'`, `'success'`, `'failed'`, `'skipped'` (a file the batch never reached after an earlier failure) | +| `skip_reason` | Which failure stopped the batch, if skipped | | `duration_ms` | Execution time | @@ -420,15 +421,14 @@ This utility is used internally by build operations when applying settings rules ## Unified executeFiles -The `executeFiles` function provides low-level file execution with full tracking. `runBuild`/`runDir`/`runFiles` funnel through it. The change module does **not** — `core/change/executor.ts` has its own private `executeFiles` so it can emit `change:file` events and honour manifest resolution; it shares only `computeChecksum`, `computeCombinedChecksum`, and the `Tracker` base class with the runner. +The `executeFiles` function provides low-level file execution with full tracking. `runBuild`/`runDir`/`runFiles`/`runFile` funnel through it. The change module does **not** — `core/change/executor.ts` has its own private `executeFiles` so it can emit `change:file` events and honour manifest resolution; it shares only `computeChecksum`, `computeCombinedChecksum`, and the `Tracker` base class with the runner. ```typescript import { executeFiles, type FileInput } from './core/runner' -// Prepare file inputs with pre-computed checksums const files: FileInput[] = [ - { path: '/project/sql/001_users.sql', type: 'sql', checksum: 'abc123' }, - { path: '/project/sql/002_posts.sql', type: 'sql', checksum: 'def456' }, + { path: '/project/sql/001_users.sql', type: 'sql' }, + { path: '/project/sql/002_posts.sql', type: 'sql' }, ] // Four arguments: run options and execution options are separate objects @@ -452,7 +452,6 @@ The `FileInput` type: interface FileInput { path: string type: 'sql' | 'txt' - checksum?: string // Pre-computed SHA-256; computed if omitted } ``` @@ -460,8 +459,8 @@ interface FileInput { This design separates file discovery (external) from execution (internal), enabling: - Pre-validation before database operations begin -- Checksum computation for all files upfront -- Full batch visibility via `createFileRecords()` +- Rendering and hashing every file before any tracking write +- One tracking query to decide skip-or-run for the whole batch ## Tracker Class @@ -469,20 +468,18 @@ This design separates file discovery (external) from execution (internal), enabl The `Tracker` class handles execution history and change detection. It serves as the base for both runner and change operations. ```typescript -import { Tracker } from './core/runner' +import { Tracker, decideNeedsRun } from './core/runner' // (db, configName, dialect) — dialect defaults to 'sqlite', which selects the // prefixed table names. Pass the real dialect or pg/mssql queries hit the // wrong identifiers. const tracker = new Tracker(db, configName, 'postgres') -// Check if file needs to run (by filepath). Returns NeedsRunResult: +// Check if files need to run: the newest row per filepath in one query, +// decided in memory. Returns NeedsRunResult: // { needsRun, reason?, skipReason?, previousChecksum? } -const { needsRun, reason, skipReason } = await tracker.needsRun(filepath, checksum, force) - -// Inside a running operation, exclude that operation's own pending rows or -// every file reads as 'new' forever -const check = await tracker.needsRun(filepath, checksum, force, operationId) +const [latest, latestErr] = await tracker.latestExecutions() +const { needsRun, reason, skipReason } = decideNeedsRun(latestErr ?? latest.get(filepath), checksum, force) // Check if file needs to run (by name only - for changes) const byName = await tracker.needsRunByName(name, checksum, force) @@ -496,29 +493,19 @@ const operationId = await tracker.createOperation({ executedBy: 'alice@example.com', }) -// Create file records upfront (for full batch visibility) — fileType required +// Create file records for the files about to run — fileType required await tracker.createFileRecords(operationId, [ { filepath: '/path/to/file1.sql', fileType: 'sql', checksum: 'abc123' }, { filepath: '/path/to/file2.sql', fileType: 'sql', checksum: 'def456' }, ]) // Update individual file after execution. Positional, not an options object: -// (operationId, filepath, status, durationMs, errorMessage?, skipReason?, checksum?) -await tracker.updateFileExecution(operationId, filepath, 'success', 45, undefined, undefined, 'abc123') +// (operationId, filepath, status, durationMs, errorMessage?, skipReason?) +await tracker.updateFileExecution(operationId, filepath, 'success', 45) // Skip remaining files (e.g., on error with abortOnError) await tracker.skipRemainingFiles(operationId, 'aborted due to previous error') -// Legacy: Record a file execution (still supported). Note the field is -// `changeId`, not `operationId`. -await tracker.recordExecution({ - changeId: operationId, - filepath, - checksum, - status: 'success', - durationMs: 45, -}) - // Finalize operation with optional checksum and error message await tracker.finalizeOperation(operationId, 'success', 1234, checksum, errorMessage) ``` @@ -542,14 +529,14 @@ The API uses `'commit'` | `'revert'` for clarity, but the database stores `'chan ### Batch Visibility -Creating file records upfront provides complete audit trails: +Every file in the run set appears as `pending` before the first one executes. Skipped files leave no row. ```typescript -// All files are visible immediately as 'pending' -await tracker.createFileRecords(operationId, files) +// Every file in the run set is visible immediately as 'pending' +await tracker.createFileRecords(operationId, runSet) // As each executes, status updates to 'success' or 'failed' -await tracker.updateFileExecution(operationId, filepath, { status: 'success', ... }) +await tracker.updateFileExecution(operationId, filepath, 'success', 45) // If aborted, remaining files marked as 'skipped' await tracker.skipRemainingFiles(operationId, 'aborted due to error') diff --git a/docs/guide/changes/history.md b/docs/guide/changes/history.md index 9954ebfe..e4f6b6b9 100644 --- a/docs/guide/changes/history.md +++ b/docs/guide/changes/history.md @@ -3,6 +3,8 @@ Every database operation noorm performs is recorded. When a change runs, a build executes, or files get deployed, that execution goes into the history log with who ran it, when, how long it took, and whether it succeeded. +A build, dir, files or file run where every file is unchanged writes no operation, so it appears neither in history nor in Recent Activity. + This gives you an audit trail for compliance, a debugging tool for failures, and a reference for understanding what happened to your database. @@ -248,12 +250,12 @@ The detail view breaks down each file: |--------|------|---------| | Success | `[OK]` | File executed without errors | | Failed | `[ERR]` | Execution threw an error | -| Skipped | `[-]` | Either a prior run already applied this exact file, or an earlier file in the change failed and this one was never reached | +| Skipped | `[-]` | An earlier file in the change failed and this one was never reached | -For skipped files, the skip reason explains why: +A file a prior run already applied leaves no row, so it does not appear here. For skipped files, the skip reason names the failure: ``` -[-] setup.sql - already applied +[-] setup.sql - 001_tables.sql failed: table users already exists ``` diff --git a/docs/guide/sql-files/execution.md b/docs/guide/sql-files/execution.md index dacda022..4f7fad81 100644 --- a/docs/guide/sql-files/execution.md +++ b/docs/guide/sql-files/execution.md @@ -145,10 +145,10 @@ noorm maintains two tables in your database to track execution history. On Postg | `filepath` | File that was executed, relative to the project root | | `checksum` | SHA-256 of the SQL that actually ran (the rendered output for a `.sql.tmpl`) | | `status` | `'pending'`, `'success'`, `'failed'`, `'skipped'` | -| `skip_reason` | `'unchanged'` when change detection skipped it, or `Skipped: failure in ` when an earlier file in the batch failed | +| `skip_reason` | `Skipped: failure in ` when an earlier file in the batch failed | | `duration_ms` | Execution time | -Every file in a batch gets a `'pending'` row before the first one executes, so an interrupted build still shows you what it was going to do. Rows move to their final status as the run proceeds. +Every file that will run gets a `'pending'` row before the first one executes, so an interrupted build still shows you what it was going to do. Rows move to their final status as the run proceeds. A file that change detection skips gets no row. These tables let noorm answer: "Has this exact file content been executed before, and did it succeed?" diff --git a/docs/reference/sdk.md b/docs/reference/sdk.md index 5f090229..e7e012ba 100644 --- a/docs/reference/sdk.md +++ b/docs/reference/sdk.md @@ -575,7 +575,9 @@ Execute a single SQL file. await ctx.noorm.run.file('seeds/test-data.sql'); ``` -**Returns:** `Promise`. On failure `result.error` carries the SQL/load error message; on skip `result.skipReason` carries `'unchanged'` or `'already-run'`. +**Options (`RunOptions`):** `{ force?: boolean; abortOnError?: boolean; dryRun?: boolean; preview?: boolean; output?: string | null }`. `preview` renders the file without executing it or writing history. + +**Returns:** `Promise`. On failure `result.error` carries the SQL/load error message; on skip `result.skipReason` carries `'unchanged'`. A skipped file writes no execution history. #### run.files(filepaths, options?) diff --git a/docs/spec/prefetch-run-gate.md b/docs/spec/prefetch-run-gate.md new file mode 100644 index 00000000..a4f73677 --- /dev/null +++ b/docs/spec/prefetch-run-gate.md @@ -0,0 +1,232 @@ +# Spec: prefetch run gate + + +## Approach + +One prefetch query per batch replaces the per-file skip lookups. A pure decision function, applied in memory, decides skip or run. See `docs/design/prefetch-run-gate.md`. + + +## Goal + +`run build`, `run dir`, `run files`, `run file`, `checkFilesStatus` and the change executor decide skip-or-run for a whole batch with a fixed number of tracking queries, independent of file count. A skipped file leaves no trace in the tracking tables. + + +## Non-goals + +- No parallel execution. Files still run sequentially, in discovery order. +- No change to the change-level `ChangeHistory.needsRun` (one query per change). +- No schema change to `__noorm_change__` or `__noorm_executions__`. +- No change to cascade skips: when `abortOnError` stops a batch, the run-set files it never reached keep their pending rows, which `skipRemainingFiles` turns into `skipped`. +- No change to dry-run or preview modes in `executeFiles`. + + +## Success criteria + +1. A pure exported function `decideNeedsRun(record, checksum, force)` holds the runner's skip rules. It returns the same `NeedsRunResult` the per-file `Tracker.needsRun` lookup returned, which is removed: + - `force` → run, reason `force` + - lookup error → run, reason `error` + - no record → run, reason `new` + - `failed` → run, reason `failed` + - `pending` → run, reason `new` + - `skipped` with `skip_reason` other than `unchanged` → run, reason `new` + - parent change `stale` → run, reason `stale` + - checksum differs → run, reason `changed` + - otherwise skip, `skipReason: 'unchanged'`. This includes a `skipped/unchanged` row with a matching checksum, a shape written by older builds. +2. `Tracker.latestExecutions()` returns the newest execution row per filepath for the config in one SELECT, as a `Map`. It uses the `MAX(id) GROUP BY filepath` form, which runs on all four dialects. On error it emits `observer` `error` and returns the error, so the gate treats every file as reason `error`. +3. `executeFiles`, for non-`change` change types, runs this gate before any tracking write: + 1. Load and render every file once, then hash the rendered SQL. A `.sql.tmpl` is decided on its rendered hash, and a plain `.sql` on its content hash. A file that fails to load goes into the run set with its load error and fails at its turn in the loop. + 2. One `latestExecutions()` call. `force` skips it. + 3. `decideNeedsRun` per file. + 4. Run set empty → return a `BatchResult` with `status: 'success'`, every file `skipped`, no `changeId`, and zero rows written to `__noorm_change__` or `__noorm_executions__`. + 5. Otherwise create the operation, then `createFileRecords` for the run set only, seeded with the rendered checksum. + 6. Walk the files in discovery order. A skip pushes a `skipped` `FileResult` and emits `file:skip` with no database call. A run executes the SQL already rendered in step 1 and updates its row. +4. Skipped files produce no `__noorm_executions__` rows of any status. +5. Tracking queries per `runBuild` are constant in file count. Going from 3 files to 30, with the same skip/run mix, adds zero SELECTs against `__noorm_executions__`. +6. A template whose raw bytes are unchanged but whose rendered output changed (a config or secret input changed) runs. A template whose rendered output is unchanged skips. +7. `runFile` routes through `executeFiles`. A skipped single file writes no operation row. `runFile(..., { preview: true })` renders without executing or writing tracking rows, the same as every other entry point. `executeSingleFile`, `Tracker.recordExecution` and `RecordExecutionData` are removed. +8. `checkFilesStatus` uses `latestExecutions()` + `decideNeedsRun`, so one query for any file count, with categories unchanged. +9. Change executor: + 1. `ChangeHistory` gains a pure `decideNeedsRunFile(record, checksum, force)` and a batch `latestFileExecutions(name, direction)`. The batch method runs one boundary SELECT plus one history SELECT per change, applying the filters the per-file lookup used (name, direction, config, change status not in `reverted`/`stale`, exec status not in `pending`/`skipped`, `id > boundary`). The per-file `needsRunFile` lookup is removed. A filepath listed more than once in a change (through `.txt` manifests) runs at its first occurrence and skips with `already applied` at each later one. + 2. Per-file decisions happen before `createFileRecords`. Pending rows are created for the run set only. Skipped files emit their `skipped` `ChangeFileResult` and make no database call. + 3. The operation row is still written when every file skips, so the change's combined checksum is recorded. + 4. The apply → revert → apply cycle and retry-after-fix behavior are unchanged. The existing `executor.test.ts`, `executor-retry.test.ts` and `history.test.ts` stay green. +10. `bun run typecheck`, `bun run typecheck:tests`, `bun run lint` and `bun test --serial` show no new type errors or test failures against the loop base `bc3ace00`. The integration suites under `tests/integration/runner` and `tests/integration/change` pass where their databases are available. + + +## Checkpoints + +| # | Checkpoint | Files/areas | Agent | Est. files | Verifies | +|---|------------|-------------|-------|------------|----------| +| 1 | Pure decision + batch prefetch | `src/core/runner/tracker.ts`, `types.ts` | atomic-implementer | 4 | 1, 2 | +| 2 | Runner gate | `src/core/runner/runner.ts`, `tracker.ts` | atomic-implementer | 10 | 3–8 | +| 3 | Change-path prefetch | `src/core/change/history.ts`, `executor.ts` | atomic-implementer | 10 | 9, 10 | + + +## Change tree + +``` +src/core/runner/ + M tracker.ts decideNeedsRun, latestExecutions; needsRun, recordExecution removed + M runner.ts gate in executeFiles; runFile via executeFiles; checkFilesStatus batched; executeSingleFile removed + M types.ts ExecutionRecord added; RecordExecutionData, FileInput.checksum removed + M index.ts export decideNeedsRun, ExecutionRecord +src/core/change/ + M history.ts decideNeedsRunFile, latestFileExecutions; needsRunFile removed + M executor.ts runFileBatch decides via findSkippedFiles before createFileRecords + M types.ts FileExecutionRecord + M index.ts export decideNeedsRunFile, FileExecutionRecord +src/core/ + M index.ts RecordExecutionData export removed + M shared/tables.ts skip_reason doc +tests/core/runner/ + A needs-run-decision.test.ts every rule in decideNeedsRun + A prefetch-gate.test.ts no skip rows, no op row when all skip, constant query count, runFile skip + M template-dedup.test.ts rendered-output change runs; unchanged output skips with no rows + M tracker.test.ts latestExecutions + decideNeedsRun +tests/core/change/ + A prefetch-gate.test.ts change path: constant query count, skipped files no rows, op row kept + M executor-retry.test.ts skipped files leave no execution rows +tests/integration/ + M runner/tracker-dialects.test.ts latestExecutions per dialect + M change/history-dialects.test.ts latestFileExecutions per dialect +tests/cli/run/ + M build.test.ts skip-on-rerun comments +tests/sdk/ + M run-build-filtering.test.ts mock comment +docs/ + M dev/runner.md + M dev/change.md + M dev/datamodel.md + M guide/changes/history.md + M guide/sql-files/execution.md + M reference/sdk.md + M tui.md + M spec/v1-49-54-cli-field-defects.md +skills/noorm/references/ + M cli.md file status covers executed files only +.changeset/ + A prefetch-run-gate.md +``` + + +## Outline + +``` +src/core/runner/tracker.ts + decideNeedsRun — pure skip/run rules from a latest record (or lookup error) and checksum + Tracker + latestExecutions — newest execution row per filepath for the config, one SELECT +src/core/runner/runner.ts + prepareFiles — render once and hash each file, carrying rendered SQL or the load error + applyRunGate — one latestExecutions call, decideNeedsRun per file + skipFile — skipped FileResult with no tracking write + executeFiles — gate, then operation and pending rows for the run set, then sequential walk + executeSingleFileWithUpdate — executes pre-rendered SQL, no lookup + runFile — thin wrapper over executeFiles returning the single FileResult + checkFilesStatus — batch decision via latestExecutions + statusCategory — maps a NeedsRunResult to a file status category +src/core/change/history.ts + decideNeedsRunFile — pure per-file rules for the change path + ChangeHistory + latestFileExecutions — boundary plus newest qualifying row per filepath, one change +src/core/change/executor.ts + findSkippedFiles — latestFileExecutions + decideNeedsRunFile, first occurrence of a repeated filepath wins + runFileBatch — decide before createFileRecords, pending rows for run set only +``` + + +## Flows + +**Build with mixed run set** + +1. Runner calls `executeFiles` with N discovered files. +2. `prepareFiles` renders and hashes each file, keeping its SQL. +3. `latestExecutions()` returns the newest row per filepath (one SELECT). +4. `decideNeedsRun` splits the files into a run set and a skip set. +5. `createOperation` inserts one `__noorm_change__` row. `createFileRecords` inserts pending rows for the run set. +6. Each file in discovery order is either skipped (`file:skip`, result pushed) or executed (`file:before`, SQL, `updateFileExecution`, `file:after`). +7. `finalizeOperation` writes the status and combined checksum. + +**Build where everything is unchanged** + +1. Steps 1–4 above. The run set is empty. +2. Every file emits `file:skip`. +3. `executeFiles` returns `status: 'success'` with no `changeId`. The tracking tables are untouched. + +**Change apply** + +1. The executor checks the change-level `needsRun` (unchanged). +2. `latestFileExecutions(name, direction)` runs the boundary SELECT and one history SELECT. +3. `decideNeedsRunFile` per file splits the files into a run set and a skip set. +4. `createOperation`, then `createFileRecords` for the run set. +5. Skipped files push a result with no database call. Run-set files execute and update their rows. +6. `finalizeOperation` records the change checksum. + + +## Risks + +- Templates render before the first file executes, so a render side effect (a helper script) now happens for files after a failing one. Accepted: rendering has no database access. +- Rendered SQL for every file is held in memory for the batch. SQL files are small, and builds run in the thousands of files at most. +- Older databases hold `skipped/unchanged` rows. `decideNeedsRun` keeps treating them as covering a skip. + + +## Change log + +### 2026-10-07 — Initial spec + +**What changed:** First version. + +**Why:** The per-file skip lookup costs two round trips per file. + +### 2026-10-07 — runFile preview, dead tracker surface + +**What changed:** Criterion 7 now states that `runFile` honors `preview`. It also removes `RecordExecutionData` and `needsRun`'s `excludeOperationId`. + +**Why:** Routing `runFile` through `executeFiles` made it honor `preview`. Before, `run.file(path, { preview: true })` executed the SQL. The CP2 review also found the parameter and the type had no callers left. + +**Superseded:** `runFile` ignored `preview` and executed the file. + +### 2026-10-07 — Remove the per-file lookups, keep duplicate-entry skip + +**What changed:** Criteria 1, 7 and 9.1 now remove `Tracker.needsRun` and `ChangeHistory.needsRunFile`. Criterion 9.1 now states that a filepath repeated in one change runs once. + +**Why:** After the gate, neither lookup had a caller in `src/`. The CP3 review found that deciding every file up front would run a duplicated manifest entry twice. Before, the second occurrence saw the first one's success row and skipped. + +**Superseded:** `needsRun` and `needsRunFile` were kept and delegated to the pure functions. + +### 2026-10-07 — Correction: criterion 10 baseline + +**What changed:** Criterion 10 now requires no new type errors or test failures against the loop base `bc3ace00`, instead of a clean pass. + +**Why:** `bun run typecheck:tests` reports 323 errors at `bc3ace00` and at the delivered HEAD, none in a file this work touched. `tests/cli` keeps one failure that predates the work: `cli: components/TextInput > rendering > should draw the cursor, placeholder and suggestion exactly as upstream does`. + +**Superseded:** Criterion 10 required all four commands to pass outright. + + +## Implementation log + +### shipped — 2026-10-07 + +Built across 7 iterations of /autopilot. Commits (chronological): + +- `1695a0e8` — CP-1 pure `decideNeedsRun`, batched `Tracker.latestExecutions` +- `adbdba89` — CP-2 runner gate, `runFile` via `executeFiles`, batched `checkFilesStatus` +- `3c00902e` — CP-3 change-path prefetch, per-file lookups removed +- `405b9418` — docs: SDK reference, changeset +- `89848b70` — audit fixes: docs, spec, changeset + +**Out-of-scope work performed during this build:** + +- `checkFilesStatus` looked up absolute paths against relative rows, so the TUI re-run prompt never fired. Fixed in CP-2 because the batched rewrite touched the same lookup. + +**Unforeseens — surprises that emerged during implementation:** + +- `runFile` ignored `preview` and executed SQL. Routing through `executeFiles` fixed it, and criterion 7 was amended. +- Deciding change files up front would run a path repeated through `.txt` manifests twice. Repeats now skip, and criterion 9.1 was amended. +- `Tracker.needsRun` and `ChangeHistory.needsRunFile` lost every caller and were removed. + +**Deferred items still open:** + +- none. `Tracker.needsRunByName` was dead before this build and is out of scope. + +**Squashed into one commit on `perf-prefetch-gate` and rebased onto `next` (2026-10-07).** The per-iteration SHAs above are historical and unreachable from any branch. The rebase merged this work with the statement-watcher cancel feature in `executeFiles`. diff --git a/docs/spec/v1-49-54-cli-field-defects.md b/docs/spec/v1-49-54-cli-field-defects.md index 9df04cf4..180a85cf 100644 --- a/docs/spec/v1-49-54-cli-field-defects.md +++ b/docs/spec/v1-49-54-cli-field-defects.md @@ -300,8 +300,8 @@ every file, so a second build against a populated database is *guaranteed* to fa first non-idempotent DDL statement. For the reporter's schema that is `CREATE TYPE mls_id` — the first statement of the second phase, exactly as reported. -Fix: exclude the current operation's own rows from the `needsRun` lookup — `change_id <> -operationId`, or `executions.id <` the first id inserted by this run. +Fix: decide skip-or-run for the whole batch before the operation inserts any pending row, +so the lookup never sees this run's rows (`docs/spec/prefetch-run-gate.md`). **Do not "scope `needsRun` to the current operation".** An earlier draft of this spec said that; it is backwards and would cement the defect while appearing to fix it. The pending row @@ -372,3 +372,7 @@ Integration tests requiring MSSQL use `TEST_MSSQL_*` from `tests/sample.env` (po citty dispatch (the design doc's D1) so they worked in either position. That created an asymmetry with `--config`/`--force`, which were never hoisted, for no reason beyond convenience. The standard is now uniform: a flag goes on the command that uses it. +- 2026-10-07 — CP10 fix replaced by the prefetch run gate (`docs/spec/prefetch-run-gate.md`): + the batch decides skip-or-run before any pending row exists. + **Superseded:** `needsRun` excluded the running operation's own rows (`change_id <> + operationId`). diff --git a/docs/tui.md b/docs/tui.md index d1277d9a..42fabf82 100644 --- a/docs/tui.md +++ b/docs/tui.md @@ -310,6 +310,8 @@ Press `h` for the execution history — what ran, when, and who ran it: ![The run menu](/image/tui/run-menu.png) +Picking a file that already ran with the same content opens **File Previously Run** and asks "Run this file again?". A directory with such files opens **Files Previously Run**. + ### Database Menu diff --git a/docs/wiki/core-change.md b/docs/wiki/core-change.md index 9e043b4d..a4263a9c 100644 --- a/docs/wiki/core-change.md +++ b/docs/wiki/core-change.md @@ -69,14 +69,14 @@ Inside the transaction, `batchResult.status !== 'success'` is the rollback trigg ### Per-file skip on retry -Per file inside `runFileBatch`, `history.needsRunFile` can still skip a file whose checksum already succeeded, even when the overall change checksum changed because a sibling file needed fixing: +`runFileBatch` first calls `findSkippedFiles`, which runs `ChangeHistory.latestFileExecutions(name, direction)` (a boundary SELECT plus one history SELECT) and the pure `decideNeedsRunFile`. A file whose checksum already succeeded skips even when the overall change checksum changed because a sibling file needed fixing. A path repeated through `.txt` manifests runs once; later positions skip as 'already applied'. Skipped files get no pending row and no DB call, and the operation row is still written when every file skips: ```mermaid flowchart TD %% source: src/core/change/executor.ts, src/core/change/history.ts - A["for each expanded file"] --> B{needsRunFile} - B -->|no| C["record status: skipped"] --> A - B -->|yes| D[loadAndRenderFile] + A["for each expanded file"] --> B{"decideNeedsRunFile via findSkippedFiles"} + B -->|skip| C["no row, no DB call"] --> A + B -->|run| D[loadAndRenderFile] D -->|error| E["record failed, break loop"] D -->|ok| F["watcher.run(file.path, executor, ...)"] F -->|error| G["record failed, break loop"] @@ -120,7 +120,7 @@ stateDiagram-v2 | [`src/core/change/parser.ts`](../../src/core/change/parser.ts) | `parseChange`/`discoverChanges`, scans a change folder, validates structure, resolves `.txt` manifests, parses sequence/date prefixes | | [`src/core/change/scaffold.ts`](../../src/core/change/scaffold.ts) | Creates/deletes/renames/reorders change files and folders on disk | | [`src/core/change/tracker.ts`](../../src/core/change/tracker.ts) | `ChangeTracker` (extends `Tracker`), `canRevert`, `markAsReverted`, `markAllAsStale` | -| [`src/core/change/history.ts`](../../src/core/change/history.ts) | `ChangeHistory`, `needsRun`/`needsRunFile`, operation/file record CRUD, `hydrateDate` UTC normalization | +| [`src/core/change/history.ts`](../../src/core/change/history.ts) | `ChangeHistory`, `needsRun`, `latestFileExecutions`, operation/file record CRUD, `hydrateDate` UTC normalization | | [`src/core/change/types.ts`](../../src/core/change/types.ts) | `Change`, `ChangeContext`, `ChangeOptions`, `ChangeResult`, error classes, `isPendingChange` | | [`src/core/change/validation.ts`](../../src/core/change/validation.ts) | `validateChangeContent`/`SQL_TEMPLATE`, used only by TUI pre-flight checks, not by the executor's own content gate | | [`src/cli/change/index.ts`](../../src/cli/change/index.ts) | `change` command group registration (`add`, `edit`, `rm`, `run`, `next`, `ff`, `revert`, `rewind`, `list`, `history`, `history-detail`) | @@ -133,7 +133,7 @@ stateDiagram-v2 - `executor.ts`'s pre-execution content gate (`hasExecutableSql`, preceded by the `files.length === 0` throw) checks for any non-blank, non-`--`-comment line; it does not call `validateChangeContent` from `validation.ts`, which is a stale check still used only by the TUI's `ChangeFFScreen`/`ChangeRunScreen`. A stub worded to pass `validateChangeContent` can still fail the executor's own gate, so TUI pre-flight and `noorm change run` can disagree about whether a change is runnable. - `createChange` always scaffolds a stub file into both `change/` and `revert/`. An empty `change/`+`revert/` pair fails `parseChange`'s validation (`scaffold.ts:146-149`), and without the stub the caller sees that misreported as "change not found" instead of "needs editing". - Change directory names follow `YYYY-MM-DD-`; a name without a date prefix parses with `date: null`, and `discoverChanges` sorts by raw name (`a.name.localeCompare(b.name)`, `parser.ts:215`), so an undated `add-users` sorts after every `2024-...` change and moves in `ff`/`next` order. Files inside are ordered by `filename.localeCompare` (`parser.ts:440`), not by parsed sequence number, so an unpadded sequence prefix (`2_foo.sql` before `10_bar.sql`) sorts and runs out of numeric order. -- `ChangeHistory.needsRunFile` bounds its lookback at the most recent opposite-direction operation, so a prior success only licenses a per-file skip while no revert/re-apply has happened since. It also retires a prior success once the parent operation's own status is `reverted` or `stale` (`history.ts:494-509`); without both conditions, every apply -> revert -> apply cycle silently no-ops its files instead of re-running them. +- `ChangeHistory.latestFileExecutions` bounds its lookback at the most recent opposite-direction operation, so a prior success only licenses a per-file skip while no revert/re-apply has happened since. It also retires a prior success once the parent operation's own status is `reverted` or `stale`; without both conditions, every apply -> revert -> apply cycle silently no-ops its files instead of re-running them. - `RESET_MARKER = '__reset__'` is a reserved change name written by `ChangeHistory.recordReset` for teardown audit rows. A user change named `__reset__` collides with it: `getAllStatuses` filters that name out, so the change disappears from `change list` even though it still shows in `getHistory`/`getUnifiedHistory`. - `history.ts`'s `hydrateDate` normalizes `executed_at` to UTC for Postgres/MySQL (reinterpreted field-by-field) and SQLite (text with `Z` appended); MSSQL is left unmodified on purpose, because its driver's behavior was never measured. On a host whose local zone isn't UTC, MSSQL's `executed_at` can render in the TUI's relative-time display shifted by the host's UTC offset. - Add a new status to `isPendingChange` only. An inlined copy of the check drifts, and `ff` then reports success while work is still outstanding. diff --git a/docs/wiki/core-runner.md b/docs/wiki/core-runner.md index 6b5e893e..6a93b481 100644 --- a/docs/wiki/core-runner.md +++ b/docs/wiki/core-runner.md @@ -14,19 +14,21 @@ Runs `.sql` and `.sql.tmpl` files against a Kysely connection with checksum-base ### Execution and change detection -A file's status decides whether it runs, and a run always records the outcome before moving to the next file. +`executeFiles` renders and hashes every file once, reads prior outcomes in one query, and decides the whole batch in memory. Only files that need a run touch the database again. ```mermaid flowchart TD - A[loadAndRenderFile] --> B[computeChecksumFromContent] - B --> C{Tracker.needsRun} - C -->|"new / changed / failed / stale / force / error"| D[runWatched: executeSqlBody via StatementWatcher] - C -->|unchanged| E[skip: skipReason unchanged] - D --> F[Tracker.updateFileExecution] - E --> F + A["prepareFiles: render + computeChecksumFromContent"] --> B["Tracker.latestExecutions (one query, skipped under force)"] + B --> C{"decideNeedsRun per file"} + C -->|"unchanged"| E["skipFile: no row, no DB call"] + C -->|"new / changed / failed / stale / force / error"| F["createOperation + createFileRecords (run set only)"] + F --> D["runWatched: executeSqlBody via StatementWatcher"] + D --> G["Tracker.updateFileExecution"] ``` -`executeSingleFileWithUpdate` ([`src/core/runner/runner.ts`](../../src/core/runner/runner.ts)) loads and renders the file first, then recomputes the checksum from the rendered content, because comparing raw `.sql.tmpl` bytes would re-execute every template on every build. The raw `computeChecksum` only seeds the pending row that `createFileRecords` inserts for every file in the batch before the batch starts. [`src/core/runner/tracker.ts`](../../src/core/runner/tracker.ts)'s `Tracker.needsRun` excludes that pending row by its own operation id (`excludeOperationId`), or every file would read as "new" forever. A prior `skipped` row with `skip_reason: 'unchanged'` counts as a valid outcome and falls through to the stale and checksum comparison; any other `skipped` row (a cascade skip after an earlier failure, or a cancelled run) and any `pending` row re-runs with `reason: 'new'`. +`prepareFiles` ([`src/core/runner/runner.ts`](../../src/core/runner/runner.ts)) loads and renders each file once and hashes the rendered content, because comparing raw `.sql.tmpl` bytes would re-execute every template on every build. The rendered SQL is kept and run as-is later. `Tracker.latestExecutions` ([`src/core/runner/tracker.ts`](../../src/core/runner/tracker.ts)) fetches the latest outcome per file in one query, and the pure `decideNeedsRun` (applied by `applyRunGate`) compares it to the new checksum. A legacy `skipped` row with `skip_reason: 'unchanged'` still counts as a prior outcome; any other `skipped` row (a cascade skip after an earlier failure, or a cancelled run) and any `pending` row re-runs with `reason: 'new'`. + +Skipped files write no rows. If every file skips, no operation row is written and `changeId` is `undefined`. Otherwise `createOperation` and `createFileRecords` cover the run set only, and `executeSingleFileWithUpdate(context, entry, tracker, operationId, watcher)` walks it sequentially. `runFile` routes through `executeFiles`, and `checkFilesStatus` uses the same batched lookup. ### Long-running statement detection @@ -76,7 +78,7 @@ Aborting `RunContext.signal` sends the dialect's `SERVER_CANCEL` from the side c | [`src/core/runner/runner.ts`](../../src/core/runner/runner.ts) | `runBuild`/`runFile`/`runDir`/`runFiles`/`preview`/`checkFilesStatus`/`discoverFiles`/`executeFiles`; all except `discoverFiles`/`executeFiles` are gated. `createWatcher` builds one `StatementWatcher` per run and routes every file's SQL through `runWatched`. | | [`src/core/runner/statement-watcher.ts`](../../src/core/runner/statement-watcher.ts) | `StatementWatcher` class: pins a file's connection, reads its session id, times the 10s delay and 10s report interval via `#report`/`#poll`, checks out and holds the side connection via `#sideConnection`, sends the cancel on abort via `#cancel`. | | [`src/core/runner/statement-probes.ts`](../../src/core/runner/statement-probes.ts) | `STATEMENT_PROBES` (postgres/mssql/mysql `StatementProbe` functions) and the `StatementStatus`/`BlockingSession`/`OperationProgress` shapes a probe returns. | -| [`src/core/runner/tracker.ts`](../../src/core/runner/tracker.ts) | `Tracker` class: `needsRun`, `needsRunByName`, `createOperation`, `recordExecution`, `createFileRecords`, `updateFileExecution`, `finalizeOperation`, `skipRemainingFiles`, `priorSuccessfulExecutions`. | +| [`src/core/runner/tracker.ts`](../../src/core/runner/tracker.ts) | `Tracker` class: `latestExecutions`, `needsRunByName`, `createOperation`, `createFileRecords`, `updateFileExecution`, `finalizeOperation`, `skipRemainingFiles`, `priorSuccessfulExecutions`. | | [`src/core/runner/checksum.ts`](../../src/core/runner/checksum.ts) | `computeChecksum`, `computeChecksumFromContent`, `computeCombinedChecksum` (SHA-256). | | [`src/core/runner/mssql-batches.ts`](../../src/core/runner/mssql-batches.ts) | `executeSqlBody` (dialect dispatch: mssql splits on line-only `GO`, sqlite splits on statement boundaries, postgres/mysql execute the body whole), `splitMssqlBatches`. | | [`src/core/runner/sqlite-statements.ts`](../../src/core/runner/sqlite-statements.ts) | `splitSqliteStatements`, a boundary scanner (not a SQL parser) tracking string/identifier quoting, comments, and `BEGIN`/`CASE`…`END` trigger bodies. | @@ -103,7 +105,7 @@ Aborting `RunContext.signal` sends the dialect's `SERVER_CANCEL` from the side c ## Constraints -- Dropping `excludeOperationId` from `Tracker.needsRun` makes every file read as "new" forever, since the batch's own `pending` rows would be the latest record. +- Reading `latestExecutions` after `createFileRecords` makes every file read as "new" forever, since the batch's own `pending` rows would be the latest record. Read prior outcomes first. - Comparing raw `.sql.tmpl` bytes instead of the rendered checksum re-executes every template on every build. - Skipping `StatementWatcher.close()` leaks any side connection checked out during the run; it never returns to the pool. - With `connection.pool.max: 1`, the side connection checkout waits `SIDE_CONNECTION_WAIT_MS` (5s) then the watcher gives it up for the rest of the run: reports carry elapsed time only, and cancel can only act between files. diff --git a/docs/wiki/index.md b/docs/wiki/index.md index 75401db0..07a8f581 100644 --- a/docs/wiki/index.md +++ b/docs/wiki/index.md @@ -90,7 +90,7 @@ CI gate: lint → typecheck → build → 5 test groups → 3 example jobs. Inte **Convention pointers:** [`.claude/rules/typescript.md`](../../.claude/rules/typescript.md) (4-block function structure, `attempt` over try-catch), [`.claude/rules/tui-development.md`](../../.claude/rules/tui-development.md) (focus system, Ink layout), [`.claude/rules/testing.md`](../../.claude/rules/testing.md) (test naming, coverage), [`.claude/rules/documentation.md`](../../.claude/rules/documentation.md) (three-pillar structure). -**Domain partitioning basis:** Domains are functional vertical slices. `core-state` groups the startup/persistence concerns (state, settings, config, lifecycle, version) because they all initialize together in `project-init.ts`. `core-identity` groups crypto identity, vault, logger, and SQL terminal because they share the "user-facing sensitive data" concern. `core-db` groups connection, explore, transfer, and teardown because they all operate against a live database connection. `core-change` and `core-runner` are separate because changes are versioned operations while runner handles idempotent file execution — they do not share an execution path (core-change implements its own `executeFiles`/`needsRun`, distinct from the core runner's same-named functions). `core-policy` is a cross-cutting domain ([`src/core/policy/`](../../src/core/policy)): domains that enforce a config-scoped action via `assertPolicy`/`checkConfigPolicy` (`core-change`, `core-runner`, `core-db`, `core-identity`, `sdk`, `cli`, `tui`, `mcp-rpc`) import from it directly, and `core-state` imports it too but only for `resolveLegacyAccess`/`guarded` — data resolution and display styling, not enforcement. `infra` absorbs the entire [`docs/`](..) tree (including the new [`docs/tapes/`](../tapes) VHS demo-recording sources, [`docs/guide/relational-design.md`](../guide/relational-design.md), and [`docs/modeling/`](../modeling) — which documents `ignatius`, a separate information-modeling tool in a separate repo, not noorm's own code) rather than splitting docs out per-domain, since the docs site is built/deployed as one VitePress unit. +**Domain partitioning basis:** Domains are functional vertical slices. `core-state` groups the startup/persistence concerns (state, settings, config, lifecycle, version) because they all initialize together in `project-init.ts`. `core-identity` groups crypto identity, vault, logger, and SQL terminal because they share the "user-facing sensitive data" concern. `core-db` groups connection, explore, transfer, and teardown because they all operate against a live database connection. `core-change` and `core-runner` are separate because changes are versioned operations while runner handles idempotent file execution — they do not share an execution path (core-change implements its own `executeFiles` and skip-or-run decision, distinct from the core runner's same-named functions). `core-policy` is a cross-cutting domain ([`src/core/policy/`](../../src/core/policy)): domains that enforce a config-scoped action via `assertPolicy`/`checkConfigPolicy` (`core-change`, `core-runner`, `core-db`, `core-identity`, `sdk`, `cli`, `tui`, `mcp-rpc`) import from it directly, and `core-state` imports it too but only for `resolveLegacyAccess`/`guarded` — data resolution and display styling, not enforcement. `infra` absorbs the entire [`docs/`](..) tree (including the new [`docs/tapes/`](../tapes) VHS demo-recording sources, [`docs/guide/relational-design.md`](../guide/relational-design.md), and [`docs/modeling/`](../modeling) — which documents `ignatius`, a separate information-modeling tool in a separate repo, not noorm's own code) rather than splitting docs out per-domain, since the docs site is built/deployed as one VitePress unit. **Access-control policy (2026-07, config-access-roles feature):** `Config.protected: boolean` was replaced by `Config.access: ConfigAccess` (per-channel `user`/`agent` roles), enforced through the `core-policy` domain. `src/core/config/protection.ts` and `src/rpc/protection.ts` were both deleted — their rule-checking is absorbed into `core/policy`. The runner/change/transfer/sql-terminal executors gate at their core seam via `assertPolicy`, so SDK/CLI/TUI/MCP callers all inherit one enforcement path. `StateManager.load()` runs the schemaVersion-keyed migration (`core/version/state/`, v2 maps `protected`→`access`, v3 maps `access.mcp`→`access.agent`) ahead of the pre-existing package-semver migration. diff --git a/docs/wiki/scan.md b/docs/wiki/scan.md index c980267d..6d5eb6b8 100644 --- a/docs/wiki/scan.md +++ b/docs/wiki/scan.md @@ -22,12 +22,25 @@ │ ├── config.json (64bb386, 11L, 307ch, 307B) │ ├── connection-error-reasons.md (d86f802, 6L, 539ch, 539B) │ ├── mssql-unprivileged-logins.md (df9693f, 6L, 489ch, 489B) +│ ├── prefetch-run-gate.md (9b2ca2f, 6L, 396ch, 396B) │ ├── slow-statement-watcher.md (7fa4155, 6L, 639ch, 639B) │ ├── tui-explore-duplicate-keys.md (80009cf, 6L, 445ch, 445B) -│ ├── tui-progress-bars.md (5dba30d, 5L, 167ch, 167B) │ └── tui-typing-blocks-shortcuts.md (c3a693c, 5L, 356ch, 356B) ├── .claude/ (3) -│ ├── rules/ (4) +│ ├── rules/ (5) +│ │ ├── wiki/ (12) +│ │ │ ├── cli.md (b4937fd, 25L, 730ch, 732B) +│ │ │ ├── core-change.md (0e1ad1a, 17L, 597ch, 599B) +│ │ │ ├── core-db.md (491e050, 28L, 947ch, 949B) +│ │ │ ├── core-identity.md (2cf5cad, 24L, 766ch, 768B) +│ │ │ ├── core-policy.md (b1a151a, 17L, 591ch, 596B) +│ │ │ ├── core-runner.md (287760f, 17L, 598ch, 600B) +│ │ │ ├── core-state.md (6f2964b, 30L, 941ch, 943B) +│ │ │ ├── infra.md (82164d2, 18L, 526ch, 528B) +│ │ │ ├── mcp-rpc.md (0d4628c, 16L, 511ch, 513B) +│ │ │ ├── sdk.md (792107d, 22L, 620ch, 622B) +│ │ │ ├── tui.md (8aac176, 20L, 631ch, 633B) +│ │ │ └── worker-bridge.md (394769b, 15L, 521ch, 523B) │ │ ├── documentation.md (72f6102, 33L, 1577ch, 1577B) │ │ ├── testing.md (87fc604, 95L, 3931ch, 3933B) │ │ ├── tui-development.md (07906c0, 422L, 18517ch, 18527B) @@ -104,18 +117,19 @@ │ │ ├── settings-secret.md (99e8f29, 23L, 664ch, 668B) │ │ ├── sql-repl.md (20a6f63, 28L, 799ch, 803B) │ │ └── sql.md (52e5a3c, 77L, 3342ch, 3374B) -│ ├── design/ (5) +│ ├── design/ (6) │ │ ├── .gitkeep (e3b0c44, 0L, 0ch, 0B) │ │ ├── config-access-roles.md (bd73baa, 116L, 6566ch, 6670B) +│ │ ├── prefetch-run-gate.md (36f0f60, 43L, 3105ch, 3105B) │ │ ├── sdk-with-schema.md (37b324b, 117L, 7194ch, 7234B) │ │ ├── update-download-progress.md (2106019, 47L, 3002ch, 3002B) │ │ └── v1-49-54-cli-field-defects.md (6f051d8, 242L, 12448ch, 12534B) │ ├── dev/ (25) -│ │ ├── change.md (89e1ed4, 556L, 19149ch, 19231B) +│ │ ├── change.md (6cb1ae4, 575L, 20544ch, 20628B) │ │ ├── ci.md (53c262d, 221L, 8752ch, 8772B) │ │ ├── config-sharing.md (36147c5, 280L, 10834ch, 10884B) │ │ ├── config.md (d1992d9, 459L, 14716ch, 14768B) -│ │ ├── datamodel.md (93e585e, 1105L, 36367ch, 36527B) +│ │ ├── datamodel.md (0e2c39e, 1105L, 36520ch, 36680B) │ │ ├── explore.md (024a815, 361L, 10798ch, 10902B) │ │ ├── headless.md (8731e15, 839L, 23000ch, 23212B) │ │ ├── identity.md (6124955, 429L, 16703ch, 16747B) @@ -125,7 +139,7 @@ │ │ ├── lock.md (2096e54, 344L, 10829ch, 10875B) │ │ ├── logger.md (e297dba, 599L, 21334ch, 22867B) │ │ ├── project-discovery.md (ac60c94, 150L, 5071ch, 5083B) -│ │ ├── runner.md (61cd053, 614L, 26307ch, 26345B) +│ │ ├── runner.md (9908c1a, 601L, 26334ch, 26372B) │ │ ├── sdk.md (b538f8f, 1159L, 31592ch, 31668B) │ │ ├── secrets.md (47a8ca1, 321L, 11587ch, 11665B) │ │ ├── settings.md (eacd272, 854L, 23279ch, 23301B) @@ -148,7 +162,7 @@ │ │ │ └── non-interactive.md (0babb24, 138L, 5135ch, 5149B) │ │ ├── changes/ (3) │ │ │ ├── forward-revert.md (e89c652, 301L, 11159ch, 11161B) -│ │ │ ├── history.md (cf1d916, 334L, 11323ch, 11325B) +│ │ │ ├── history.md (ce7de9c, 336L, 11526ch, 11528B) │ │ │ └── overview.md (4bc55b3, 409L, 16143ch, 16485B) │ │ ├── database/ (5) │ │ │ ├── create.md (061870f, 169L, 6551ch, 6573B) @@ -162,7 +176,7 @@ │ │ │ ├── stages.md (14f1041, 252L, 8279ch, 8283B) │ │ │ └── vault.md (b1af624, 258L, 8719ch, 9267B) │ │ ├── sql-files/ (3) -│ │ │ ├── execution.md (5bc154f, 265L, 11279ch, 11421B) +│ │ │ ├── execution.md (059d29c, 265L, 11279ch, 11421B) │ │ │ ├── organization.md (5febfd5, 351L, 9226ch, 9692B) │ │ │ └── templates.md (c8f060b, 508L, 16419ch, 16547B) │ │ ├── deployment.md (0e61d4d, 190L, 8503ch, 8530B) @@ -253,14 +267,15 @@ │ │ │ └── ignatius.mp4 (f46adb8, 26426L, 6528910ch, 6776756B) │ │ └── install.sh (0cc90a2, 116L, 2925ch, 2925B) │ ├── reference/ (1) -│ │ └── sdk.md (be03cd8, 1705L, 63186ch, 63417B) -│ ├── spec/ (6) +│ │ └── sdk.md (a8c2d94, 1707L, 63416ch, 63647B) +│ ├── spec/ (7) │ │ ├── .gitkeep (e3b0c44, 0L, 0ch, 0B) │ │ ├── config-access-roles.md (40ef290, 162L, 21805ch, 21929B) +│ │ ├── prefetch-run-gate.md (f939bfe, 232L, 13949ch, 14025B) │ │ ├── sdk-with-schema.md (593829a, 174L, 13384ch, 13504B) │ │ ├── update-download-progress.md (85fec84, 152L, 9599ch, 9677B) │ │ ├── v1-45-rewind-tiebreak.md (0e35550, 61L, 5465ch, 5507B) -│ │ └── v1-49-54-cli-field-defects.md (438757f, 374L, 23347ch, 23495B) +│ │ └── v1-49-54-cli-field-defects.md (bf30805, 378L, 23615ch, 23763B) │ ├── superpowers/ (1) │ │ └── specs/ (1) │ │ └── 2026-04-19-cli-ci-identity-design.md (6c9cc80, 938L, 32762ch, 32918B) @@ -287,7 +302,7 @@ │ ├── headless.md (a76c0a6, 1870L, 55232ch, 55376B) │ ├── index.md (376b0eb, 166L, 6438ch, 6452B) │ ├── package.json (b4778f0, 24L, 657ch, 657B) -│ ├── tui.md (da1aad9, 537L, 18451ch, 19040B) +│ ├── tui.md (5ae509e, 539L, 18629ch, 19218B) │ └── why-noorm.md (dc7e978, 109L, 7078ch, 7078B) ├── examples/ (3) │ ├── llm-memory-db-mssql/ (16) @@ -561,13 +576,13 @@ │ │ │ └── views.test.ts (0ce91fc, 746L, 22141ch, 22887B) │ │ ├── .gitignore (ccb61cd, 40L, 446ch, 446B) │ │ ├── .mcp.json (14f011f, 11L, 174ch, 174B) -│ │ ├── CHANGELOG.md (4f6a37d, 127L, 2547ch, 2547B) +│ │ ├── CHANGELOG.md (a10ab3e, 142L, 2752ch, 2752B) │ │ ├── CLAUDE.md (1f39d31, 111L, 2676ch, 2676B) │ │ ├── README.md (c228f5b, 121L, 6833ch, 6869B) │ │ ├── REPORT.md (4f3efcd, 161L, 12957ch, 13004B) │ │ ├── mcp-config.json (14f011f, 11L, 174ch, 174B) │ │ ├── mssql-problems.md (5083524, 328L, 23834ch, 23934B) -│ │ ├── package.json (09c895f, 23L, 623ch, 623B) +│ │ ├── package.json (0e20903, 23L, 623ch, 623B) │ │ └── tsconfig.json (7be3bae, 27L, 736ch, 736B) │ ├── llm-memory-db-pg/ (17) │ │ ├── .cursor/ (1) @@ -773,13 +788,13 @@ │ │ │ └── mcp-discovery.test.ts (02d4035, 765L, 24559ch, 24603B) │ │ ├── .gitignore (a94396a, 36L, 397ch, 397B) │ │ ├── .mcp.json (14f011f, 11L, 174ch, 174B) -│ │ ├── CHANGELOG.md (2ff7198, 127L, 2544ch, 2544B) +│ │ ├── CHANGELOG.md (97e9542, 142L, 2749ch, 2749B) │ │ ├── CLAUDE.md (1f39d31, 111L, 2676ch, 2676B) │ │ ├── README.md (b97f95b, 205L, 10359ch, 10665B) │ │ ├── REPORT-PHASE-1.md (59b7d41, 103L, 9610ch, 9670B) │ │ ├── REPORT.md (98ee180, 140L, 17043ch, 17113B) │ │ ├── mcp-config.json (14f011f, 11L, 174ch, 174B) -│ │ ├── package.json (2eae516, 23L, 662ch, 662B) +│ │ ├── package.json (987b772, 23L, 662ch, 662B) │ │ ├── postgres-problems.md (1cbb5b5, 200L, 13674ch, 13799B) │ │ └── tsconfig.json (4dc04b1, 30L, 735ch, 735B) │ └── todo-db/ (10) @@ -866,26 +881,26 @@ │ │ │ └── todos_with_details.test.ts (76bc932, 215L, 6840ch, 6842B) │ │ └── preload.ts (0d97cd6, 25L, 658ch, 660B) │ ├── .gitignore (81531bd, 5L, 57ch, 57B) -│ ├── CHANGELOG.md (e765a76, 133L, 2604ch, 2604B) +│ ├── CHANGELOG.md (446f579, 148L, 2809ch, 2809B) │ ├── bunfig.toml (e10e7cb, 5L, 89ch, 89B) -│ ├── package.json (bc715ae, 22L, 573ch, 573B) +│ ├── package.json (198dc00, 22L, 573ch, 573B) │ └── tsconfig.json (efeae48, 17L, 484ch, 484B) ├── packages/ (2) │ ├── cli/ (7) │ │ ├── scripts/ (1) │ │ │ └── postinstall.js (b82655f, 411L, 12511ch, 12513B) -│ │ ├── CHANGELOG.md (ceae00f, 1603L, 112727ch, 113265B) +│ │ ├── CHANGELOG.md (85afed4, 1619L, 114971ch, 115509B) │ │ ├── LICENSE (cfc7749, 202L, 11358ch, 11358B) │ │ ├── NOTICE (d464ce1, 2L, 43ch, 43B) │ │ ├── README.md (5ec4b13, 65L, 1728ch, 1734B) │ │ ├── noorm.js (e3d76e8, 41L, 1041ch, 1043B) -│ │ └── package.json (5b81351, 32L, 563ch, 563B) +│ │ └── package.json (10704f8, 32L, 563ch, 563B) │ └── sdk/ (5) -│ ├── CHANGELOG.md (146ccc1, 1223L, 78839ch, 79281B) +│ ├── CHANGELOG.md (35f5c52, 1233L, 80220ch, 80662B) │ ├── LICENSE (cfc7749, 202L, 11358ch, 11358B) │ ├── NOTICE (5efbb3e, 2L, 43ch, 43B) │ ├── README.md (4cd7cfa, 104L, 3525ch, 3541B) -│ └── package.json (85ffb22, 62L, 1135ch, 1135B) +│ └── package.json (9630b93, 62L, 1135ch, 1135B) ├── scripts/ (5) │ ├── Dockerfile (5fe0d7a, 51L, 1595ch, 1595B) │ ├── build-binary.mjs (3003f29, 41L, 1627ch, 1631B) @@ -895,7 +910,7 @@ ├── skills/ (1) │ └── noorm/ (2) │ ├── references/ (4) -│ │ ├── cli.md (c2b2dbb, 1013L, 29905ch, 29959B) +│ │ ├── cli.md (df77aa4, 1013L, 29910ch, 29964B) │ │ ├── config.md (1248d65, 300L, 10761ch, 11471B) │ │ ├── sdk.md (0722f21, 650L, 22066ch, 22165B) │ │ └── templates.md (20dbd54, 386L, 11061ch, 11161B) @@ -1021,14 +1036,14 @@ │ │ └── version.ts (ca59073, 273L, 6880ch, 6890B) │ ├── core/ (30) │ │ ├── change/ (9) -│ │ │ ├── executor.ts (948525e, 1340L, 36909ch, 38393B) -│ │ │ ├── history.ts (1ef7798, 1266L, 38071ch, 39941B) -│ │ │ ├── index.ts (b0a2184, 131L, 3071ch, 4779B) +│ │ │ ├── executor.ts (0f32b4f, 1358L, 37264ch, 38748B) +│ │ │ ├── history.ts (b55db34, 1283L, 37958ch, 40068B) +│ │ │ ├── index.ts (ec2928e, 132L, 3116ch, 4824B) │ │ │ ├── manager.ts (05fe831, 653L, 17287ch, 19599B) │ │ │ ├── parser.ts (24d6396, 576L, 13658ch, 15124B) │ │ │ ├── scaffold.ts (155fb34, 649L, 15641ch, 17593B) │ │ │ ├── tracker.ts (5752ecd, 266L, 8102ch, 8592B) -│ │ │ ├── types.ts (f0cda30, 742L, 18037ch, 20735B) +│ │ │ ├── types.ts (05976cc, 748L, 18242ch, 20940B) │ │ │ └── validation.ts (ca6f086, 90L, 2247ch, 2247B) │ │ ├── config/ (5) │ │ │ ├── index.ts (3d6e44e, 127L, 3484ch, 3484B) @@ -1145,14 +1160,14 @@ │ │ │ └── types.ts (5b1fc76, 82L, 2738ch, 2744B) │ │ ├── runner/ (9) │ │ │ ├── checksum.ts (d21f46d, 99L, 2650ch, 2650B) -│ │ │ ├── index.ts (ff52cef, 58L, 1106ch, 1106B) +│ │ │ ├── index.ts (c681b6a, 58L, 1118ch, 1118B) │ │ │ ├── mssql-batches.ts (3a929cd, 180L, 5670ch, 5684B) -│ │ │ ├── runner.ts (ee1701e, 1681L, 45992ch, 47712B) +│ │ │ ├── runner.ts (9edaa8b, 1405L, 38723ch, 40441B) │ │ │ ├── sqlite-statements.ts (74e41da, 252L, 6407ch, 6419B) │ │ │ ├── statement-probes.ts (7c20808, 335L, 10843ch, 10843B) │ │ │ ├── statement-watcher.ts (1ff301a, 341L, 10073ch, 10073B) -│ │ │ ├── tracker.ts (8a25bc7, 743L, 23769ch, 24003B) -│ │ │ └── types.ts (da308ee, 443L, 11695ch, 13655B) +│ │ │ ├── tracker.ts (fa6d03e, 705L, 21804ch, 22034B) +│ │ │ └── types.ts (2ed8de2, 424L, 11368ch, 13328B) │ │ ├── settings/ (7) │ │ │ ├── defaults.ts (fc071d6, 143L, 3677ch, 3679B) │ │ │ ├── events.ts (f8588b1, 108L, 2257ch, 2257B) @@ -1169,7 +1184,7 @@ │ │ │ ├── index.ts (c6bfa1d, 71L, 1623ch, 1623B) │ │ │ ├── mask.ts (0e78786, 88L, 3439ch, 3447B) │ │ │ ├── operation-id.ts (7feb6c9, 197L, 6858ch, 6874B) -│ │ │ └── tables.ts (06b23dd, 487L, 12754ch, 14716B) +│ │ │ └── tables.ts (ac34950, 487L, 12825ch, 14787B) │ │ ├── sql-terminal/ (4) │ │ │ ├── executor.ts (1a634a5, 334L, 10634ch, 10642B) │ │ │ ├── history.ts (edcbcea, 428L, 11003ch, 11007B) @@ -1265,7 +1280,7 @@ │ │ │ ├── pool.ts (aea709d, 86L, 2314ch, 2314B) │ │ │ └── types.ts (c142541, 46L, 1770ch, 1776B) │ │ ├── environment.ts (08ebb56, 141L, 3015ch, 3017B) -│ │ ├── index.ts (aeb0945, 423L, 9045ch, 9045B) +│ │ ├── index.ts (9306009, 422L, 9020ch, 9020B) │ │ ├── observer.ts (ecc7e41, 283L, 10724ch, 10726B) │ │ ├── project-init.ts (3a2c70e, 174L, 5041ch, 5043B) │ │ ├── project.ts (18af87e, 255L, 6533ch, 6533B) @@ -1373,12 +1388,12 @@ │ │ │ │ └── viewport.ts (bfd2be1, 199L, 6213ch, 6227B) │ │ │ └── index.ts (bb7cbf7, 97L, 2635ch, 2635B) │ │ ├── hooks/ (18) -│ │ │ ├── index.ts (0648b0b, 87L, 1811ch, 1811B) +│ │ │ ├── index.ts (ec744a8, 87L, 1835ch, 1835B) │ │ │ ├── useAbortableTask.ts (f290efa, 112L, 2908ch, 2908B) │ │ │ ├── useAsyncEffect.ts (9e1d1d5, 43L, 903ch, 903B) │ │ │ ├── useChangeProgress.ts (d2c0fc3, 125L, 3773ch, 3773B) │ │ │ ├── useConnection.ts (516f590, 229L, 6526ch, 6526B) -│ │ │ ├── useDoublePress.ts (6b96d6a, 40L, 1066ch, 1066B) +│ │ │ ├── useDoublePress.ts (9564684, 43L, 1195ch, 1195B) │ │ │ ├── useLoadGuard.ts (e4d449c, 51L, 1054ch, 1054B) │ │ │ ├── useLockStatus.ts (91d7117, 145L, 3664ch, 3664B) │ │ │ ├── useObserver.ts (e5db37d, 158L, 4195ch, 4197B) @@ -1457,9 +1472,9 @@ │ │ │ │ ├── LockStatusScreen.tsx (34ab652, 347L, 9918ch, 9918B) │ │ │ │ └── index.ts (144f483, 27L, 835ch, 835B) │ │ │ ├── run/ (7) -│ │ │ │ ├── RunBuildScreen.tsx (9b42729, 571L, 20572ch, 20574B) +│ │ │ │ ├── RunBuildScreen.tsx (6bef16c, 571L, 20614ch, 20616B) │ │ │ │ ├── RunDirScreen.tsx (fcb4c49, 798L, 27739ch, 27741B) -│ │ │ │ ├── RunExecScreen.tsx (366caa5, 432L, 13581ch, 13581B) +│ │ │ │ ├── RunExecScreen.tsx (678b006, 432L, 13623ch, 13623B) │ │ │ │ ├── RunFileScreen.tsx (6682193, 618L, 19931ch, 19931B) │ │ │ │ ├── RunInspectScreen.tsx (9d12203, 995L, 33278ch, 33327B) │ │ │ │ ├── RunListScreen.tsx (fba2854, 264L, 8125ch, 8125B) @@ -1588,7 +1603,7 @@ │ │ ├── run/ (13) │ │ │ ├── _setup.ts (35ce510, 167L, 4749ch, 4753B) │ │ │ ├── build-unmatched-exclude.test.ts (1563076, 109L, 3256ch, 3262B) -│ │ │ ├── build.test.ts (5e20267, 367L, 11053ch, 11067B) +│ │ │ ├── build.test.ts (3c114ef, 361L, 10657ch, 10671B) │ │ │ ├── change-ff-dryrun.test.ts (963e4be, 197L, 5697ch, 5701B) │ │ │ ├── change-ff.test.ts (58aa688, 126L, 3522ch, 3526B) │ │ │ ├── change-rewind.test.ts (c9f324a, 109L, 3291ch, 3297B) @@ -1664,16 +1679,17 @@ │ │ ├── types.test.ts (c9a86fb, 136L, 4786ch, 4786B) │ │ └── yes-flag.test.ts (7eef63b, 476L, 12490ch, 12492B) │ ├── core/ (30) -│ │ ├── change/ (9) +│ │ ├── change/ (10) │ │ │ ├── fixtures/ (3) │ │ │ │ ├── changes/ (0 files, 9 dirs) │ │ │ │ ├── manifests/ (5 files, 0 dirs) │ │ │ │ └── schema/ (0 files, 2 dirs) -│ │ │ ├── executor-retry.test.ts (812eeff, 287L, 11181ch, 11193B) +│ │ │ ├── executor-retry.test.ts (0056899, 279L, 10531ch, 10541B) │ │ │ ├── executor.test.ts (59f53c9, 388L, 14130ch, 14134B) │ │ │ ├── history.test.ts (9b7b5dd, 202L, 6278ch, 6282B) │ │ │ ├── manager.test.ts (3ab8020, 704L, 25970ch, 25980B) │ │ │ ├── parser.test.ts (a2896bd, 397L, 12524ch, 12526B) +│ │ │ ├── prefetch-gate.test.ts (ac565b0, 324L, 11161ch, 11161B) │ │ │ ├── scaffold.test.ts (011896d, 468L, 15139ch, 15139B) │ │ │ ├── tracker.test.ts (c867ae3, 181L, 6283ch, 6283B) │ │ │ └── types.test.ts (6dac816, 150L, 4669ch, 4669B) @@ -1778,7 +1794,7 @@ │ │ │ ├── session-not-found.test.ts (1e7939f, 76L, 2663ch, 2665B) │ │ │ ├── session-status.test.ts (a9a1f86, 230L, 7452ch, 7454B) │ │ │ └── session.test.ts (196c2e2, 194L, 5760ch, 5760B) -│ │ ├── runner/ (11) +│ │ ├── runner/ (13) │ │ │ ├── fixtures/ (6) │ │ │ │ ├── create-table.sql (5ea918f, 1L, 28ch, 28B) │ │ │ │ ├── raw.sql (d1ec24b, 1L, 20ch, 20B) @@ -1790,12 +1806,14 @@ │ │ │ ├── dry-run-output.test.ts (3f9e652, 136L, 4787ch, 4789B) │ │ │ ├── execute-files.test.ts (0eb62b0, 111L, 3835ch, 3837B) │ │ │ ├── mssql-batches.test.ts (13579fd, 285L, 8093ch, 8099B) +│ │ │ ├── needs-run-decision.test.ts (a15db6b, 123L, 3540ch, 3540B) +│ │ │ ├── prefetch-gate.test.ts (4acfb43, 319L, 11288ch, 11288B) │ │ │ ├── runner.test.ts (9a75b44, 248L, 7884ch, 7886B) │ │ │ ├── sqlite-multi-statement.test.ts (ae75da5, 194L, 6351ch, 6355B) │ │ │ ├── sqlite-statements.test.ts (6e299e0, 144L, 4338ch, 4340B) -│ │ │ ├── statement-watcher.test.ts (86b069c, 243L, 8019ch, 8019B) -│ │ │ ├── template-dedup.test.ts (434e39a, 165L, 5880ch, 5888B) -│ │ │ └── tracker.test.ts (9098589, 382L, 14862ch, 14870B) +│ │ │ ├── statement-watcher.test.ts (79a0066, 282L, 9495ch, 9495B) +│ │ │ ├── template-dedup.test.ts (371d079, 202L, 7307ch, 7313B) +│ │ │ └── tracker.test.ts (7567f8b, 417L, 14553ch, 14559B) │ │ ├── secrets/ (1) │ │ │ └── leakage.test.ts (e06aa00, 174L, 5317ch, 5321B) │ │ ├── settings/ (5) @@ -1932,7 +1950,7 @@ │ │ └── silent.ts (77e9168, 7L, 276ch, 278B) │ ├── integration/ (12) │ │ ├── change/ (3) -│ │ │ ├── history-dialects.test.ts (99fb7a6, 179L, 6550ch, 6556B) +│ │ │ ├── history-dialects.test.ts (7f9a70e, 244L, 9432ch, 9438B) │ │ │ ├── mysql-lifecycle.test.ts (b8c508f, 178L, 5956ch, 5960B) │ │ │ └── postgres-transaction.test.ts (abb5d62, 285L, 9557ch, 9567B) │ │ ├── cli/ (3) @@ -1961,7 +1979,7 @@ │ │ ├── runner/ (3) │ │ │ ├── mssql-batches.test.ts (b60b9e8, 268L, 8063ch, 8065B) │ │ │ ├── statement-watcher.test.ts (388cdfd, 327L, 10559ch, 10559B) -│ │ │ └── tracker-dialects.test.ts (377c0de, 159L, 5889ch, 5899B) +│ │ │ └── tracker-dialects.test.ts (8952118, 219L, 8394ch, 8404B) │ │ ├── sdk/ (8) │ │ │ ├── db-reset.test.ts (ddfaa6e, 119L, 3933ch, 3939B) │ │ │ ├── dt-namespace.test.ts (3dee923, 146L, 6011ch, 6513B) @@ -2005,7 +2023,7 @@ │ │ ├── lifecycle.test.ts (371af51, 126L, 3617ch, 4353B) │ │ ├── noorm-ops.test.ts (9a800e1, 345L, 9648ch, 11064B) │ │ ├── render-vault-tier.test.ts (df82adb, 142L, 4738ch, 5230B) -│ │ ├── run-build-filtering.test.ts (9b4fc21, 407L, 14317ch, 14833B) +│ │ ├── run-build-filtering.test.ts (39d1539, 408L, 14330ch, 14846B) │ │ ├── sql.test.ts (959c16c, 1035L, 32675ch, 34387B) │ │ ├── templates-policy.test.ts (7d94491, 68L, 2329ch, 2333B) │ │ ├── transfer-dt-namespace.test.ts (fb55bb4, 120L, 3861ch, 4351B) @@ -2049,17 +2067,17 @@ ## Manifests - docs/package.json: name=@noormdev/docs, scripts=[build, dev, preview] -- examples/llm-memory-db-mssql/package.json: name=@noormdev/example-llm-memory-db-mssql, version=0.0.7, scripts=[test, test:watch, typecheck] -- examples/llm-memory-db-pg/package.json: name=@noormdev/example-llm-memory-db-pg, version=0.0.7, scripts=[test, test:watch, typecheck] -- examples/todo-db/package.json: name=@noormdev/example-todo-db, version=0.0.7, scripts=[test, test:watch, typecheck] +- examples/llm-memory-db-mssql/package.json: name=@noormdev/example-llm-memory-db-mssql, version=0.0.9, scripts=[test, test:watch, typecheck] +- examples/llm-memory-db-pg/package.json: name=@noormdev/example-llm-memory-db-pg, version=0.0.9, scripts=[test, test:watch, typecheck] +- examples/todo-db/package.json: name=@noormdev/example-todo-db, version=0.0.9, scripts=[test, test:watch, typecheck] - package.json: name=@noormdev/main, version=0.0.1, scripts=[build, build:binary, build:packages, changeset, clean, dev, lint, lint:docs, lint:fix, prepublishOnly, release, start, test, test:coverage, test:watch, typecheck, typecheck:tests, version] -- packages/cli/package.json: name=@noormdev/cli, version=1.4.0, scripts=[postinstall] -- packages/sdk/package.json: name=@noormdev/sdk, version=1.4.0 +- packages/cli/package.json: name=@noormdev/cli, version=1.4.2, scripts=[postinstall] +- packages/sdk/package.json: name=@noormdev/sdk, version=1.4.2 ## Languages -- TypeScript: 264274 LOC (81%), 1097 files (78%) -- Markdown: 51566 LOC (15%), 192 files (13%) +- TypeScript: 264974 LOC (81%), 1100 files (77%) +- Markdown: 52178 LOC (16%), 206 files (14%) - HTML: 2977 LOC (0%), 30 files (2%) - JavaScript: 1265 LOC (0%), 22 files (1%) - YAML: 1186 LOC (0%), 19 files (1%) diff --git a/skills/noorm/references/cli.md b/skills/noorm/references/cli.md index 9eec9c87..8e7456a7 100644 --- a/skills/noorm/references/cli.md +++ b/skills/noorm/references/cli.md @@ -544,7 +544,7 @@ noorm change history --json ### change history-detail -Per-file execution history for a specific change — every operation record plus each SQL file's status, duration, and error/skip reason. Omit the name on a TTY to pick from changes that have a history record. +Per-file execution history for a specific change — every operation record plus each executed file's status, duration, and error/skip reason. Omit the name on a TTY to pick from changes that have a history record. ```bash noorm change history-detail # Interactive picker diff --git a/src/core/change/executor.ts b/src/core/change/executor.ts index edf03b10..86d439b7 100644 --- a/src/core/change/executor.ts +++ b/src/core/change/executor.ts @@ -38,7 +38,7 @@ import { StatementWatcher } from '../runner/statement-watcher.js'; import { getSqlErrorMessage } from '../shared/index.js'; import type { NoormDatabase } from '../shared/index.js'; import { getLockManager } from '../lock/index.js'; -import { ChangeHistory } from './history.js'; +import { ChangeHistory, decideNeedsRunFile } from './history.js'; import { ChangeTracker } from './tracker.js'; import { resolveManifest, validateChange, hasRevertFiles } from './parser.js'; import type { @@ -572,7 +572,18 @@ async function runFileBatch( watcher: StatementWatcher, ): Promise { - // Create operation record + const fileChecksums = new Map(); + + for (const file of expandedFiles) { + + const [cs] = await attempt(() => computeChecksum(file.path)); + fileChecksums.set(file.path, cs ?? ''); + + } + + const skipReasons = await findSkippedFiles(context, change.name, direction, expandedFiles, fileChecksums, force, history); + + // Written even when every file skips: finalizeOperation records the change checksum on it. const [operationId, createErr] = await attempt(() => history.createOperation({ name: change.name, @@ -593,20 +604,10 @@ async function runFileBatch( } - // Compute checksums for all files - const fileChecksums = new Map(); - - for (const file of expandedFiles) { - - const [cs] = await attempt(() => computeChecksum(file.path)); - fileChecksums.set(file.path, cs ?? ''); - - } - // Create pending file records (use relative paths to avoid leaking absolute paths) const createRecordsErr = await history.createFileRecords( operationId, - expandedFiles.map((f) => ({ + expandedFiles.filter((_, i) => !skipReasons[i]).map((f) => ({ filepath: path.relative(context.projectRoot, f.path), fileType: f.type, checksum: fileChecksums.get(f.path) ?? '', @@ -662,48 +663,18 @@ async function runFileBatch( const fileStart = performance.now(); const relPath = path.relative(context.projectRoot, file.path); const fileChecksum = fileChecksums.get(file.path) ?? ''; + const skipReason = skipReasons[i]; - // Per-file skip: a prior success with a matching checksum means this - // file doesn't need to run again, even though the overall change's - // checksum differs because another file needed fixing. - const needsRunFileResult = await history.needsRunFile( - change.name, - direction, - relPath, - fileChecksum, - force, - ); - - if (!needsRunFileResult.needsRun) { + if (skipReason) { results.push({ filepath: file.path, checksum: fileChecksum, status: 'skipped', - skipReason: needsRunFileResult.skipReason, + skipReason, durationMs: 0, }); - const skipUpdateErr = await history.updateFileExecution( - operationId, - relPath, - 'skipped', - 0, - undefined, - needsRunFileResult.skipReason, - ); - - if (skipUpdateErr) { - - // Log but continue - the skip decision itself is sound - observer.emit('error', { - source: 'change', - error: new Error(skipUpdateErr), - context: { filepath: relPath, operation: 'update-skipped-record' }, - }); - - } - continue; } @@ -916,6 +887,53 @@ async function runFileBatch( } +/** + * Skip reason per file, by position in `files`; `undefined` means run. + * + * Decided per file because a retry after a fix changes the change's + * checksum while the files that already succeeded stay applied. A path + * repeated through `.txt` manifests runs only at its first position. + */ +async function findSkippedFiles( + context: ChangeContext, + changeName: string, + direction: 'change' | 'revert', + files: ChangeFile[], + fileChecksums: Map, + force: boolean, + history: ChangeHistory, +): Promise> { + + const skipReasons: Array = []; + const seen = new Set(); + + const [latest, latestErr] = force + ? [null, null] + : await history.latestFileExecutions(changeName, direction); + + for (const file of files) { + + const relPath = path.relative(context.projectRoot, file.path); + + if (seen.has(relPath)) { + + skipReasons.push('already applied'); + continue; + + } + + seen.add(relPath); + + const decision = decideNeedsRunFile(latestErr ?? latest?.get(relPath), fileChecksums.get(file.path) ?? '', force); + + skipReasons.push(decision.needsRun ? undefined : decision.skipReason); + + } + + return skipReasons; + +} + // ───────────────────────────────────────────────────────────── // Internal: Preview // ───────────────────────────────────────────────────────────── diff --git a/src/core/change/history.ts b/src/core/change/history.ts index f9e7df7d..622d080f 100644 --- a/src/core/change/history.ts +++ b/src/core/change/history.ts @@ -17,7 +17,7 @@ * const history = new ChangeHistory(db, 'production', 'postgres') * * // Check if change needs to run - * const result = await history.needsRun('2024-01-15-add-users', 'abc123...') + * const result = await history.needsRun('2024-01-15-add-users', 'abc123...', false) * * // Get status * const status = await history.getStatus('2024-01-15-add-users') @@ -43,6 +43,7 @@ import type { ChangeHistoryRecord, UnifiedHistoryRecord, FileHistoryRecord, + FileExecutionRecord, NeedsRunResult, } from './types.js'; import type { ChangeType } from '../shared/index.js'; @@ -130,6 +131,74 @@ export function hydrateDate( } +// ───────────────────────────────────────────────────────────── +// File Decision +// ───────────────────────────────────────────────────────────── + +/** + * Decide whether one file of a change must run, given its newest + * qualifying execution row. + * + * A lookup error runs the file: a redundant run is recoverable, a wrongly + * skipped one is not. + * + * @param record - Newest qualifying row, `undefined` when there is none, or the lookup error + * @param checksum - Current checksum of the file + * @param force - Force re-run regardless of history + * @returns Whether the file needs to run and why + * + * @example + * ```typescript + * const [latest, err] = await history.latestFileExecutions('2024-01-15-add-users', 'change') + * const result = decideNeedsRunFile(err ?? latest.get('changes/2024-01-15-add-users/change/001.sql'), checksum, false) + * ``` + */ +export function decideNeedsRunFile( + record: FileExecutionRecord | Error | undefined, + checksum: string, + force: boolean, +): NeedsRunResult { + + if (force) { + + return { needsRun: true, reason: 'force' }; + + } + + if (record instanceof Error || !record) { + + return { needsRun: true, reason: 'new' }; + + } + + if (record.exec_status === 'failed') { + + return { + needsRun: true, + reason: 'failed', + previousChecksum: record.checksum, + }; + + } + + if (record.checksum !== checksum) { + + return { + needsRun: true, + reason: 'changed', + previousChecksum: record.checksum, + }; + + } + + return { + needsRun: false, + skipReason: 'already applied', + previousChecksum: record.checksum, + }; + +} + // ───────────────────────────────────────────────────────────── // History Class // ───────────────────────────────────────────────────────────── @@ -150,13 +219,11 @@ export function hydrateDate( * executedBy: 'Alice ', * }) * - * // Record file executions - * await history.recordFileExecution(opId, { - * filepath: '/path/to/001.sql', - * checksum: 'abc123', - * status: 'success', - * durationMs: 42, - * }) + * // Seed a pending row per file that will run, then record each outcome + * await history.createFileRecords(opId, [ + * { filepath: 'changes/2024-01-15-add-users/change/001.sql', fileType: 'sql', checksum: 'abc123' }, + * ]) + * await history.updateFileExecution(opId, 'changes/2024-01-15-add-users/change/001.sql', 'success', 42) * * // Finalize * await history.finalizeOperation(opId, 'success', 'xyz789', 1234) @@ -466,31 +533,19 @@ export class ChangeHistory { } /** - * Check if a single file within a change needs to run. + * Fetch the newest qualifying execution row per filepath for one + * change and direction. * - * Mirrors `Tracker.needsRun` (runner/tracker.ts) but scoped to this - * change's name+direction+config instead of a global filepath lookup, - * so file A's success under one change never satisfies file A's check - * under a different change. + * Two SELECTs for any number of files: the direction boundary, then the + * same query form as `Tracker.latestExecutions`. * - * Excludes `pending` rows from consideration: `createFileRecords` - * inserts a fresh pending row for every file before the per-file loop - * runs, and that row (always the highest id for this filepath) would - * otherwise shadow the prior operation's real success/failure record, - * making retries re-run every file instead of just the one that failed. + * Rows are scoped to this change's name+direction+config, so file A's + * success under one change never satisfies file A's check under another. * - * Also excludes `skipped` rows: `status: 'skipped'` is written for two - * different meanings — `skipRemainingFiles` writes it for files never - * reached after an earlier failure (must re-run), while this method's - * own per-file-skip path (called from `executor.ts`) writes it for a - * file that matched a prior success (must stay skipped). A `skipped` - * row is never itself a decision basis: a success-match skip still has - * its covering `success` row further back in history (found once the - * `skipped` row is excluded), and a never-reached skip has no terminal - * row at all, so the lookup falls through to `{ needsRun: true, reason: - * 'new' }` below and the file runs. Both resolve correctly without - * consulting the ambiguous row — excluding it here is what prevents a - * third attempt from re-running a file a prior success already covered. + * `pending` and `skipped` rows never count. Neither records an outcome: + * `pending` is a file its batch never reached, and `skipped` is written by + * `skipRemainingFiles` for the same reason, or by older versions for a + * file that matched a success still further back in history. * * A prior success only licenses a skip while it is still *standing*. * Two things retire it, and neither is visible on the execution row @@ -511,33 +566,25 @@ export class ChangeHistory { * * @param name - Change name * @param direction - 'change' or 'revert' - * @param filepath - Relative filepath as stored in execution records - * @param checksum - Current checksum of the file - * @param force - Force re-run regardless of status - * @returns Whether the file needs to run and why + * @returns Map of relative filepath to its newest qualifying row, or the lookup error + * + * @example + * ```typescript + * const [latest, err] = await history.latestFileExecutions('2024-01-15-add-users', 'change') + * const result = decideNeedsRunFile(err ?? latest.get(filepath), checksum, false) + * ``` */ - async needsRunFile( + async latestFileExecutions( name: string, direction: Direction, - filepath: string, - checksum: string, - force: boolean, - ): Promise { - - // Force always runs - if (force) { - - return { needsRun: true, reason: 'force' }; - - } + ): Promise<[Map, null] | [null, Error]> { + const { executions, change } = this.#tables; const opposite: Direction = direction === 'change' ? 'revert' : 'change'; - // Newest operation that ran the other way; anything at or before it - // has since been undone. const [boundary, boundaryErr] = await attempt(() => this.#ndb - .selectFrom(this.#tables.change) + .selectFrom(change) .select(['id']) .where('name', '=', name) .where('change_type', '=', 'change') @@ -553,100 +600,69 @@ export class ChangeHistory { observer.emit('error', { source: 'change', error: boundaryErr, - context: { name, filepath, operation: 'needs-run-file-boundary' }, + context: { name, operation: 'latest-file-executions-boundary' }, }); - // Can't prove the prior success still stands, so re-run rather - // than skip: a redundant run is recoverable, a skipped one is not. - return { needsRun: true, reason: 'new' }; + return [null, boundaryErr]; } - // Get most recent completed execution record for this file, scoped - // to this change's name+direction+config - const [record, err] = await attempt(() => { - - let query = (this.#ndb - .selectFrom(this.#tables.executions) - .innerJoin( - this.#tables.change, - `${this.#tables.change}.id`, - `${this.#tables.executions}.change_id`, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ) as any) - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .select((eb: any) => [ - eb.ref(`${this.#tables.executions}.checksum`).as('checksum'), - eb.ref(`${this.#tables.executions}.status`).as('exec_status'), - ]) - .where(`${this.#tables.change}.name`, '=', name) - .where(`${this.#tables.change}.direction`, '=', direction) - .where(`${this.#tables.change}.config_name`, '=', this.#configName) - .where(`${this.#tables.change}.status`, 'not in', ['reverted', 'stale']) - .where(`${this.#tables.executions}.filepath`, '=', filepath) - .where(`${this.#tables.executions}.status`, 'not in', ['pending', 'skipped']); + let qualifying = (this.#ndb + .selectFrom(executions) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .innerJoin(change, `${change}.id`, `${executions}.change_id`) as any) + .where(`${change}.name`, '=', name) + .where(`${change}.direction`, '=', direction) + .where(`${change}.config_name`, '=', this.#configName) + .where(`${change}.status`, 'not in', ['reverted', 'stale']) + .where(`${executions}.status`, 'not in', ['pending', 'skipped']); - if (boundary) { + if (boundary) { - query = query.where(`${this.#tables.change}.id`, '>', boundary.id); + qualifying = qualifying.where(`${change}.id`, '>', boundary.id); - } + } - return query - .orderBy(`${this.#tables.executions}.id`, 'desc') - .limit(1) - .executeTakeFirst(); + const newestIds = qualifying + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .select((eb: any) => eb.fn.max(`${executions}.id`).as('id')) + .groupBy(`${executions}.filepath`) + .as('newest'); - }); + const [rows, err] = await attempt(() => + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (this.#ndb.selectFrom(executions) as any) + .innerJoin(newestIds, 'newest.id', `${executions}.id`) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .select((eb: any) => [ + eb.ref(`${executions}.filepath`).as('filepath'), + eb.ref(`${executions}.checksum`).as('checksum'), + eb.ref(`${executions}.status`).as('exec_status'), + ]) + .execute(), + ); if (err) { observer.emit('error', { source: 'change', error: err, - context: { name, filepath, operation: 'needs-run-file-check' }, + context: { name, operation: 'latest-file-executions' }, }); - // On error, assume needs to run - return { needsRun: true, reason: 'new' }; + return [null, err]; } - // No previous completed record - new file - if (!record) { + const latest = new Map(); - return { needsRun: true, reason: 'new' }; + for (const { filepath, ...record } of rows) { - } - - // Previous execution failed - retry - if (record.exec_status === 'failed') { - - return { - needsRun: true, - reason: 'failed', - previousChecksum: record.checksum, - }; + latest.set(filepath, record); } - // Checksum changed since the last recorded attempt - if (record.checksum !== checksum) { - - return { - needsRun: true, - reason: 'changed', - previousChecksum: record.checksum, - }; - - } - - // Success and unchanged - skip - return { - needsRun: false, - skipReason: 'already applied', - previousChecksum: record.checksum, - }; + return [latest, null]; } @@ -701,9 +717,10 @@ export class ChangeHistory { } /** - * Create pending file records for all files. + * Create pending file records for the files about to run. * - * Creates records upfront so we can mark remaining as skipped on failure. + * The pending rows let `skipRemainingFiles` mark the files a + * failure left unreached. * * @returns Error message if creation failed, null on success */ diff --git a/src/core/change/index.ts b/src/core/change/index.ts index 391c0b51..ce75edbd 100644 --- a/src/core/change/index.ts +++ b/src/core/change/index.ts @@ -56,6 +56,7 @@ export type { // Change detection ChangeRunReason, NeedsRunResult, + FileExecutionRecord, // Scaffold CreateChangeOptions, @@ -109,7 +110,7 @@ export { // History & Tracking // ───────────────────────────────────────────────────────────── -export { ChangeHistory } from './history.js'; +export { ChangeHistory, decideNeedsRunFile } from './history.js'; export { ChangeTracker, type CanRevertResult } from './tracker.js'; // ───────────────────────────────────────────────────────────── diff --git a/src/core/change/types.ts b/src/core/change/types.ts index 0d3897c7..a00a26a9 100644 --- a/src/core/change/types.ts +++ b/src/core/change/types.ts @@ -606,6 +606,12 @@ export interface NeedsRunResult { previousStatus?: OperationStatus; } +/** Newest qualifying execution row for one file of a change; see `ChangeHistory.latestFileExecutions`. */ +export interface FileExecutionRecord { + checksum: string; + exec_status: ExecutionStatus; +} + // ───────────────────────────────────────────────────────────── // Scaffold Types // ───────────────────────────────────────────────────────────── diff --git a/src/core/index.ts b/src/core/index.ts index f8c2d6e3..f1129ced 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -255,7 +255,6 @@ export type { RunReason, NeedsRunResult, CreateOperationData, - RecordExecutionData, } from './runner/index.js'; // Database lifecycle diff --git a/src/core/runner/index.ts b/src/core/runner/index.ts index f42d85ff..76e87ff2 100644 --- a/src/core/runner/index.ts +++ b/src/core/runner/index.ts @@ -17,7 +17,7 @@ export { } from './runner.js'; // Tracker -export { Tracker } from './tracker.js'; +export { Tracker, decideNeedsRun } from './tracker.js'; // Checksum utilities export { @@ -39,8 +39,8 @@ export type { SkipReason, RunReason, NeedsRunResult, + ExecutionRecord, CreateOperationData, - RecordExecutionData, // New unified types ChangeType, Direction, diff --git a/src/core/runner/runner.ts b/src/core/runner/runner.ts index d28c2b9a..3ab691cd 100644 --- a/src/core/runner/runner.ts +++ b/src/core/runner/runner.ts @@ -28,7 +28,7 @@ import path from 'node:path'; import { readFile, readdir, writeFile as fsWriteFile, mkdir } from 'node:fs/promises'; -import { attempt, attemptSync } from '@logosdx/utils'; +import { attempt } from '@logosdx/utils'; import { observer } from '../observer.js'; import { formatIdentity } from '../identity/resolver.js'; @@ -38,7 +38,7 @@ import type { Permission } from '../policy/index.js'; import { computeChecksum, computeChecksumFromContent, computeCombinedChecksum } from './checksum.js'; import { executeSqlBody } from './mssql-batches.js'; import { StatementWatcher } from './statement-watcher.js'; -import { Tracker } from './tracker.js'; +import { Tracker, decideNeedsRun } from './tracker.js'; import { getSqlErrorMessage } from '../shared/index.js'; import { OperationAbortedError } from '../shared/abort.js'; import type { NoormDatabase } from '../shared/index.js'; @@ -50,10 +50,11 @@ import type { BatchStatus, FileInput, ExecuteFilesOptions, - ChangeType, FilesStatusResult, FileStatusResult, FileStatusCategory, + NeedsRunResult, + SkipReason, } from './types.js'; // ───────────────────────────────────────────────────────────── @@ -202,70 +203,22 @@ export async function runFile( assertRunPolicy(context, 'run:file'); - const opts = { ...DEFAULT_RUN_OPTIONS_INTERNAL, ...options }; - observer.emit('run:file', { filepath, configName: context.configName, }); - // Dry run renders to tmp/ and never reaches the database. This has to sit - // ahead of the tracker: executeFiles returns before creating an operation, - // and runFile must match or a dry run leaves history claiming the file ran. - // The policy assertion above still applies — rendering resolves secrets to - // disk, so it is gated the same as a real run. - if (opts.dryRun) { - - const results = await executeDryRun(context, [filepath]); - - return results[0]!; - - } - - // For single file, we still create an operation record for tracking - const tracker = new Tracker(context.db, context.configName, context.dialect ?? 'postgres'); - const operationName = `run:${new Date().toISOString()}`; - - const [operationId, createErr] = await attempt(() => - tracker.createOperation({ - name: operationName, - changeType: 'run', - configName: context.configName, - executedBy: formatIdentity(context.identity), - }), - ); - - if (createErr) { - - observer.emit('error', { - source: 'runner', - error: createErr, - context: { filepath, operation: 'create-operation' }, - }); - - return { - filepath, - checksum: '', - status: 'failed', - error: createErr.message, - }; - - } - - const watcher = createWatcher(context); - const result = await executeSingleFile(context, filepath, opts, tracker, operationId!, watcher) - .finally(() => watcher.close()); - - // Finalize operation - await tracker.finalizeOperation( - operationId!, - result.status === 'failed' || result.error ? 'failed' : 'success', - Math.round(result.durationMs ?? 0), - result.checksum, - result.error, - ); + const batch = await executeFiles(context, [{ path: filepath, type: 'sql' }], options, { + changeType: 'run', + operationName: `run:${new Date().toISOString()}`, + }); - return result; + return batch.files[0] ?? { + filepath, + checksum: '', + status: batch.error === RUN_CANCELLED ? 'skipped' : 'failed', + error: batch.error, + }; } @@ -483,83 +436,27 @@ export async function checkFilesStatus( assertRunPolicy(context, 'run:file'); const tracker = new Tracker(context.db, context.configName, context.dialect ?? 'postgres'); - const results: FileStatusResult[] = []; + const prepared = await prepareFiles(context, files.map((filepath): FileInput => ({ path: filepath, type: 'sql' }))); + const [latest, latestErr] = await tracker.latestExecutions(); - for (const filepath of files) { + const results = prepared.map((entry): FileStatusResult => { - // Load and render file to compute checksum - // Templates need rendering before checksum computation - const [sqlContent, loadErr] = await attempt(() => loadAndRenderFile(context, filepath)); + if (entry.loadError) { - if (loadErr) { - - // Can't check status if we can't read the file - results.push({ - filepath, - checksum: '', - category: 'new', - wouldSkip: false, - }); - continue; + return { filepath: entry.file.path, checksum: '', category: 'new', wouldSkip: false }; } - // Compute checksum from rendered content - const [checksum, checksumErr] = attemptSync(() => computeChecksumFromContent(sqlContent)); + const decision = decideNeedsRun(latestErr ?? latest.get(entry.relFilepath), entry.checksum, false); - if (checksumErr) { - - results.push({ - filepath, - checksum: '', - category: 'new', - wouldSkip: false, - }); - continue; - - } - - // Check if file needs to run (without force flag) - const needsRunResult = await tracker.needsRun(filepath, checksum, false); - - let category: FileStatusCategory; - - if (!needsRunResult.needsRun) { - - // File would be skipped - it was previously run with same content - category = 'previously-run'; - - } - else if (needsRunResult.reason === 'new') { - - category = 'new'; - - } - else if (needsRunResult.reason === 'changed') { - - category = 'changed'; - - } - else if (needsRunResult.reason === 'failed') { - - category = 'failed'; - - } - else { - - // stale, force, etc. - treat as new - category = 'new'; - - } - - results.push({ - filepath, - checksum, - category, - wouldSkip: !needsRunResult.needsRun, - }); + return { + filepath: entry.file.path, + checksum: entry.checksum, + category: statusCategory(decision), + wouldSkip: !decision.needsRun, + }; - } + }); // Categorize results const newFiles = results.filter((r) => r.category === 'new').map((r) => r.filepath); @@ -579,6 +476,17 @@ export async function checkFilesStatus( } +/** Stale and error reasons have no category of their own, so they read as new. */ +function statusCategory(decision: NeedsRunResult): FileStatusCategory { + + if (!decision.needsRun) return 'previously-run'; + + if (decision.reason === 'changed' || decision.reason === 'failed') return decision.reason; + + return 'new'; + +} + // ───────────────────────────────────────────────────────────── // Internal Helpers // ───────────────────────────────────────────────────────────── @@ -653,15 +561,25 @@ async function skipRemaining(tracker: Tracker, operationId: number, reason: stri /** * Execute multiple files with tracking. * - * This is the unified execution function used by both runner and change modules. - * It creates pending records upfront for full batch visibility, then executes - * files sequentially, updating records as it goes. + * Every file is rendered and hashed once, then a single history prefetch + * decides skip-or-run for the whole batch before any tracking write, so a + * skipped file costs no round trip and leaves no row. The operation and its + * pending rows exist only when something runs. `change` callers decide per + * change, so the per-file gate does not apply. * * @param context - Run context * @param files - Files to execute (gathered externally) * @param runOptions - Execution options (force, dryRun, etc.) * @param execOptions - Operation metadata (changeType, operationName, etc.) - * @returns Batch result with all file results + * @returns Batch result with all file results; no `changeId` when every file skipped + * + * @example + * ```typescript + * const result = await executeFiles(context, [{ path: '/project/sql/001.sql', type: 'sql' }], {}, { + * changeType: 'run', + * operationName: `run:${new Date().toISOString()}`, + * }) + * ``` */ export async function executeFiles( context: RunContext, @@ -733,6 +651,23 @@ export async function executeFiles( // Use provided tracker or create new one const tracker = (execOptions.tracker as Tracker) ?? new Tracker(context.db, context.configName, context.dialect ?? 'postgres'); + const isChange = execOptions.changeType === 'change'; + + const prepared = await prepareFiles(context, files); + const gated: GatedFile[] = isChange ? prepared : await applyRunGate(tracker, prepared, opts.force); + + if (!isChange && gated.every((entry): entry is SkippedFile => entry.skipReason !== undefined)) { + + return { + status: 'success', + files: gated.map(skipFile), + filesRun: 0, + filesSkipped: gated.length, + filesFailed: 0, + durationMs: performance.now() - start, + }; + + } // Create operation record const [operationId, createErr] = await attempt(() => @@ -760,31 +695,11 @@ export async function executeFiles( } - // Compute checksums for files that don't have them - const fileRecords: Array<{ filepath: string; fileType: 'sql' | 'txt'; checksum: string }> = []; - - for (const file of files) { - - let checksum = file.checksum; - - if (!checksum) { - - const [computed, err] = await attempt(() => computeChecksum(file.path)); - checksum = err ? '' : computed; - - } - - fileRecords.push({ - filepath: path.relative(context.projectRoot, file.path), - fileType: file.type, - checksum, - }); - - } - - // Create ALL file records upfront (pending status) - // This gives full visibility into the batch before execution starts - const createRecordsErr = await tracker.createFileRecords(operationId!, fileRecords); + const runSet = gated.filter((entry) => !entry.skipReason); + const createRecordsErr = await tracker.createFileRecords( + operationId!, + runSet.map((entry) => ({ filepath: entry.relFilepath, fileType: entry.file.type, checksum: entry.checksum })), + ); if (createRecordsErr) { @@ -808,13 +723,20 @@ export async function executeFiles( let failed = false; let cancelled = false; const watcher = createWatcher(context); + const lastRunIndex = gated.map((entry) => !entry.skipReason).lastIndexOf(true); try { - for (let i = 0; i < files.length; i++) { + for (let i = 0; i < gated.length; i++) { - const file = files[i]!; - const fileRecord = fileRecords[i]!; + const entry = gated[i]!; + + if (entry.skipReason) { + + results.push(skipFile(entry)); + continue; + + } if (context.signal?.aborted) { @@ -826,16 +748,7 @@ export async function executeFiles( } - const result = await executeSingleFileWithUpdate( - context, - file.path, - fileRecord.checksum, - opts, - tracker, - operationId!, - execOptions.changeType, - watcher, - ); + const result = await executeSingleFileWithUpdate(context, entry, tracker, operationId!, watcher); // Refused before its SQL was sent: its pending row is skipped with the rest. if (!result) { @@ -850,10 +763,10 @@ export async function executeFiles( results.push(result); - // A last file that finished despite the abort (mssql, sqlite, or a - // cancel that missed) completed the run; nothing was cut short. + // The last file that runs finished despite the abort (mssql, sqlite, or a missed + // cancel), and every file after it is a skip, so nothing was cut short. cancelled = (context.signal?.aborted ?? false) - && (i < files.length - 1 || result.status === 'failed'); + && (i < lastRunIndex || result.status === 'failed'); if (cancelled || (result.status === 'failed' && opts.abortOnError)) { @@ -862,7 +775,7 @@ export async function executeFiles( await skipRemaining( tracker, operationId!, - cancelled ? RUN_CANCELLED : `Skipped: failure in ${path.basename(file.path)}`, + cancelled ? RUN_CANCELLED : `Skipped: failure in ${path.basename(entry.file.path)}`, ); break; @@ -878,11 +791,8 @@ export async function executeFiles( } - // When abortOnError stops the loop early, fewer results than files is - // by design -- the remaining files were never attempted, only marked - // skipped above. The invariant "one result per file" only holds for a - // full pass; this guards against a future regression re-introducing - // double (or dropped) execution within that pass. + // One result per file holds only for a full pass: after a cancel or abortOnError + // break, later gate-skipped files are not in results and have no row. if (!failed && results.length !== files.length) { throw new Error( @@ -908,7 +818,7 @@ export async function executeFiles( // Compute combined checksum (or use provided) const combinedChecksum = - execOptions.checksum ?? computeCombinedChecksum(fileRecords.map((f) => f.checksum)); + execOptions.checksum ?? computeCombinedChecksum(gated.map((entry) => entry.checksum)); // Finalize operation (partial failures count as failed) // Compute final status AFTER all operations @@ -945,6 +855,84 @@ export async function executeFiles( } +interface PreparedFile { + file: FileInput; + relFilepath: string; + checksum: string; + sql: string; + loadError?: Error; +} + +type SkippedFile = PreparedFile & { skipReason: SkipReason }; +type GatedFile = (PreparedFile & { skipReason?: undefined }) | SkippedFile; + +/** Renders each file once: the gate keys a template on its rendered output, and execution reuses the render. */ +async function prepareFiles(context: RunContext, files: FileInput[]): Promise { + + const prepared: PreparedFile[] = []; + + for (const file of files) { + + const relFilepath = path.relative(context.projectRoot, file.path); + const [loaded, loadError] = await attempt(async () => { + + const sql = await loadAndRenderFile(context, file.path); + + return { sql, checksum: computeChecksumFromContent(sql) }; + + }); + + if (loadError) { + + prepared.push({ file, relFilepath, checksum: '', sql: '', loadError }); + continue; + + } + + prepared.push({ file, relFilepath, ...loaded }); + + } + + return prepared; + +} + +/** A file that failed to load never skips, so its error surfaces at its turn in the walk. */ +async function applyRunGate(tracker: Tracker, prepared: PreparedFile[], force: boolean): Promise { + + if (force) return prepared; + + const [latest, latestErr] = await tracker.latestExecutions(); + + return prepared.map((entry) => { + + if (entry.loadError) return entry; + + const decision = decideNeedsRun(latestErr ?? latest.get(entry.relFilepath), entry.checksum, false); + + return decision.skipReason ? { ...entry, skipReason: decision.skipReason } : entry; + + }); + +} + +/** Deliberately writes nothing: a skipped file leaves no tracking row. */ +function skipFile(entry: SkippedFile): FileResult { + + observer.emit('file:skip', { + filepath: entry.file.path, + reason: entry.skipReason, + }); + + return { + filepath: entry.file.path, + checksum: entry.checksum, + status: 'skipped', + skipReason: entry.skipReason, + }; + +} + /** * Internal wrapper for legacy callers. * @@ -977,46 +965,34 @@ async function executeFilesInternal( } /** - * Execute a single file with upfront record update. - * - * This version uses updateFileExecution (records created upfront) - * instead of recordExecution (insert on execution). + * Execute one run-set file's pre-rendered SQL and update its pending row. * * @param context - Run context - * @param filepath - File to execute - * @param checksum - Pre-computed checksum - * @param options - Run options + * @param entry - Prepared file carrying its rendered SQL or load error * @param tracker - Tracker instance * @param operationId - Parent operation ID - * @param changeType - Type of operation (affects needsRun behavior) + * @param watcher - Statement watcher the SQL runs through + * @returns null when the run was cancelled before the file's SQL was sent */ async function executeSingleFileWithUpdate( context: RunContext, - filepath: string, - checksum: string, - options: Required> & { output: string | null }, + entry: PreparedFile, tracker: Tracker, operationId: number, - changeType: ChangeType, watcher: StatementWatcher, ): Promise { const start = performance.now(); + const { file: { path: filepath }, relFilepath, checksum } = entry; - // Relative path for DB storage (avoids leaking absolute paths) - const relFilepath = path.relative(context.projectRoot, filepath); - - // Load and render file - const [sqlContent, loadErr] = await attempt(() => loadAndRenderFile(context, filepath)); - - if (loadErr) { + if (entry.loadError) { const durationMs = performance.now() - start; const result: FileResult = { filepath, - checksum: checksum || '', + checksum, status: 'failed', - error: loadErr.message, + error: entry.loadError.message, durationMs, }; @@ -1025,76 +1001,28 @@ async function executeSingleFileWithUpdate( relFilepath, 'failed', Math.round(durationMs), - loadErr.message, + entry.loadError.message, ); observer.emit('file:after', { filepath, status: 'failed', durationMs, - error: loadErr.message, + error: entry.loadError.message, }); return result; } - // Recompute checksum from rendered content for templates - const [renderedChecksum, checksumErr] = attemptSync(() => computeChecksumFromContent(sqlContent)); - const finalChecksum = checksumErr ? checksum : renderedChecksum; - observer.emit('file:before', { filepath, - checksum: finalChecksum, + checksum, configName: context.configName, }); - // For 'build' and 'run', check if individual file needs to run - // For 'change', the change-level check was already done by the caller - if (changeType !== 'change') { - - // Exclude this operation's own rows -- createFileRecords already - // inserted a pending row for every file in this batch before this - // loop started, so without the exclusion the newest row is always - // this run's own pending record and every file reads as "new". - const needsRunResult = await tracker.needsRun(relFilepath, finalChecksum, options.force, operationId); - - if (!needsRunResult.needsRun) { - - const result: FileResult = { - filepath, - checksum: finalChecksum, - status: 'skipped', - skipReason: needsRunResult.skipReason, - }; - - // The rendered checksum is written even on a skip: the pending - // row seeded by createFileRecords holds the raw file hash, and - // it is the newest row the *next* build will compare against. - // Leaving it raw makes the file re-run one build later. - await tracker.updateFileExecution( - operationId, - relFilepath, - 'skipped', - 0, - undefined, - needsRunResult.skipReason, - finalChecksum, - ); - - observer.emit('file:skip', { - filepath, - reason: needsRunResult.skipReason!, - }); - - return result; - - } - - } - // Execute SQL (MSSQL splits on `GO` batches; other dialects run as one) - const execErrMsg = await runWatched(context, watcher, filepath, sqlContent); + const execErrMsg = await runWatched(context, watcher, filepath, entry.sql); if (execErrMsg === NOT_STARTED) return null; @@ -1106,7 +1034,7 @@ async function executeSingleFileWithUpdate( const result: FileResult = { filepath, - checksum: finalChecksum, + checksum, status: 'failed', error, durationMs, @@ -1118,8 +1046,6 @@ async function executeSingleFileWithUpdate( 'failed', Math.round(durationMs), error, - undefined, - finalChecksum, ); observer.emit('file:after', { @@ -1136,7 +1062,7 @@ async function executeSingleFileWithUpdate( // Success const result: FileResult = { filepath, - checksum: finalChecksum, + checksum, status: 'success', durationMs, }; @@ -1146,9 +1072,6 @@ async function executeSingleFileWithUpdate( relFilepath, 'success', Math.round(durationMs), - undefined, - undefined, - finalChecksum, ); observer.emit('file:after', { @@ -1161,205 +1084,6 @@ async function executeSingleFileWithUpdate( } -/** - * Execute a single file (legacy version for runFile). - * - * Uses recordExecution (insert) instead of updateFileExecution. - */ -async function executeSingleFile( - context: RunContext, - filepath: string, - options: Required> & { output: string | null }, - tracker: Tracker, - operationId: number, - watcher: StatementWatcher, -): Promise { - - const start = performance.now(); - - // Relative path for DB storage (avoids leaking absolute paths) - const relFilepath = path.relative(context.projectRoot, filepath); - - // Load and render file - // Needed before checksum to support templates - const [sqlContent, loadErr] = await attempt(() => loadAndRenderFile(context, filepath)); - - if (loadErr) { - - const durationMs = performance.now() - start; - const result: FileResult = { - filepath, - checksum: '', - status: 'failed', - error: loadErr.message, - durationMs, - }; - - await tracker.recordExecution({ - changeId: operationId, - filepath: relFilepath, - checksum: '', - status: 'failed', - errorMessage: loadErr.message, - durationMs: Math.round(durationMs), - }); - - observer.emit('file:after', { - filepath, - status: 'failed', - durationMs, - error: loadErr.message, - }); - - return result; - - } - - // Compute checksum - const [checksum, checksumErr] = attemptSync(() => computeChecksumFromContent(sqlContent)); - - if (checksumErr) { - - const result: FileResult = { - filepath, - checksum: '', - status: 'failed', - error: checksumErr.message, - durationMs: performance.now() - start, - }; - - await tracker.recordExecution({ - changeId: operationId, - filepath: relFilepath, - checksum: '', - status: 'failed', - errorMessage: checksumErr.message, - durationMs: Math.round(result.durationMs ?? 0), - }); - - observer.emit('file:after', { - filepath, - status: 'failed', - durationMs: result.durationMs ?? 0, - error: checksumErr.message, - }); - - return result; - - } - - observer.emit('file:before', { - filepath, - checksum, - configName: context.configName, - }); - - // Check if file needs to run - const needsRunResult = await tracker.needsRun(relFilepath, checksum, options.force); - - if (!needsRunResult.needsRun) { - - const result: FileResult = { - filepath, - checksum, - status: 'skipped', - skipReason: needsRunResult.skipReason, - }; - - await tracker.recordExecution({ - changeId: operationId, - filepath: relFilepath, - checksum, - status: 'skipped', - skipReason: needsRunResult.skipReason, - }); - - observer.emit('file:skip', { - filepath, - reason: needsRunResult.skipReason!, - }); - - return result; - - } - - // Execute SQL (MSSQL splits on `GO` batches; other dialects run as one) - const execErrMsg = await runWatched(context, watcher, filepath, sqlContent); - - const durationMs = performance.now() - start; - - if (execErrMsg === NOT_STARTED) { - - await tracker.recordExecution({ - changeId: operationId, - filepath: relFilepath, - checksum, - status: 'skipped', - skipReason: RUN_CANCELLED, - }); - - return { filepath, checksum, status: 'skipped', error: RUN_CANCELLED }; - - } - - if (execErrMsg) { - - const error = execErrMsg + (await describePriorSuccesses(tracker, relFilepath, operationId)); - - const result: FileResult = { - filepath, - checksum, - status: 'failed', - error, - durationMs, - }; - - await tracker.recordExecution({ - changeId: operationId, - filepath: relFilepath, - checksum, - status: 'failed', - errorMessage: error, - durationMs: Math.round(durationMs), - }); - - observer.emit('file:after', { - filepath, - status: 'failed', - durationMs, - error, - }); - - return result; - - } - - // Success - const result: FileResult = { - filepath, - checksum, - status: 'success', - durationMs, - }; - - await tracker.recordExecution({ - changeId: operationId, - filepath: relFilepath, - checksum, - status: 'success', - durationMs: Math.round(durationMs), - }); - - observer.emit('file:after', { - filepath, - status: 'success', - durationMs, - }); - - return result; - -} - /** * Load and optionally render a SQL file. */ diff --git a/src/core/runner/tracker.ts b/src/core/runner/tracker.ts index 8fefa923..c7f2e4e6 100644 --- a/src/core/runner/tracker.ts +++ b/src/core/runner/tracker.ts @@ -15,12 +15,14 @@ * * const tracker = new Tracker(db, 'dev') * - * // Check if file needs to run - * const result = await tracker.needsRun('/path/to/file.sql', 'abc123', false) + * // Decide every file of a batch from one prefetch + * const [latest, err] = await tracker.latestExecutions() + * const result = decideNeedsRun(err ?? latest.get('sql/001.sql'), 'abc123', false) * - * // Create operation and record executions + * // Create operation and pending rows, then record each outcome * const opId = await tracker.createOperation({ name: 'build:...', ... }) - * await tracker.recordExecution({ changeId: opId, filepath: '...', ... }) + * await tracker.createFileRecords(opId, [{ filepath: '...', fileType: 'sql', checksum: '...' }]) + * await tracker.updateFileExecution(opId, '...', 'success', 42) * ``` */ import type { Kysely } from 'kysely'; @@ -31,7 +33,100 @@ import { observer } from '../observer.js'; import { getNoormTables, insertOperationRecord, noormDb } from '../shared/index.js'; import type { NoormDatabase, ChangeType, ExecutionStatus, FileType } from '../shared/index.js'; import type { Dialect } from '../connection/types.js'; -import type { NeedsRunResult, CreateOperationData, RecordExecutionData, Direction } from './types.js'; +import type { NeedsRunResult, CreateOperationData, Direction, ExecutionRecord } from './types.js'; + +/** + * Decide whether a file must run, given its newest execution record. + * + * @param record - Newest execution record, `undefined` when the file never ran, or the lookup error + * @param checksum - Current file checksum + * @param force - Force re-run regardless of history + * @returns Whether the file needs to run and why + * + * @example + * ```typescript + * const [latest, err] = await tracker.latestExecutions() + * const result = decideNeedsRun(err ?? latest.get('sql/001.sql'), checksum, false) + * ``` + */ +export function decideNeedsRun( + record: ExecutionRecord | Error | undefined, + checksum: string, + force: boolean, +): NeedsRunResult { + + if (force) { + + return { needsRun: true, reason: 'force' }; + + } + + // Distinct from 'new': the SELECT itself failed, so whether a + // record exists is genuinely unknown. Reporting this as 'new' + // would make a transient read failure indistinguishable from a + // first-ever run in logs and audits. + if (record instanceof Error) { + + return { needsRun: true, reason: 'error' }; + + } + + if (!record) { + + return { needsRun: true, reason: 'new' }; + + } + + if (record.exec_status === 'failed') { + + return { + needsRun: true, + reason: 'failed', + previousChecksum: record.checksum, + }; + + } + + // A `pending` row belongs to a file its batch never reached, so it + // carries no outcome to compare against and must run. + // + // A cascade `skipped` row never executed either. An `unchanged` skip (older + // builds only) recorded a real skip, so fall through and re-compare the checksum. + const isUnchangedSkip = record.exec_status === 'skipped' && record.skip_reason === 'unchanged'; + + if (record.exec_status === 'pending' || (record.exec_status === 'skipped' && !isUnchangedSkip)) { + + return { needsRun: true, reason: 'new' }; + + } + + if (record.change_status === 'stale') { + + return { + needsRun: true, + reason: 'stale', + previousChecksum: record.checksum, + }; + + } + + if (record.checksum !== checksum) { + + return { + needsRun: true, + reason: 'changed', + previousChecksum: record.checksum, + }; + + } + + return { + needsRun: false, + skipReason: 'unchanged', + previousChecksum: record.checksum, + }; + +} /** * Execution tracker for change detection and audit logging. @@ -48,14 +143,11 @@ import type { NeedsRunResult, CreateOperationData, RecordExecutionData, Directio * executedBy: 'Alice ', * }) * - * // Record each file execution - * await tracker.recordExecution({ - * changeId: opId, - * filepath: '/project/sql/001.sql', - * checksum: 'abc123...', - * status: 'success', - * durationMs: 42, - * }) + * // Seed a pending row per file, then record each outcome + * await tracker.createFileRecords(opId, [ + * { filepath: 'sql/001.sql', fileType: 'sql', checksum: 'abc123...' }, + * ]) + * await tracker.updateFileExecution(opId, 'sql/001.sql', 'success', 42) * * // Finalize the operation * await tracker.finalizeOperation(opId, 'success', 1234) @@ -80,72 +172,49 @@ export class Tracker { } /** - * Check if a file needs to run. + * Fetch the newest execution row per filepath for this config. * - * A file needs to run if: - * - Force flag is set - * - No previous execution exists (new file) - * - Previous execution failed - * - Parent change is stale (schema was torn down) - * - Checksum differs (file changed) + * One SELECT for any number of files, so a batch can decide every file + * with `decideNeedsRun` before writing any tracking rows. Query form: + * `docs/design/prefetch-run-gate.md`. * - * @param filepath - File path to check - * @param checksum - Current file checksum - * @param force - Force re-run regardless of status - * @param excludeOperationId - Operation whose own rows should be ignored. - * `executeFiles` inserts a `pending` row for every discovered file - * *before* running any of them (for batch visibility), so within that - * same operation the newest row for a file is always its own pending - * record — reading as "new" forever and making checksum-based skipping - * unreachable. Passing the running operation's id here excludes those - * rows so the lookup finds the last *completed* operation instead. - * @returns Whether file needs to run and why + * @returns Map of filepath to its newest record, or the lookup error + * + * @example + * ```typescript + * const [latest, err] = await tracker.latestExecutions() + * const result = decideNeedsRun(err ?? latest.get(filepath), checksum, false) + * ``` */ - async needsRun( - filepath: string, - checksum: string, - force: boolean, - excludeOperationId?: number, - ): Promise { - - // Force always runs - if (force) { + async latestExecutions(): Promise<[Map, null] | [null, Error]> { - return { needsRun: true, reason: 'force' }; - - } + const { executions, change } = this.#tables; - // Find most recent execution for this file and config - // Also fetch the parent change status to check for stale - let query = (this.#ndb - .selectFrom(this.#tables.executions) - .innerJoin( - this.#tables.change, - `${this.#tables.change}.id`, - `${this.#tables.executions}.change_id`, + const newestIds = (this.#ndb + .selectFrom(executions) // eslint-disable-next-line @typescript-eslint/no-explicit-any - ) as any) + .innerJoin(change, `${change}.id`, `${executions}.change_id`) as any) + .where(`${change}.config_name`, '=', this.#configName) // eslint-disable-next-line @typescript-eslint/no-explicit-any - .select((eb: any) => [ - eb.ref(`${this.#tables.executions}.checksum`).as('checksum'), - eb.ref(`${this.#tables.executions}.status`).as('exec_status'), - eb.ref(`${this.#tables.executions}.skip_reason`).as('skip_reason'), - eb.ref(`${this.#tables.change}.status`).as('change_status'), - ]) - .where(`${this.#tables.executions}.filepath`, '=', filepath) - .where(`${this.#tables.change}.config_name`, '=', this.#configName); - - if (excludeOperationId !== undefined) { + .select((eb: any) => eb.fn.max(`${executions}.id`).as('id')) + .groupBy(`${executions}.filepath`) + .as('newest'); - query = query.where(`${this.#tables.executions}.change_id`, '<>', excludeOperationId); - - } - - const [record, err] = await attempt(() => - query - .orderBy(`${this.#tables.executions}.id`, 'desc') - .limit(1) - .executeTakeFirst(), + const [rows, err] = await attempt(() => + (this.#ndb + .selectFrom(executions) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .innerJoin(change, `${change}.id`, `${executions}.change_id`) as any) + .innerJoin(newestIds, 'newest.id', `${executions}.id`) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .select((eb: any) => [ + eb.ref(`${executions}.filepath`).as('filepath'), + eb.ref(`${executions}.checksum`).as('checksum'), + eb.ref(`${executions}.status`).as('exec_status'), + eb.ref(`${executions}.skip_reason`).as('skip_reason'), + eb.ref(`${change}.status`).as('change_status'), + ]) + .execute(), ); if (err) { @@ -153,81 +222,22 @@ export class Tracker { observer.emit('error', { source: 'runner', error: err, - context: { filepath, operation: 'needs-run-check' }, + context: { operation: 'latest-executions' }, }); - // Distinct from 'new': the SELECT itself failed, so whether a - // record exists is genuinely unknown. Reporting this as 'new' - // would make a transient read failure indistinguishable from a - // first-ever run in logs and audits. - return { needsRun: true, reason: 'error' }; + return [null, err]; } - // No previous record - new file - if (!record) { + const latest = new Map(); - return { needsRun: true, reason: 'new' }; + for (const { filepath, ...record } of rows) { - } - - // Previous execution failed - retry - if (record.exec_status === 'failed') { - - return { - needsRun: true, - reason: 'failed', - previousChecksum: record.checksum, - }; - - } - - // A `pending` row is an upfront placeholder for batch visibility whose - // file never reached execution, so it carries no outcome to compare - // against and must run. - // - // `skipped` means two different things. A cascade skip (an earlier file - // in the batch failed) also never executed, so it must run too. But an - // `unchanged` skip is the recorded outcome of a correct decision, and - // treating it as "never ran" made the third consecutive build re-execute - // a file the second build had rightly skipped -- failing on any DDL that - // is not idempotent. Fall through and re-compare the checksum instead. - const isUnchangedSkip = record.exec_status === 'skipped' && record.skip_reason === 'unchanged'; - - if (record.exec_status === 'pending' || (record.exec_status === 'skipped' && !isUnchangedSkip)) { - - return { needsRun: true, reason: 'new' }; - - } - - // Parent change is stale (schema was torn down) - needs re-run - if (record.change_status === 'stale') { - - return { - needsRun: true, - reason: 'stale', - previousChecksum: record.checksum, - }; + latest.set(filepath, record); } - // Checksum changed - if (record.checksum !== checksum) { - - return { - needsRun: true, - reason: 'changed', - previousChecksum: record.checksum, - }; - - } - - // Unchanged - skip - return { - needsRun: false, - skipReason: 'unchanged', - previousChecksum: record.checksum, - }; + return [latest, null]; } @@ -280,44 +290,6 @@ export class Tracker { } - /** - * Record a file execution. - * - * Creates a child record in __noorm_executions__ linked - * to the parent operation. - * - * @param data - Execution data - */ - async recordExecution(data: RecordExecutionData): Promise { - - const [, err] = await attempt(() => - this.#ndb - .insertInto(this.#tables.executions) - .values({ - change_id: data.changeId, - filepath: data.filepath, - file_type: 'sql', - checksum: data.checksum, - status: data.status as ExecutionStatus, - skip_reason: data.skipReason ?? '', - error_message: data.errorMessage ?? '', - duration_ms: Math.round(data.durationMs ?? 0), - }) - .execute(), - ); - - if (err) { - - observer.emit('error', { - source: 'runner', - error: err, - context: { filepath: data.filepath, operation: 'record-execution' }, - }); - - } - - } - /** * Finalize an operation. * @@ -394,10 +366,10 @@ export class Tracker { // ───────────────────────────────────────────────────────── /** - * Create pending file records for all files upfront. + * Create pending file records for the files about to run. * - * Creates records so the batch is fully visible. On failure, - * remaining files can be marked as skipped. + * The pending rows let `skipRemainingFiles` mark the files a + * failure left unreached. * * @param operationId - Parent operation ID * @param files - Files to create records for @@ -455,13 +427,6 @@ export class Tracker { * @param durationMs - Execution time * @param errorMessage - Error message if failed * @param skipReason - Skip reason if skipped - * @param checksum - Checksum of the SQL that actually reached the - * database. `createFileRecords` seeds the pending row with the *raw* - * file hash because rendering every template upfront would execute them - * twice; for a `.sql.tmpl` that hash is not what `needsRun` compares - * against, so leaving it in place made template dedup unreachable. - * Omitted leaves the seeded value alone — correct only where no render - * happened (e.g. the file could not be read). * @returns Error message if update failed, null on success */ async updateFileExecution( @@ -471,7 +436,6 @@ export class Tracker { durationMs: number, errorMessage?: string, skipReason?: string, - checksum?: string, ): Promise { const [result, err] = await attempt(() => @@ -482,7 +446,6 @@ export class Tracker { duration_ms: Math.round(durationMs), error_message: errorMessage ?? '', skip_reason: skipReason ?? '', - ...(checksum === undefined ? {} : { checksum }), }) .where('change_id', '=', operationId) .where('filepath', '=', filepath) @@ -636,7 +599,6 @@ export class Tracker { /** * Check if a change needs to run by name. * - * Similar to needsRun but checks by change name instead of filepath. * Used for change sets where we track by change name, not individual files. * * @param name - Change name diff --git a/src/core/runner/types.ts b/src/core/runner/types.ts index a748d616..3a8a5b85 100644 --- a/src/core/runner/types.ts +++ b/src/core/runner/types.ts @@ -8,7 +8,7 @@ */ import type { Kysely } from 'kysely'; -import type { NoormDatabase, ExecutionStatus } from '../shared/index.js'; +import type { NoormDatabase, ExecutionStatus, OperationStatus } from '../shared/index.js'; import type { Identity } from '../identity/index.js'; import type { Channel, ConfigAccess } from '../policy/index.js'; @@ -280,6 +280,16 @@ export interface NeedsRunResult { previousChecksum?: string; } +/** + * Newest execution row for a file, joined to its parent change. + */ +export interface ExecutionRecord { + checksum: string; + exec_status: ExecutionStatus; + skip_reason: string; + change_status: OperationStatus; +} + // ───────────────────────────────────────────────────────────── // Operation Types (Shared by Runner and Change) // ───────────────────────────────────────────────────────────── @@ -313,9 +323,6 @@ export interface FileInput { /** File type */ type: 'sql' | 'txt'; - - /** Pre-computed checksum (optional, computed if not provided) */ - checksum?: string; } /** @@ -367,32 +374,6 @@ export interface CreateOperationData { executedBy: string; } -/** - * Data for recording a file execution. - */ -export interface RecordExecutionData { - /** Parent operation ID */ - changeId: number; - - /** File path */ - filepath: string; - - /** File checksum */ - checksum: string; - - /** Execution status */ - status: ExecutionStatus; - - /** Skip reason if skipped */ - skipReason?: string; - - /** Error message if failed */ - errorMessage?: string; - - /** Duration in milliseconds */ - durationMs?: number; -} - // ───────────────────────────────────────────────────────────── // File Status Check (Pre-execution) // ───────────────────────────────────────────────────────────── diff --git a/src/core/shared/tables.ts b/src/core/shared/tables.ts index 69f9c663..edfaaba2 100644 --- a/src/core/shared/tables.ts +++ b/src/core/shared/tables.ts @@ -307,7 +307,7 @@ export interface NoormExecutionsTable { /** Error details (empty = no error) */ error_message: Generated; - /** 'unchanged', 'already-run', 'change failed' */ + /** Why a failure left this file unreached, e.g. 'change failed'; older rows may hold 'unchanged' or 'already applied' */ skip_reason: Generated; /** Execution time (0 = never ran) */ diff --git a/tests/cli/run/build.test.ts b/tests/cli/run/build.test.ts index d71e2e6c..58044357 100644 --- a/tests/cli/run/build.test.ts +++ b/tests/cli/run/build.test.ts @@ -145,12 +145,8 @@ describe('cli: noorm run build — per-file error and skip surfacing', () => { describe('cli: noorm run build — skip-on-rerun (CP10, v1/49-54)', () => { - // Previously untestable: `executeFiles` creates an upfront 'pending' - // execution record for every file before `needsRun` ever runs, and - // `needsRun` picked the newest row by id -- always that same pending - // record -- so it always read as 'new'. Checksum-based skipping was - // structurally unreachable for build/dir/files. Fixed by excluding the - // running operation's own rows from the `needsRun` lookup. + // Each build is a separate CLI process, so the skip must come from the + // previous build's rows; re-running unchanged DDL fails the second build. let project: TestProject; @@ -255,8 +251,6 @@ describe('cli: noorm run build — skip-on-rerun (CP10, v1/49-54)', () => { const first = runCli(project, ['run', 'build']); expect(first.status).toBe(0); - // A content change is a different checksum, distinct from the - // upfront-pending-row bug CP10 fixes. await writeFile( filePath, 'CREATE TABLE noorm_build_changed_b (id INTEGER PRIMARY KEY);\n', diff --git a/tests/core/change/executor-retry.test.ts b/tests/core/change/executor-retry.test.ts index bce52a89..66c7aa07 100644 --- a/tests/core/change/executor-retry.test.ts +++ b/tests/core/change/executor-retry.test.ts @@ -163,8 +163,8 @@ describe('change: executor retry', () => { expect(aResult?.status).toBe('skipped'); expect(bResult?.status).toBe('success'); - // A's SQL was never re-submitted: exactly one success row for A - // across both operations (op1: success, op2: skipped). + // A's SQL was never re-submitted, and its skip left no row: op1's + // success is A's only execution row. const relA = relative(tempDir, fileA.path); const aExecutions = await db .selectFrom('__noorm_executions__') @@ -173,9 +173,8 @@ describe('change: executor retry', () => { .orderBy('id', 'asc') .execute(); - expect(aExecutions).toHaveLength(2); + expect(aExecutions).toHaveLength(1); expect(aExecutions[0]?.status).toBe('success'); - expect(aExecutions[1]?.status).toBe('skipped'); // Cross-check via ChangeHistory's own query surface const history = new ChangeHistory(db, 'test', 'sqlite'); @@ -184,8 +183,9 @@ describe('change: executor retry', () => { expect(op1Files.find((f) => f.filepath === relA)?.status).toBe('success'); const op2Files = await history.getFileHistory(result2.operationId!); - expect(op2Files.find((f) => f.filepath === relA)?.status).toBe('skipped'); - expect(op2Files.find((f) => f.filepath === relative(tempDir, fileB.path))?.status).toBe('success'); + expect(op2Files.map((f) => ({ filepath: f.filepath, status: f.status }))).toEqual([ + { filepath: relative(tempDir, fileB.path), status: 'success' }, + ]); const statuses = await history.getAllStatuses(); expect(statuses.get('retry-fix-and-rerun')?.status).toBe('success'); @@ -195,11 +195,8 @@ describe('change: executor retry', () => { it('should keep a succeeded file skipped across a THIRD attempt, not just a second', async () => { // File A always succeeds. File B fails on attempts 1 and 2 (syntax - // error, not fixed yet), then is fixed before attempt 3. This - // reproduces the third-attempt regression: needsRunFile must not - // find attempt 2's `skipped` row for A (written by the per-file - // skip path) and mistake it for a "never reached" skip that needs - // re-running — A's real success is two operations back. + // error, not fixed yet), then is fixed before attempt 3, when A's + // covering success is two operations back and must still license the skip. const change = await createTestChange('retry-three-attempts', [ { name: '001_a.sql', content: 'CREATE TABLE retry3_test_a (id INTEGER PRIMARY KEY)' }, { name: '002_b.sql', content: 'CREATE TALBE retry3_test_b (id INTEGER PRIMARY KEY)' }, @@ -230,10 +227,8 @@ describe('change: executor retry', () => { expect(result3.files[0]?.status).toBe('skipped'); expect(result3.files[1]?.status).toBe('success'); - // A's SQL was submitted exactly once across all three attempts: - // one success row, and every other row skipped. On the buggy - // code, op3 re-runs A (finding op2's ambiguous `skipped` row), - // producing a second success (or a failure) instead. + // A's SQL was submitted exactly once across all three attempts, + // and its two skips left no rows. const fileA = change.changeFiles[0]!; const relA = relative(tempDir, fileA.path); const aExecutions = await db @@ -243,11 +238,8 @@ describe('change: executor retry', () => { .orderBy('id', 'asc') .execute(); - expect(aExecutions).toHaveLength(3); - expect(aExecutions.filter((e) => e.status === 'success')).toHaveLength(1); + expect(aExecutions).toHaveLength(1); expect(aExecutions[0]?.status).toBe('success'); - expect(aExecutions[1]?.status).toBe('skipped'); - expect(aExecutions[2]?.status).toBe('skipped'); }); @@ -276,8 +268,8 @@ describe('change: executor retry', () => { // Third run with force: per-file skip must be bypassed even // though file A has a prior success record with a matching - // checksum — force short-circuits needsRunFile before any DB - // lookup, so the file actually re-runs (not 'skipped') + // checksum — force bypasses the per-file history lookup, so the + // file actually re-runs (not 'skipped') const result3 = await executeChange(context, change, { force: true }); expect(result3.files).toHaveLength(1); expect(result3.files[0]?.status).toBe('success'); diff --git a/tests/core/change/prefetch-gate.test.ts b/tests/core/change/prefetch-gate.test.ts new file mode 100644 index 00000000..051fa350 --- /dev/null +++ b/tests/core/change/prefetch-gate.test.ts @@ -0,0 +1,324 @@ +// A skipped change file must leave no execution row and cost no per-file +// query (docs/spec/prefetch-run-gate.md). +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { mkdtemp, rm, mkdir, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { Kysely, SqliteDialect, sql } from 'kysely'; + +import { BunSqliteDatabase } from '../../../src/core/connection/dialects/sqlite-bun.js'; +import { executeChange, revertChange } from '../../../src/core/change/executor.js'; +import { decideNeedsRunFile } from '../../../src/core/change/history.js'; +import { v1 } from '../../../src/core/version/schema/migrations/v1.js'; +import { resetLockManager } from '../../../src/core/lock/index.js'; +import type { NoormDatabase } from '../../../src/core/shared/index.js'; +import type { Change, ChangeContext, ChangeFile, FileExecutionRecord } from '../../../src/core/change/types.js'; + +describe('change: decideNeedsRunFile', () => { + + const success: FileExecutionRecord = { checksum: 'abc', exec_status: 'success' }; + + it('should run with reason force when forced, whatever the history', () => { + + expect(decideNeedsRunFile(success, 'abc', true)).toEqual({ needsRun: true, reason: 'force' }); + + }); + + it('should run with reason new when the lookup failed', () => { + + expect(decideNeedsRunFile(new Error('boom'), 'abc', false)).toEqual({ needsRun: true, reason: 'new' }); + + }); + + it('should run with reason new when the file never ran', () => { + + expect(decideNeedsRunFile(undefined, 'abc', false)).toEqual({ needsRun: true, reason: 'new' }); + + }); + + it('should retry a file whose last attempt failed, even with a matching checksum', () => { + + const failed: FileExecutionRecord = { checksum: 'abc', exec_status: 'failed' }; + + expect(decideNeedsRunFile(failed, 'abc', false)).toEqual({ + needsRun: true, + reason: 'failed', + previousChecksum: 'abc', + }); + + }); + + it('should run a file whose checksum changed since its last success', () => { + + expect(decideNeedsRunFile(success, 'def', false)).toEqual({ + needsRun: true, + reason: 'changed', + previousChecksum: 'abc', + }); + + }); + + it('should skip a file whose last success has a matching checksum', () => { + + expect(decideNeedsRunFile(success, 'abc', false)).toEqual({ + needsRun: false, + skipReason: 'already applied', + previousChecksum: 'abc', + }); + + }); + +}); + +describe('change: prefetch run gate', () => { + + let db: Kysely; + let tempDir: string; + let changesDir: string; + let queries: string[]; + + function context(): ChangeContext { + + return { + db, + configName: 'test', + identity: { name: 'Test User', email: 'test@example.com', source: 'config' }, + projectRoot: tempDir, + changesDir, + sqlDir: join(tempDir, 'sql'), + access: { user: 'admin', agent: 'admin' }, + channel: 'user', + dialect: 'sqlite', + }; + + } + + async function writeFiles( + dir: string, + files: Array<{ name: string; content: string }>, + ): Promise { + + await mkdir(dir, { recursive: true }); + + const written: ChangeFile[] = []; + + for (const file of files) { + + const filepath = join(dir, file.name); + await writeFile(filepath, file.content, 'utf-8'); + written.push({ filename: file.name, path: filepath, type: 'sql' }); + + } + + return written; + + } + + async function createChange( + name: string, + files: Array<{ name: string; content: string }>, + revert: Array<{ name: string; content: string }> = [], + ): Promise { + + const changePath = join(changesDir, name); + + return { + name, + path: changePath, + date: null, + description: name, + changeFiles: await writeFiles(join(changePath, 'change'), files), + revertFiles: await writeFiles(join(changePath, 'revert'), revert), + hasChangelog: false, + }; + + } + + function tables(prefix: string, count: number): Array<{ name: string; content: string }> { + + return Array.from({ length: count }, (_, i) => ({ + name: `${String(i).padStart(3, '0')}_t.sql`, + content: `CREATE TABLE ${prefix}_${i} (id INTEGER PRIMARY KEY)`, + })); + + } + + async function executionRows(operationId: number): Promise> { + + const { rows } = await sql<{ filepath: string; status: string }>` + SELECT filepath, status FROM __noorm_executions__ WHERE change_id = ${operationId} ORDER BY id + `.execute(db); + + return rows; + + } + + async function operationStatus(operationId: number): Promise { + + const { rows } = await sql<{ status: string }>` + SELECT status FROM __noorm_change__ WHERE id = ${operationId} + `.execute(db); + + return rows[0]?.status; + + } + + function executionSelects(): number { + + return queries.filter((q) => /^\s*select/i.test(q) && q.includes('__noorm_executions__')).length; + + } + + beforeEach(async () => { + + resetLockManager(); + + queries = []; + tempDir = await mkdtemp(join(tmpdir(), 'noorm-change-prefetch-gate-')); + changesDir = join(tempDir, 'changes'); + + db = new Kysely({ + dialect: new SqliteDialect({ + database: new BunSqliteDatabase(':memory:') as never, + }), + log: (event) => { + + if (event.level === 'query') queries.push(event.query.sql); + + }, + }); + + await v1.up(db as Kysely, 'sqlite'); + + }); + + afterEach(async () => { + + resetLockManager(); + await db.destroy(); + await rm(tempDir, { recursive: true, force: true }); + + }); + + it('should write an execution row only for the file that changed on re-apply', async () => { + + const change = await createChange('reapply-one', [ + { name: '001_a.sql', content: 'CREATE TABLE reapply_a (id INTEGER PRIMARY KEY)' }, + { name: '002_b.sql', content: 'CREATE TABLE reapply_b (id INTEGER PRIMARY KEY)' }, + { name: '003_c.sql', content: 'CREATE TABLE reapply_c (id INTEGER PRIMARY KEY)' }, + ]); + + await executeChange(context(), change); + await writeFile(change.changeFiles[1]!.path, 'CREATE TABLE IF NOT EXISTS reapply_b (id INTEGER PRIMARY KEY)'); + + const second = await executeChange(context(), change); + + expect(second.status).toBe('success'); + expect(second.files.map((f) => f.status)).toEqual(['skipped', 'success', 'skipped']); + expect(await executionRows(second.operationId!)).toEqual([ + { filepath: 'changes/reapply-one/change/002_b.sql', status: 'success' }, + ]); + + }); + + it('should record the operation but no execution rows when every file skips', async () => { + + const change = await createChange('all-skip', [ + { name: '001_a.sql', content: 'CREATE TABLE all_skip_a (id INTEGER PRIMARY KEY)' }, + { name: '002_b.sql', content: 'CREATE TABLE all_skip_b (id INTEGER PRIMARY KEY)' }, + ]); + + await executeChange(context(), change); + + // Adding a file then dropping it from the change changes the change checksum twice, so the + // change-level check lets the third apply through while A and B are unchanged. + const [extra] = await writeFiles(join(change.path, 'change'), [ + { name: '003_c.sql', content: 'CREATE TABLE all_skip_c (id INTEGER PRIMARY KEY)' }, + ]); + + await executeChange(context(), { ...change, changeFiles: [...change.changeFiles, extra!] }); + + const third = await executeChange(context(), change); + + expect(third.status).toBe('success'); + expect(third.files.map((f) => f.status)).toEqual(['skipped', 'skipped']); + expect(third.operationId).toBeDefined(); + expect(await operationStatus(third.operationId!)).toBe('success'); + expect(await executionRows(third.operationId!)).toEqual([]); + + }); + + it('should not add execution-history SELECTs as a change grows from 3 files to 30', async () => { + + const small = await createChange('three-files', tables('small', 3)); + const large = await createChange('thirty-files', tables('large', 30)); + + queries = []; + await executeChange(context(), small); + const smallSelects = executionSelects(); + + queries = []; + await executeChange(context(), large); + const largeSelects = executionSelects(); + + expect(largeSelects).toBe(smallSelects); + + }); + + it('should run a file listed by two manifests once in the same apply', async () => { + + const sqlDir = join(tempDir, 'sql'); + await mkdir(sqlDir, { recursive: true }); + await writeFile(join(sqlDir, 'dup.sql'), 'CREATE TABLE dup_t (id INTEGER PRIMARY KEY)'); + + const changePath = join(changesDir, 'dup-manifest'); + const manifests = await writeFiles(join(changePath, 'change'), [ + { name: '001_first.txt', content: 'dup.sql' }, + { name: '002_second.txt', content: 'dup.sql' }, + ]); + + const result = await executeChange(context(), { + name: 'dup-manifest', + path: changePath, + date: null, + description: 'dup-manifest', + changeFiles: manifests.map((m) => ({ ...m, type: 'txt' })), + revertFiles: [], + hasChangelog: false, + }); + + expect(result.status).toBe('success'); + expect(result.files.map((f) => [f.status, f.skipReason])).toEqual([ + ['success', undefined], + ['skipped', 'already applied'], + ]); + expect(await executionRows(result.operationId!)).toEqual([ + { filepath: 'sql/dup.sql', status: 'success' }, + ]); + + }); + + it('should re-run every file on apply after a revert', async () => { + + const change = await createChange( + 'apply-revert-apply', + tables('cycle', 2), + [ + { name: '001_drop.sql', content: 'DROP TABLE cycle_0' }, + { name: '002_drop.sql', content: 'DROP TABLE cycle_1' }, + ], + ); + + const applied = await executeChange(context(), change); + const reverted = await revertChange(context(), change); + const reapplied = await executeChange(context(), change); + + expect(applied.files.map((f) => f.status)).toEqual(['success', 'success']); + expect(reverted.files.map((f) => f.status)).toEqual(['success', 'success']); + expect(reapplied.status).toBe('success'); + expect(reapplied.files.map((f) => f.status)).toEqual(['success', 'success']); + expect((await executionRows(reapplied.operationId!)).map((r) => r.status)).toEqual(['success', 'success']); + + }); + +}); diff --git a/tests/core/runner/needs-run-decision.test.ts b/tests/core/runner/needs-run-decision.test.ts new file mode 100644 index 00000000..47e44e95 --- /dev/null +++ b/tests/core/runner/needs-run-decision.test.ts @@ -0,0 +1,123 @@ +import { describe, it, expect } from 'bun:test'; + +import { decideNeedsRun } from '../../../src/core/runner/tracker.js'; +import type { ExecutionRecord, NeedsRunResult } from '../../../src/core/runner/types.js'; + +const success: ExecutionRecord = { + checksum: 'abc123', + exec_status: 'success', + skip_reason: '', + change_status: 'success', +}; + +describe('runner: decideNeedsRun', () => { + + it('should run on force even when the record says unchanged', () => { + + const result: NeedsRunResult = decideNeedsRun(success, 'abc123', true); + + expect(result).toEqual({ needsRun: true, reason: 'force' }); + + }); + + it('should run with reason error when the lookup failed', () => { + + expect(decideNeedsRun(new Error('db down'), 'abc123', false)).toEqual({ needsRun: true, reason: 'error' }); + + }); + + it('should run a file with no record as new', () => { + + expect(decideNeedsRun(undefined, 'abc123', false)).toEqual({ needsRun: true, reason: 'new' }); + + }); + + it('should retry a failed file', () => { + + const record: ExecutionRecord = { ...success, exec_status: 'failed' }; + + expect(decideNeedsRun(record, 'abc123', false)).toEqual({ + needsRun: true, + reason: 'failed', + previousChecksum: 'abc123', + }); + + }); + + it('should run a file whose newest row is a pending placeholder', () => { + + const record: ExecutionRecord = { ...success, exec_status: 'pending' }; + + expect(decideNeedsRun(record, 'abc123', false)).toEqual({ needsRun: true, reason: 'new' }); + + }); + + it('should run a file cascade-skipped by an earlier failure', () => { + + const record: ExecutionRecord = { + ...success, + exec_status: 'skipped', + skip_reason: 'Skipped: failure in 000_first.sql', + }; + + expect(decideNeedsRun(record, 'abc123', false)).toEqual({ needsRun: true, reason: 'new' }); + + }); + + it('should run a file whose parent change is stale', () => { + + const record: ExecutionRecord = { ...success, change_status: 'stale' }; + + expect(decideNeedsRun(record, 'abc123', false)).toEqual({ + needsRun: true, + reason: 'stale', + previousChecksum: 'abc123', + }); + + }); + + it('should run a file whose checksum changed', () => { + + expect(decideNeedsRun(success, 'def456', false)).toEqual({ + needsRun: true, + reason: 'changed', + previousChecksum: 'abc123', + }); + + }); + + it('should skip an unchanged file', () => { + + expect(decideNeedsRun(success, 'abc123', false)).toEqual({ + needsRun: false, + skipReason: 'unchanged', + previousChecksum: 'abc123', + }); + + }); + + it('should skip on a matching unchanged-skip row written by older builds', () => { + + const record: ExecutionRecord = { ...success, exec_status: 'skipped', skip_reason: 'unchanged' }; + + expect(decideNeedsRun(record, 'abc123', false)).toEqual({ + needsRun: false, + skipReason: 'unchanged', + previousChecksum: 'abc123', + }); + + }); + + it('should run an unchanged-skip row whose checksum no longer matches', () => { + + const record: ExecutionRecord = { ...success, exec_status: 'skipped', skip_reason: 'unchanged' }; + + expect(decideNeedsRun(record, 'def456', false)).toEqual({ + needsRun: true, + reason: 'changed', + previousChecksum: 'abc123', + }); + + }); + +}); diff --git a/tests/core/runner/prefetch-gate.test.ts b/tests/core/runner/prefetch-gate.test.ts new file mode 100644 index 00000000..9da45491 --- /dev/null +++ b/tests/core/runner/prefetch-gate.test.ts @@ -0,0 +1,319 @@ +// A skipped file must leave no tracking row and cost no per-file query +// (docs/spec/prefetch-run-gate.md). +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { mkdtemp, rm, mkdir, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { Kysely, SqliteDialect, sql } from 'kysely'; + +import { BunSqliteDatabase } from '../../../src/core/connection/dialects/sqlite-bun.js'; +import { runBuild, runFile, runFiles, checkFilesStatus } from '../../../src/core/runner/runner.js'; +import { observer } from '../../../src/core/observer.js'; +import { v1 } from '../../../src/core/version/schema/migrations/v1.js'; +import type { NoormDatabase } from '../../../src/core/shared/index.js'; +import type { RunContext } from '../../../src/core/runner/types.js'; + +describe('runner: prefetch run gate', () => { + + let db: Kysely; + let tempDir: string; + let sqlDir: string; + let queries: string[]; + + function context(): RunContext { + + return { + db, + configName: 'test', + identity: { name: 'Test User', email: 'test@example.com', source: 'config' }, + projectRoot: tempDir, + access: { user: 'admin', agent: 'admin' }, + channel: 'user', + dialect: 'sqlite', + }; + + } + + async function rowCounts(): Promise<{ operations: number; executions: number }> { + + const ops = await sql<{ n: number }>`SELECT COUNT(*) AS n FROM __noorm_change__`.execute(db); + const execs = await sql<{ n: number }>`SELECT COUNT(*) AS n FROM __noorm_executions__`.execute(db); + + return { operations: Number(ops.rows[0]!.n), executions: Number(execs.rows[0]!.n) }; + + } + + async function executionRows(): Promise> { + + const { rows } = await sql<{ filepath: string; status: string }>` + SELECT filepath, status FROM __noorm_executions__ ORDER BY id + `.execute(db); + + return rows; + + } + + function executionSelects(): number { + + return queries.filter((q) => /^\s*select/i.test(q) && q.includes('__noorm_executions__')).length; + + } + + async function writeTables(dir: string, count: number, prefix: string): Promise { + + await mkdir(dir, { recursive: true }); + + const paths: string[] = []; + + for (let i = 0; i < count; i++) { + + const filepath = join(dir, `${String(i).padStart(3, '0')}_t.sql`); + await writeFile(filepath, `CREATE TABLE ${prefix}_${i} (id INTEGER PRIMARY KEY);\n`, 'utf-8'); + paths.push(filepath); + + } + + return paths; + + } + + beforeEach(async () => { + + queries = []; + tempDir = await mkdtemp(join(tmpdir(), 'noorm-prefetch-gate-')); + sqlDir = join(tempDir, 'sql'); + await mkdir(sqlDir, { recursive: true }); + + db = new Kysely({ + dialect: new SqliteDialect({ + database: new BunSqliteDatabase(':memory:') as never, + }), + log: (event) => { + + if (event.level === 'query') queries.push(event.query.sql); + + }, + }); + + await v1.up(db as Kysely, 'sqlite'); + + }); + + afterEach(async () => { + + await db.destroy(); + await rm(tempDir, { recursive: true, force: true }); + + }); + + it('should write no tracking rows and no operation when every file is unchanged', async () => { + + await writeTables(sqlDir, 3, 'unchanged'); + await runBuild(context(), sqlDir); + + const before = await rowCounts(); + const skipped: string[] = []; + const off = observer.on('file:skip', (data) => skipped.push(data.filepath)); + + const second = await runBuild(context(), sqlDir); + + off(); + + expect(second.status).toBe('success'); + expect(second.files.map((f) => f.status)).toEqual(['skipped', 'skipped', 'skipped']); + expect(second.changeId).toBeUndefined(); + expect(skipped).toHaveLength(3); + expect(await rowCounts()).toEqual(before); + + }); + + it('should report success and write nothing when a cancelled run has only unchanged files', async () => { + + await writeTables(sqlDir, 2, 'cancelled_unchanged'); + await runBuild(context(), sqlDir); + + const before = await rowCounts(); + const controller = new AbortController(); + controller.abort(); + + const second = await runBuild({ ...context(), signal: controller.signal }, sqlDir); + + expect(second.status).toBe('success'); + expect(second.files.map((f) => f.status)).toEqual(['skipped', 'skipped']); + expect(await rowCounts()).toEqual(before); + + }); + + it('should write no operation for an empty file list', async () => { + + const before = await rowCounts(); + const result = await runFiles(context(), []); + + expect(result.status).toBe('success'); + expect(result.changeId).toBeUndefined(); + expect(await rowCounts()).toEqual(before); + + }); + + it('should record only the changed file in a mixed build', async () => { + + const [first] = await writeTables(sqlDir, 3, 'mixed'); + await runBuild(context(), sqlDir); + + await writeFile(first!, 'CREATE TABLE mixed_0b (id INTEGER PRIMARY KEY);\n', 'utf-8'); + + const before = await rowCounts(); + const second = await runBuild(context(), sqlDir); + + expect(second.files.map((f) => f.status)).toEqual(['success', 'skipped', 'skipped']); + expect(second.changeId).toBeDefined(); + expect(await rowCounts()).toEqual({ + operations: before.operations + 1, + executions: before.executions + 1, + }); + + const rows = await executionRows(); + + expect(rows.at(-1)).toEqual({ filepath: 'sql/000_t.sql', status: 'success' }); + + }); + + it('should rerun a previously failed file whose content did not change', async () => { + + await writeFile(join(sqlDir, '001_ok.sql'), 'CREATE TABLE ok_t (id INTEGER PRIMARY KEY);\n', 'utf-8'); + await writeFile(join(sqlDir, '002_needs.sql'), 'INSERT INTO later_t (id) VALUES (1);\n', 'utf-8'); + + const first = await runBuild(context(), sqlDir); + + expect(first.files.map((f) => f.status)).toEqual(['success', 'failed']); + + await sql`CREATE TABLE later_t (id INTEGER PRIMARY KEY)`.execute(db); + + const second = await runBuild(context(), sqlDir); + + expect(second.files.map((f) => f.status)).toEqual(['skipped', 'success']); + + }); + + it('should rerun an unchanged file whose parent operation went stale', async () => { + + await writeTables(sqlDir, 2, 'stale'); + const first = await runBuild(context(), sqlDir); + + await sql`UPDATE __noorm_change__ SET status = 'stale' WHERE id = ${first.changeId}`.execute(db); + await sql`DROP TABLE stale_0`.execute(db); + await sql`DROP TABLE stale_1`.execute(db); + + const second = await runBuild(context(), sqlDir); + + expect(second.files.map((f) => f.status)).toEqual(['success', 'success']); + + }); + + it('should fail a file that cannot render at its turn and cascade-skip the files after it', async () => { + + await writeFile(join(sqlDir, '001_ok.sql'), 'CREATE TABLE load_ok (id INTEGER PRIMARY KEY);\n', 'utf-8'); + await writeFile(join(sqlDir, '002_bad.sql.tmpl'), "SELECT '{%~ $.secrets.MISSING_KEY %}';\n", 'utf-8'); + await writeFile(join(sqlDir, '003_later.sql'), 'CREATE TABLE load_later (id INTEGER PRIMARY KEY);\n', 'utf-8'); + + const result = await runBuild(context(), sqlDir); + + expect(result.files.map((f) => f.status)).toEqual(['success', 'failed']); + expect(result.files[1]!.error).toContain('MISSING_KEY'); + expect((await rowCounts()).operations).toBe(1); + expect(await executionRows()).toEqual([ + { filepath: 'sql/001_ok.sql', status: 'success' }, + { filepath: 'sql/002_bad.sql.tmpl', status: 'failed' }, + { filepath: 'sql/003_later.sql', status: 'skipped' }, + ]); + + }); + + it('should not add executions SELECTs going from 3 files to 30', async () => { + + async function selectsForMixedRebuild(dir: string, count: number): Promise { + + const [first] = await writeTables(dir, count, `q${count}`); + await runBuild(context(), dir); + await writeFile(first!, `CREATE TABLE q${count}_0b (id INTEGER PRIMARY KEY);\n`, 'utf-8'); + + queries = []; + await runBuild(context(), dir); + + return executionSelects(); + + } + + const small = await selectsForMixedRebuild(join(tempDir, 'sql3'), 3); + const large = await selectsForMixedRebuild(join(tempDir, 'sql30'), 30); + + expect(large).toBe(small); + + }); + + it('should write nothing when runFile is given an unchanged file', async () => { + + const [filepath] = await writeTables(sqlDir, 1, 'single'); + + expect((await runFile(context(), filepath!)).status).toBe('success'); + + const before = await rowCounts(); + const second = await runFile(context(), filepath!); + + expect(second.status).toBe('skipped'); + expect(second.skipReason).toBe('unchanged'); + expect(await rowCounts()).toEqual(before); + + }); + + it('should write one operation and one execution when runFile runs a changed file', async () => { + + const [filepath] = await writeTables(sqlDir, 1, 'single_changed'); + await runFile(context(), filepath!); + await writeFile(filepath!, 'CREATE TABLE single_changed_b (id INTEGER PRIMARY KEY);\n', 'utf-8'); + + const before = await rowCounts(); + const second = await runFile(context(), filepath!); + + expect(second.status).toBe('success'); + expect(await rowCounts()).toEqual({ + operations: before.operations + 1, + executions: before.executions + 1, + }); + + }); + + it('should neither execute nor record a file when runFile is given preview', async () => { + + const [filepath] = await writeTables(sqlDir, 1, 'previewed'); + + await runFile(context(), filepath!, { preview: true }); + + const { rows } = await sql<{ n: number }>` + SELECT COUNT(*) AS n FROM sqlite_master WHERE type = 'table' AND name = 'previewed_0' + `.execute(db); + + expect(Number(rows[0]!.n)).toBe(0); + expect(await rowCounts()).toEqual({ operations: 0, executions: 0 }); + + }); + + it('should categorize files with a single executions SELECT', async () => { + + const [unchanged, changed] = await writeTables(sqlDir, 2, 'status'); + await runBuild(context(), sqlDir); + await writeFile(changed!, 'CREATE TABLE status_1b (id INTEGER PRIMARY KEY);\n', 'utf-8'); + const [fresh] = await writeTables(join(tempDir, 'extra'), 1, 'status_new'); + + queries = []; + const status = await checkFilesStatus(context(), [unchanged!, changed!, fresh!]); + + expect(executionSelects()).toBe(1); + expect(status.previouslyRunFiles).toEqual([unchanged!]); + expect(status.changedFiles).toEqual([changed!]); + expect(status.newFiles).toEqual([fresh!]); + expect(status.wouldSkipCount).toBe(1); + + }); + +}); diff --git a/tests/core/runner/statement-watcher.test.ts b/tests/core/runner/statement-watcher.test.ts index c81556d4..907603c2 100644 --- a/tests/core/runner/statement-watcher.test.ts +++ b/tests/core/runner/statement-watcher.test.ts @@ -16,7 +16,7 @@ import { BunSqliteDatabase } from '../../../src/core/connection/dialects/sqlite- import { observer } from '../../../src/core/observer.js'; import { OperationAbortedError } from '../../../src/core/shared/abort.js'; -import { runFiles } from '../../../src/core/runner/runner.js'; +import { runFile, runFiles } from '../../../src/core/runner/runner.js'; import { StatementWatcher } from '../../../src/core/runner/statement-watcher.js'; import { v1 } from '../../../src/core/version/schema/migrations/v1.js'; import type { NoormDatabase } from '../../../src/core/shared/index.js'; @@ -240,4 +240,43 @@ describe('runner: run cancellation', () => { }); + it('should report success when the cancel lands after the last file that runs, before an unchanged one', async () => { + + await runFiles(buildContext(new AbortController().signal), files); + await writeFile(files[0]!, 'CREATE TABLE cancel_a2 (id INTEGER PRIMARY KEY);\n', 'utf-8'); + + const controller = new AbortController(); + const stop = observer.on('file:after', ({ filepath }) => { + + if (filepath === files[0]) controller.abort(); + + }); + + const result = await runFiles(buildContext(controller.signal), files); + stop(); + + expect(result.status).toBe('success'); + expect(result.error).toBeUndefined(); + expect(result.files.map((f) => f.status)).toEqual(['success', 'skipped']); + + }); + + it('should skip and finalize the operation as cancelled when runFile gets an aborted signal', async () => { + + const controller = new AbortController(); + controller.abort(); + + const result = await runFile(buildContext(controller.signal), files[0]!); + + const { rows } = await sql<{ status: string; error_message: string }>` + SELECT status, error_message FROM __noorm_change__ + `.execute(db); + + expect(result.status).toBe('skipped'); + expect(result.error).toBe('Run cancelled'); + expect(await tableExists('cancel_a')).toBe(false); + expect(rows).toEqual([{ status: 'failed', error_message: 'Run cancelled' }]); + + }); + }); diff --git a/tests/core/runner/template-dedup.test.ts b/tests/core/runner/template-dedup.test.ts index 829e8a5d..b9409ea7 100644 --- a/tests/core/runner/template-dedup.test.ts +++ b/tests/core/runner/template-dedup.test.ts @@ -2,14 +2,14 @@ * Checksum dedup for `.sql.tmpl` files through `run build`. * * The runner's headline promise is "run build as often as you like, only - * changed files execute". For templates it was false on every build: the - * checksum persisted upfront was the *raw file* hash while `needsRun` - * compared the *rendered* hash, so the two could never match and every - * template re-executed forever — failing on any non-idempotent DDL. + * changed files execute". For a template that holds only if the recorded + * checksum and the compared checksum are both the *rendered* hash; mixing in + * the raw file hash means they never match, and every template re-executes + * forever — failing on any non-idempotent DDL. * - * `tests/core/runner/tracker.test.ts` cannot see this: it hands `needsRun` - * checksums directly, so the raw-vs-rendered mismatch is invisible by - * construction. These tests drive `runBuild` end to end instead, and pin + * `tests/core/runner/tracker.test.ts` cannot see this: it hands + * `latestExecutions` rows and checksums to `decideNeedsRun` directly, + * so the raw-vs-rendered mismatch is invisible by construction. These tests drive `runBuild` end to end instead, and pin * the *decision* the fix encodes — the rendered SQL is the dedup key, so a * template whose inputs changed re-runs even though its bytes did not. */ @@ -22,6 +22,7 @@ import { Kysely, SqliteDialect, sql } from 'kysely'; import { BunSqliteDatabase } from '../../../src/core/connection/dialects/sqlite-bun.js'; import { runBuild } from '../../../src/core/runner/runner.js'; import { computeChecksumFromContent } from '../../../src/core/runner/checksum.js'; +import { observer } from '../../../src/core/observer.js'; import { v1 } from '../../../src/core/version/schema/migrations/v1.js'; import type { NoormDatabase } from '../../../src/core/shared/index.js'; import type { RunContext } from '../../../src/core/runner/types.js'; @@ -103,8 +104,8 @@ describe('runner: template checksum dedup', () => { expect(second.filesSkipped).toBe(1); expect(second.files[0]?.skipReason).toBe('unchanged'); - // The third build is the one the previous skip-reason fix (dd7e387) - // was about — a recorded `unchanged` skip must not read as "never ran". + // A skip writes no row, so the third build still compares against + // the first build's success. const third = await runBuild(context(), sqlDir); expect(third.status).toBe('success'); @@ -162,4 +163,40 @@ describe('runner: template checksum dedup', () => { }); + it('should write no tracking rows when the rendered output is unchanged', async () => { + + await writeFile(join(sqlDir, 'seed.json'), JSON.stringify({ label: 'same' }), 'utf-8'); + await writeFile(join(sqlDir, '001_t.sql.tmpl'), "SELECT '{%~ $.seed.label %}' AS label;", 'utf-8'); + + await runBuild(context(), sqlDir); + + const before = await sql<{ n: number }>`SELECT COUNT(*) AS n FROM __noorm_executions__`.execute(db); + const second = await runBuild(context(), sqlDir); + const after = await sql<{ n: number }>`SELECT COUNT(*) AS n FROM __noorm_executions__`.execute(db); + + expect(second.filesSkipped).toBe(1); + expect(after.rows[0]!.n).toBe(before.rows[0]!.n); + + }); + + it('should render each template once per build, whether it runs or skips', async () => { + + await writeFile(join(sqlDir, '001_t.sql.tmpl'), 'CREATE TABLE tmpl_once (id INTEGER PRIMARY KEY);', 'utf-8'); + + const renders: string[] = []; + const off = observer.on('template:render', (data) => renders.push(data.filepath)); + + await runBuild(context(), sqlDir); + const rendersWhenRun = renders.length; + + await runBuild(context(), sqlDir); + const rendersWhenSkipped = renders.length - rendersWhenRun; + + off(); + + expect(rendersWhenRun).toBe(1); + expect(rendersWhenSkipped).toBe(1); + + }); + }); diff --git a/tests/core/runner/tracker.test.ts b/tests/core/runner/tracker.test.ts index b82c1eff..f3f8eeeb 100644 --- a/tests/core/runner/tracker.test.ts +++ b/tests/core/runner/tracker.test.ts @@ -1,23 +1,25 @@ /** - * Runner tracker tests (v1/49-54 CP9/CP10). + * Runner tracker tests. * - * Uses a real in-memory SQLite database, not a mock -- the CP10 defect - * lives in `needsRun`'s `ORDER BY id DESC` picking up a row that a mocked - * tracker would never reproduce. + * Uses a real in-memory SQLite database, not a mock -- `decideNeedsRun` + * decides from whichever row `latestExecutions` picks as newest, which a mock + * can't reproduce. */ import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; import { Kysely, SqliteDialect, sql } from 'kysely'; import { BunSqliteDatabase } from '../../../src/core/connection/dialects/sqlite-bun.js'; -import { Tracker } from '../../../src/core/runner/tracker.js'; +import { Tracker, decideNeedsRun } from '../../../src/core/runner/tracker.js'; +import { observer } from '../../../src/core/observer.js'; import { v1 } from '../../../src/core/version/schema/migrations/v1.js'; import type { NoormDatabase } from '../../../src/core/shared/index.js'; -import type { CreateOperationData } from '../../../src/core/runner/types.js'; +import type { CreateOperationData, NeedsRunResult } from '../../../src/core/runner/types.js'; describe('runner: tracker', () => { let db: Kysely; let tracker: Tracker; + let queries: string[]; const baseOp: Omit = { changeType: 'build', @@ -27,10 +29,17 @@ describe('runner: tracker', () => { beforeEach(async () => { + queries = []; + db = new Kysely({ dialect: new SqliteDialect({ database: new BunSqliteDatabase(':memory:') as never, }), + log: (event) => { + + if (event.level === 'query') queries.push(event.query.sql); + + }, }); await v1.up(db as Kysely, 'sqlite'); @@ -45,86 +54,22 @@ describe('runner: tracker', () => { }); - describe('needsRun — excludeOperationId (CP10)', () => { - - it('should not treat the current operation\'s own upfront pending row as the newest record', async () => { - - // A prior, completed build ran this file successfully. - const priorOpId = await tracker.createOperation({ ...baseOp, name: 'build:prior' }); - await tracker.createFileRecords(priorOpId, [ - { filepath: 'sql/001.sql', fileType: 'sql', checksum: 'abc123' }, - ]); - await tracker.updateFileExecution(priorOpId, 'sql/001.sql', 'success', 10); - - // A second build's `executeFiles` inserts a pending row for - // every file upfront (createFileRecords), before needsRun runs. - const currentOpId = await tracker.createOperation({ ...baseOp, name: 'build:current' }); - await tracker.createFileRecords(currentOpId, [ - { filepath: 'sql/001.sql', fileType: 'sql', checksum: 'abc123' }, - ]); - - // Reproduces the defect: without exclusion, the newest row by id - // is this run's own pending record, and 'pending' always reads - // as 'new' -- checksum-based skipping is unreachable. - const withoutExclusion = await tracker.needsRun('sql/001.sql', 'abc123', false); - expect(withoutExclusion).toEqual({ needsRun: true, reason: 'new' }); - - // With exclusion, the lookup skips this operation's own pending - // row and finds the prior success -- the file is unchanged. - const result = await tracker.needsRun('sql/001.sql', 'abc123', false, currentOpId); - - expect(result.needsRun).toBe(false); - expect(result.skipReason).toBe('unchanged'); - - }); - - it('should still report a changed file as needing to run when excluding the current operation', async () => { - - const priorOpId = await tracker.createOperation({ ...baseOp, name: 'build:prior' }); - await tracker.createFileRecords(priorOpId, [ - { filepath: 'sql/001.sql', fileType: 'sql', checksum: 'old-checksum' }, - ]); - await tracker.updateFileExecution(priorOpId, 'sql/001.sql', 'success', 10); - - const currentOpId = await tracker.createOperation({ ...baseOp, name: 'build:current' }); - await tracker.createFileRecords(currentOpId, [ - { filepath: 'sql/001.sql', fileType: 'sql', checksum: 'new-checksum' }, - ]); - - const result = await tracker.needsRun('sql/001.sql', 'new-checksum', false, currentOpId); - - expect(result).toEqual({ - needsRun: true, - reason: 'changed', - previousChecksum: 'old-checksum', - }); - - }); - - it('should force re-run regardless of exclusion', async () => { + const decide = async (filepath: string, checksum: string, force: boolean): Promise => { - const priorOpId = await tracker.createOperation({ ...baseOp, name: 'build:prior' }); - await tracker.createFileRecords(priorOpId, [ - { filepath: 'sql/001.sql', fileType: 'sql', checksum: 'abc123' }, - ]); - await tracker.updateFileExecution(priorOpId, 'sql/001.sql', 'success', 10); + const [latest, err] = await tracker.latestExecutions(); - const result = await tracker.needsRun('sql/001.sql', 'abc123', true, priorOpId); + return decideNeedsRun(err ?? latest.get(filepath), checksum, force); - expect(result).toEqual({ needsRun: true, reason: 'force' }); - - }); - - }); + }; - describe('needsRun — skipped means two different things', () => { + describe('latestExecutions + decideNeedsRun — skipped means two different things', () => { const filepath = 'sql/001.sql'; /** - * Replays the first two builds of the sequence that used to break: - * build 1 executes the file, build 2 correctly skips it as unchanged. - * What build 3 should do with that history is what each test asks. + * Seeds the history an older build left: build 1 executes the file, + * build 2 skips it and records an `unchanged` skip row. Current builds + * write no row for a skip, but these rows remain in existing databases. */ const seedSuccessThenUnchangedSkip = async (checksum: string) => { @@ -148,16 +93,9 @@ describe('runner: tracker', () => { await seedSuccessThenUnchangedSkip('abc123'); - // Build 3 inserts its own pending row upfront, then asks. Before - // the fix this returned `{ needsRun: true, reason: 'new' }` off - // build 2's skip and re-executed the file -- fatal for any DDL - // that isn't idempotent, and deterministic on every third build. - const thirdOpId = await tracker.createOperation({ ...baseOp, name: 'build:3' }); - await tracker.createFileRecords(thirdOpId, [ - { filepath, fileType: 'sql', checksum: 'abc123' }, - ]); - - const result = await tracker.needsRun(filepath, 'abc123', false, thirdOpId); + // Reading build 2's skip as 'new' re-executes the file on every + // third build -- fatal for any DDL that isn't idempotent. + const result = await decide(filepath, 'abc123', false); expect(result).toEqual({ needsRun: false, @@ -174,25 +112,25 @@ describe('runner: tracker', () => { { filepath, fileType: 'sql', checksum: 'abc123' }, ]); - // The batch stopped before reaching this file, so its upfront - // pending row becomes a skip carrying a cascade reason. Unlike an + // The batch stopped before reaching this file, so its pending + // row becomes a skip carrying a cascade reason. Unlike an // 'unchanged' skip, nothing ever ran -- the file still owes a run. await tracker.skipRemainingFiles(opId, 'Skipped: failure in 000_first.sql'); - const result = await tracker.needsRun(filepath, 'abc123', false); + const result = await decide(filepath, 'abc123', false); expect(result).toEqual({ needsRun: true, reason: 'new' }); }); - it('should re-run a file whose only record is an upfront pending placeholder', async () => { + it('should re-run a file whose only record is a pending placeholder', async () => { const opId = await tracker.createOperation({ ...baseOp, name: 'build:crashed' }); await tracker.createFileRecords(opId, [ { filepath, fileType: 'sql', checksum: 'abc123' }, ]); - const result = await tracker.needsRun(filepath, 'abc123', false); + const result = await decide(filepath, 'abc123', false); expect(result).toEqual({ needsRun: true, reason: 'new' }); @@ -204,7 +142,7 @@ describe('runner: tracker', () => { // Proves the unchanged skip falls through to the checksum // comparison rather than short-circuiting into a blanket skip. - const result = await tracker.needsRun(filepath, 'def456', false); + const result = await decide(filepath, 'def456', false); expect(result).toEqual({ needsRun: true, @@ -226,7 +164,7 @@ describe('runner: tracker', () => { .where('id', '=', secondOpId) .execute(); - const result = await tracker.needsRun(filepath, 'abc123', false); + const result = await decide(filepath, 'abc123', false); expect(result).toEqual({ needsRun: true, @@ -240,7 +178,7 @@ describe('runner: tracker', () => { await seedSuccessThenUnchangedSkip('abc123'); - const result = await tracker.needsRun(filepath, 'abc123', true); + const result = await decide(filepath, 'abc123', true); expect(result).toEqual({ needsRun: true, reason: 'force' }); @@ -248,7 +186,7 @@ describe('runner: tracker', () => { }); - describe('needsRun — DB error path (CP9.3)', () => { + describe('latestExecutions + decideNeedsRun — DB error path', () => { it('should distinguish a failed read from a genuinely new file', async () => { @@ -256,7 +194,7 @@ describe('runner: tracker', () => { // fails -- distinct from "no matching row". await sql`DROP TABLE __noorm_executions__`.execute(db); - const result = await tracker.needsRun('sql/001.sql', 'abc123', false); + const result = await decide('sql/001.sql', 'abc123', false); expect(result.needsRun).toBe(true); expect(result.reason).toBe('error'); @@ -266,6 +204,103 @@ describe('runner: tracker', () => { }); + describe('latestExecutions', () => { + + const seedRun = async (configName: string, filepath: string, checksum: string) => { + + const opId = await tracker.createOperation({ ...baseOp, configName, name: `build:${checksum}` }); + await tracker.createFileRecords(opId, [{ filepath, fileType: 'sql', checksum }]); + await tracker.updateFileExecution(opId, filepath, 'success', 1); + + return opId; + + }; + + it('should return the newest row per filepath for the config', async () => { + + await seedRun('test', 'sql/001.sql', 'old'); + await seedRun('test', 'sql/001.sql', 'new'); + await seedRun('test', 'sql/002.sql', 'only'); + + const [latest, err] = await tracker.latestExecutions(); + + expect(err).toBeNull(); + expect(latest?.size).toBe(2); + expect(latest?.get('sql/001.sql')?.checksum).toBe('new'); + expect(latest?.get('sql/002.sql')?.checksum).toBe('only'); + + }); + + it('should exclude rows recorded under another config', async () => { + + await seedRun('test', 'sql/001.sql', 'mine'); + await seedRun('other', 'sql/001.sql', 'theirs'); + await seedRun('other', 'sql/002.sql', 'theirs'); + + const [latest] = await tracker.latestExecutions(); + + expect(latest?.size).toBe(1); + expect(latest?.get('sql/001.sql')?.checksum).toBe('mine'); + + }); + + it('should carry the parent change status so stale rows rerun', async () => { + + const opId = await seedRun('test', 'sql/001.sql', 'abc'); + + await db + .updateTable('__noorm_change__') + .set({ status: 'stale' }) + .where('id', '=', opId) + .execute(); + + const [latest] = await tracker.latestExecutions(); + + expect(latest?.get('sql/001.sql')).toEqual({ + checksum: 'abc', + exec_status: 'success', + skip_reason: '', + change_status: 'stale', + }); + + }); + + it('should issue exactly one query regardless of file count', async () => { + + for (let i = 0; i < 5; i++) { + + await seedRun('test', `sql/00${i}.sql`, `c${i}`); + + } + + queries = []; + + const [latest] = await tracker.latestExecutions(); + + expect(latest?.size).toBe(5); + expect(queries).toHaveLength(1); + + }); + + it('should emit and return the error when the lookup fails', async () => { + + const events: unknown[] = []; + const unsub = observer.on('error', (data) => events.push(data)); + + await sql`DROP TABLE __noorm_executions__`.execute(db); + + const [latest, err] = await tracker.latestExecutions(); + + unsub(); + + expect(latest).toBeNull(); + expect(err).toBeInstanceOf(Error); + expect(events).toHaveLength(1); + + }); + + }); + describe('updateFileExecution — row count enforcement (CP9.2)', () => { it('should fail when no row matches', async () => { diff --git a/tests/integration/change/history-dialects.test.ts b/tests/integration/change/history-dialects.test.ts index 6c66c7ea..4477ea19 100644 --- a/tests/integration/change/history-dialects.test.ts +++ b/tests/integration/change/history-dialects.test.ts @@ -14,6 +14,9 @@ * needs, these methods must hand back a usable primary key that child * rows can reference. Anything else means no operation record exists. * + * `latestFileExecutions` is here for the same reason: its `MAX(id) GROUP BY` + * subquery join is dialect SQL that the sqlite unit tests cannot vouch for. + * * Requires the docker-compose.test.yml containers (postgres 15432, * mysql 13306, mssql 11433). Each dialect skips itself when unreachable. */ @@ -27,7 +30,7 @@ import { ChangeHistory } from '../../../src/core/change/history.js'; import { migrateSchema } from '../../../src/core/version/schema/index.js'; import { v1 } from '../../../src/core/version/schema/migrations/v1.js'; import { getNoormTables, noormDb } from '../../../src/core/shared/index.js'; -import type { NoormDatabase } from '../../../src/core/shared/index.js'; +import type { Direction, ExecutionStatus, NoormDatabase } from '../../../src/core/shared/index.js'; import type { Dialect } from '../../../src/core/connection/types.js'; import type { ConnectionResult } from '../../../src/core/connection/types.js'; @@ -172,6 +175,68 @@ describe('change: history id retrieval across dialects', () => { }); + it('should return the newest standing row per file of one change', async () => { + + if (!reachable) { + + console.warn(`Skipping ${dialect}: container not reachable`); + + return; + + } + + const db = conn!.db as unknown as Kysely; + const history = new ChangeHistory(db, CONFIG_NAME, dialect); + const name = `change:latest:${dialect}:${Date.now()}`; + const fileA = `changes/${name}/change/001_a.sql`; + const fileB = `changes/${name}/change/002_b.sql`; + + async function record( + changeName: string, + direction: Direction, + rows: Array<{ filepath: string; checksum: string; status: ExecutionStatus }>, + ): Promise { + + const opId = await history.createOperation({ name: changeName, direction, executedBy: 'test@example.com' }); + + await history.createFileRecords(opId, rows.map((row) => ({ ...row, fileType: 'sql' }))); + + for (const row of rows) { + + await history.updateFileExecution(opId, row.filepath, row.status, 5); + + } + + await history.finalizeOperation(opId, 'success', `${changeName}:checksum`, 5); + + } + + await record(name, 'change', [ + { filepath: fileA, checksum: 'a-old', status: 'success' }, + { filepath: fileB, checksum: 'b-only', status: 'success' }, + ]); + await record(name, 'change', [ + { filepath: fileA, checksum: 'a-new', status: 'failed' }, + { filepath: fileB, checksum: 'b-unreached', status: 'skipped' }, + ]); + await record(`${name}:other`, 'change', [{ filepath: fileA, checksum: 'a-other', status: 'success' }]); + + const [latest, err] = await history.latestFileExecutions(name, 'change'); + + expect(err).toBeNull(); + expect(latest!.size).toBe(2); + expect(latest!.get(fileA)).toEqual({ checksum: 'a-new', exec_status: 'failed' }); + expect(latest!.get(fileB)).toEqual({ checksum: 'b-only', exec_status: 'success' }); + + await record(name, 'revert', [{ filepath: `changes/${name}/revert/001_a.sql`, checksum: 'r', status: 'success' }]); + + const [afterRevert, revertErr] = await history.latestFileExecutions(name, 'change'); + + expect(revertErr).toBeNull(); + expect(afterRevert!.size).toBe(0); + + }); + }); } diff --git a/tests/integration/runner/tracker-dialects.test.ts b/tests/integration/runner/tracker-dialects.test.ts index 75c1b6a7..781a09e1 100644 --- a/tests/integration/runner/tracker-dialects.test.ts +++ b/tests/integration/runner/tracker-dialects.test.ts @@ -14,6 +14,9 @@ * usable primary key that later rows can reference. Anything else means the * operation record was never created. * + * `latestExecutions` is here for the same reason: its `MAX(id) GROUP BY` + * subquery join is dialect SQL that the sqlite unit tests cannot vouch for. + * * Requires the docker-compose.test.yml containers (postgres 15432, * mysql 13306, mssql 11433). Each dialect skips itself when unreachable. */ @@ -36,8 +39,9 @@ import { createTestConnection, isContainerRunning } from '../../utils/db.js'; const LIVE_DIALECTS: Dialect[] = ['postgres', 'mysql', 'mssql']; const CONFIG_NAME = '__tracker_dialects__'; +const OTHER_CONFIG_NAME = '__tracker_dialects_other__'; -describe('runner: tracker.createOperation across dialects', () => { +describe('runner: tracker across dialects', () => { it('should return a usable operation id on sqlite', async () => { @@ -99,7 +103,7 @@ describe('runner: tracker.createOperation across dialects', () => { (noormDb(db, dialect) as any) .selectFrom(tables.change) .select('id') - .where('config_name', '=', CONFIG_NAME), + .where('config_name', 'in', [CONFIG_NAME, OTHER_CONFIG_NAME]), ) .execute(), ); @@ -108,7 +112,7 @@ describe('runner: tracker.createOperation across dialects', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any (noormDb(db, dialect) as any) .deleteFrom(tables.change) - .where('config_name', '=', CONFIG_NAME) + .where('config_name', 'in', [CONFIG_NAME, OTHER_CONFIG_NAME]) .execute(), ); @@ -152,6 +156,62 @@ describe('runner: tracker.createOperation across dialects', () => { }); + it('should return the newest execution per filepath for its own config only', async () => { + + if (!reachable) { + + console.warn(`Skipping ${dialect}: container not reachable`); + + return; + + } + + const db = conn!.db as unknown as Kysely; + const tracker = new Tracker(db, CONFIG_NAME, dialect); + const otherTracker = new Tracker(db, OTHER_CONFIG_NAME, dialect); + const run = Date.now(); + const fileA = `sql/latest-a-${run}.sql`; + const fileB = `sql/latest-b-${run}.sql`; + + async function record( + target: Tracker, + configName: string, + rows: Array<{ filepath: string; checksum: string }>, + status: 'success' | 'failed', + ): Promise { + + const opId = await target.createOperation({ + name: `build:${dialect}:${run}`, + changeType: 'build', + configName, + executedBy: 'test@example.com', + }); + + await target.createFileRecords(opId, rows.map((row) => ({ ...row, fileType: 'sql' }))); + + for (const row of rows) { + + await target.updateFileExecution(opId, row.filepath, status, 5); + + } + + } + + await record(tracker, CONFIG_NAME, [ + { filepath: fileA, checksum: 'a-old' }, + { filepath: fileB, checksum: 'b-only' }, + ], 'success'); + await record(tracker, CONFIG_NAME, [{ filepath: fileA, checksum: 'a-new' }], 'failed'); + await record(otherTracker, OTHER_CONFIG_NAME, [{ filepath: fileA, checksum: 'a-other' }], 'success'); + + const [latest, err] = await tracker.latestExecutions(); + + expect(err).toBeNull(); + expect(latest!.get(fileA)).toMatchObject({ checksum: 'a-new', exec_status: 'failed' }); + expect(latest!.get(fileB)).toMatchObject({ checksum: 'b-only', exec_status: 'success' }); + + }); + }); } diff --git a/tests/sdk/run-build-filtering.test.ts b/tests/sdk/run-build-filtering.test.ts index 3afe02f3..7fca2b2d 100644 --- a/tests/sdk/run-build-filtering.test.ts +++ b/tests/sdk/run-build-filtering.test.ts @@ -86,8 +86,9 @@ function makeKysely(): Kysely { * Tracker.createOperation's `insert ... returning "id"` (verified via a * compiled-query sanity check — it's the only query shape containing both * substrings) gets an incrementing id, or the whole batch aborts before - * touching any file. Everything else — the needsRun lookup, file-record - * inserts/updates, and each fixture file's own SQL body — gets empty rows, + * touching any file. Everything else — the `latestExecutions` lookup, + * file-record inserts/updates, and each fixture file's own SQL body — + * gets empty rows, * which reads as "no prior execution" / "no-op success". This is enough to * prove which files build() attempted to run; simulating real execution * semantics is core/runner's own test surface, not this one's.