diff --git a/.changeset/migrate-occupancy-and-deferred-ddl.md b/.changeset/migrate-occupancy-and-deferred-ddl.md new file mode 100644 index 0000000000..16628352c1 --- /dev/null +++ b/.changeset/migrate-occupancy-and-deferred-ddl.md @@ -0,0 +1,37 @@ +--- +'@objectstack/driver-sql': minor +'@objectstack/runtime': minor +'@objectstack/cli': minor +--- + +`os migrate` no longer touches the database before you confirm, and refuses a +SQLite database another process is using (#3917). + +**Nothing is written before the prompt.** `plan` called itself a dry run and +`apply` gated on `[y/N]`, but both booted the full plugin set first — and boot +schema-sync issued create-table/add-column DDL (plus the artifact's inline seed +wrote rows) against the target database before either promise was kept. +`SqlDriver` gains `setDeferredDdl` / `previewDeferredSchemaWork` / +`flushDeferredSchemaDdl`: while armed, `initObjects` still registers every +in-memory map drift detection depends on but records the physical work instead +of performing it. Both commands boot with it armed, render the held-back work +as a `New (additive)` section of the plan, and `apply` performs it only after +confirmation. `os meta resync` / `os migrate files-to-references` keep the old +behaviour — they need the tables to exist. + +**Occupancy check.** A live `os dev`/`os serve` holding the same SQLite file is +the usual way a migration goes wrong: the migration is transactional and swaps +tables inside the file, but the running server keeps prepared statements and a +schema cookie the migration invalidates. `os migrate` now probes the target +before booting — `PRAGMA locking_mode = EXCLUSIVE` + `BEGIN IMMEDIATE` under +`busy_timeout = 0`, which reports `SQLITE_BUSY` when another connection is +*attached*, not merely writing. (`wal_checkpoint(TRUNCATE)` only sees an active +writer, and `-wal`/`-shm` presence cannot tell a live server from a crashed one; +both are encoded as tests.) `apply` refuses with exit 1 — `error: database_busy` +under `--json` — unless the new `--force` flag is passed; `plan` warns and +continues, since it writes nothing either way. SQLite only: Postgres and MySQL +take their own server-side locks. + +`@objectstack/runtime` also exports `resolveStandaloneDatabase()`, so a caller +can resolve the database target with the same precedence the boot uses without +building the stack, and `createStandaloneStack` accepts `skipSeedData`. diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 2e862af038..4f0334cae0 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -494,9 +494,49 @@ os migrate plan # Preview drift (no changes) os migrate apply # Apply safe (loosening) changes, with a confirm prompt os migrate apply --yes # Skip the prompt (CI / scripts) os migrate apply --allow-destructive --yes # Also drop orphaned columns, tighten NOT NULL, narrow types +os migrate apply --force # Migrate even though another process is using the database os migrate plan --json # Machine-readable output ``` +#### Nothing is written before you confirm + +Both commands boot your app to read its metadata. That boot no longer touches +the target database: the additive schema sync (create missing tables, add +missing columns) and the artifact's inline seed data are **deferred**, not +performed. So `plan` really is a dry run, and everything `apply` is about to do +— additive work included — is on screen before the `[y/N]` prompt: + +```text + New (additive — created when you apply) + + crm_quote [create_table, 9 column(s)] + + crm_contact [add_columns: nickname, region] + + Safe (loosening — applied without --allow-destructive) + ✓ crm_contact.email [relax_not_null] +``` + +Answering `n` leaves the database exactly as it was. + +#### Occupancy check (SQLite) + +A running `os dev` / `os serve` holding the same SQLite file open is the usual +way a migration goes wrong: the migration itself is transactional and swaps +tables *inside* the file, but the live server keeps prepared statements and a +schema cookie that the migration invalidates, and its writes can collide as +`SQLITE_BUSY`. Before booting, `os migrate` asks the database whether anyone +else is attached (`PRAGMA locking_mode = EXCLUSIVE` under `busy_timeout = 0` — +non-destructive, and unlike a WAL checkpoint it also sees a connection that is +merely *open* rather than actively writing). + +| Command | If the database is in use | +|---------|---------------------------| +| `os migrate plan` | Warns and continues — a plan writes nothing either way | +| `os migrate apply` | **Refuses** (exit 1, `error: database_busy` under `--json`). Stop the other process, or pass `--force` | + +The check applies to SQLite only. Postgres and MySQL take their own server-side +locks, and a `-wal`/`-shm` left behind by a crashed process is deliberately not +treated as occupancy on its own. + | Category | Examples | Applied by | |----------|----------|------------| | `safe` | relax `NOT NULL` → nullable, widen a `varchar`, create a declared index, replace a legacy global unique with its tenant-scoped composite | `os migrate apply` (and dev auto-reconcile) | diff --git a/packages/cli/package.json b/packages/cli/package.json index a9e7b45eb9..2a08e3ca01 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -100,9 +100,13 @@ "yaml": "^2.9.0", "zod": "^4.4.3" }, + "optionalDependencies": { + "better-sqlite3": "^13.0.1" + }, "devDependencies": { "@oclif/plugin-help": "^6.2.55", "@oclif/plugin-plugins": "^5.4.86", + "@types/better-sqlite3": "^7.6.13", "@types/node": "^26.1.1", "tsup": "^8.5.1", "typescript": "^6.0.3", diff --git a/packages/cli/src/commands/migrate/apply.ts b/packages/cli/src/commands/migrate/apply.ts index f082e3010f..acb2ee41cd 100644 --- a/packages/cli/src/commands/migrate/apply.ts +++ b/packages/cli/src/commands/migrate/apply.ts @@ -16,9 +16,13 @@ import { import { bootSchemaStack, renderPlan, + renderPendingSchemaWork, summarize, + summarizePendingSchemaWork, groupByCategory, } from '../../utils/schema-migrate.js'; +import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; +import { describeOccupancy } from '../../utils/sqlite-occupancy.js'; async function confirm(question: string): Promise { if (!process.stdin.isTTY) return false; // non-interactive → require --yes @@ -36,6 +40,17 @@ async function confirm(question: string): Promise { * Applies safe (loosening) + needs-confirm changes by default; destructive * changes (drop column, tighten NOT NULL, narrow type) require * `--allow-destructive`. + * + * Two operational-safety guarantees, both added by #3917: + * + * 1. **Nothing is written before you say yes.** The stack boots with schema + * DDL deferred and the artifact seed suppressed, so the additive + * create-table / add-column work that used to run during boot is now part + * of the plan you confirm — not something that already happened by the time + * the prompt appeared. + * 2. **A database somebody else is using is not migrated by accident.** The + * SQLite target is probed for other attached connections before boot, and a + * busy database refuses without `--force`. */ export default class MigrateApply extends Command { static override description = @@ -45,6 +60,7 @@ export default class MigrateApply extends Command { '$ os migrate apply', '$ os migrate apply --yes', '$ os migrate apply --allow-destructive --yes', + '$ os migrate apply --force', '$ os migrate apply --json', ]; @@ -58,6 +74,10 @@ export default class MigrateApply extends Command { default: false, }), yes: Flags.boolean({ char: 'y', description: 'Skip the confirmation prompt', default: false }), + force: Flags.boolean({ + description: 'Migrate even when another process is using the database (SQLite occupancy check)', + default: false, + }), json: Flags.boolean({ description: 'Output as JSON (implies non-interactive; requires --yes to mutate)' }), }; @@ -68,12 +88,45 @@ export default class MigrateApply extends Command { if (!flags.json) { printHeader('Migrate · apply'); + printStep('Checking whether the database is in use…'); + } + + // Occupancy gate — BEFORE the stack boots, or our own pooled connections + // are what the probe finds (#3917). + const occupancy = await probeMigrationTarget(flags['database-url']); + if (occupancy.status === 'busy' && !flags.force) { + if (flags.json) { + await emitJson({ + error: 'database_busy', + database: occupancy.filename, + signal: occupancy.signal, + detail: occupancy.detail, + hint: OCCUPANCY_HINT, + }, 0, { compact: true }); + this.exit(1); + return; + } + printError(describeOccupancy(occupancy)); + printWarning(OCCUPANCY_HINT); + this.exit(1); + return; + } + if (occupancy.status === 'busy' && flags.force && !flags.json) { + printWarning(`--force: ${describeOccupancy(occupancy)} Migrating anyway — the live process may see stale schema or SQLITE_BUSY.`); + } + if (occupancy.status === 'unknown' && !flags.json) { + printWarning(`Could not check whether the database is in use — ${occupancy.detail}`); + } + + if (!flags.json) { printStep('Booting schema stack…'); } let stack; try { - stack = await bootSchemaStack({ databaseUrl: flags['database-url'] }); + // `deferSchemaDdl` is what makes the prompt below meaningful: without it + // the boot has already created tables and added columns by this point. + stack = await bootSchemaStack({ databaseUrl: flags['database-url'], deferSchemaDdl: true }); } catch (error: any) { if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); @@ -90,9 +143,13 @@ export default class MigrateApply extends Command { const drift = await stack.driver.detectManagedDrift(); const grouped = groupByCategory(drift); + // Additive work the boot sync was held back from doing. Not drift — it + // is what `initObjects` does on its own — but it IS a change to the + // target database, so it belongs in the plan and behind the prompt. + const pending = stack.pendingSchemaWork; - if (drift.length === 0) { - if (flags.json) { await emitJson({ applied: [], skipped: [], message: 'in_sync' }, 0, { compact: true }); return; } + if (drift.length === 0 && pending.length === 0) { + if (flags.json) { await emitJson({ applied: [], skipped: [], created: [], message: 'in_sync' }, 0, { compact: true }); return; } printSuccess('Physical schema is already in sync with metadata — nothing to apply.'); return; } @@ -104,7 +161,9 @@ export default class MigrateApply extends Command { if (!flags.json) { printInfo(`Database: ${chalk.white(stack.dbLabel)}`); console.log(''); + renderPendingSchemaWork(pending); renderPlan(drift); + if (pending.length > 0) printInfo(summarizePendingSchemaWork(pending)); printInfo(summarize(drift)); if (deferred.length > 0) { printWarning(`${deferred.length} destructive change(s) will be SKIPPED (re-run with --allow-destructive to include them).`); @@ -114,28 +173,35 @@ export default class MigrateApply extends Command { } } - if (intended.length === 0) { - if (flags.json) { await emitJson({ applied: [], skipped: deferred, message: 'nothing_safe_to_apply' }, 0, { compact: true }); return; } + const totalIntended = intended.length + pending.length; + if (totalIntended === 0) { + if (flags.json) { await emitJson({ applied: [], skipped: deferred, created: [], message: 'nothing_safe_to_apply' }, 0, { compact: true }); return; } printWarning('No changes to apply without --allow-destructive.'); return; } - // Confirmation gate. + // Confirmation gate. Nothing above this line has touched the database. if (!flags.yes) { if (flags.json || !process.stdin.isTTY) { - if (flags.json) { await emitJson({ applied: [], skipped: drift, message: 'confirmation_required', hint: 'pass --yes' }, 0, { compact: true }); return; } + if (flags.json) { await emitJson({ applied: [], skipped: drift, pending, message: 'confirmation_required', hint: 'pass --yes' }, 0, { compact: true }); return; } printWarning('Confirmation required. Re-run with --yes to apply, or use "os migrate plan" to preview.'); return; } - const ok = await confirm(chalk.bold(`\nApply ${intended.length} change(s) to ${stack.dbLabel}? [y/N] `)); + const ok = await confirm(chalk.bold(`\nApply ${totalIntended} change(s) to ${stack.dbLabel}? [y/N] `)); if (!ok) { printInfo('Aborted — no changes made.'); return; } } + // Additive work first: a table has to exist before its columns can be + // reconciled. Drift was detected against the pre-flush database, and a + // just-created table matches metadata by construction, so the two sets + // never overlap. + const created = await stack.flushSchemaDdl(); const { applied, skipped } = await stack.driver.applyMigrationEntries(drift, { allowDestructive }); if (flags.json) { await emitJson({ database: stack.dbLabel, + created, applied, skipped, duration: timer.elapsed(), @@ -144,6 +210,9 @@ export default class MigrateApply extends Command { } console.log(''); + if (created.length > 0) { + printSuccess(`Created/extended ${created.length} table(s): ${summarizePendingSchemaWork(created)}.`); + } printSuccess(`Applied ${applied.length} change(s).`); if (skipped.length > 0) { printWarning(`Skipped ${skipped.length} change(s) (destructive without --allow-destructive, or unsupported on this dialect).`); diff --git a/packages/cli/src/commands/migrate/plan.ts b/packages/cli/src/commands/migrate/plan.ts index 1e8fa79c1f..5bfcfa1a74 100644 --- a/packages/cli/src/commands/migrate/plan.ts +++ b/packages/cli/src/commands/migrate/plan.ts @@ -12,12 +12,27 @@ import { createTimer, emitJson, } from '../../utils/format.js'; -import { bootSchemaStack, renderPlan, summarize } from '../../utils/schema-migrate.js'; +import { + bootSchemaStack, + renderPlan, + renderPendingSchemaWork, + summarize, + summarizePendingSchemaWork, +} from '../../utils/schema-migrate.js'; +import { probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; +import { describeOccupancy } from '../../utils/sqlite-occupancy.js'; /** * `os migrate plan` — dry-run diff of metadata vs the physical database, * categorised safe / needs-confirm / destructive (issue #2186). Never mutates * the schema. + * + * "Never mutates" is enforced rather than merely documented since #3917: the + * stack boots with schema DDL deferred and the artifact seed suppressed, so the + * boot-time create-table / add-column sync that used to run before this command + * printed a single line is now REPORTED as pending work instead of performed. + * A database another process is using is reported too — as a warning, not a + * refusal, since a plan writes nothing either way. */ export default class MigratePlan extends Command { static override description = @@ -46,9 +61,16 @@ export default class MigratePlan extends Command { printStep('Booting schema stack…'); } + // Probed before boot so the answer is about somebody else's connections, + // not our own pool. + const occupancy = await probeMigrationTarget(flags['database-url']); + if (occupancy.status === 'busy' && !flags.json) { + printWarning(`${describeOccupancy(occupancy)} The plan below is still accurate — nothing is written — but "os migrate apply" will refuse until it is free (or you pass --force).`); + } + let stack; try { - stack = await bootSchemaStack({ databaseUrl: flags['database-url'] }); + stack = await bootSchemaStack({ databaseUrl: flags['database-url'], deferSchemaDdl: true }); } catch (error: any) { if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); @@ -64,6 +86,7 @@ export default class MigratePlan extends Command { } const drift = await stack.driver.detectManagedDrift(); + const pending = stack.pendingSchemaWork; if (flags.json) { await emitJson({ @@ -71,6 +94,10 @@ export default class MigratePlan extends Command { managedTables: stack.managedTableCount, total: drift.length, changes: drift, + pending, + ...(occupancy.status === 'busy' + ? { occupancy: { status: 'busy', signal: occupancy.signal, detail: occupancy.detail } } + : {}), duration: timer.elapsed(), }); return; @@ -80,13 +107,15 @@ export default class MigratePlan extends Command { printInfo(`Examined ${chalk.white(String(stack.managedTableCount))} managed table(s).`); console.log(''); - if (drift.length === 0) { + if (drift.length === 0 && pending.length === 0) { printSuccess('Physical schema is in sync with metadata — nothing to migrate.'); console.log(''); return; } + renderPendingSchemaWork(pending); renderPlan(drift); + if (pending.length > 0) printInfo(summarizePendingSchemaWork(pending)); printInfo(summarize(drift)); console.log(chalk.dim(' Apply with: ') + chalk.white('os migrate apply') + chalk.dim(' (add --allow-destructive for drops / tightenings)')); diff --git a/packages/cli/src/utils/migrate-occupancy-gate.ts b/packages/cli/src/utils/migrate-occupancy-gate.ts new file mode 100644 index 0000000000..97d4db471f --- /dev/null +++ b/packages/cli/src/utils/migrate-occupancy-gate.ts @@ -0,0 +1,41 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The occupancy gate shared by `os migrate plan` and `os migrate apply` (#3917). + * + * Resolving the target and probing it happens BEFORE the migrate stack boots — + * once the stack is up, its own pooled connections are attached to the file and + * every probe answers "busy" about ourselves. + */ + +import type { SqliteOccupancy } from './sqlite-occupancy.js'; +import { probeSqliteOccupancy } from './sqlite-occupancy.js'; + +/** + * Probe whatever database this invocation would open. Resolution mirrors the + * boot exactly (same config → the same `resolveStandaloneDatabase` the stack + * uses), so the file probed is the file migrated. + * + * Non-SQLite targets return `not_applicable`: Postgres and MySQL take their own + * locks and report their own `SQLITE_BUSY` equivalents server-side, and this + * probe has nothing to say about them. + */ +export async function probeMigrationTarget(databaseUrl?: string): Promise { + try { + const { resolveStandaloneDatabase } = await import('@objectstack/runtime'); + const target = resolveStandaloneDatabase({ + projectRoot: process.cwd(), + ...(databaseUrl ? { databaseUrl } : {}), + }); + return await probeSqliteOccupancy(target.sqliteFile); + } catch { + // An unresolvable URL is the boot's problem to report, with its own much + // better message. Never let the probe be the thing that fails the command. + return { status: 'not_applicable' }; + } +} + +/** The operator-facing hint attached to every refusal and warning. */ +export const OCCUPANCY_HINT = + 'Stop the process using it (a running "os dev"/"os serve" is the usual one) and re-run, ' + + 'or pass --force to migrate anyway.'; diff --git a/packages/cli/src/utils/schema-migrate.deferred-ddl.integration.test.ts b/packages/cli/src/utils/schema-migrate.deferred-ddl.integration.test.ts new file mode 100644 index 0000000000..8ca50501a7 --- /dev/null +++ b/packages/cli/src/utils/schema-migrate.deferred-ddl.integration.test.ts @@ -0,0 +1,169 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * End-to-end acceptance for #3917: booting the migrate stack must not touch the + * target database. + * + * The report's second finding was that `runtime.start()` boots the full plugin + * set, `syncRegisteredSchemas` runs create-table/add-column DDL, and all of that + * happens BEFORE `os migrate plan` renders a line or `os migrate apply` shows + * its `[y/N]`. So the test that matters is not "does the driver have a flag" — + * it is "after a real `bootSchemaStack`, is the database byte-for-byte what it + * was?" That is what this asserts, through the same + * createStandaloneStack → AppPlugin → ObjectQL → driver path the commands use. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, writeFileSync, mkdirSync, rmSync, existsSync, statSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { bootSchemaStack } from './schema-migrate.js'; + +const ARTIFACT = { + id: 'defer_smoke', + name: 'Defer Smoke', + objects: [ + { + name: 'defer_widget', + fields: { + name: { type: 'text', required: true }, + // Absent from the pre-existing table below — the additive sync would + // have silently ALTERed it in at boot. + colour: { type: 'text' }, + }, + }, + // No physical counterpart at all — the additive sync would have CREATEd it. + { name: 'defer_gadget', fields: { label: { type: 'text' } } }, + ], + // A seed the boot would otherwise write into the operator's live database. + data: [{ object: 'defer_widget', records: [{ id: 'seed-1', name: 'Seeded' }] }], +}; + +describe('bootSchemaStack({ deferSchemaDdl }) — the boot writes nothing (#3917)', () => { + let dir: string; + let dbFile: string; + const savedEnv: Record = {}; + + beforeEach(async () => { + dir = mkdtempSync(join(tmpdir(), 'os-defer-')); + mkdirSync(join(dir, 'dist'), { recursive: true }); + mkdirSync(join(dir, 'data'), { recursive: true }); + dbFile = join(dir, 'data', 'app.db'); + writeFileSync(join(dir, 'dist', 'objectstack.json'), JSON.stringify(ARTIFACT)); + + // One pre-existing table, missing the `colour` column. + const seed = new SqlDriver({ client: 'better-sqlite3', connection: { filename: dbFile }, useNullAsDefault: true }); + const k = (seed as any).knex; + await k.schema.createTable('defer_widget', (t: any) => { + t.string('id').primary(); + t.timestamp('created_at'); + t.timestamp('updated_at'); + t.string('name'); + }); + await k('defer_widget').insert({ id: 'pre-1', name: 'Existing' }); + await k.destroy(); + + savedEnv.OS_ARTIFACT_PATH = process.env.OS_ARTIFACT_PATH; + savedEnv.NODE_ENV = process.env.NODE_ENV; + process.env.OS_ARTIFACT_PATH = join(dir, 'dist', 'objectstack.json'); + process.env.NODE_ENV = 'production'; // no dev auto-reconcile + }); + + afterEach(() => { + process.env.OS_ARTIFACT_PATH = savedEnv.OS_ARTIFACT_PATH; + process.env.NODE_ENV = savedEnv.NODE_ENV; + try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + /** Read the physical shape straight from the file, outside the booted stack. */ + async function inspect(): Promise<{ tables: string[]; widgetColumns: string[]; widgetRows: number }> { + const d = new SqlDriver({ client: 'better-sqlite3', connection: { filename: dbFile }, useNullAsDefault: true }); + const k = (d as any).knex; + try { + const rows = await k.raw("SELECT name FROM sqlite_master WHERE type = 'table'"); + const tables = (rows as Array<{ name: string }>).map((r) => r.name).filter((n) => !n.startsWith('sqlite_')); + const widgetColumns = tables.includes('defer_widget') + ? Object.keys(await k('defer_widget').columnInfo()).sort() + : []; + const widgetRows = tables.includes('defer_widget') + ? Number((await k('defer_widget').count({ n: '*' }))[0].n) + : 0; + return { tables: tables.sort(), widgetColumns, widgetRows }; + } finally { + await k.destroy(); + } + } + + it('leaves the schema and the rows exactly as they were, and reports what it held back', async () => { + const before = await inspect(); + expect(before.tables).toEqual(['defer_widget']); + expect(before.widgetColumns).toEqual(['created_at', 'id', 'name', 'updated_at']); + + const stack = await bootSchemaStack({ databaseUrl: `file:${dbFile}`, deferSchemaDdl: true }); + try { + const after = await inspect(); + // The core assertion: boot created no table, added no column, wrote no + // seed row. Before #3917 all three happened here. + expect(after.tables).toEqual(before.tables); + expect(after.widgetColumns).toEqual(before.widgetColumns); + expect(after.widgetRows).toBe(before.widgetRows); + + // …and the work it held back is reported, so the plan can show it. + const pending = stack.pendingSchemaWork; + expect(pending.find((p) => p.table === 'defer_gadget')).toMatchObject({ kind: 'create_table' }); + const widget = pending.find((p) => p.table === 'defer_widget'); + expect(widget?.kind).toBe('add_columns'); + // `colour` plus the platform-injected fields (organization_id, owner_id, + // …) the pre-existing table predates — every one of them an ALTER the + // boot used to run unannounced. + expect(widget?.columns).toContain('colour'); + + // Drift detection still works — deferring the DDL must not blind it. + expect(stack.managedTableCount).toBeGreaterThan(0); + await expect(stack.driver!.detectManagedDrift()).resolves.toBeInstanceOf(Array); + } finally { + await stack.shutdown(); + } + }, 60_000); + + it('flushSchemaDdl performs exactly the work that was reported', async () => { + const stack = await bootSchemaStack({ databaseUrl: `file:${dbFile}`, deferSchemaDdl: true }); + try { + const pending = stack.pendingSchemaWork; + const performed = await stack.flushSchemaDdl(); + expect(performed).toEqual(pending); + + const after = await inspect(); + expect(after.tables).toContain('defer_gadget'); + expect(after.widgetColumns).toContain('colour'); + // Still no seed row — suppressing the seed is not undone by the flush. + expect(after.widgetRows).toBe(1); + + // A second boot finds nothing left to do. + await stack.shutdown(); + const again = await bootSchemaStack({ databaseUrl: `file:${dbFile}`, deferSchemaDdl: true }); + try { + expect(again.pendingSchemaWork).toEqual([]); + } finally { + await again.shutdown(); + } + } catch (e) { + await stack.shutdown().catch(() => { /* already down */ }); + throw e; + } + }, 60_000); + + it('without the flag, the boot syncs as it always did', async () => { + const stack = await bootSchemaStack({ databaseUrl: `file:${dbFile}` }); + try { + expect(stack.pendingSchemaWork).toEqual([]); + const after = await inspect(); + expect(after.tables).toContain('defer_gadget'); + expect(after.widgetColumns).toContain('colour'); + expect(existsSync(dbFile) && statSync(dbFile).size).toBeGreaterThan(0); + } finally { + await stack.shutdown(); + } + }, 60_000); +}); diff --git a/packages/cli/src/utils/schema-migrate.ts b/packages/cli/src/utils/schema-migrate.ts index 850697e6b6..4f058e87cf 100644 --- a/packages/cli/src/utils/schema-migrate.ts +++ b/packages/cli/src/utils/schema-migrate.ts @@ -13,15 +13,21 @@ * not in the loaded metadata are never examined or altered. */ import chalk from 'chalk'; -import type { ManagedDriftEntry, DriftCategory } from '@objectstack/driver-sql'; +import type { ManagedDriftEntry, DriftCategory, PendingSchemaWork } from '@objectstack/driver-sql'; import { describeDriverConnection } from './connection-display.js'; +export type { PendingSchemaWork }; + export interface SqlDriverLike { detectManagedDrift(): Promise; applyMigrationEntries( entries: ManagedDriftEntry[], opts: { allowDestructive?: boolean }, ): Promise<{ applied: ManagedDriftEntry[]; skipped: ManagedDriftEntry[] }>; + /** Deferred-DDL surface (#3917) — optional, so a driver without it still boots. */ + setDeferredDdl?: (deferred: boolean) => void; + previewDeferredSchemaWork?: () => Promise; + flushDeferredSchemaDdl?: () => Promise; config?: any; disconnect?: () => Promise; } @@ -33,6 +39,16 @@ export interface SchemaStack { /** The booted kernel — `getService('objectql')` etc. for one-shot commands * beyond schema migration (e.g. `os meta resync`, #2705). */ kernel: any; + /** + * Create-table / add-column work the boot sync was held back from running + * (#3917). Always `[]` unless the stack was booted with `deferSchemaDdl`. + */ + pendingSchemaWork: PendingSchemaWork[]; + /** + * Perform the deferred sync — call only once the operator has confirmed the + * plan. Returns the work it actually ran (`[]` when nothing was deferred). + */ + flushSchemaDdl: () => Promise; shutdown: () => Promise; } @@ -42,10 +58,11 @@ const SQL_DRIVER_SERVICES = [ 'driver.sql', ]; -function findSqlDriver(kernel: any): SqlDriverLike | null { +/** Locate the SQL driver behind any `getService`-shaped lookup (kernel or plugin ctx). */ +function findSqlDriverVia(getService: (name: string) => any): SqlDriverLike | null { for (const name of SQL_DRIVER_SERVICES) { let d: any; - try { d = kernel?.getService?.(name); } catch { /* not registered */ } + try { d = getService(name); } catch { /* not registered */ } if (d && typeof d.detectManagedDrift === 'function' && typeof d.applyMigrationEntries === 'function') { return d as SqlDriverLike; } @@ -53,6 +70,49 @@ function findSqlDriver(kernel: any): SqlDriverLike | null { return null; } +function findSqlDriver(kernel: any): SqlDriverLike | null { + return findSqlDriverVia((name) => kernel?.getService?.(name)); +} + +/** + * Arms the SQL driver's deferred-DDL mode before boot schema-sync can run + * (#3917). + * + * Timing is the whole point, and it is why this is a plugin rather than a call + * in `bootSchemaStack`. The kernel runs **every** plugin's `init()` (Phase 1) + * before **any** `start()` (Phase 2). `DefaultDatasourcePlugin` connects the + * driver and registers it as `driver.*` in its `init()`; `ObjectQLPlugin` runs + * `syncRegisteredSchemas` — the create-table/add-column DDL this issue is about + * — in its `start()`. An `init()` that depends on the datasource plugin + * therefore lands in the one window where the driver exists and no DDL has run. + */ +class DeferSchemaDdlPlugin { + name = 'com.objectstack.cli.defer-schema-ddl'; + version = '1.0.0'; + /** Ordering, not optionality: our init must follow the one that registers `driver.*`. */ + dependencies = ['com.objectstack.runtime.default-datasource']; + + driver: SqlDriverLike | null = null; + + init = async (ctx: any) => { + this.driver = findSqlDriverVia((name) => ctx.getService(name)); + if (!this.driver) { + // No SQL driver (memory/mongo) — nothing issues DDL, nothing to defer. + ctx.logger?.debug?.('[defer-schema-ddl] no SQL driver — deferral not armed'); + return; + } + if (typeof this.driver.setDeferredDdl !== 'function') { + // Fail loudly rather than silently boot-syncing: the caller asked for a + // dry run and this driver cannot give one. + throw new Error( + 'The active SQL driver does not support deferred schema DDL, so this command cannot ' + + 'guarantee a dry run. Upgrade @objectstack/driver-sql.', + ); + } + this.driver.setDeferredDdl(true); + }; +} + /** * Name the database the migrate/resync commands are about to write to. * @@ -79,13 +139,32 @@ export async function bootSchemaStack( * adapter are present. Plain schema commands pass nothing. */ extraPlugins?: unknown[]; + /** + * Boot WITHOUT touching the target database (#3917). + * + * Boot schema-sync issues create-table / add-column DDL, and the artifact's + * inline seed writes rows — both used to happen before `os migrate plan` + * rendered its "dry run" and before `os migrate apply` asked `[y/N]`. With + * this set, the driver registers metadata but records the physical work + * instead of performing it ({@link SchemaStack.pendingSchemaWork}), and the + * seed is suppressed, so the boot is read-only and the plan describes the + * database as it actually is. Call {@link SchemaStack.flushSchemaDdl} after + * confirmation to perform the work. + * + * Commands that boot in order to READ AND WRITE DATA (`os meta resync`, + * `os migrate files-to-references`) must leave this off — they need the + * tables to exist. + */ + deferSchemaDdl?: boolean; } = {}, ): Promise { const { createStandaloneStack, Runtime } = await import('@objectstack/runtime'); + const defer = opts.deferSchemaDdl === true; const stack = await createStandaloneStack({ projectRoot: process.cwd(), ...(opts.databaseUrl ? { databaseUrl: opts.databaseUrl } : {}), + ...(defer ? { skipSeedData: true } : {}), }); // No HTTP, no cluster — this is a one-shot schema operation. @@ -94,6 +173,9 @@ export async function bootSchemaStack( for (const plugin of stack.plugins) { await kernel.use(plugin); } + if (defer) { + await kernel.use(new DeferSchemaDdlPlugin() as any); + } for (const plugin of opts.extraPlugins ?? []) { await kernel.use(plugin as any); } @@ -101,12 +183,19 @@ export async function bootSchemaStack( const driver = findSqlDriver(kernel); const managedTableCount = driver ? (driver as any).managedObjectFields?.size ?? 0 : 0; + const pendingSchemaWork = defer && driver?.previewDeferredSchemaWork + ? await driver.previewDeferredSchemaWork() + : []; return { driver, dbLabel: describeDb(driver), managedTableCount, kernel, + pendingSchemaWork, + flushSchemaDdl: async () => (defer && driver?.flushDeferredSchemaDdl + ? await driver.flushDeferredSchemaDdl() + : []), shutdown: async () => { try { await (runtime as any).stop?.(); } catch { /* ignore */ } try { await driver?.disconnect?.(); } catch { /* ignore */ } @@ -160,3 +249,31 @@ export function summarize(drift: ManagedDriftEntry[]): string { const g = groupByCategory(drift); return `${drift.length} change(s): ${g.safe.length} safe, ${g.needs_confirm.length} needs-confirm, ${g.destructive.length} destructive`; } + +/** + * Render the additive work the boot sync was held back from doing (#3917). + * + * Deliberately its own section rather than a `DriftCategory`: this is not + * divergence between metadata and an existing column — it is the create/add + * that used to happen silently at boot, now shown before it runs. Purely + * additive and never data-losing, so it carries no `--allow-destructive` gate. + */ +export function renderPendingSchemaWork(pending: PendingSchemaWork[]): void { + if (pending.length === 0) return; + console.log(` ${chalk.bold('New (additive — created when you apply)')}`); + for (const p of pending) { + const detail = p.kind === 'create_table' + ? `[create_table, ${p.columns.length} column(s)]` + : `[add_columns: ${p.columns.join(', ')}]`; + console.log(` ${chalk.cyan('+')} ${chalk.cyan(p.table)} ${chalk.dim(detail)}`); + } + console.log(''); +} + +export function summarizePendingSchemaWork(pending: PendingSchemaWork[]): string { + const creates = pending.filter((p) => p.kind === 'create_table').length; + const columns = pending + .filter((p) => p.kind === 'add_columns') + .reduce((n, p) => n + p.columns.length, 0); + return `${creates} table(s) to create, ${columns} column(s) to add`; +} diff --git a/packages/cli/src/utils/sqlite-occupancy.test.ts b/packages/cli/src/utils/sqlite-occupancy.test.ts new file mode 100644 index 0000000000..1a992e679c --- /dev/null +++ b/packages/cli/src/utils/sqlite-occupancy.test.ts @@ -0,0 +1,194 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * SQLite occupancy detection (#3917). + * + * The scenario these encode is the one that produced the report: a dev server + * holding `.objectstack/data/standalone.db` open while `os migrate apply` runs + * in another terminal. The probe has to tell that apart from a database nobody + * is attached to — including a database that merely has `-wal`/`-shm` left over + * from a crash, which sidecar presence alone cannot do. + */ + +import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest'; +import { mkdtempSync, rmSync, writeFileSync, existsSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import Database from 'better-sqlite3'; + +import { probeSqliteOccupancy, sqliteSidecars, describeOccupancy } from './sqlite-occupancy.js'; +import { probeMigrationTarget } from './migrate-occupancy-gate.js'; + +let dir: string; +const openHandles: any[] = []; + +function newDb(name: string, journalMode: 'wal' | 'delete'): string { + const file = join(dir, name); + const db = new Database(file); + db.pragma(`journal_mode = ${journalMode}`); + db.exec('CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)'); + db.exec("INSERT INTO t (v) VALUES ('a')"); + db.close(); + return file; +} + +/** Keep a connection attached for the duration of one test. */ +function attach(file: string): any { + const db = new Database(file); + openHandles.push(db); + return db; +} + +beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'os-occupancy-')); +}); + +afterEach(() => { + while (openHandles.length > 0) { + try { openHandles.pop()?.close(); } catch { /* already closed */ } + } +}); + +afterAll(() => { + try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } +}); + +describe('probeSqliteOccupancy', () => { + it('is not applicable to a non-file target', async () => { + expect(await probeSqliteOccupancy(null)).toEqual({ status: 'not_applicable' }); + expect(await probeSqliteOccupancy(undefined)).toEqual({ status: 'not_applicable' }); + expect(await probeSqliteOccupancy(':memory:')).toEqual({ status: 'not_applicable' }); + }); + + it('treats a database that does not exist yet as idle', async () => { + const res = await probeSqliteOccupancy(join(dir, 'never-created.db')); + expect(res.status).toBe('idle'); + }); + + it('reports idle for a WAL database nobody has open', async () => { + const file = newDb('idle-wal.db', 'wal'); + const res = await probeSqliteOccupancy(file); + expect(res.status).toBe('idle'); + }); + + it('reports busy for a WAL database an ATTACHED-BUT-IDLE connection holds open', async () => { + // The reported scenario: a dev server between requests. It is holding no + // lock and running no transaction — only `-shm` and the exclusive-locking + // probe can see it, which is the whole reason the checkpoint probe was + // rejected. + const file = newDb('busy-wal-idle.db', 'wal'); + const other = attach(file); + other.prepare('SELECT * FROM t').all(); + + const res = await probeSqliteOccupancy(file); + expect(res.status).toBe('busy'); + if (res.status === 'busy') { + expect(res.signal).toBe('wal_attached'); + expect(describeOccupancy(res)).toContain(file); + } + }); + + it('reports busy for a WAL database another connection is reading in a transaction', async () => { + const file = newDb('busy-wal.db', 'wal'); + const other = attach(file); + other.exec('BEGIN'); + other.prepare('SELECT * FROM t').all(); + + const res = await probeSqliteOccupancy(file); + expect(res.status).toBe('busy'); + if (res.status === 'busy') expect(res.signal).toBe('wal_attached'); + }); + + it('reports busy for a WAL database another connection is writing', async () => { + const file = newDb('busy-wal-write.db', 'wal'); + const other = attach(file); + other.exec('BEGIN IMMEDIATE'); + other.exec("INSERT INTO t (v) VALUES ('b')"); + + const res = await probeSqliteOccupancy(file); + expect(res.status).toBe('busy'); + }); + + it('reports busy for a rollback-journal database under a write lock', async () => { + const file = newDb('busy-journal.db', 'delete'); + const other = attach(file); + other.exec('BEGIN IMMEDIATE'); + other.exec("INSERT INTO t (v) VALUES ('b')"); + + const res = await probeSqliteOccupancy(file); + expect(res.status).toBe('busy'); + if (res.status === 'busy') expect(res.signal).toBe('write_lock'); + }); + + it('reports idle for a rollback-journal database nobody is writing', async () => { + const file = newDb('idle-journal.db', 'delete'); + expect((await probeSqliteOccupancy(file)).status).toBe('idle'); + }); + + it('does not mistake crash-left sidecars for a live connection', async () => { + // The exact false positive sidecar-presence-only detection would produce: + // files left behind by a process that died, with nobody attached now. + const file = newDb('stale-sidecars.db', 'wal'); + writeFileSync(`${file}-wal`, ''); + writeFileSync(`${file}-shm`, ''); + expect(sqliteSidecars(file)).toHaveLength(2); + + const res = await probeSqliteOccupancy(file); + expect(res.status).toBe('idle'); + }); + + it('leaves the data intact — the probe locks, it does not modify', async () => { + const file = newDb('intact.db', 'wal'); + const live = attach(file); + live.exec("INSERT INTO t (v) VALUES ('b')"); + live.close(); + openHandles.pop(); + + await probeSqliteOccupancy(file); + + const check = attach(file); + expect(check.prepare('SELECT COUNT(*) AS n FROM t').get().n).toBe(2); + expect(existsSync(file)).toBe(true); + }); + + it('describeOccupancy says nothing about a database that is not busy', async () => { + const file = newDb('quiet.db', 'wal'); + expect(describeOccupancy(await probeSqliteOccupancy(file))).toBe(''); + }); +}); + +/** + * The gate the migrate commands actually call. Its job is to resolve the target + * the same way the boot does — probing a different file than the one about to + * be migrated would be worse than not probing at all. + */ +describe('probeMigrationTarget', () => { + // The first call cold-loads @objectstack/runtime (the resolver lives there), + // which dwarfs the probe itself. Warm it once so the per-test budgets measure + // the gate rather than module resolution. + beforeAll(async () => { + await probeMigrationTarget('memory://warmup'); + }, 60_000); + + it('resolves --database-url to the file it names and probes THAT file', async () => { + const file = newDb('gate-target.db', 'wal'); + const other = attach(file); + other.prepare('SELECT * FROM t').all(); + + const res = await probeMigrationTarget(`file:${file}`); + expect(res.status).toBe('busy'); + if (res.status === 'busy') expect(res.filename).toBe(file); + }); + + it('has nothing to say about a non-SQLite target', async () => { + expect(await probeMigrationTarget('postgres://localhost:5432/app')) + .toEqual({ status: 'not_applicable' }); + expect(await probeMigrationTarget('memory://gate')) + .toEqual({ status: 'not_applicable' }); + }); + + it('never fails the command over an unusable URL', async () => { + expect(await probeMigrationTarget('nonsense://not-a-database')) + .toEqual({ status: 'not_applicable' }); + }); +}); diff --git a/packages/cli/src/utils/sqlite-occupancy.ts b/packages/cli/src/utils/sqlite-occupancy.ts new file mode 100644 index 0000000000..a53c76f84f --- /dev/null +++ b/packages/cli/src/utils/sqlite-occupancy.ts @@ -0,0 +1,183 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * "Is somebody else using this SQLite database right now?" (#3917) + * + * `os migrate apply` used to gate only on `--allow-destructive` and the `[y/N]` + * prompt. Neither says anything about *occupancy*: the overwhelmingly common + * shape of a dev machine is a `pnpm dev` server holding the same + * `.objectstack/data/standalone.db` open while the operator runs a migration in + * another terminal. What that costs is not a swapped-out file — the SQLite + * column-op rebuild swaps tables *inside* the file, in one transaction — it is + * `SQLITE_BUSY` mid-migration, stale prepared statements in the live server, + * and schema-cookie churn under its feet. + * + * ## The probe + * + * On a WAL database (the default for every ObjectStack sqlite deployment), + * `PRAGMA locking_mode = EXCLUSIVE` followed by `BEGIN IMMEDIATE` under + * `busy_timeout = 0`. Exclusive locking mode makes SQLite stop using the `-shm` + * shared-memory file, which it can only do by taking exclusive locks on it — + * so the first transaction after the pragma fails with `SQLITE_BUSY` when, and + * only when, another connection is attached. Attached is the right question, + * not writing: a dev server sitting idle between requests still holds prepared + * statements and a schema cookie that a migration invalidates. + * + * Two rejected alternatives, both measured rather than assumed: + * + * - **`-wal` / `-shm` presence alone.** Correct while a connection is open + * (SQLite creates them with the first attach and removes them with the last + * clean close) but it cannot tell a live server from a crashed one, and + * refusing to migrate because a dead process left files behind is its own + * operational bug. Sidecars are reported as supporting evidence, never as the + * verdict. + * - **`PRAGMA wal_checkpoint(TRUNCATE)`.** Reports `busy = 1` only when a + * *writer* is mid-transaction — an attached-but-idle connection checkpoints + * cleanly. It misses precisely the common case. + * + * A rollback-journal database has no persistent record of who is attached + * (locks exist only for the duration of a transaction), so there the probe + * degrades honestly to `BEGIN IMMEDIATE`: it detects an active writer and + * nothing more. + * + * The probe is non-destructive — an empty transaction, rolled back, with + * locking mode restored — and it reads and writes no row. + * + * ## What it deliberately does NOT do + * + * No conclusion is invented when the probe cannot run — a missing + * `better-sqlite3`, an unreadable file, a non-SQLite target. Those return + * `unknown` / `not_applicable`, and the caller proceeds (a warning at most). + * Refusing a migration because we failed to look would be worse than the bug + * this guards. + */ + +import { existsSync } from 'node:fs'; + +export type SqliteOccupancy = + /** Not a SQLite file target (postgres, mongo, `:memory:`) — nothing to probe. */ + | { status: 'not_applicable' } + /** Probed successfully; no other connection is attached. */ + | { status: 'idle'; filename: string; sidecars: string[] } + /** Another connection is using this database right now. */ + | { status: 'busy'; filename: string; signal: 'wal_attached' | 'write_lock'; detail: string; sidecars: string[] } + /** Could not tell — the probe itself failed. Never a reason to refuse. */ + | { status: 'unknown'; filename: string; detail: string; sidecars: string[] }; + +/** The `-wal` / `-shm` companions that exist next to `filename`, if any. */ +export function sqliteSidecars(filename: string): string[] { + return ['-wal', '-shm'].map((s) => `${filename}${s}`).filter((p) => existsSync(p)); +} + +/** + * Probe `filename` for other attached connections. Never throws: every failure + * mode collapses into `unknown`. + * + * MUST run before the caller's own stack connects — once our pool is attached, + * the probe is answering about us. + */ +export async function probeSqliteOccupancy(filename: string | null | undefined): Promise { + if (!filename || filename === ':memory:' || filename.startsWith(':')) { + return { status: 'not_applicable' }; + } + const sidecars = sqliteSidecars(filename); + + // A database that does not exist yet cannot be occupied. + if (!existsSync(filename)) return { status: 'idle', filename, sidecars }; + + let Database: any; + try { + Database = (await import('better-sqlite3')).default; + } catch (e: any) { + // The wasm step-down (#2229) and a never-built native addon both land here. + return { + status: 'unknown', + filename, + sidecars, + detail: `better-sqlite3 is not loadable, so occupancy could not be checked (${e?.message ?? e})`, + }; + } + + let db: any; + try { + db = new Database(filename, { fileMustExist: true }); + // Fail FAST rather than wait out a live writer — the question is "is it + // busy", not "wait until it isn't". + db.pragma('busy_timeout = 0'); + + const journalMode = String(db.pragma('journal_mode', { simple: true }) ?? '').toLowerCase(); + const wal = journalMode === 'wal'; + + // Exclusive locking mode is what makes an ATTACHED-but-idle connection + // visible on WAL; on a rollback journal it is equivalent to a plain + // `BEGIN IMMEDIATE` and detects only an active writer. + if (wal) db.pragma('locking_mode = EXCLUSIVE'); + + try { + db.exec('BEGIN IMMEDIATE'); + db.exec('ROLLBACK'); + } catch (e: any) { + if (!isBusyError(e)) throw e; + return wal + ? { + status: 'busy', + filename, + signal: 'wal_attached', + sidecars, + detail: 'another connection is attached to it (its WAL shared-memory is in use)', + } + : { + status: 'busy', + filename, + signal: 'write_lock', + sidecars, + detail: 'another connection holds a write lock on it', + }; + } + + return { status: 'idle', filename, sidecars }; + } catch (e: any) { + if (isBusyError(e)) { + return { + status: 'busy', + filename, + signal: 'write_lock', + sidecars, + detail: 'it reported SQLITE_BUSY while being probed', + }; + } + return { + status: 'unknown', + filename, + sidecars, + detail: `occupancy probe failed: ${e?.message ?? e}`, + }; + } finally { + // Hand the exclusive lock back before closing. Closing releases it anyway; + // doing it explicitly keeps the window shut even if `close()` is delayed. + try { db?.pragma('locking_mode = NORMAL'); } catch { /* never opened */ } + try { db?.close(); } catch { /* already closed */ } + } +} + +function isBusyError(e: unknown): boolean { + const code = (e as { code?: string } | null | undefined)?.code ?? ''; + const message = e instanceof Error ? e.message : String(e ?? ''); + return code === 'SQLITE_BUSY' + || code === 'SQLITE_BUSY_SNAPSHOT' + || /database is locked/i.test(message) + || /SQLITE_BUSY/i.test(message); +} + +/** + * One line an operator can act on. Names the file and the evidence — the + * sidecars are included because "which process?" is answered by looking for + * whoever holds them open. + */ +export function describeOccupancy(occupancy: SqliteOccupancy): string { + if (occupancy.status !== 'busy') return ''; + const sidecars = occupancy.sidecars.length > 0 + ? ` (${occupancy.sidecars.map((s) => s.slice(s.lastIndexOf('/') + 1)).join(', ')} present)` + : ''; + return `${occupancy.filename} is in use — ${occupancy.detail}${sidecars}.`; +} diff --git a/packages/plugins/driver-sql/src/index.ts b/packages/plugins/driver-sql/src/index.ts index 0abff59050..edb983fff9 100644 --- a/packages/plugins/driver-sql/src/index.ts +++ b/packages/plugins/driver-sql/src/index.ts @@ -37,6 +37,7 @@ export type { PhysicalIndex, ExpectedIndex, LegacyUniqueReplacement, + PendingSchemaWork, FieldDef as DriftFieldDef, } from './schema-drift.js'; diff --git a/packages/plugins/driver-sql/src/schema-drift.ts b/packages/plugins/driver-sql/src/schema-drift.ts index d44c6d8550..05cdca7399 100644 --- a/packages/plugins/driver-sql/src/schema-drift.ts +++ b/packages/plugins/driver-sql/src/schema-drift.ts @@ -90,6 +90,23 @@ export type DriftOp = unique: boolean; }; +/** + * Physical schema work the *additive* boot sync is holding back (#3917). + * + * Distinct from {@link DriftOp}: drift is divergence between metadata and an + * EXISTING column/index that only a deliberate reconcile may resolve, whereas + * this is the create-table / add-column work `initObjects` performs on its own + * — captured rather than executed while the driver runs with DDL deferred, so + * `os migrate plan` can show it and `os migrate apply` can gate it behind the + * confirmation prompt. + */ +export interface PendingSchemaWork { + table: string; + kind: 'create_table' | 'add_columns'; + /** Declared columns for a create; the missing ones for an add. */ + columns: string[]; +} + /** Ops that act on an index rather than a column — reconciled without a table rebuild. */ export const INDEX_DRIFT_OPS: ReadonlySet = new Set([ 'replace_unique_index', diff --git a/packages/plugins/driver-sql/src/sql-driver-deferred-ddl.test.ts b/packages/plugins/driver-sql/src/sql-driver-deferred-ddl.test.ts new file mode 100644 index 0000000000..dfc8141292 --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-deferred-ddl.test.ts @@ -0,0 +1,139 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Deferred schema DDL (#3917). + * + * `os migrate plan` promises a dry run and `os migrate apply` promises a + * confirmation prompt, but boot schema-sync used to run create-table / + * add-column DDL against the target database before either promise was kept. + * With the deferral armed, `initObjects` must register everything drift + * detection depends on and change nothing physical until it is flushed. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { SqlDriver } from './sql-driver.js'; + +function makeDriver() { + return new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); +} + +const WIDGET = { + name: 'widgets', + fields: { + sku: { type: 'text' }, + qty: { type: 'number' }, + }, +}; + +describe('SqlDriver deferred schema DDL (#3917)', () => { + let driver: SqlDriver; + + afterEach(async () => { + await driver.disconnect(); + }); + + it('creates nothing while deferred, and still registers drift metadata', async () => { + driver = makeDriver(); + driver.setDeferredDdl(true); + await driver.initObjects([WIDGET]); + + const k = (driver as any).knex; + expect(await k.schema.hasTable('widgets')).toBe(false); + // The authoritative field set drift detection diffs against is populated — + // deferring the DDL must not blind `detectManagedDrift`. + expect((driver as any).managedObjectFields.has('widgets')).toBe(true); + expect(driver.deferredSchemaObjectCount).toBe(1); + }); + + it('previews a missing table as create_table without creating it', async () => { + driver = makeDriver(); + driver.setDeferredDdl(true); + await driver.initObjects([WIDGET]); + + const pending = await driver.previewDeferredSchemaWork(); + expect(pending).toEqual([ + { table: 'widgets', kind: 'create_table', columns: ['sku', 'qty'] }, + ]); + expect(await (driver as any).knex.schema.hasTable('widgets')).toBe(false); + }); + + it('previews only the MISSING columns of an existing table', async () => { + driver = makeDriver(); + // Table exists with `sku` only. + await driver.initObjects([{ name: 'widgets', fields: { sku: { type: 'text' } } }]); + + driver.setDeferredDdl(true); + await driver.initObjects([WIDGET]); + + expect(await driver.previewDeferredSchemaWork()).toEqual([ + { table: 'widgets', kind: 'add_columns', columns: ['qty'] }, + ]); + const info = await (driver as any).knex('widgets').columnInfo(); + expect(Object.keys(info)).not.toContain('qty'); + }); + + it('reports nothing pending when the database already matches metadata', async () => { + driver = makeDriver(); + await driver.initObjects([WIDGET]); + + driver.setDeferredDdl(true); + await driver.initObjects([WIDGET]); + + expect(await driver.previewDeferredSchemaWork()).toEqual([]); + }); + + it('flush performs the deferred work, reports it, and disarms the deferral', async () => { + driver = makeDriver(); + driver.setDeferredDdl(true); + await driver.initObjects([WIDGET]); + + const performed = await driver.flushDeferredSchemaDdl(); + expect(performed).toEqual([ + { table: 'widgets', kind: 'create_table', columns: ['sku', 'qty'] }, + ]); + + const k = (driver as any).knex; + expect(await k.schema.hasTable('widgets')).toBe(true); + expect(Object.keys(await k('widgets').columnInfo()).sort()) + .toEqual(['created_at', 'id', 'qty', 'sku', 'updated_at']); + expect(driver.deferredSchemaObjectCount).toBe(0); + + // Disarmed: a later sync goes straight through again. + await driver.initObjects([{ name: 'gadgets', fields: { label: { type: 'text' } } }]); + expect(await k.schema.hasTable('gadgets')).toBe(true); + }); + + it('flush is a no-op when nothing was deferred', async () => { + driver = makeDriver(); + driver.setDeferredDdl(true); + expect(await driver.flushDeferredSchemaDdl()).toEqual([]); + }); + + it('holds back the auto_number sequences table too', async () => { + driver = makeDriver(); + driver.setDeferredDdl(true); + await driver.initObjects([ + { name: 'invoices', fields: { no: { type: 'auto_number', format: '{0000}' } } }, + ]); + + const k = (driver as any).knex; + // `ensureSequencesTable` is DDL and rode outside the per-object loop — + // deferral has to cover it or "nothing is written" is a lie. + expect(await k.schema.hasTable('_objectstack_sequences')).toBe(false); + + await driver.flushDeferredSchemaDdl(); + expect(await k.schema.hasTable('_objectstack_sequences')).toBe(true); + }); + + it('leaves the default (undeferred) path exactly as before', async () => { + driver = makeDriver(); + await driver.initObjects([WIDGET]); + expect(await (driver as any).knex.schema.hasTable('widgets')).toBe(true); + expect(driver.deferredSchemaObjectCount).toBe(0); + expect(await driver.previewDeferredSchemaWork()).toEqual([]); + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index 8b7a311195..6e841af3f2 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -28,6 +28,7 @@ import { type PhysicalIndex, type SqlDialectName, type PhysicalColumn, + type PendingSchemaWork, } from './schema-drift.js'; import knex, { Knex } from 'knex'; import { nanoid } from 'nanoid'; @@ -580,6 +581,12 @@ export class SqlDriver implements IDataDriver { /** De-dup set for boot-time drift warnings (keyed by {@link driftKey}). */ protected driftWarned = new Set(); + /** Deferred-DDL mode (#3917) — see {@link setDeferredDdl}. */ + protected deferredDdl = false; + + /** Object defs `initObjects` registered but did not physically sync while {@link deferredDdl}. */ + protected deferredSchemaObjects = new Map }>(); + constructor(config: SqlDriverConfig) { // `schemaMode` / `autoMigrate` are ObjectStack concerns, not Knex options — // strip them before handing the config to Knex. @@ -2365,6 +2372,17 @@ export class SqlDriver implements IDataDriver { this.autoNumberFields[tableName] = autoNumberCols; this.tenantFieldByTable[tableName] = tenantField; + // Deferred-DDL mode (#3917): everything above is in-memory metadata + // registration — coercion maps, tenancy, and the `managedObjectFields` + // entry `detectManagedDrift()` diffs against. Everything below issues + // DDL. `os migrate plan` / `apply` boot with the deferral armed so the + // plan is computed against the database as it actually is, and nothing + // is created until the operator has seen (and confirmed) the plan. + if (this.deferredDdl) { + this.deferredSchemaObjects.set(tableName, { ...obj, name: tableName }); + continue; + } + // ADR-0057 P2: rotation-declared telemetry is physically time-sharded — // the Rotator owns its DDL (shard tables + a read view under the base // name); the plain create/alter path below would collide with the view. @@ -2471,11 +2489,84 @@ export class SqlDriver implements IDataDriver { const usesAutoNumber = Object.values(this.autoNumberFields).some( (cols) => Array.isArray(cols) && cols.length > 0, ); - if (usesAutoNumber) { + if (usesAutoNumber && !this.deferredDdl) { await this.ensureSequencesTable(); } } + // ── Deferred schema DDL (#3917) ──────────────────────────────────────────── + + /** + * Arm/disarm DDL deferral for {@link initObjects}. + * + * `os migrate plan` promises a dry run and `os migrate apply` promises a + * confirmation prompt, but both booted the full plugin set first — and boot + * schema-sync ran create-table / add-column DDL against the target database + * *before* either promise was kept (#3917). With the deferral armed, + * `initObjects` still registers all in-memory metadata (so drift detection + * sees the same authoritative field set) but records the physical work + * instead of performing it. {@link previewDeferredSchemaWork} renders it into + * the plan; {@link flushDeferredSchemaDdl} performs it once the operator has + * said yes. + * + * Off by default: every other boot (serve/dev/start) wants the additive sync + * to run exactly as before. + */ + setDeferredDdl(deferred: boolean): void { + this.deferredDdl = deferred; + } + + /** How many objects are waiting for {@link flushDeferredSchemaDdl}. */ + get deferredSchemaObjectCount(): number { + return this.deferredSchemaObjects.size; + } + + /** + * What the deferred sync *would* do, without doing it. + * + * Read-only: `hasTable` + `columnInfo`, the same two probes the additive sync + * uses to decide between create and alter. Tables and columns that already + * match metadata produce no entry, so an in-sync database returns `[]`. + */ + async previewDeferredSchemaWork(): Promise { + const out: PendingSchemaWork[] = []; + for (const [tableName, obj] of this.deferredSchemaObjects) { + const declared = Object.keys(obj.fields ?? {}); + if (!(await this.knex.schema.hasTable(tableName))) { + out.push({ table: tableName, kind: 'create_table', columns: declared }); + continue; + } + const existing = new Set(Object.keys(await this.knex(tableName).columnInfo())); + const missing = declared.filter((c) => !existing.has(c)); + if (missing.length > 0) { + out.push({ table: tableName, kind: 'add_columns', columns: missing }); + } + } + out.sort((a, b) => a.table.localeCompare(b.table)); + return out; + } + + /** + * Run the deferred sync and disarm the deferral. Returns the work that was + * outstanding (captured before the DDL ran, so the caller can report what it + * just did). A no-op when nothing was deferred. + */ + async flushDeferredSchemaDdl(): Promise { + const pending = [...this.deferredSchemaObjects.values()]; + if (pending.length === 0) { + this.deferredDdl = false; + return []; + } + const performed = await this.previewDeferredSchemaWork(); + this.deferredSchemaObjects.clear(); + this.deferredDdl = false; + // Re-entering initObjects re-registers the same metadata (idempotent) and + // this time takes the DDL path, so create/alter/index/rotation handling + // stays in exactly one place. + await this.initObjects(pending); + return performed; + } + // ── Managed-schema drift & reconcile (#2186) ─────────────────────────────── /** Canonical dialect name for the drift differ. */ diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index 773c33c466..c73ae4e3dc 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -59,10 +59,24 @@ export class AppPlugin implements Plugin { private projectContext?: AppPluginProjectContext; /** When true, init/start become no-ops — env has no app payload. */ private readonly empty: boolean = false; + /** + * Suppress the inline boot seed (#3917). One-shot schema commands + * (`os migrate plan` / `os migrate apply`) boot the full plugin set purely + * to read metadata, and a boot that writes demo rows into the operator's + * live database before they have confirmed anything is the same class of + * bug as boot-time DDL. The `seed-replayer` service is still registered — + * it only writes when something calls it. + */ + private readonly skipSeedData: boolean; - constructor(bundle: any, projectContext?: AppPluginProjectContext) { + constructor( + bundle: any, + projectContext?: AppPluginProjectContext, + opts: { skipSeedData?: boolean } = {}, + ) { this.bundle = bundle; this.projectContext = projectContext; + this.skipSeedData = opts.skipSeedData ?? false; // Support both direct manifest (legacy) and Stack Definition (nested manifest) const sys = bundle?.manifest || bundle; const appId = sys?.id || sys?.name; @@ -871,7 +885,11 @@ export class AppPlugin implements Plugin { // legacy behaviour: seed immediately at boot so there's // always demo data without needing an org insert. const multiTenant = resolveMultiOrgEnabled(); - if (multiTenant) { + if (this.skipSeedData) { + // #3917: this boot exists to READ metadata (os migrate + // plan/apply). It must not write to the target database. + ctx.logger.info('[Seeder] skipSeedData — inline seed suppressed; no rows written by this boot'); + } else if (multiTenant) { ctx.logger.info('[Seeder] multi-tenant mode — skipping inline seed; per-org replay will run on sys_organization insert'); } else { // Inline seed budget: large bundles (e.g. CRM Starter's 10 diff --git a/packages/runtime/src/index.ts b/packages/runtime/src/index.ts index 4893862cb0..278ca20e5d 100644 --- a/packages/runtime/src/index.ts +++ b/packages/runtime/src/index.ts @@ -8,8 +8,8 @@ export { Runtime } from './runtime.js'; export type { RuntimeConfig } from './runtime.js'; // Export Standalone Stack -export { createStandaloneStack, resolveObjectStackHome } from './standalone-stack.js'; -export type { StandaloneStackConfig, StandaloneStackResult } from './standalone-stack.js'; +export { createStandaloneStack, resolveObjectStackHome, resolveStandaloneDatabase } from './standalone-stack.js'; +export type { StandaloneStackConfig, StandaloneStackResult, ResolvedStandaloneDatabase } from './standalone-stack.js'; // Export Default Host (artifact-first, no objectstack.config.ts required) export { createDefaultHostConfig, resolveDefaultArtifactPath } from './default-host.js'; diff --git a/packages/runtime/src/standalone-stack.ts b/packages/runtime/src/standalone-stack.ts index 149bbba7b1..a4d4f2b3d6 100644 --- a/packages/runtime/src/standalone-stack.ts +++ b/packages/runtime/src/standalone-stack.ts @@ -83,6 +83,13 @@ export const StandaloneStackConfigSchema = z.object({ * failure is NOT silently swapped for wasm/mingo (fail-closed). */ dev: z.boolean().optional(), + /** + * Suppress the artifact's inline boot seed (#3917). Set by one-shot + * commands that boot the stack only to READ metadata — `os migrate plan` / + * `os migrate apply` — so the boot cannot write demo rows into the + * operator's live database before they have confirmed anything. + */ + skipSeedData: z.boolean().optional(), }); export type StandaloneStackConfig = z.input; @@ -136,6 +143,70 @@ function detectDriverFromUrl(dbUrl: string): ResolvedDriverKind { ); } +/** URL→filename for the two sqlite kinds. Throws on a URL that isn't a path. */ +function sqliteFilenameFromUrl(dbUrl: string, kind: 'sqlite' | 'sqlite-wasm'): string { + if (kind === 'sqlite-wasm') { + return dbUrl + .replace(/^wasm-sqlite:(\/\/)?/i, '') + .replace(/^file:(\/\/)?/i, '') || ':memory:'; + } + const filename = dbUrl.replace(/^file:(\/\/)?/, ''); + if (!filename || /^[a-z][a-z0-9+.-]*:\/\//i.test(filename)) { + throw new Error( + `[StandaloneStack] sqlite driver was selected but the URL does not look like a file path: "${dbUrl}". ` + + `Use file:/path/to/db.sqlite, or set OS_DATABASE_DRIVER explicitly.` + ); + } + return filename; +} + +/** Which database a standalone boot would talk to, and how. */ +export interface ResolvedStandaloneDatabase { + url: string; + driver: ResolvedDriverKind; + /** + * The sqlite file this boot would open, or `null` for every non-sqlite + * target and for `:memory:`. Callers that must inspect the file BEFORE a + * boot — `os migrate`'s occupancy probe (#3917) — need the path without + * the side effects of building the stack. + */ + sqliteFile: string | null; +} + +/** + * Resolve the database target WITHOUT building anything. + * + * Same precedence `createStandaloneStack` applies (explicit config → + * `OS_DATABASE_URL`/`DATABASE_URL` → `TURSO_DATABASE_URL` → `OS_HOME` → + * project root → user home), factored out so a caller can answer "which file + * am I about to open?" first. Pure: reads env, touches no filesystem. + */ +export function resolveStandaloneDatabase(config?: StandaloneStackConfig): ResolvedStandaloneDatabase { + const cfg = StandaloneStackConfigSchema.parse(config ?? {}); + const url = resolveDatabaseUrl(cfg); + const explicitDriver = cfg.databaseDriver + ?? (process.env.OS_DATABASE_DRIVER?.trim() as ResolvedDriverKind | undefined); + const driver: ResolvedDriverKind = explicitDriver || detectDriverFromUrl(url); + const isSqlite = driver === 'sqlite' || driver === 'sqlite-wasm'; + const filename = isSqlite ? sqliteFilenameFromUrl(url, driver) : null; + return { + url, + driver, + sqliteFile: filename && filename !== ':memory:' && !filename.startsWith(':') ? filename : null, + }; +} + +function resolveDatabaseUrl(cfg: z.output): string { + return cfg.databaseUrl + ?? readEnvWithDeprecation('OS_DATABASE_URL', 'DATABASE_URL', { silent: true })?.trim() + ?? process.env.TURSO_DATABASE_URL?.trim() + ?? (process.env.OS_HOME?.trim() + ? `file:${resolvePath(resolveObjectStackHome(), 'data/standalone.db')}` + : (cfg.projectRoot + ? `file:${resolvePath(cfg.projectRoot, '.objectstack/data/standalone.db')}` + : `file:${resolvePath(resolveObjectStackHome(), 'data/standalone.db')}`)); +} + export async function createStandaloneStack(config?: StandaloneStackConfig): Promise { const cfg = StandaloneStackConfigSchema.parse(config ?? {}); @@ -155,20 +226,10 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro ? artifactPathInput : resolvePath(cwd, artifactPathInput)); - const dbUrl = cfg.databaseUrl - ?? readEnvWithDeprecation('OS_DATABASE_URL', 'DATABASE_URL', { silent: true })?.trim() - ?? process.env.TURSO_DATABASE_URL?.trim() - ?? (process.env.OS_HOME?.trim() - ? `file:${resolvePath(resolveObjectStackHome(), 'data/standalone.db')}` - : (cfg.projectRoot - ? `file:${resolvePath(cfg.projectRoot, '.objectstack/data/standalone.db')}` - : `file:${resolvePath(resolveObjectStackHome(), 'data/standalone.db')}`)); // `databaseAuthToken` / `OS_DATABASE_AUTH_TOKEN` are preserved in the // config schema for cloud builds that compose their own turso driver; // the standalone (open-core) runtime no longer consumes them directly. - const explicitDriver = cfg.databaseDriver - ?? (process.env.OS_DATABASE_DRIVER?.trim() as ResolvedDriverKind | undefined); - const dbDriver: ResolvedDriverKind = explicitDriver ?? detectDriverFromUrl(dbUrl); + const { url: dbUrl, driver: dbDriver } = resolveStandaloneDatabase(cfg); // Translate the database URL into the `default` datasource DEFINITION // (ADR-0062 D1, #3826). The stack no longer builds a driver: the definition @@ -200,9 +261,7 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro driverConfig = { url: dbUrl }; } else if (dbDriver === 'sqlite-wasm') { driverId = 'sqlite-wasm'; - const filename = dbUrl - .replace(/^wasm-sqlite:(\/\/)?/i, '') - .replace(/^file:(\/\/)?/i, '') || ':memory:'; + const filename = sqliteFilenameFromUrl(dbUrl, 'sqlite-wasm'); if (filename !== ':memory:') { mkdirSync(resolvePath(filename, '..'), { recursive: true }); } @@ -210,13 +269,7 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro } else { // sqlite (better-sqlite3) driverId = 'sqlite'; - const filename = dbUrl.replace(/^file:(\/\/)?/, ''); - if (!filename || /^[a-z][a-z0-9+.-]*:\/\//i.test(filename)) { - throw new Error( - `[StandaloneStack] sqlite driver was selected but the URL does not look like a file path: "${dbUrl}". ` + - `Use file:/path/to/db.sqlite, or set OS_DATABASE_DRIVER explicitly.` - ); - } + const filename = sqliteFilenameFromUrl(dbUrl, 'sqlite'); mkdirSync(resolvePath(filename, '..'), { recursive: true }); driverConfig = { filename }; } @@ -254,7 +307,9 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro }), new ObjectQLPlugin({ environmentId }), ]; - if (artifactBundle) plugins.push(new AppPlugin(artifactBundle)); + if (artifactBundle) { + plugins.push(new AppPlugin(artifactBundle, undefined, { skipSeedData: cfg.skipSeedData ?? false })); + } // Surface artifact-declared metadata so a caller using this result // directly as a `defineStack()`-shaped config (no host diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b5d45aa711..bf49655cc9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -541,6 +541,9 @@ importers: '@oclif/plugin-plugins': specifier: ^5.4.86 version: 5.4.86 + '@types/better-sqlite3': + specifier: ^7.6.13 + version: 7.6.13 '@types/node': specifier: ^26.1.1 version: 26.1.1 @@ -553,6 +556,10 @@ importers: vitest: specifier: ^4.1.10 version: 4.1.10(@opentelemetry/api@1.9.1)(@types/node@26.1.1)(@vitest/coverage-v8@4.1.10)(happy-dom@20.10.2)(msw@2.14.6(@types/node@26.1.1)(typescript@6.0.3))(vite@8.0.16(@types/node@26.1.1)(esbuild@0.28.1)(jiti@2.7.0)(tsx@4.23.1)(yaml@2.9.0)) + optionalDependencies: + better-sqlite3: + specifier: ^13.0.1 + version: 13.0.1 packages/client: dependencies: @@ -4305,6 +4312,9 @@ packages: '@tybys/wasm-util@0.10.3': resolution: {integrity: sha512-F3fo1MYrRJYL3zER0OUOmkutjr1Vp23m7OsSgp7nq4SP6OqX6C/56XFIPAl5bt3zaBRjmW7SGz3u/6LwFpYcOg==} + '@types/better-sqlite3@7.6.13': + resolution: {integrity: sha512-NMv9ASNARoKksWtsq/SHakpYAYnhBrQgGD8zkLYk/jaK8jUGn08CfEdTRgYhMypUQAfzSP8W6gNLe0q19/t4VA==} + '@types/chai@5.2.3': resolution: {integrity: sha512-Mw558oeA9fFbv65/y4mHtXDs9bPnFMZAL/jxdPFUpOHHIXX91mcgEHbS5Lahr+pwZFR8A7GQleRWeI6cGFC2UA==} @@ -10947,6 +10957,10 @@ snapshots: tslib: 2.8.1 optional: true + '@types/better-sqlite3@7.6.13': + dependencies: + '@types/node': 26.1.1 + '@types/chai@5.2.3': dependencies: '@types/deep-eql': 4.0.2