Skip to content

Commit 517dad9

Browse files
xuyushun441-sysos-zhuangclaude
authored
feat: schema-drift detection + dev auto-reconcile + os migrate (#2186) (#2196)
The metadata→DB sync was additive-only (create table / add column), so any non-additive metadata change — relax `required`, change type/length, drop or rename 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. Root-cause note: that 400 is NOT a validator bug. `record-validator` already reads `required` from metadata and passes once the field is optional; the 400 is a DB NOT NULL violation that `rest-server` paraphrases as "is required". So the fix is reconciling the physical schema + de-misleading the message, not changing the validator's source of truth. P1 — detection + warnings (schema-drift.ts, wired into SqlDriver.initObjects): diff metadata vs physical columns, categorise safe / needs_confirm / destructive (reusing SchemaDiffEntry). Boot warns once per divergence with an actionable `os migrate` hint. rest-server's NOT NULL translation now carries a drift-aware `hint` (VALIDATION_FAILED / fields envelope unchanged for back-compat). P2 — dev auto-reconcile (SqlDriverConfig.autoMigrate:'safe', wired into serve/dev in dev only): 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. P3 — os migrate plan/apply (cli/commands/migrate/*): categorised dry-run + reconcile, --allow-destructive gate, confirm prompt, --json. SQLite reconciles via the official table-rebuild (copy→swap, data preserved); Postgres/MySQL alter in place. Only examines objects in the loaded artifact; never auto-drops a table absent from metadata. Tests: driver-sql +12 (incl. #2178 repro self-heal with data preserved, prod guard, destructive gating, PG/MySQL DDL-gen), cli integration (real createStandaloneStack boot → detect → apply → in-sync), rest hint. Full build green. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent de9508d commit 517dad9

14 files changed

Lines changed: 1410 additions & 5 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
"@objectstack/cli": minor
4+
"@objectstack/rest": patch
5+
---
6+
7+
Schema drift detection + `os migrate` for non-additive metadata changes (#2186).
8+
9+
The metadata→DB schema sync was additive-only: it created tables and added
10+
columns but never altered/dropped existing ones, so relaxing `required`,
11+
changing a type/length, or dropping a field silently diverged from an existing
12+
database. The physical column won at write time, surfacing a misleading
13+
`organization_id is required` 400 even though `/meta` reported the field
14+
optional.
15+
16+
- **driver-sql** — the SQL driver now detects managed-schema drift (metadata is
17+
the source of truth) and categorises each divergence `safe` / `needs_confirm`
18+
/ `destructive`. `initObjects` warns once per divergence with an actionable
19+
hint. A new opt-in `SqlDriverConfig.autoMigrate: 'safe'` auto-applies the
20+
*loosening* subset (relax `NOT NULL`, widen varchar) so an existing dev DB
21+
self-heals on restart — never destructive, force-disabled under
22+
`NODE_ENV=production`. New public methods `detectManagedDrift()` /
23+
`applyMigrationEntries()`. SQLite reconciles via the official table-rebuild
24+
(copy → swap), preserving data; Postgres/MySQL alter in place.
25+
- **cli** — new `os migrate plan` (dry-run, categorised diff) and
26+
`os migrate apply` (`--allow-destructive` for drops/tightenings, confirm gate,
27+
`--json`). `os dev`/`serve` now pass `autoMigrate: 'safe'` in dev only.
28+
- **rest** — a `NOT NULL` violation that reaches the driver (metadata validation
29+
already passed) now carries a drift-aware `hint` pointing at `os migrate`,
30+
instead of only the misleading "field is required" message. The
31+
`VALIDATION_FAILED` / `fields` envelope is unchanged for back-compat.
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { Command, Flags } from '@oclif/core';
4+
import chalk from 'chalk';
5+
import { createInterface } from 'node:readline';
6+
import {
7+
printHeader,
8+
printSuccess,
9+
printWarning,
10+
printError,
11+
printInfo,
12+
printStep,
13+
createTimer,
14+
} from '../../utils/format.js';
15+
import {
16+
bootSchemaStack,
17+
renderPlan,
18+
summarize,
19+
groupByCategory,
20+
} from '../../utils/schema-migrate.js';
21+
22+
async function confirm(question: string): Promise<boolean> {
23+
if (!process.stdin.isTTY) return false; // non-interactive → require --yes
24+
const rl = createInterface({ input: process.stdin, output: process.stdout });
25+
try {
26+
const answer: string = await new Promise((resolve) => rl.question(question, resolve));
27+
return /^y(es)?$/i.test(answer.trim());
28+
} finally {
29+
rl.close();
30+
}
31+
}
32+
33+
/**
34+
* `os migrate apply` — reconcile the physical database to metadata (#2186).
35+
* Applies safe (loosening) + needs-confirm changes by default; destructive
36+
* changes (drop column, tighten NOT NULL, narrow type) require
37+
* `--allow-destructive`.
38+
*/
39+
export default class MigrateApply extends Command {
40+
static override description =
41+
'Reconcile the physical database to metadata (safe by default; destructive changes need --allow-destructive)';
42+
43+
static override examples = [
44+
'$ os migrate apply',
45+
'$ os migrate apply --yes',
46+
'$ os migrate apply --allow-destructive --yes',
47+
'$ os migrate apply --json',
48+
];
49+
50+
static override flags = {
51+
'database-url': Flags.string({
52+
description: 'Database URL to reconcile (defaults to $OS_DATABASE_URL / the project DB)',
53+
env: 'OS_DATABASE_URL',
54+
}),
55+
'allow-destructive': Flags.boolean({
56+
description: 'Also apply destructive changes (drop column, tighten NOT NULL, narrow type)',
57+
default: false,
58+
}),
59+
yes: Flags.boolean({ char: 'y', description: 'Skip the confirmation prompt', default: false }),
60+
json: Flags.boolean({ description: 'Output as JSON (implies non-interactive; requires --yes to mutate)' }),
61+
};
62+
63+
async run(): Promise<void> {
64+
const { flags } = await this.parse(MigrateApply);
65+
const timer = createTimer();
66+
const allowDestructive = flags['allow-destructive'];
67+
68+
if (!flags.json) {
69+
printHeader('Migrate · apply');
70+
printStep('Booting schema stack…');
71+
}
72+
73+
let stack;
74+
try {
75+
stack = await bootSchemaStack({ databaseUrl: flags['database-url'] });
76+
} catch (error: any) {
77+
if (flags.json) { console.log(JSON.stringify({ error: error.message })); this.exit(1); }
78+
printError(error.message || String(error));
79+
this.exit(1);
80+
return;
81+
}
82+
83+
try {
84+
if (!stack.driver) {
85+
if (flags.json) { console.log(JSON.stringify({ error: 'no_sql_driver' })); return; }
86+
printWarning('Schema migration is only supported on SQL drivers (SQLite / Postgres). No SQL driver is active.');
87+
return;
88+
}
89+
90+
const drift = await stack.driver.detectManagedDrift();
91+
const grouped = groupByCategory(drift);
92+
93+
if (drift.length === 0) {
94+
if (flags.json) { console.log(JSON.stringify({ applied: [], skipped: [], message: 'in_sync' })); return; }
95+
printSuccess('Physical schema is already in sync with metadata — nothing to apply.');
96+
return;
97+
}
98+
99+
// Entries we intend to apply this run.
100+
const intended = drift.filter((d) => d.category !== 'destructive' || allowDestructive);
101+
const deferred = drift.filter((d) => d.category === 'destructive' && !allowDestructive);
102+
103+
if (!flags.json) {
104+
printInfo(`Database: ${chalk.white(stack.dbLabel)}`);
105+
console.log('');
106+
renderPlan(drift);
107+
printInfo(summarize(drift));
108+
if (deferred.length > 0) {
109+
printWarning(`${deferred.length} destructive change(s) will be SKIPPED (re-run with --allow-destructive to include them).`);
110+
}
111+
if (allowDestructive && grouped.destructive.length > 0) {
112+
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.');
113+
}
114+
}
115+
116+
if (intended.length === 0) {
117+
if (flags.json) { console.log(JSON.stringify({ applied: [], skipped: deferred, message: 'nothing_safe_to_apply' })); return; }
118+
printWarning('No changes to apply without --allow-destructive.');
119+
return;
120+
}
121+
122+
// Confirmation gate.
123+
if (!flags.yes) {
124+
if (flags.json || !process.stdin.isTTY) {
125+
if (flags.json) { console.log(JSON.stringify({ applied: [], skipped: drift, message: 'confirmation_required', hint: 'pass --yes' })); return; }
126+
printWarning('Confirmation required. Re-run with --yes to apply, or use "os migrate plan" to preview.');
127+
return;
128+
}
129+
const ok = await confirm(chalk.bold(`\nApply ${intended.length} change(s) to ${stack.dbLabel}? [y/N] `));
130+
if (!ok) { printInfo('Aborted — no changes made.'); return; }
131+
}
132+
133+
const { applied, skipped } = await stack.driver.applyMigrationEntries(drift, { allowDestructive });
134+
135+
if (flags.json) {
136+
console.log(JSON.stringify({
137+
database: stack.dbLabel,
138+
applied,
139+
skipped,
140+
duration: timer.elapsed(),
141+
}, null, 2));
142+
return;
143+
}
144+
145+
console.log('');
146+
printSuccess(`Applied ${applied.length} change(s).`);
147+
if (skipped.length > 0) {
148+
printWarning(`Skipped ${skipped.length} change(s) (destructive without --allow-destructive, or unsupported on this dialect).`);
149+
}
150+
console.log(chalk.dim(` ${timer.display()}`));
151+
console.log('');
152+
} catch (error: any) {
153+
if (flags.json) { console.log(JSON.stringify({ error: error.message })); this.exit(1); }
154+
printError(error.message || String(error));
155+
this.exit(1);
156+
} finally {
157+
await stack.shutdown();
158+
}
159+
}
160+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import MigratePlan from './plan.js';
4+
5+
/**
6+
* `os migrate` with no subcommand defaults to the (read-only) plan, so the
7+
* bare command can never mutate the schema by surprise (issue #2186).
8+
*/
9+
export default class Migrate extends MigratePlan {
10+
static override description =
11+
'Inspect / reconcile physical-database drift from metadata. Defaults to a dry-run plan; use "os migrate apply" to reconcile.';
12+
}
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { Command, Flags } from '@oclif/core';
4+
import chalk from 'chalk';
5+
import {
6+
printHeader,
7+
printSuccess,
8+
printWarning,
9+
printError,
10+
printInfo,
11+
printStep,
12+
createTimer,
13+
} from '../../utils/format.js';
14+
import { bootSchemaStack, renderPlan, summarize } from '../../utils/schema-migrate.js';
15+
16+
/**
17+
* `os migrate plan` — dry-run diff of metadata vs the physical database,
18+
* categorised safe / needs-confirm / destructive (issue #2186). Never mutates
19+
* the schema.
20+
*/
21+
export default class MigratePlan extends Command {
22+
static override description =
23+
'Show how the physical database has drifted from metadata (dry run; no changes applied)';
24+
25+
static override examples = [
26+
'$ os migrate plan',
27+
'$ os migrate plan --json',
28+
'$ os migrate plan --database-url postgres://localhost/app',
29+
];
30+
31+
static override flags = {
32+
'database-url': Flags.string({
33+
description: 'Database URL to inspect (defaults to $OS_DATABASE_URL / the project DB)',
34+
env: 'OS_DATABASE_URL',
35+
}),
36+
json: Flags.boolean({ description: 'Output as JSON' }),
37+
};
38+
39+
async run(): Promise<void> {
40+
const { flags } = await this.parse(MigratePlan);
41+
const timer = createTimer();
42+
43+
if (!flags.json) {
44+
printHeader('Migrate · plan');
45+
printStep('Booting schema stack…');
46+
}
47+
48+
let stack;
49+
try {
50+
stack = await bootSchemaStack({ databaseUrl: flags['database-url'] });
51+
} catch (error: any) {
52+
if (flags.json) { console.log(JSON.stringify({ error: error.message })); this.exit(1); }
53+
printError(error.message || String(error));
54+
this.exit(1);
55+
return;
56+
}
57+
58+
try {
59+
if (!stack.driver) {
60+
if (flags.json) { console.log(JSON.stringify({ error: 'no_sql_driver', changes: [] })); return; }
61+
printWarning('Schema migration is only supported on SQL drivers (SQLite / Postgres). No SQL driver is active.');
62+
return;
63+
}
64+
65+
const drift = await stack.driver.detectManagedDrift();
66+
67+
if (flags.json) {
68+
console.log(JSON.stringify({
69+
database: stack.dbLabel,
70+
managedTables: stack.managedTableCount,
71+
total: drift.length,
72+
changes: drift,
73+
duration: timer.elapsed(),
74+
}, null, 2));
75+
return;
76+
}
77+
78+
printInfo(`Database: ${chalk.white(stack.dbLabel)}`);
79+
printInfo(`Examined ${chalk.white(String(stack.managedTableCount))} managed table(s).`);
80+
console.log('');
81+
82+
if (drift.length === 0) {
83+
printSuccess('Physical schema is in sync with metadata — nothing to migrate.');
84+
console.log('');
85+
return;
86+
}
87+
88+
renderPlan(drift);
89+
printInfo(summarize(drift));
90+
console.log(chalk.dim(' Apply with: ') + chalk.white('os migrate apply') +
91+
chalk.dim(' (add --allow-destructive for drops / tightenings)'));
92+
console.log(chalk.dim(` ${timer.display()}`));
93+
console.log('');
94+
} catch (error: any) {
95+
if (flags.json) { console.log(JSON.stringify({ error: error.message })); this.exit(1); }
96+
printError(error.message || String(error));
97+
this.exit(1);
98+
} finally {
99+
await stack.shutdown();
100+
}
101+
}
102+
}

‎packages/cli/src/commands/serve.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,9 @@ export default class Serve extends Command {
606606
client: 'better-sqlite3',
607607
connection: { filename: filePath },
608608
useNullAsDefault: true,
609+
// #2186: in dev, self-heal a persisted DB when a metadata change
610+
// relaxes a constraint (loosen-only; never destructive / never in prod).
611+
autoMigrate: isDev ? 'safe' : undefined,
609612
}) as any));
610613
trackPlugin('SqlDriver');
611614
resolvedDriverLabel = 'SqlDriver(sqlite)';
@@ -626,6 +629,7 @@ export default class Serve extends Command {
626629
client: 'pg',
627630
connection: databaseUrl,
628631
pool: { min: 0, max: 5 },
632+
autoMigrate: isDev ? 'safe' : undefined, // #2186 dev loosen-only self-heal
629633
}) as any));
630634
trackPlugin('PostgresDriver');
631635
resolvedDriverLabel = 'SqlDriver(pg)';
@@ -636,6 +640,7 @@ export default class Serve extends Command {
636640
client: 'mysql2',
637641
connection: databaseUrl,
638642
pool: { min: 0, max: 5 },
643+
autoMigrate: isDev ? 'safe' : undefined, // #2186 dev loosen-only self-heal
639644
}) as any));
640645
trackPlugin('MySQLDriver');
641646
resolvedDriverLabel = 'SqlDriver(mysql2)';
@@ -666,6 +671,7 @@ export default class Serve extends Command {
666671
client: 'better-sqlite3',
667672
connection: { filename: ':memory:' },
668673
useNullAsDefault: true,
674+
autoMigrate: 'safe', // #2186 dev loosen-only self-heal
669675
});
670676
await sqliteDriver.connect();
671677
sqliteOk = true;

‎packages/cli/src/index.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ export { default as StartCommand } from './commands/start.js';
1717
export { default as TestCommand } from './commands/test.js';
1818
export { default as DoctorCommand } from './commands/doctor.js';
1919

20+
// ─── Migrate topic subcommands (#2186) ──────────────────────────────
21+
export { default as MigrateCommand } from './commands/migrate/index.js';
22+
export { default as MigratePlanCommand } from './commands/migrate/plan.js';
23+
export { default as MigrateApplyCommand } from './commands/migrate/apply.js';
24+
2025
// ─── Environments topic subcommands ─────────────────────────────────
2126
export { default as EnvironmentsListCommand } from './commands/environments/list.js';
2227
export { default as EnvironmentsShowCommand } from './commands/environments/show.js';

0 commit comments

Comments
 (0)