Skip to content

Commit dac6a08

Browse files
authored
feat(driver-sql)!: index drift is planned, not silently executed at boot (#3728) (#3737)
The #3696 unique-scope migration converged in place: `syncTableIndexes` ran a `DROP` + `CREATE UNIQUE INDEX` during `initObjects`, in every environment, leaving one log line behind. `os migrate plan` showed nothing, because `detectManagedDrift` was column-only. An operator had no way to review the DDL before it reached their database, and a managed schema was being auto-altered in production, which the #2186 contract forbids. Index drift is now a first-class dimension, reconciled through the same path as column drift. - `syncTableIndexes` is ADDITIVE ONLY; `dropLegacyGlobalUniques` is gone. - New `DriftOp` variants: `replace_unique_index` (safe), `create_index` (safe), `recreate_index` (needs-confirm; destructive when it tightens to UNIQUE) and `drop_index` (destructive). - `detectManagedDrift` reports them, `os migrate plan` renders them, `os migrate apply` executes them — portable DDL, no SQLite table rebuild. - `replace_unique_index` creates before it drops, and only drops once the replacement is confirmed present. - Declared `indexes[]` drift is covered: missing, and redefined-under-the-same- name (which the name-idempotent sync could never self-heal). - Orphan detection is limited to ObjectStack's own generated naming, so a hand-rolled operational index is never reported or deleted. Behaviour change: dev (`autoMigrate: 'safe'`) still self-heals on restart; production warns with an `os migrate` hint and leaves the schema alone. Also fixed: `managedObjectIndexes` was never cleared when an object dropped its `indexes[]`. `SchemaDiffEntryKind` gains `index_mismatch` / `unmapped_index`.
1 parent 7180ed5 commit dac6a08

11 files changed

Lines changed: 1229 additions & 169 deletions

File tree

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
"@objectstack/spec": patch
4+
"@objectstack/cli": patch
5+
---
6+
7+
feat(driver-sql)!: make index drift visible to `os migrate plan` — no more silent DDL at boot (#3728)
8+
9+
The #3696 unique-scope migration converged **in place**: `syncTableIndexes` ran a
10+
`DROP` + `CREATE UNIQUE INDEX` during `initObjects`, in every environment,
11+
leaving one log line behind. `os migrate plan` showed nothing, because
12+
`detectManagedDrift` was column-only — `ManagedDriftOp` had no index dimension at
13+
all. An operator who wanted to review the DDL before it reached their database
14+
had no way to, and a managed schema was being auto-altered in production, which
15+
the #2186 contract explicitly forbids.
16+
17+
Index drift is now a first-class dimension, reconciled through the same path as
18+
column drift:
19+
20+
- **`syncTableIndexes` is additive only.** It creates indexes; it never drops or
21+
rewrites one. `dropLegacyGlobalUniques` is gone.
22+
- **New `DriftOp` variants**`replace_unique_index` (safe: retire the legacy
23+
platform-wide unique in favour of the tenant composite), `create_index` (safe),
24+
`recreate_index` (needs-confirm; destructive when it tightens to `UNIQUE`), and
25+
`drop_index` (destructive).
26+
- **`detectManagedDrift` reports them**, `os migrate plan` renders them (index
27+
ops display as `table [index_name]`), and `os migrate apply` executes them.
28+
Index DDL is portable, so it applies directly on every dialect — no SQLite
29+
table rebuild.
30+
- **`replace_unique_index` creates before it drops**, so uniqueness is never
31+
unenforced mid-migration and a failed create leaves the schema untouched.
32+
- **Declared `indexes[]` drift is covered too**: an index metadata declares but
33+
the database lacks, and one whose definition no longer matches the declaration
34+
(the additive sync skips those by name, so they could never self-heal).
35+
- **Orphan detection is limited to ObjectStack's own generated naming**
36+
(`uniq_…` / `idx_…`, plus the pre-#3696 `<table>_<column>_unique` knex
37+
spelling). A hand-rolled operational index is never reported as drift and
38+
`--allow-destructive` will not delete it.
39+
40+
**Behaviour change.** Boot no longer rewrites the index unconditionally. Dev
41+
(`autoMigrate: 'safe'`, what `os dev` / `os serve` use) still self-heals on
42+
restart, so local workflows are unchanged. Production now **warns** with an
43+
actionable `os migrate` hint and leaves the schema alone — the deployment stays
44+
on the legacy global unique (multi-tenant inserts still collide) until someone
45+
runs `os migrate apply`. That is the deliberate trade: a visible, pre-inspectable
46+
migration instead of an invisible one.
47+
48+
Also fixed: `managedObjectIndexes` was never cleared when an object dropped its
49+
`indexes[]`, so drift detection kept expecting an index nobody declared.
50+
51+
`SchemaDiffEntryKind` gains `index_mismatch` and `unmapped_index`.

content/docs/deployment/cli.mdx

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -477,12 +477,12 @@ os info --json # JSON output for tooling
477477
### Schema migrations
478478

479479
The metadata→database sync is **additive-only**: on boot it creates missing
480-
tables and adds new columns, but never alters or drops existing ones. So a
481-
*non-additive* change to an object already backed by a database — relaxing
482-
`required` (drop `NOT NULL`), changing a field's type/length, or removing a
483-
field — silently diverges from the live schema, and the physical column wins at
484-
write time. `os migrate` reconciles the database to the metadata (the source of
485-
truth).
480+
tables, adds new columns and creates missing indexes, but never alters or drops
481+
existing ones. So a *non-additive* change to an object already backed by a
482+
database — relaxing `required` (drop `NOT NULL`), changing a field's
483+
type/length, removing a field, or re-scoping a `unique` constraint — silently
484+
diverges from the live schema, and the physical column wins at write time.
485+
`os migrate` reconciles the database to the metadata (the source of truth).
486486

487487
| Command | Description |
488488
|---------|-------------|
@@ -499,16 +499,32 @@ os migrate plan --json # Machine-readable output
499499

500500
| Category | Examples | Applied by |
501501
|----------|----------|------------|
502-
| `safe` | relax `NOT NULL` → nullable, widen a `varchar` | `os migrate apply` (and dev auto-reconcile) |
503-
| `needs_confirm` | non-narrowing type change | `os migrate apply` |
504-
| `destructive` | drop an orphaned column, tighten `NOT NULL`, narrow a type | `os migrate apply --allow-destructive` |
502+
| `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) |
503+
| `needs_confirm` | non-narrowing type change, rebuild a non-unique index whose columns changed | `os migrate apply` |
504+
| `destructive` | drop an orphaned column or index, tighten `NOT NULL`, narrow a type, rebuild an index as `UNIQUE` | `os migrate apply --allow-destructive` |
505+
506+
#### Index drift
507+
508+
`plan` covers indexes as well as columns:
509+
510+
| Op | What it means |
511+
|----|---------------|
512+
| `create_index` | Metadata declares an index the database does not have |
513+
| `replace_unique_index` | A field's `unique` used to be enforced platform-wide, but metadata now scopes it per tenant — the legacy single-column index is swapped for the `(tenantField, field)` composite. A pure relaxation: it creates before it drops, and cannot fail |
514+
| `recreate_index` | An index exists under the declared name but with different columns/uniqueness. The additive sync skips it by name, so it must be dropped and rebuilt |
515+
| `drop_index` | An index carrying ObjectStack's generated naming (`uniq_…` / `idx_…`) that metadata no longer declares |
516+
517+
Orphan detection is deliberately limited to indexes ObjectStack itself
518+
generated. A hand-rolled covering index you added in `psql` is never reported as
519+
drift, and `--allow-destructive` will not delete it.
505520

506521
<Callout type="tip">
507522
**Dev self-heal.** `os dev` runs the SQL driver with `autoMigrate: 'safe'`, so
508-
loosening changes (e.g. you just made a field optional) are applied to your
509-
existing dev database automatically on restart — no `os migrate` needed, no data
510-
loss. Auto-reconcile is **dev-only and never destructive**; it is force-disabled
511-
under `NODE_ENV=production`, where you run `os migrate` deliberately.
523+
safe changes (you just made a field optional; a `unique` field became
524+
tenant-scoped) are applied to your existing dev database automatically on
525+
restart — no `os migrate` needed, no data loss. Auto-reconcile is **dev-only and
526+
never destructive**; it is force-disabled under `NODE_ENV=production`, where
527+
every change is shown by `os migrate plan` before you apply it deliberately.
512528
</Callout>
513529

514530
<Callout type="warn">

packages/cli/src/utils/schema-migrate.integration.test.ts

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,16 @@ import { SqlDriver } from '@objectstack/driver-sql';
88
import { bootSchemaStack } from './schema-migrate.js';
99

1010
/**
11-
* End-to-end (#2186): boot the real standalone stack via `bootSchemaStack`
12-
* against a pre-seeded "legacy" SQLite DB (organization_id created NOT NULL),
13-
* then verify `os migrate`'s engine detects the drift and reconciles it —
14-
* exercising the full createStandaloneStack → AppPlugin → ObjectQL → driver path.
11+
* End-to-end (#2186, #3728): boot the real standalone stack via
12+
* `bootSchemaStack` against a pre-seeded "legacy" SQLite DB — `organization_id`
13+
* created NOT NULL, and a platform-wide UNIQUE index on a field metadata now
14+
* scopes per tenant — then verify `os migrate`'s engine detects BOTH drifts and
15+
* reconciles them, exercising the full createStandaloneStack → AppPlugin →
16+
* ObjectQL → driver path.
17+
*
18+
* The index half is the #3728 acceptance: under `NODE_ENV=production` nothing
19+
* self-heals at boot, so whatever `plan` reports here is exactly what an
20+
* operator would see before any DDL touches their database.
1521
*/
1622
describe('bootSchemaStack + migrate engine (integration)', () => {
1723
let dir: string;
@@ -36,13 +42,15 @@ describe('bootSchemaStack + migrate engine (integration)', () => {
3642
fields: {
3743
name: { type: 'text', required: true },
3844
organization_id: { type: 'text', required: false }, // optional now
45+
code: { type: 'text', unique: true }, // tenant-scoped since #3696
3946
},
4047
},
4148
],
4249
}),
4350
);
4451

45-
// Seed a "legacy" DB where organization_id is NOT NULL (the #2178 shape).
52+
// Seed a "legacy" DB: organization_id NOT NULL (the #2178 shape) plus the
53+
// platform-wide unique index on `code` the pre-#3696 driver emitted.
4654
const seed = new SqlDriver({ client: 'better-sqlite3', connection: { filename: dbFile }, useNullAsDefault: true });
4755
const k = (seed as any).knex;
4856
await k.schema.createTable('mig_biz_unit', (t: any) => {
@@ -51,8 +59,10 @@ describe('bootSchemaStack + migrate engine (integration)', () => {
5159
t.timestamp('updated_at');
5260
t.string('name').notNullable();
5361
t.string('organization_id').notNullable();
62+
t.string('code');
5463
});
55-
await k('mig_biz_unit').insert({ id: '1', name: 'Acme', organization_id: 'org1' });
64+
await k.raw('CREATE UNIQUE INDEX mig_biz_unit_code_unique ON mig_biz_unit (code)');
65+
await k('mig_biz_unit').insert({ id: '1', name: 'Acme', organization_id: 'org1', code: 'BU-00001' });
5666
await k.destroy();
5767

5868
savedEnv.OS_ARTIFACT_PATH = process.env.OS_ARTIFACT_PATH;
@@ -67,24 +77,41 @@ describe('bootSchemaStack + migrate engine (integration)', () => {
6777
try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ }
6878
});
6979

70-
it('detects the NOT NULL drift, applies it, and self-verifies in-sync', async () => {
80+
it('detects the NOT NULL + legacy-unique drift, applies both, and self-verifies in-sync', async () => {
7181
const stack = await bootSchemaStack({ databaseUrl: `file:${dbFile}` });
7282
try {
7383
expect(stack.driver).toBeTruthy();
7484
expect(stack.managedTableCount).toBeGreaterThan(0);
7585

86+
// Boot changed nothing — this is exactly what `os migrate plan` renders.
7687
const drift = await stack.driver!.detectManagedDrift();
88+
7789
const org = drift.find((d) => d.table === 'mig_biz_unit' && d.column === 'organization_id');
7890
expect(org, 'expected drift on mig_biz_unit.organization_id').toBeDefined();
7991
expect(org!.category).toBe('safe');
8092
expect(org!.op.type).toBe('relax_not_null');
8193

94+
// #3728: the legacy platform-wide unique is PLANNED, not silently rewritten.
95+
const idx = drift.find((d) => d.op.type === 'replace_unique_index');
96+
expect(idx, 'expected replace_unique_index drift on mig_biz_unit.code').toBeDefined();
97+
expect(idx!.category).toBe('safe');
98+
expect(idx!.op).toMatchObject({
99+
table: 'mig_biz_unit',
100+
dropIndexNames: ['mig_biz_unit_code_unique'],
101+
createColumns: ['organization_id', 'code'],
102+
});
103+
82104
const { applied, skipped } = await stack.driver!.applyMigrationEntries(drift, { allowDestructive: false });
83105
expect(applied.some((d) => d.op.type === 'relax_not_null')).toBe(true);
106+
expect(applied.some((d) => d.op.type === 'replace_unique_index')).toBe(true);
84107
expect(skipped).toHaveLength(0);
85108

86109
const after = await stack.driver!.detectManagedDrift();
87110
expect(after.find((d) => d.table === 'mig_biz_unit' && d.column === 'organization_id')).toBeUndefined();
111+
expect(after.find((d) => d.op.type === 'replace_unique_index')).toBeUndefined();
112+
113+
// The seeded row survived the reconcile.
114+
expect(await (stack.driver as any).count('mig_biz_unit', {})).toBe(1);
88115
} finally {
89116
await stack.shutdown();
90117
}

packages/cli/src/utils/schema-migrate.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,17 @@ export function groupByCategory(drift: ManagedDriftEntry[]): Record<DriftCategor
136136
return out;
137137
}
138138

139+
/**
140+
* What a drift entry acts on. Column ops read `table.column`; index ops (#3728)
141+
* name the index instead — a composite unique spans several columns, so the
142+
* leading column alone would misrepresent what is about to change.
143+
*/
144+
export function driftTarget(d: ManagedDriftEntry): string {
145+
const op = d.op as { indexName?: string; createIndexName?: string };
146+
const indexName = op.indexName ?? op.createIndexName;
147+
return indexName ? `${d.table} [${indexName}]` : `${d.table}.${d.column ?? ''}`;
148+
}
149+
139150
export function renderPlan(drift: ManagedDriftEntry[]): void {
140151
const grouped = groupByCategory(drift);
141152
for (const cat of CATEGORY_ORDER) {
@@ -144,7 +155,7 @@ export function renderPlan(drift: ManagedDriftEntry[]): void {
144155
const meta = CATEGORY_META[cat];
145156
console.log(` ${chalk.bold(meta.label)}`);
146157
for (const d of items) {
147-
console.log(` ${meta.color(meta.icon)} ${meta.color(`${d.table}.${d.column ?? ''}`)} ${chalk.dim(`[${d.op.type}]`)}`);
158+
console.log(` ${meta.color(meta.icon)} ${meta.color(driftTarget(d))} ${chalk.dim(`[${d.op.type}]`)}`);
148159
console.log(` ${chalk.dim(d.message)}`);
149160
}
150161
console.log('');

packages/plugins/driver-sql/src/index.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,32 @@ export type {
1111
IntrospectedForeignKey,
1212
} from './sql-driver.js';
1313

14-
// Managed-schema drift / reconcile (#2186)
14+
// Managed-schema drift / reconcile (#2186), incl. the index dimension (#3728)
1515
export {
1616
diffManagedTable,
1717
driftKey,
1818
fieldHasColumn,
1919
BUILTIN_COLUMNS,
20+
buildIndexName,
21+
diffManagedIndexes,
22+
expectedIndexes,
23+
isIndexDriftOp,
24+
isManagedIndexName,
25+
legacyUniqueIndexNames,
26+
legacyUniqueReplacements,
27+
normalizeDeclaredIndex,
28+
uniqueIndexesFromFields,
29+
INDEX_DRIFT_OPS,
2030
} from './schema-drift.js';
2131
export type {
2232
ManagedDriftEntry,
2333
DriftOp,
2434
DriftCategory,
2535
SqlDialectName,
2636
PhysicalColumn,
37+
PhysicalIndex,
38+
ExpectedIndex,
39+
LegacyUniqueReplacement,
2740
FieldDef as DriftFieldDef,
2841
} from './schema-drift.js';
2942

0 commit comments

Comments
 (0)