diff --git a/.changeset/issue-2186-schema-migrate.md b/.changeset/issue-2186-schema-migrate.md new file mode 100644 index 0000000000..332e412a03 --- /dev/null +++ b/.changeset/issue-2186-schema-migrate.md @@ -0,0 +1,31 @@ +--- +"@objectstack/driver-sql": minor +"@objectstack/cli": minor +"@objectstack/rest": patch +--- + +Schema drift detection + `os migrate` for non-additive metadata changes (#2186). + +The metadata→DB schema sync was additive-only: it created tables and added +columns but never altered/dropped existing ones, so relaxing `required`, +changing a type/length, or dropping a field silently diverged from an existing +database. The physical column won at write time, surfacing a misleading +`organization_id is required` 400 even though `/meta` reported the field +optional. + +- **driver-sql** — the SQL driver now detects managed-schema drift (metadata is + the source of truth) and categorises each divergence `safe` / `needs_confirm` + / `destructive`. `initObjects` warns once per divergence with an actionable + hint. A new opt-in `SqlDriverConfig.autoMigrate: 'safe'` auto-applies the + *loosening* subset (relax `NOT NULL`, widen varchar) so an existing dev DB + self-heals on restart — never destructive, force-disabled under + `NODE_ENV=production`. New public methods `detectManagedDrift()` / + `applyMigrationEntries()`. SQLite reconciles via the official table-rebuild + (copy → swap), preserving data; Postgres/MySQL alter in place. +- **cli** — new `os migrate plan` (dry-run, categorised diff) and + `os migrate apply` (`--allow-destructive` for drops/tightenings, confirm gate, + `--json`). `os dev`/`serve` now pass `autoMigrate: 'safe'` in dev only. +- **rest** — a `NOT NULL` violation that reaches the driver (metadata validation + already passed) now carries a drift-aware `hint` pointing at `os migrate`, + instead of only the misleading "field is required" message. The + `VALIDATION_FAILED` / `fields` envelope is unchanged for back-compat. diff --git a/packages/cli/src/commands/migrate/apply.ts b/packages/cli/src/commands/migrate/apply.ts new file mode 100644 index 0000000000..2004ad4932 --- /dev/null +++ b/packages/cli/src/commands/migrate/apply.ts @@ -0,0 +1,160 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { Command, Flags } from '@oclif/core'; +import chalk from 'chalk'; +import { createInterface } from 'node:readline'; +import { + printHeader, + printSuccess, + printWarning, + printError, + printInfo, + printStep, + createTimer, +} from '../../utils/format.js'; +import { + bootSchemaStack, + renderPlan, + summarize, + groupByCategory, +} from '../../utils/schema-migrate.js'; + +async function confirm(question: string): Promise { + if (!process.stdin.isTTY) return false; // non-interactive → require --yes + const rl = createInterface({ input: process.stdin, output: process.stdout }); + try { + const answer: string = await new Promise((resolve) => rl.question(question, resolve)); + return /^y(es)?$/i.test(answer.trim()); + } finally { + rl.close(); + } +} + +/** + * `os migrate apply` — reconcile the physical database to metadata (#2186). + * Applies safe (loosening) + needs-confirm changes by default; destructive + * changes (drop column, tighten NOT NULL, narrow type) require + * `--allow-destructive`. + */ +export default class MigrateApply extends Command { + static override description = + 'Reconcile the physical database to metadata (safe by default; destructive changes need --allow-destructive)'; + + static override examples = [ + '$ os migrate apply', + '$ os migrate apply --yes', + '$ os migrate apply --allow-destructive --yes', + '$ os migrate apply --json', + ]; + + static override flags = { + 'database-url': Flags.string({ + description: 'Database URL to reconcile (defaults to $OS_DATABASE_URL / the project DB)', + env: 'OS_DATABASE_URL', + }), + 'allow-destructive': Flags.boolean({ + description: 'Also apply destructive changes (drop column, tighten NOT NULL, narrow type)', + default: false, + }), + yes: Flags.boolean({ char: 'y', description: 'Skip the confirmation prompt', default: false }), + json: Flags.boolean({ description: 'Output as JSON (implies non-interactive; requires --yes to mutate)' }), + }; + + async run(): Promise { + const { flags } = await this.parse(MigrateApply); + const timer = createTimer(); + const allowDestructive = flags['allow-destructive']; + + if (!flags.json) { + printHeader('Migrate · apply'); + printStep('Booting schema stack…'); + } + + let stack; + try { + stack = await bootSchemaStack({ databaseUrl: flags['database-url'] }); + } catch (error: any) { + if (flags.json) { console.log(JSON.stringify({ error: error.message })); this.exit(1); } + printError(error.message || String(error)); + this.exit(1); + return; + } + + try { + if (!stack.driver) { + if (flags.json) { console.log(JSON.stringify({ error: 'no_sql_driver' })); return; } + printWarning('Schema migration is only supported on SQL drivers (SQLite / Postgres). No SQL driver is active.'); + return; + } + + const drift = await stack.driver.detectManagedDrift(); + const grouped = groupByCategory(drift); + + if (drift.length === 0) { + if (flags.json) { console.log(JSON.stringify({ applied: [], skipped: [], message: 'in_sync' })); return; } + printSuccess('Physical schema is already in sync with metadata — nothing to apply.'); + return; + } + + // Entries we intend to apply this run. + const intended = drift.filter((d) => d.category !== 'destructive' || allowDestructive); + const deferred = drift.filter((d) => d.category === 'destructive' && !allowDestructive); + + if (!flags.json) { + printInfo(`Database: ${chalk.white(stack.dbLabel)}`); + console.log(''); + renderPlan(drift); + printInfo(summarize(drift)); + if (deferred.length > 0) { + printWarning(`${deferred.length} destructive change(s) will be SKIPPED (re-run with --allow-destructive to include them).`); + } + if (allowDestructive && grouped.destructive.length > 0) { + printWarning('Destructive changes assume your full app/plugin set is loaded. A column that looks "orphaned" here may belong to a plugin that is not part of this build.'); + } + } + + if (intended.length === 0) { + if (flags.json) { console.log(JSON.stringify({ applied: [], skipped: deferred, message: 'nothing_safe_to_apply' })); return; } + printWarning('No changes to apply without --allow-destructive.'); + return; + } + + // Confirmation gate. + if (!flags.yes) { + if (flags.json || !process.stdin.isTTY) { + if (flags.json) { console.log(JSON.stringify({ applied: [], skipped: drift, message: 'confirmation_required', hint: 'pass --yes' })); 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] `)); + if (!ok) { printInfo('Aborted — no changes made.'); return; } + } + + const { applied, skipped } = await stack.driver.applyMigrationEntries(drift, { allowDestructive }); + + if (flags.json) { + console.log(JSON.stringify({ + database: stack.dbLabel, + applied, + skipped, + duration: timer.elapsed(), + }, null, 2)); + return; + } + + console.log(''); + 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).`); + } + console.log(chalk.dim(` ${timer.display()}`)); + console.log(''); + } catch (error: any) { + if (flags.json) { console.log(JSON.stringify({ error: error.message })); this.exit(1); } + printError(error.message || String(error)); + this.exit(1); + } finally { + await stack.shutdown(); + } + } +} diff --git a/packages/cli/src/commands/migrate/index.ts b/packages/cli/src/commands/migrate/index.ts new file mode 100644 index 0000000000..6875e2747b --- /dev/null +++ b/packages/cli/src/commands/migrate/index.ts @@ -0,0 +1,12 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import MigratePlan from './plan.js'; + +/** + * `os migrate` with no subcommand defaults to the (read-only) plan, so the + * bare command can never mutate the schema by surprise (issue #2186). + */ +export default class Migrate extends MigratePlan { + static override description = + 'Inspect / reconcile physical-database drift from metadata. Defaults to a dry-run plan; use "os migrate apply" to reconcile.'; +} diff --git a/packages/cli/src/commands/migrate/plan.ts b/packages/cli/src/commands/migrate/plan.ts new file mode 100644 index 0000000000..911482b964 --- /dev/null +++ b/packages/cli/src/commands/migrate/plan.ts @@ -0,0 +1,102 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { Command, Flags } from '@oclif/core'; +import chalk from 'chalk'; +import { + printHeader, + printSuccess, + printWarning, + printError, + printInfo, + printStep, + createTimer, +} from '../../utils/format.js'; +import { bootSchemaStack, renderPlan, summarize } from '../../utils/schema-migrate.js'; + +/** + * `os migrate plan` — dry-run diff of metadata vs the physical database, + * categorised safe / needs-confirm / destructive (issue #2186). Never mutates + * the schema. + */ +export default class MigratePlan extends Command { + static override description = + 'Show how the physical database has drifted from metadata (dry run; no changes applied)'; + + static override examples = [ + '$ os migrate plan', + '$ os migrate plan --json', + '$ os migrate plan --database-url postgres://localhost/app', + ]; + + static override flags = { + 'database-url': Flags.string({ + description: 'Database URL to inspect (defaults to $OS_DATABASE_URL / the project DB)', + env: 'OS_DATABASE_URL', + }), + json: Flags.boolean({ description: 'Output as JSON' }), + }; + + async run(): Promise { + const { flags } = await this.parse(MigratePlan); + const timer = createTimer(); + + if (!flags.json) { + printHeader('Migrate · plan'); + printStep('Booting schema stack…'); + } + + let stack; + try { + stack = await bootSchemaStack({ databaseUrl: flags['database-url'] }); + } catch (error: any) { + if (flags.json) { console.log(JSON.stringify({ error: error.message })); this.exit(1); } + printError(error.message || String(error)); + this.exit(1); + return; + } + + try { + if (!stack.driver) { + if (flags.json) { console.log(JSON.stringify({ error: 'no_sql_driver', changes: [] })); return; } + printWarning('Schema migration is only supported on SQL drivers (SQLite / Postgres). No SQL driver is active.'); + return; + } + + const drift = await stack.driver.detectManagedDrift(); + + if (flags.json) { + console.log(JSON.stringify({ + database: stack.dbLabel, + managedTables: stack.managedTableCount, + total: drift.length, + changes: drift, + duration: timer.elapsed(), + }, null, 2)); + return; + } + + printInfo(`Database: ${chalk.white(stack.dbLabel)}`); + printInfo(`Examined ${chalk.white(String(stack.managedTableCount))} managed table(s).`); + console.log(''); + + if (drift.length === 0) { + printSuccess('Physical schema is in sync with metadata — nothing to migrate.'); + console.log(''); + return; + } + + renderPlan(drift); + printInfo(summarize(drift)); + console.log(chalk.dim(' Apply with: ') + chalk.white('os migrate apply') + + chalk.dim(' (add --allow-destructive for drops / tightenings)')); + console.log(chalk.dim(` ${timer.display()}`)); + console.log(''); + } catch (error: any) { + if (flags.json) { console.log(JSON.stringify({ error: error.message })); this.exit(1); } + printError(error.message || String(error)); + this.exit(1); + } finally { + await stack.shutdown(); + } + } +} diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index a4404ff760..c7a6d3a07f 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -606,6 +606,9 @@ export default class Serve extends Command { client: 'better-sqlite3', connection: { filename: filePath }, useNullAsDefault: true, + // #2186: in dev, self-heal a persisted DB when a metadata change + // relaxes a constraint (loosen-only; never destructive / never in prod). + autoMigrate: isDev ? 'safe' : undefined, }) as any)); trackPlugin('SqlDriver'); resolvedDriverLabel = 'SqlDriver(sqlite)'; @@ -626,6 +629,7 @@ export default class Serve extends Command { client: 'pg', connection: databaseUrl, pool: { min: 0, max: 5 }, + autoMigrate: isDev ? 'safe' : undefined, // #2186 dev loosen-only self-heal }) as any)); trackPlugin('PostgresDriver'); resolvedDriverLabel = 'SqlDriver(pg)'; @@ -636,6 +640,7 @@ export default class Serve extends Command { client: 'mysql2', connection: databaseUrl, pool: { min: 0, max: 5 }, + autoMigrate: isDev ? 'safe' : undefined, // #2186 dev loosen-only self-heal }) as any)); trackPlugin('MySQLDriver'); resolvedDriverLabel = 'SqlDriver(mysql2)'; @@ -666,6 +671,7 @@ export default class Serve extends Command { client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true, + autoMigrate: 'safe', // #2186 dev loosen-only self-heal }); await sqliteDriver.connect(); sqliteOk = true; diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 74bf4b1b57..c2916767bd 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -17,6 +17,11 @@ export { default as StartCommand } from './commands/start.js'; export { default as TestCommand } from './commands/test.js'; export { default as DoctorCommand } from './commands/doctor.js'; +// ─── Migrate topic subcommands (#2186) ────────────────────────────── +export { default as MigrateCommand } from './commands/migrate/index.js'; +export { default as MigratePlanCommand } from './commands/migrate/plan.js'; +export { default as MigrateApplyCommand } from './commands/migrate/apply.js'; + // ─── Environments topic subcommands ───────────────────────────────── export { default as EnvironmentsListCommand } from './commands/environments/list.js'; export { default as EnvironmentsShowCommand } from './commands/environments/show.js'; diff --git a/packages/cli/src/utils/schema-migrate.integration.test.ts b/packages/cli/src/utils/schema-migrate.integration.test.ts new file mode 100644 index 0000000000..9ef6ab0d77 --- /dev/null +++ b/packages/cli/src/utils/schema-migrate.integration.test.ts @@ -0,0 +1,92 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { mkdtempSync, writeFileSync, mkdirSync, rmSync } 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'; + +/** + * End-to-end (#2186): boot the real standalone stack via `bootSchemaStack` + * against a pre-seeded "legacy" SQLite DB (organization_id created NOT NULL), + * then verify `os migrate`'s engine detects the drift and reconciles it — + * exercising the full createStandaloneStack → AppPlugin → ObjectQL → driver path. + */ +describe('bootSchemaStack + migrate engine (integration)', () => { + let dir: string; + let dbFile: string; + const savedEnv: Record = {}; + + beforeAll(async () => { + dir = mkdtempSync(join(tmpdir(), 'os-mig-')); + mkdirSync(join(dir, 'dist'), { recursive: true }); + mkdirSync(join(dir, 'data'), { recursive: true }); + dbFile = join(dir, 'data', 'app.db'); + + // Compiled-artifact stand-in: id at top level so AppPlugin registers it. + writeFileSync( + join(dir, 'dist', 'objectstack.json'), + JSON.stringify({ + id: 'mig_smoke', + name: 'Migrate Smoke', + objects: [ + { + name: 'mig_biz_unit', + fields: { + name: { type: 'text', required: true }, + organization_id: { type: 'text', required: false }, // optional now + }, + }, + ], + }), + ); + + // Seed a "legacy" DB where organization_id is NOT NULL (the #2178 shape). + const seed = new SqlDriver({ client: 'better-sqlite3', connection: { filename: dbFile }, useNullAsDefault: true }); + const k = (seed as any).knex; + await k.schema.createTable('mig_biz_unit', (t: any) => { + t.string('id').primary(); + t.timestamp('created_at'); + t.timestamp('updated_at'); + t.string('name').notNullable(); + t.string('organization_id').notNullable(); + }); + await k('mig_biz_unit').insert({ id: '1', name: 'Acme', organization_id: 'org1' }); + 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'; // ensure no auto-reconcile masks the drift + }); + + afterAll(() => { + 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 */ } + }); + + it('detects the NOT NULL drift, applies it, and self-verifies in-sync', async () => { + const stack = await bootSchemaStack({ databaseUrl: `file:${dbFile}` }); + try { + expect(stack.driver).toBeTruthy(); + expect(stack.managedTableCount).toBeGreaterThan(0); + + const drift = await stack.driver!.detectManagedDrift(); + const org = drift.find((d) => d.table === 'mig_biz_unit' && d.column === 'organization_id'); + expect(org, 'expected drift on mig_biz_unit.organization_id').toBeDefined(); + expect(org!.category).toBe('safe'); + expect(org!.op.type).toBe('relax_not_null'); + + const { applied, skipped } = await stack.driver!.applyMigrationEntries(drift, { allowDestructive: false }); + expect(applied.some((d) => d.op.type === 'relax_not_null')).toBe(true); + expect(skipped).toHaveLength(0); + + const after = await stack.driver!.detectManagedDrift(); + expect(after.find((d) => d.table === 'mig_biz_unit' && d.column === 'organization_id')).toBeUndefined(); + } finally { + await stack.shutdown(); + } + }, 30_000); +}); diff --git a/packages/cli/src/utils/schema-migrate.ts b/packages/cli/src/utils/schema-migrate.ts new file mode 100644 index 0000000000..24787e5c09 --- /dev/null +++ b/packages/cli/src/utils/schema-migrate.ts @@ -0,0 +1,139 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Shared boot + rendering for `os migrate` (issue #2186). + * + * Boots the data stack (driver + ObjectQL + the compiled artifact's objects) + * via the supported `createStandaloneStack` programmatic entry, runs schema + * sync, and hands back the live SQL driver so the command can call + * `detectManagedDrift()` / `applyMigrationEntries()`. + * + * Migration only sees the objects present in the loaded metadata (compiled + * artifact). Run `os build` first so your objects are visible; tables/columns + * not in the loaded metadata are never examined or altered. + */ +import chalk from 'chalk'; +import type { ManagedDriftEntry, DriftCategory } from '@objectstack/driver-sql'; + +export interface SqlDriverLike { + detectManagedDrift(): Promise; + applyMigrationEntries( + entries: ManagedDriftEntry[], + opts: { allowDestructive?: boolean }, + ): Promise<{ applied: ManagedDriftEntry[]; skipped: ManagedDriftEntry[] }>; + config?: any; + disconnect?: () => Promise; +} + +export interface SchemaStack { + driver: SqlDriverLike | null; + dbLabel: string; + managedTableCount: number; + shutdown: () => Promise; +} + +const SQL_DRIVER_SERVICES = [ + 'driver.com.objectstack.driver.sql', + 'driver.com.objectstack.driver.turso', + 'driver.sql', +]; + +function findSqlDriver(kernel: any): SqlDriverLike | null { + for (const name of SQL_DRIVER_SERVICES) { + let d: any; + try { d = kernel?.getService?.(name); } catch { /* not registered */ } + if (d && typeof d.detectManagedDrift === 'function' && typeof d.applyMigrationEntries === 'function') { + return d as SqlDriverLike; + } + } + return null; +} + +function describeDb(driver: SqlDriverLike | null): string { + const cfg: any = driver?.config; + if (!cfg) return 'unknown'; + const conn = cfg.connection; + if (typeof conn === 'string') return redactUrl(conn); + if (conn && typeof conn === 'object') { + if (conn.filename) return `sqlite:${conn.filename}`; + if (conn.host) return `${cfg.client}://${conn.host}${conn.database ? '/' + conn.database : ''}`; + } + return String(cfg.client ?? 'unknown'); +} + +function redactUrl(url: string): string { + try { + const u = new URL(url); + if (u.password) u.password = '***'; + return u.toString(); + } catch { + return url.replace(/:\/\/[^@]*@/, '://***@'); + } +} + +/** Boot the schema stack. Caller MUST call `shutdown()` when done. */ +export async function bootSchemaStack(opts: { databaseUrl?: string } = {}): Promise { + const { createStandaloneStack, Runtime } = await import('@objectstack/runtime'); + + const stack = await createStandaloneStack({ + projectRoot: process.cwd(), + ...(opts.databaseUrl ? { databaseUrl: opts.databaseUrl } : {}), + }); + + // No HTTP, no cluster — this is a one-shot schema operation. + const runtime = new Runtime({ cluster: false }); + const kernel = runtime.getKernel(); + for (const plugin of stack.plugins) { + await kernel.use(plugin); + } + await runtime.start(); + + const driver = findSqlDriver(kernel); + const managedTableCount = driver ? (driver as any).managedObjectFields?.size ?? 0 : 0; + + return { + driver, + dbLabel: describeDb(driver), + managedTableCount, + shutdown: async () => { + try { await (runtime as any).stop?.(); } catch { /* ignore */ } + try { await driver?.disconnect?.(); } catch { /* ignore */ } + }, + }; +} + +// ── Rendering ─────────────────────────────────────────────────────── + +const CATEGORY_ORDER: DriftCategory[] = ['safe', 'needs_confirm', 'destructive']; + +const CATEGORY_META: Record string; icon: string }> = { + safe: { label: 'Safe (loosening — applied without --allow-destructive)', color: chalk.green, icon: '✓' }, + needs_confirm: { label: 'Needs confirmation', color: chalk.yellow, icon: '~' }, + destructive: { label: 'Destructive (requires --allow-destructive)', color: chalk.red, icon: '✗' }, +}; + +export function groupByCategory(drift: ManagedDriftEntry[]): Record { + const out: Record = { safe: [], needs_confirm: [], destructive: [] }; + for (const d of drift) out[d.category].push(d); + return out; +} + +export function renderPlan(drift: ManagedDriftEntry[]): void { + const grouped = groupByCategory(drift); + for (const cat of CATEGORY_ORDER) { + const items = grouped[cat]; + if (items.length === 0) continue; + const meta = CATEGORY_META[cat]; + console.log(` ${chalk.bold(meta.label)}`); + for (const d of items) { + console.log(` ${meta.color(meta.icon)} ${meta.color(`${d.table}.${d.column ?? ''}`)} ${chalk.dim(`[${d.op.type}]`)}`); + console.log(` ${chalk.dim(d.message)}`); + } + console.log(''); + } +} + +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`; +} diff --git a/packages/plugins/driver-sql/src/index.ts b/packages/plugins/driver-sql/src/index.ts index 5610028a9b..6c18793f53 100644 --- a/packages/plugins/driver-sql/src/index.ts +++ b/packages/plugins/driver-sql/src/index.ts @@ -11,6 +11,22 @@ export type { IntrospectedForeignKey, } from './sql-driver.js'; +// Managed-schema drift / reconcile (#2186) +export { + diffManagedTable, + driftKey, + fieldHasColumn, + BUILTIN_COLUMNS, +} from './schema-drift.js'; +export type { + ManagedDriftEntry, + DriftOp, + DriftCategory, + SqlDialectName, + PhysicalColumn, + FieldDef as DriftFieldDef, +} from './schema-drift.js'; + export default { id: 'com.objectstack.driver.sql', version: '1.0.0', diff --git a/packages/plugins/driver-sql/src/schema-drift.ts b/packages/plugins/driver-sql/src/schema-drift.ts new file mode 100644 index 0000000000..2a52ab4a25 --- /dev/null +++ b/packages/plugins/driver-sql/src/schema-drift.ts @@ -0,0 +1,220 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Managed-datasource schema drift (issue #2186). + * + * The driver's `initObjects` sync is *additive-only*: it creates missing + * tables and adds missing columns, but never alters or drops existing ones. + * So a non-additive metadata change (relax `required`, change a type/length, + * drop or rename a field) silently diverges from an existing database — the + * served metadata says one thing and the physical column enforces another. + * + * This module is the single source of truth for *detecting* that divergence + * (metadata is authoritative on a `managed` datasource) and for *categorising* + * each divergence by how dangerous it is to reconcile: + * + * - `safe` — loosening that cannot lose data and cannot fail: + * relax NOT NULL → NULL, widen a varchar. Applied + * automatically by dev auto-reconcile (P2). + * - `needs_confirm`— a change a human should eyeball but that does not + * destroy data (e.g. a non-narrowing type change). + * - `destructive` — drops or tightenings that can lose data or fail: + * drop an orphaned column, narrow a varchar, add a + * NOT NULL constraint over possibly-null data. Only + * applied by `os migrate apply --allow-destructive`. + * + * The detector reuses {@link SchemaDiffEntry} (the same shape the external / + * federated validator emits, ADR-0015 §5.2) so CLI / Studio / audit can render + * managed and external drift uniformly. + */ + +import type { SchemaDiffEntry } from '@objectstack/spec/shared'; + +export type SqlDialectName = 'sqlite' | 'postgres' | 'mysql' | 'unknown'; + +export type DriftCategory = 'safe' | 'needs_confirm' | 'destructive'; + +/** A reconcilable schema operation, machine-readable for the reconciler. */ +export type DriftOp = + | { type: 'relax_not_null'; table: string; column: string } + | { type: 'tighten_not_null'; table: string; column: string } + | { type: 'widen_varchar'; table: string; column: string; to: number; from?: number } + | { type: 'narrow_varchar'; table: string; column: string; to: number; from?: number } + | { type: 'drop_column'; table: string; column: string }; + +/** + * A managed-schema drift finding: a {@link SchemaDiffEntry} enriched with the + * owning table, a reconcile {@link DriftOp}, and a {@link DriftCategory}. + */ +export interface ManagedDriftEntry extends SchemaDiffEntry { + table: string; + category: DriftCategory; + op: DriftOp; + /** Human one-liner with an actionable hint. */ + message: string; +} + +/** Columns the driver creates unconditionally — never metadata fields. */ +export const BUILTIN_COLUMNS = new Set(['id', 'created_at', 'updated_at']); + +/** Minimal shape of an introspected physical column (see SqlDriver.introspectColumns). */ +export interface PhysicalColumn { + name: string; + type: string; + nullable: boolean; + maxLength?: number; +} + +/** Minimal shape of a metadata field definition. */ +export interface FieldDef { + type?: string; + required?: boolean; + multiple?: boolean; + maxLength?: number; +} + +/** + * Does this metadata field materialise a physical column? Mirrors + * `SqlDriver.createColumn` exactly: `formula` is virtual (computed, no column); + * everything else — including `multiple` (a JSON column) — gets one. + */ +export function fieldHasColumn(field: FieldDef): boolean { + if (field?.multiple) return true; + return (field?.type ?? 'string') !== 'formula'; +} + +/** Whether the dialect physically enforces varchar length (SQLite does not). */ +function enforcesVarcharLength(dialect: SqlDialectName): boolean { + return dialect === 'postgres' || dialect === 'mysql'; +} + +/** + * Diff one table's metadata fields against its physical columns and return the + * set of *drift* findings. Metadata is authoritative. + * + * Note: a metadata field with no physical column is NOT reported — the + * additive sync (`ALTER TABLE ADD COLUMN`) already covers added fields, so by + * the time this runs every expected column exists. We only surface the + * non-additive divergences the additive sync can never fix. + */ +export function diffManagedTable(args: { + table: string; + fields: Record; + columns: PhysicalColumn[]; + dialect: SqlDialectName; +}): ManagedDriftEntry[] { + const { table, fields, columns, dialect } = args; + const out: ManagedDriftEntry[] = []; + + const columnsByName = new Map(columns.map((c) => [c.name, c])); + // Field name → physical column it should produce. Built only for fields that + // materialise a column, so orphan detection below treats virtual fields as + // "no column expected". + const expectedColumns = new Set(); + + for (const [fieldName, field] of Object.entries(fields ?? {})) { + if (BUILTIN_COLUMNS.has(fieldName)) continue; + if (!fieldHasColumn(field)) continue; + expectedColumns.add(fieldName); + + const col = columnsByName.get(fieldName); + if (!col) continue; // additive sync adds it; not drift + + // ── nullability ────────────────────────────────────────────────── + const expectNullable = field.required !== true; + if (expectNullable && !col.nullable) { + out.push({ + kind: 'nullability_mismatch', + remoteName: table, + table, + column: fieldName, + expected: 'NULL', + actual: 'NOT NULL', + severity: 'warning', + category: 'safe', + op: { type: 'relax_not_null', table, column: fieldName }, + message: + `${table}.${fieldName}: metadata is optional but the column is NOT NULL ` + + `— writes that omit it fail. Run "os migrate" to relax it.`, + }); + } else if (!expectNullable && col.nullable) { + out.push({ + kind: 'nullability_mismatch', + remoteName: table, + table, + column: fieldName, + expected: 'NOT NULL', + actual: 'NULL', + severity: 'error', + category: 'destructive', + op: { type: 'tighten_not_null', table, column: fieldName }, + message: + `${table}.${fieldName}: metadata is required but the column is nullable ` + + `— existing nulls must be backfilled. Run "os migrate apply --allow-destructive".`, + }); + } + + // ── varchar length (only where the dialect enforces it) ────────── + if ( + enforcesVarcharLength(dialect) && + typeof field.maxLength === 'number' && + typeof col.maxLength === 'number' && + field.maxLength !== col.maxLength + ) { + if (field.maxLength > col.maxLength) { + out.push({ + kind: 'type_mismatch', + remoteName: table, + table, + column: fieldName, + expected: `varchar(${field.maxLength})`, + actual: `varchar(${col.maxLength})`, + severity: 'warning', + category: 'safe', + op: { type: 'widen_varchar', table, column: fieldName, to: field.maxLength, from: col.maxLength }, + message: `${table}.${fieldName}: metadata allows ${field.maxLength} chars but the column caps at ${col.maxLength} — widen via "os migrate".`, + }); + } else { + out.push({ + kind: 'type_mismatch', + remoteName: table, + table, + column: fieldName, + expected: `varchar(${field.maxLength})`, + actual: `varchar(${col.maxLength})`, + severity: 'error', + category: 'destructive', + op: { type: 'narrow_varchar', table, column: fieldName, to: field.maxLength, from: col.maxLength }, + message: `${table}.${fieldName}: metadata caps at ${field.maxLength} chars but the column allows ${col.maxLength} — narrowing may truncate. "os migrate apply --allow-destructive".`, + }); + } + } + } + + // ── orphaned columns (physical column, no metadata field) ────────── + for (const col of columns) { + if (BUILTIN_COLUMNS.has(col.name)) continue; + if (expectedColumns.has(col.name)) continue; + out.push({ + kind: 'unmapped_column', + remoteName: table, + table, + column: col.name, + expected: '(absent)', + actual: col.type, + severity: 'warning', + category: 'destructive', + op: { type: 'drop_column', table, column: col.name }, + message: + `${table}.${col.name}: column exists in the database but not in metadata (orphaned) ` + + `— "os migrate apply --allow-destructive" to drop it.`, + }); + } + + return out; +} + +/** Stable de-dup / sort key for a drift entry. */ +export function driftKey(d: ManagedDriftEntry): string { + return `${d.table}.${d.column ?? ''}:${d.kind}`; +} diff --git a/packages/plugins/driver-sql/src/sql-driver-schema-drift.test.ts b/packages/plugins/driver-sql/src/sql-driver-schema-drift.test.ts new file mode 100644 index 0000000000..22111532a7 --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-schema-drift.test.ts @@ -0,0 +1,256 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { SqlDriver } from '../src/index.js'; + +/** + * Managed-schema drift detection + reconcile (#2186). + * + * The driver's `initObjects` sync is additive-only. These tests exercise the + * non-additive paths it could never fix before: detecting divergence, the + * dev-only loosen auto-reconcile, and the destructive reconcile `os migrate` + * uses. + */ +describe('SqlDriver managed-schema drift (#2186)', () => { + let knexInstance: any; + + const makeDriver = (opts: any = {}) => { + const d = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + ...opts, + }); + knexInstance = (d as any).knex; + (d as any).logger = { warn: vi.fn(), info: vi.fn() }; + return d; + }; + + afterEach(async () => { + await knexInstance?.destroy(); + }); + + // Build an "existing DB" table where organization_id is NOT NULL (the shape + // a DB created while the field was `required: true` has — the #2178 repro). + const seedLegacyTable = async (driver: SqlDriver) => { + await knexInstance.schema.createTable('biz_unit', (t: any) => { + t.string('id').primary(); + t.timestamp('created_at'); + t.timestamp('updated_at'); + t.string('name').notNullable(); + t.string('organization_id').notNullable(); + }); + await knexInstance('biz_unit').insert({ id: '1', name: 'Acme', organization_id: 'org1' }); + }; + + // Metadata after #2178: organization_id is now optional. + const relaxedMeta = [ + { + name: 'biz_unit', + fields: { + name: { type: 'string', required: true }, + organization_id: { type: 'string', required: false }, + }, + }, + ]; + + describe('detectManagedDrift', () => { + it('flags a NOT NULL column that metadata says is optional (safe / relax_not_null)', async () => { + const driver = makeDriver(); + await seedLegacyTable(driver); + await driver.initObjects(relaxedMeta); + + const drift = await driver.detectManagedDrift(); + const orgDrift = drift.find((d) => d.column === 'organization_id'); + expect(orgDrift).toBeDefined(); + expect(orgDrift!.kind).toBe('nullability_mismatch'); + expect(orgDrift!.category).toBe('safe'); + expect(orgDrift!.op.type).toBe('relax_not_null'); + }); + + it('flags an orphaned physical column as destructive (unmapped_column)', async () => { + const driver = makeDriver(); + await knexInstance.schema.createTable('biz_unit', (t: any) => { + t.string('id').primary(); + t.string('name'); + t.string('legacy_code'); // not in metadata + }); + await driver.initObjects([{ name: 'biz_unit', fields: { name: { type: 'string' } } }]); + + const drift = await driver.detectManagedDrift(); + const orphan = drift.find((d) => d.column === 'legacy_code'); + expect(orphan).toBeDefined(); + expect(orphan!.kind).toBe('unmapped_column'); + expect(orphan!.category).toBe('destructive'); + expect(orphan!.op.type).toBe('drop_column'); + }); + + it('flags a required metadata field over a nullable column as destructive (tighten)', async () => { + const driver = makeDriver(); + await knexInstance.schema.createTable('biz_unit', (t: any) => { + t.string('id').primary(); + t.string('name'); // nullable + }); + await driver.initObjects([{ name: 'biz_unit', fields: { name: { type: 'string', required: true } } }]); + + const drift = await driver.detectManagedDrift(); + const d = drift.find((x) => x.column === 'name'); + expect(d?.category).toBe('destructive'); + expect(d?.op.type).toBe('tighten_not_null'); + }); + + it('does not flag varchar length on SQLite (no length enforcement)', async () => { + const driver = makeDriver(); + await knexInstance.schema.createTable('biz_unit', (t: any) => { + t.string('id').primary(); + t.string('name'); + }); + await driver.initObjects([{ name: 'biz_unit', fields: { name: { type: 'string', maxLength: 999 } } }]); + const drift = await driver.detectManagedDrift(); + expect(drift.filter((d) => d.kind === 'type_mismatch')).toHaveLength(0); + }); + + it('reports no drift when metadata and physical schema agree', async () => { + const driver = makeDriver(); + await driver.initObjects(relaxedMeta); // fresh table, built to spec + const drift = await driver.detectManagedDrift(); + expect(drift).toHaveLength(0); + }); + }); + + describe("dev auto-reconcile (autoMigrate: 'safe')", () => { + it('self-heals a NOT NULL→NULL relax on restart, preserving data, so an optional-field insert succeeds (#2178 repro)', async () => { + const driver = makeDriver({ autoMigrate: 'safe' }); + await seedLegacyTable(driver); + + await driver.initObjects(relaxedMeta); // simulates restart after pull+rebuild + + // Column is now nullable... + const info = await knexInstance('biz_unit').columnInfo(); + expect(info.organization_id.nullable).toBe(true); + + // ...the pre-existing row survived the rebuild... + const rows = await knexInstance('biz_unit').select('*'); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ id: '1', name: 'Acme', organization_id: 'org1' }); + + // ...and a write that omits the now-optional field succeeds. + await expect( + knexInstance('biz_unit').insert({ id: '2', name: 'Beta' }), + ).resolves.toBeDefined(); + + // No residual drift. + expect(await driver.detectManagedDrift()).toHaveLength(0); + }); + + it('does NOT auto-apply destructive drift (orphan column kept; warned instead)', async () => { + const driver = makeDriver({ autoMigrate: 'safe' }); + await knexInstance.schema.createTable('biz_unit', (t: any) => { + t.string('id').primary(); + t.string('name'); + t.string('legacy_code'); + }); + await driver.initObjects([{ name: 'biz_unit', fields: { name: { type: 'string' } } }]); + + const info = await knexInstance('biz_unit').columnInfo(); + expect(info).toHaveProperty('legacy_code'); // not dropped + // P1 acceptance: boot logs a clear, actionable warning per divergence. + const warn = (driver as any).logger.warn as ReturnType; + expect(warn).toHaveBeenCalled(); + const warnedDrift = warn.mock.calls.map((c: any[]) => String(c[0])).join('\n'); + expect(warnedDrift).toMatch(/os migrate/); + }); + + it('is force-disabled under NODE_ENV=production (warns, does not alter)', async () => { + const prev = process.env.NODE_ENV; + process.env.NODE_ENV = 'production'; + try { + const driver = makeDriver({ autoMigrate: 'safe' }); + await seedLegacyTable(driver); + await driver.initObjects(relaxedMeta); + const info = await knexInstance('biz_unit').columnInfo(); + expect(info.organization_id.nullable).toBe(false); // unchanged + } finally { + process.env.NODE_ENV = prev; + } + }); + }); + + describe('in-place ALTER SQL (Postgres / MySQL)', () => { + // No live PG/MySQL in CI — assert the generated DDL by spying on knex.raw. + const opFor = (type: string): any => ({ + kind: 'x', table: 'biz_unit', column: 'organization_id', severity: 'warning', + category: 'safe', message: 'm', op: { type, table: 'biz_unit', column: 'organization_id', to: 120 }, + }); + + const spyDriver = (client: string, columnInfo?: any) => { + const d = new SqlDriver({ client, connection: { host: 'x', database: 'y', user: 'u' } } as any); + knexInstance = (d as any).knex; + (d as any).logger = { warn: vi.fn(), info: vi.fn() }; + const calls: string[] = []; + (d as any).knex = Object.assign( + (_t: string) => ({ columnInfo: async () => columnInfo ?? {} }), + { raw: vi.fn(async (sql: string) => { calls.push(sql); return {}; }) }, + ); + return { d, calls }; + }; + + it('Postgres emits standard ALTER COLUMN statements', async () => { + const { d, calls } = spyDriver('pg'); + expect(await (d as any).applyDriftOpInPlace(opFor('relax_not_null').op)).toBe(true); + expect(await (d as any).applyDriftOpInPlace(opFor('tighten_not_null').op)).toBe(true); + expect(await (d as any).applyDriftOpInPlace(opFor('widen_varchar').op)).toBe(true); + expect(await (d as any).applyDriftOpInPlace(opFor('drop_column').op)).toBe(true); + expect(calls[0]).toMatch(/ALTER COLUMN \?\? DROP NOT NULL/); + expect(calls[1]).toMatch(/ALTER COLUMN \?\? SET NOT NULL/); + expect(calls[2]).toMatch(/TYPE varchar\(120\)/); + expect(calls[3]).toMatch(/DROP COLUMN/); + }); + + it('MySQL MODIFY reconstructs char length so a nullability change keeps it', async () => { + const { d, calls } = spyDriver('mysql2', { organization_id: { type: 'varchar', maxLength: 255 } }); + expect(await (d as any).applyDriftOpInPlace(opFor('relax_not_null').op)).toBe(true); + expect(calls[0]).toMatch(/MODIFY \?\? varchar\(255\) NULL/); + }); + }); + + describe('applyMigrationEntries (os migrate apply core)', () => { + it('relaxes NOT NULL without allowDestructive', async () => { + const driver = makeDriver(); + await seedLegacyTable(driver); + await driver.initObjects(relaxedMeta); + + const drift = await driver.detectManagedDrift(); + const { applied, skipped } = await driver.applyMigrationEntries(drift, { allowDestructive: false }); + expect(applied.some((d) => d.op.type === 'relax_not_null')).toBe(true); + expect(skipped).toHaveLength(0); + expect((await knexInstance('biz_unit').columnInfo()).organization_id.nullable).toBe(true); + }); + + it('skips destructive drops unless allowDestructive, then drops with it', async () => { + const driver = makeDriver(); + await knexInstance.schema.createTable('biz_unit', (t: any) => { + t.string('id').primary(); + t.string('name'); + t.string('legacy_code'); + }); + await knexInstance('biz_unit').insert({ id: '1', name: 'Acme', legacy_code: 'x' }); + await driver.initObjects([{ name: 'biz_unit', fields: { name: { type: 'string' } } }]); + + const drift = await driver.detectManagedDrift(); + + // Without the flag: orphan is skipped, column kept. + const r1 = await driver.applyMigrationEntries(drift, { allowDestructive: false }); + expect(r1.skipped.some((d) => d.op.type === 'drop_column')).toBe(true); + expect(await knexInstance('biz_unit').columnInfo()).toHaveProperty('legacy_code'); + + // With the flag: orphan dropped, data preserved. + const r2 = await driver.applyMigrationEntries(drift, { allowDestructive: true }); + expect(r2.applied.some((d) => d.op.type === 'drop_column')).toBe(true); + const info = await knexInstance('biz_unit').columnInfo(); + expect(info).not.toHaveProperty('legacy_code'); + const rows = await knexInstance('biz_unit').select('*'); + expect(rows[0]).toMatchObject({ id: '1', name: 'Acme' }); + }); + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index 70d328f930..146e981133 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -12,6 +12,14 @@ import { parseAutonumberFormat, renderAutonumber, missingFieldValues, type Auton import type { IDataDriver } from '@objectstack/spec/contracts'; import { StorageNameMapping } from '@objectstack/spec/system'; import { ExternalSchemaModeViolationError } from '@objectstack/spec/shared'; +import { + diffManagedTable, + driftKey, + type ManagedDriftEntry, + type DriftOp, + type SqlDialectName, + type PhysicalColumn, +} from './schema-drift.js'; import knex, { Knex } from 'knex'; import { nanoid } from 'nanoid'; import { createHash } from 'node:crypto'; @@ -115,7 +123,17 @@ export interface IntrospectedSchema { * all schema-mutating DDL. Defaults to `'managed'` when omitted, preserving * legacy behaviour. */ -export type SqlDriverConfig = Knex.Config & { schemaMode?: SchemaMode }; +export type SqlDriverConfig = Knex.Config & { + schemaMode?: SchemaMode; + /** + * Dev-only schema auto-reconcile (issue #2186). When `'safe'`, `initObjects` + * automatically applies *non-destructive* alters (relax NOT NULL, widen + * varchar) so an existing database self-heals after a metadata change + * loosens a constraint. `'off'` (default) only warns. Never applies + * destructive DDL, and is force-disabled when `NODE_ENV==='production'`. + */ + autoMigrate?: 'off' | 'safe'; +}; // ── SQL Driver ─────────────────────────────────────────────────────────────── @@ -267,7 +285,10 @@ export class SqlDriver implements IDataDriver { * production callers wire in their preferred logger. Defaults to * `console.warn` so warnings surface even without setup. */ - protected logger: { warn: (msg: string, meta?: any) => void } = { + protected logger: { + warn: (msg: string, meta?: any) => void; + info?: (msg: string, meta?: any) => void; + } = { warn: (msg, meta) => console.warn(msg, meta ?? ''), }; @@ -373,11 +394,30 @@ export class SqlDriver implements IDataDriver { */ protected readonly schemaMode: SchemaMode; + /** + * Dev-only auto-reconcile policy (issue #2186). See {@link SqlDriverConfig.autoMigrate}. + */ + protected readonly autoMigrate: 'off' | 'safe'; + + /** + * Metadata field defs for every table this driver manages, captured during + * `initObjects` (tableName → fields). The source of truth that + * {@link detectManagedDrift} diffs the physical schema against. + */ + protected managedObjectFields = new Map>(); + + /** Declared indexes per managed table (tableName → indexes[]), captured in `initObjects`. Used to recreate indexes after a SQLite table rebuild. */ + protected managedObjectIndexes = new Map(); + + /** De-dup set for boot-time drift warnings (keyed by {@link driftKey}). */ + protected driftWarned = new Set(); + constructor(config: SqlDriverConfig) { - // `schemaMode` is an ObjectStack concern, not a Knex option — strip it - // before handing the config to Knex. - const { schemaMode, ...knexConfig } = config; + // `schemaMode` / `autoMigrate` are ObjectStack concerns, not Knex options — + // strip them before handing the config to Knex. + const { schemaMode, autoMigrate, ...knexConfig } = config; this.schemaMode = schemaMode ?? 'managed'; + this.autoMigrate = autoMigrate ?? 'off'; this.config = knexConfig; this.knex = knex(knexConfig); } @@ -1374,6 +1414,12 @@ export class SqlDriver implements IDataDriver { for (const obj of objects) { const tableName = StorageNameMapping.resolveTableName(obj); + // #2186: remember the authoritative metadata field set for this table so + // drift detection / `os migrate` can diff the physical schema against it. + this.managedObjectFields.set(tableName, obj.fields ?? {}); + if (Array.isArray((obj as any).indexes)) { + this.managedObjectIndexes.set(tableName, (obj as any).indexes); + } const jsonCols: string[] = []; const booleanCols: string[] = []; @@ -1503,9 +1549,313 @@ export class SqlDriver implements IDataDriver { const physicalColumns = new Set(Object.keys(colInfo)); await this.syncDeclaredIndexes(tableName, declaredIndexes, physicalColumns); } + + // #2186: the additive sync above only ever ADDs tables/columns. For a + // table that already existed, detect (and in dev, auto-reconcile) any + // non-additive divergence (relaxed NOT NULL, widened varchar, orphaned + // column) between metadata and the physical schema. + if (exists) { + await this.reconcileAndWarnDrift(tableName, obj.fields ?? {}); + } + } + } + + // ── Managed-schema drift & reconcile (#2186) ─────────────────────────────── + + /** Canonical dialect name for the drift differ. */ + protected get dialectName(): SqlDialectName { + if (this.isSqlite) return 'sqlite'; + if (this.isPostgres) return 'postgres'; + if (this.isMysql) return 'mysql'; + return 'unknown'; + } + + /** True only when running under `NODE_ENV=production` — auto-DDL is force-disabled there. */ + protected isProductionEnv(): boolean { + try { + return (process.env.NODE_ENV ?? '').toLowerCase() === 'production'; + } catch { + return false; } } + /** Diff one table's metadata fields against its physical columns. */ + protected async detectTableDrift( + tableName: string, + fields: Record, + ): Promise { + const cols = await this.introspectColumns(tableName); + const physical: PhysicalColumn[] = cols.map((c) => ({ + name: c.name, + type: c.type, + nullable: c.nullable, + maxLength: c.maxLength, + })); + return diffManagedTable({ table: tableName, fields, columns: physical, dialect: this.dialectName }); + } + + /** + * Detect every managed-schema divergence between metadata and the physical + * database. Metadata is the source of truth. Returns one entry per drift, + * sorted by table then column. Used by `os migrate` (P3) and tests. + * + * @param objects optional explicit object list; defaults to whatever + * `initObjects` last synced (captured in {@link managedObjectFields}). + */ + async detectManagedDrift( + objects?: Array<{ name: string; fields?: Record }>, + ): Promise { + const tables = new Map>(); + if (objects) { + for (const o of objects) tables.set(StorageNameMapping.resolveTableName(o), o.fields ?? {}); + } else { + for (const [t, f] of this.managedObjectFields) tables.set(t, f); + } + + const out: ManagedDriftEntry[] = []; + for (const [tableName, fields] of tables) { + if (!(await this.knex.schema.hasTable(tableName))) continue; + out.push(...(await this.detectTableDrift(tableName, fields))); + } + out.sort((a, b) => (a.table === b.table ? (a.column ?? '').localeCompare(b.column ?? '') : a.table.localeCompare(b.table))); + return out; + } + + /** + * Boot-time per-table drift handling (P1 + P2): detect divergence, in dev + * auto-reconcile the *safe* (loosening) subset when `autoMigrate==='safe'`, + * then WARN once per remaining divergence with an actionable hint. + */ + protected async reconcileAndWarnDrift(tableName: string, fields: Record): Promise { + let drift: ManagedDriftEntry[]; + try { + drift = await this.detectTableDrift(tableName, fields); + } catch (e: any) { + this.logger.warn(`[schema-drift] could not introspect '${tableName}' for drift detection`, e?.message ?? e); + return; + } + if (drift.length === 0) return; + + const autoOn = this.autoMigrate === 'safe' && this.schemaMode === 'managed'; + if (autoOn && this.isProductionEnv()) { + this.logger.warn( + `[schema-drift] autoMigrate='safe' is ignored under NODE_ENV=production — schema is never auto-altered in production. Run 'os migrate' deliberately.`, + ); + } else if (autoOn) { + const safe = drift.filter((d) => d.category === 'safe'); + if (safe.length > 0) { + try { + const { applied } = await this.applyMigrationEntries(safe, { allowDestructive: false }); + for (const d of applied) { + (this.logger.info ?? this.logger.warn)(`[schema-drift] auto-reconciled ${d.op.type} on ${d.table}.${d.column}`); + } + // Re-detect so the warnings below reflect the post-reconcile state. + drift = await this.detectTableDrift(tableName, fields); + } catch (e: any) { + this.logger.warn(`[schema-drift] dev auto-reconcile failed for '${tableName}' — falling back to warning`, e?.message ?? e); + } + } + } + + for (const d of drift) { + const k = driftKey(d); + if (this.driftWarned.has(k)) continue; + this.driftWarned.add(k); + this.logger.warn(`[schema-drift] ${d.message}`); + } + } + + /** + * Apply a set of drift entries to the physical schema. Destructive entries + * are skipped unless `allowDestructive` is set. Postgres/MySQL alter columns + * in place; SQLite (which cannot alter constraints in place) rebuilds each + * affected table (copy → swap) applying only the requested edits. + * + * @returns the entries actually applied and those skipped (e.g. destructive + * without `allowDestructive`, or unsupported on the dialect). + */ + async applyMigrationEntries( + entries: ManagedDriftEntry[], + opts: { allowDestructive?: boolean } = {}, + ): Promise<{ applied: ManagedDriftEntry[]; skipped: ManagedDriftEntry[] }> { + this.assertSchemaMutable('reconcileManagedSchema'); + const allowDestructive = opts.allowDestructive === true; + + const applied: ManagedDriftEntry[] = []; + const skipped: ManagedDriftEntry[] = []; + + const candidates = entries.filter((d) => { + if (d.category === 'destructive' && !allowDestructive) { + skipped.push(d); + return false; + } + return true; + }); + if (candidates.length === 0) return { applied, skipped }; + + // Group by table — SQLite reconciles a whole table in one rebuild. + const byTable = new Map(); + for (const d of candidates) { + (byTable.get(d.table) ?? byTable.set(d.table, []).get(d.table)!).push(d); + } + + for (const [table, ents] of byTable) { + try { + if (this.isSqlite) { + await this.rebuildSqliteTablePatched(table, ents); + applied.push(...ents); + } else { + for (const d of ents) { + const ok = await this.applyDriftOpInPlace(d.op); + (ok ? applied : skipped).push(d); + } + } + } catch (e: any) { + this.logger.warn(`[schema-drift] failed to reconcile '${table}'`, e?.message ?? e); + for (const d of ents) if (!applied.includes(d)) skipped.push(d); + } + } + return { applied, skipped }; + } + + /** Apply a single drift op in place (Postgres / MySQL). Returns false if unsupported. */ + protected async applyDriftOpInPlace(op: DriftOp): Promise { + const { table, column } = op; + if (this.isPostgres) { + switch (op.type) { + case 'relax_not_null': + await this.knex.raw('ALTER TABLE ?? ALTER COLUMN ?? DROP NOT NULL', [table, column]); + return true; + case 'tighten_not_null': + await this.knex.raw('ALTER TABLE ?? ALTER COLUMN ?? SET NOT NULL', [table, column]); + return true; + case 'widen_varchar': + case 'narrow_varchar': + await this.knex.raw(`ALTER TABLE ?? ALTER COLUMN ?? TYPE varchar(${op.to})`, [table, column]); + return true; + case 'drop_column': + await this.knex.raw('ALTER TABLE ?? DROP COLUMN ??', [table, column]); + return true; + } + } + if (this.isMysql) { + // MySQL MODIFY restates the FULL column definition — reconstruct the + // type (with length for char types, so a nullability change never + // silently drops a varchar's declared length) from columnInfo. + const info: any = await this.knex(table).columnInfo(); + const ci: any = info?.[column]; + const colType: string | undefined = ci?.type + ? (/char/i.test(ci.type) && ci.maxLength ? `${ci.type}(${ci.maxLength})` : ci.type) + : undefined; + switch (op.type) { + case 'relax_not_null': + if (!colType) return false; + await this.knex.raw(`ALTER TABLE ?? MODIFY ?? ${colType} NULL`, [table, column]); + return true; + case 'tighten_not_null': + if (!colType) return false; + await this.knex.raw(`ALTER TABLE ?? MODIFY ?? ${colType} NOT NULL`, [table, column]); + return true; + case 'widen_varchar': + case 'narrow_varchar': + await this.knex.raw(`ALTER TABLE ?? MODIFY ?? varchar(${op.to})`, [table, column]); + return true; + case 'drop_column': + await this.knex.raw('ALTER TABLE ?? DROP COLUMN ??', [table, column]); + return true; + } + } + this.logger.warn(`[schema-drift] ${op.type} on ${table}.${column} is unsupported on dialect '${this.dialectName}' — skipped`); + return false; + } + + /** + * Rebuild a SQLite table applying a set of column edits (relax/tighten NOT + * NULL, drop column), preserving all other columns and their data. Follows + * the official SQLite procedure: create patched table → copy → drop → rename. + * varchar widen/narrow are no-ops on SQLite (dynamic typing) and ignored. + * + * Unique field-level constraints and declared indexes are recreated from + * metadata afterwards (the source of truth). DB-level foreign keys declared + * by `lookup` fields are not re-added (ObjectStack enforces relationships at + * the application layer, not via SQLite FK constraints). + */ + protected async rebuildSqliteTablePatched(table: string, ents: ManagedDriftEntry[]): Promise { + const relax = new Set(); + const tighten = new Set(); + const drop = new Set(); + for (const e of ents) { + if (e.op.type === 'relax_not_null') relax.add(e.op.column); + else if (e.op.type === 'tighten_not_null') tighten.add(e.op.column); + else if (e.op.type === 'drop_column') drop.add(e.op.column); + // widen/narrow varchar: SQLite ignores declared length — nothing to do. + } + + const physical = await this.introspectColumns(table); + const kept = physical.filter((c) => !drop.has(c.name)); + const keptNames = kept.map((c) => c.name); + const fields = this.managedObjectFields.get(table) ?? {}; + const tmp = `__os_mig_${table}`; + + // FK enforcement must be toggled OUTSIDE the transaction (SQLite ignores + // the PRAGMA inside one). Off during the swap so the rename doesn't trip + // any dangling references mid-flight. + await this.knex.raw('PRAGMA foreign_keys = OFF'); + try { + await this.knex.transaction(async (trx) => { + await trx.schema.dropTableIfExists(tmp); + await trx.schema.createTable(tmp, (t) => { + for (const c of kept) { + const col = this.buildRebuiltColumn(t, c); + if (!col) continue; + const nullable = relax.has(c.name) ? true : tighten.has(c.name) ? false : c.nullable; + if (!nullable && c.name !== 'id') col.notNullable(); + if (c.name === 'created_at' || c.name === 'updated_at') col.defaultTo(this.knex.fn.now()); + } + }); + const colList = keptNames.map((n) => `"${n}"`).join(', '); + await trx.raw(`INSERT INTO "${tmp}" (${colList}) SELECT ${colList} FROM "${table}"`); + await trx.schema.dropTable(table); + await trx.schema.renameTable(tmp, table); + }); + } finally { + await this.knex.raw('PRAGMA foreign_keys = ON'); + } + + // Recreate unique constraints + declared indexes from metadata. + try { + const keptSet = new Set(keptNames); + for (const [name, field] of Object.entries(fields)) { + if (field?.unique && keptSet.has(name)) { + const idx = `uniq_${table}_${name}`; + await this.knex.raw('CREATE UNIQUE INDEX IF NOT EXISTS ?? ON ?? (??)', [idx, table, name]); + } + } + const declared = this.managedObjectIndexes.get(table); + if (Array.isArray(declared) && declared.length > 0) { + await this.syncDeclaredIndexes(table, declared, keptSet); + } + } catch (e: any) { + this.logger.warn(`[schema-drift] could not fully recreate indexes for '${table}' after rebuild`, e?.message ?? e); + } + } + + /** Map an introspected SQLite column to a knex builder for the rebuilt table. */ + protected buildRebuiltColumn(t: Knex.CreateTableBuilder, c: IntrospectedColumn): any { + if (c.name === 'id') return t.string('id').primary(); + const ty = (c.type || 'text').toLowerCase(); + if (ty.includes('int')) return t.integer(c.name); + if (/(real|floa|doub|num|dec)/.test(ty)) return t.float(c.name); + if (ty.includes('bool')) return t.boolean(c.name); + if (ty.includes('datetime') || ty.includes('timestamp')) return t.timestamp(c.name); + if (ty === 'date') return t.date(c.name); + if (ty === 'time') return t.time(c.name); + if (ty.includes('json')) return t.json(c.name); + if (ty.includes('blob') || ty.includes('binary')) return t.binary(c.name); + if (ty.includes('text')) return t.text(c.name); + return t.string(c.name); + } + /** * Build a deterministic index name for a declared index so repeated * `initObjects` runs converge on the same identifier (and can detect an diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index dd2d9aecc0..f8d77a2af8 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -196,12 +196,23 @@ export function mapDataError(error: any, object?: string): { status: number; bod /column\s+["'`]([a-z0-9_]+)["'`]\s+cannot be null/i.exec(raw); if (notNull) { const field = notNull[1]; + // The metadata required-check (`record-validator`) runs BEFORE the + // driver, so a NOT NULL violation that reaches this far means metadata + // did NOT consider the field required — i.e. the physical column has + // drifted from metadata (#2186), not a genuine missing-required-field. + // We keep the `VALIDATION_FAILED` / `required` envelope for back-compat + // (form UIs key off it) but add an actionable `hint` so the message + // stops being misleading. return { status: 400, body: { error: `${field} is required`, code: 'VALIDATION_FAILED', fields: [{ field, code: 'required', message: `${field} is required` }], + hint: + `If '${field}' is optional in your object metadata, the database column is still NOT NULL — ` + + `the physical schema has drifted from metadata. Run 'os migrate' to reconcile ` + + `(or reset the dev database).`, ...(object ? { object } : {}), }, }; diff --git a/packages/rest/src/rest.test.ts b/packages/rest/src/rest.test.ts index a66a73a32f..7060350697 100644 --- a/packages/rest/src/rest.test.ts +++ b/packages/rest/src/rest.test.ts @@ -1681,6 +1681,11 @@ describe('mapDataError — schema/constraint envelopes', () => { expect(r.body.fields).toEqual([ { field: 'organization_id', code: 'required', message: 'organization_id is required' }, ]); + // #2186: reaching this branch means metadata did not require the field, so + // it is physical-schema drift — surface an actionable hint without breaking + // the back-compat envelope. + expect(String(r.body.hint)).toMatch(/os migrate/); + expect(String(r.body.hint)).toMatch(/drifted from metadata/); }); it('maps Postgres not-null violation → 400 VALIDATION_FAILED', () => {