feat(driver-sql)!: index drift is planned, not silently executed at boot (#3728)

.changeset/index-drift-migrate-plan.md

@@ -0,0 +1,51 @@
+---
+"@objectstack/driver-sql": minor
+"@objectstack/spec": patch
+"@objectstack/cli": patch
+---
+
+feat(driver-sql)!: make index drift visible to `os migrate plan` — no more silent DDL at boot (#3728)
+
+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 — `ManagedDriftOp` had no index dimension at
+all. An operator who wanted to review the DDL before it reached their database
+had no way to, and a managed schema was being auto-altered in production, which
+the #2186 contract explicitly forbids.
+
+Index drift is now a first-class dimension, reconciled through the same path as
+column drift:
+
+- **`syncTableIndexes` is additive only.** It creates indexes; it never drops or
+  rewrites one. `dropLegacyGlobalUniques` is gone.
+- **New `DriftOp` variants** — `replace_unique_index` (safe: retire the legacy
+  platform-wide unique in favour of the tenant composite), `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 (index
+  ops display as `table [index_name]`), and `os migrate apply` executes them.
+  Index DDL is portable, so it applies directly on every dialect — no SQLite
+  table rebuild.
+- **`replace_unique_index` creates before it drops**, so uniqueness is never
+  unenforced mid-migration and a failed create leaves the schema untouched.
+- **Declared `indexes[]` drift is covered too**: an index metadata declares but
+  the database lacks, and one whose definition no longer matches the declaration
+  (the additive sync skips those by name, so they could never self-heal).
+- **Orphan detection is limited to ObjectStack's own generated naming**
+  (`uniq_…` / `idx_…`, plus the pre-#3696 `<table>_<column>_unique` knex
+  spelling). A hand-rolled operational index is never reported as drift and
+  `--allow-destructive` will not delete it.
+
+**Behaviour change.** Boot no longer rewrites the index unconditionally. Dev
+(`autoMigrate: 'safe'`, what `os dev` / `os serve` use) still self-heals on
+restart, so local workflows are unchanged. Production now **warns** with an
+actionable `os migrate` hint and leaves the schema alone — the deployment stays
+on the legacy global unique (multi-tenant inserts still collide) until someone
+runs `os migrate apply`. That is the deliberate trade: a visible, pre-inspectable
+migration instead of an invisible one.
+
+Also fixed: `managedObjectIndexes` was never cleared when an object dropped its
+`indexes[]`, so drift detection kept expecting an index nobody declared.
+
+`SchemaDiffEntryKind` gains `index_mismatch` and `unmapped_index`.

content/docs/deployment/cli.mdx

@@ -477,12 +477,12 @@ os info --json         # JSON output for tooling
 ### Schema migrations
 
 The metadata→database sync is **additive-only**: on boot it creates missing
-tables and adds new columns, but never alters or drops existing ones. So a
-*non-additive* change to an object already backed by a database — relaxing
-`required` (drop `NOT NULL`), changing a field's type/length, or removing a
-field — silently diverges from the live schema, and the physical column wins at
-write time. `os migrate` reconciles the database to the metadata (the source of
-truth).
+tables, adds new columns and creates missing indexes, but never alters or drops
+existing ones. So a *non-additive* change to an object already backed by a
+database — relaxing `required` (drop `NOT NULL`), changing a field's
+type/length, removing a field, or re-scoping a `unique` constraint — silently
+diverges from the live schema, and the physical column wins at write time.
+`os migrate` reconciles the database to the metadata (the source of truth).
 
 | Command | Description |
 |---------|-------------|
@@ -499,16 +499,32 @@ os migrate plan --json                       # Machine-readable output
 
 | Category | Examples | Applied by |
 |----------|----------|------------|
-| `safe` | relax `NOT NULL` → nullable, widen a `varchar` | `os migrate apply` (and dev auto-reconcile) |
-| `needs_confirm` | non-narrowing type change | `os migrate apply` |
-| `destructive` | drop an orphaned column, tighten `NOT NULL`, narrow a type | `os migrate apply --allow-destructive` |
+| `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) |
+| `needs_confirm` | non-narrowing type change, rebuild a non-unique index whose columns changed | `os migrate apply` |
+| `destructive` | drop an orphaned column or index, tighten `NOT NULL`, narrow a type, rebuild an index as `UNIQUE` | `os migrate apply --allow-destructive` |
+
+#### Index drift
+
+`plan` covers indexes as well as columns:
+
+| Op | What it means |
+|----|---------------|
+| `create_index` | Metadata declares an index the database does not have |
+| `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 |
+| `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 |
+| `drop_index` | An index carrying ObjectStack's generated naming (`uniq_…` / `idx_…`) that metadata no longer declares |
+
+Orphan detection is deliberately limited to indexes ObjectStack itself
+generated. A hand-rolled covering index you added in `psql` is never reported as
+drift, and `--allow-destructive` will not delete it.
 
 <Callout type="tip">
 **Dev self-heal.** `os dev` runs the SQL driver with `autoMigrate: 'safe'`, so
-loosening changes (e.g. you just made a field optional) are applied to your
-existing dev database automatically on restart — no `os migrate` needed, no data
-loss. Auto-reconcile is **dev-only and never destructive**; it is force-disabled
-under `NODE_ENV=production`, where you run `os migrate` deliberately.
+safe changes (you just made a field optional; a `unique` field became
+tenant-scoped) are applied to your existing dev database automatically on
+restart — no `os migrate` needed, no data loss. Auto-reconcile is **dev-only and
+never destructive**; it is force-disabled under `NODE_ENV=production`, where
+every change is shown by `os migrate plan` before you apply it deliberately.
 </Callout>
 
 <Callout type="warn">

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

@@ -8,10 +8,16 @@ 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.
+ * End-to-end (#2186, #3728): boot the real standalone stack via
+ * `bootSchemaStack` against a pre-seeded "legacy" SQLite DB — `organization_id`
+ * created NOT NULL, and a platform-wide UNIQUE index on a field metadata now
+ * scopes per tenant — then verify `os migrate`'s engine detects BOTH drifts and
+ * reconciles them, exercising the full createStandaloneStack → AppPlugin →
+ * ObjectQL → driver path.
+ *
+ * The index half is the #3728 acceptance: under `NODE_ENV=production` nothing
+ * self-heals at boot, so whatever `plan` reports here is exactly what an
+ * operator would see before any DDL touches their database.
  */
 describe('bootSchemaStack + migrate engine (integration)', () => {
   let dir: string;
@@ -36,13 +42,15 @@ describe('bootSchemaStack + migrate engine (integration)', () => {
             fields: {
               name: { type: 'text', required: true },
               organization_id: { type: 'text', required: false }, // optional now
+              code: { type: 'text', unique: true }, // tenant-scoped since #3696
             },
           },
         ],
       }),
     );
 
-    // Seed a "legacy" DB where organization_id is NOT NULL (the #2178 shape).
+    // Seed a "legacy" DB: organization_id NOT NULL (the #2178 shape) plus the
+    // platform-wide unique index on `code` the pre-#3696 driver emitted.
     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) => {
@@ -51,8 +59,10 @@ describe('bootSchemaStack + migrate engine (integration)', () => {
       t.timestamp('updated_at');
       t.string('name').notNullable();
       t.string('organization_id').notNullable();
+      t.string('code');
     });
-    await k('mig_biz_unit').insert({ id: '1', name: 'Acme', organization_id: 'org1' });
+    await k.raw('CREATE UNIQUE INDEX mig_biz_unit_code_unique ON mig_biz_unit (code)');
+    await k('mig_biz_unit').insert({ id: '1', name: 'Acme', organization_id: 'org1', code: 'BU-00001' });
     await k.destroy();
 
     savedEnv.OS_ARTIFACT_PATH = process.env.OS_ARTIFACT_PATH;
@@ -67,24 +77,41 @@ describe('bootSchemaStack + migrate engine (integration)', () => {
     try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ }
   });
 
-  it('detects the NOT NULL drift, applies it, and self-verifies in-sync', async () => {
+  it('detects the NOT NULL + legacy-unique drift, applies both, and self-verifies in-sync', async () => {
     const stack = await bootSchemaStack({ databaseUrl: `file:${dbFile}` });
     try {
       expect(stack.driver).toBeTruthy();
       expect(stack.managedTableCount).toBeGreaterThan(0);
 
+      // Boot changed nothing — this is exactly what `os migrate plan` renders.
       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');
 
+      // #3728: the legacy platform-wide unique is PLANNED, not silently rewritten.
+      const idx = drift.find((d) => d.op.type === 'replace_unique_index');
+      expect(idx, 'expected replace_unique_index drift on mig_biz_unit.code').toBeDefined();
+      expect(idx!.category).toBe('safe');
+      expect(idx!.op).toMatchObject({
+        table: 'mig_biz_unit',
+        dropIndexNames: ['mig_biz_unit_code_unique'],
+        createColumns: ['organization_id', 'code'],
+      });
+
       const { applied, skipped } = await stack.driver!.applyMigrationEntries(drift, { allowDestructive: false });
       expect(applied.some((d) => d.op.type === 'relax_not_null')).toBe(true);
+      expect(applied.some((d) => d.op.type === 'replace_unique_index')).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();
+      expect(after.find((d) => d.op.type === 'replace_unique_index')).toBeUndefined();
+
+      // The seeded row survived the reconcile.
+      expect(await (stack.driver as any).count('mig_biz_unit', {})).toBe(1);
     } finally {
       await stack.shutdown();
     }

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

@@ -136,6 +136,17 @@ export function groupByCategory(drift: ManagedDriftEntry[]): Record<DriftCategor
   return out;
 }
 
+/**
+ * What a drift entry acts on. Column ops read `table.column`; index ops (#3728)
+ * name the index instead — a composite unique spans several columns, so the
+ * leading column alone would misrepresent what is about to change.
+ */
+export function driftTarget(d: ManagedDriftEntry): string {
+  const op = d.op as { indexName?: string; createIndexName?: string };
+  const indexName = op.indexName ?? op.createIndexName;
+  return indexName ? `${d.table} [${indexName}]` : `${d.table}.${d.column ?? ''}`;
+}
+
 export function renderPlan(drift: ManagedDriftEntry[]): void {
   const grouped = groupByCategory(drift);
   for (const cat of CATEGORY_ORDER) {
@@ -144,7 +155,7 @@ export function renderPlan(drift: ManagedDriftEntry[]): void {
     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(`    ${meta.color(meta.icon)} ${meta.color(driftTarget(d))} ${chalk.dim(`[${d.op.type}]`)}`);
       console.log(`        ${chalk.dim(d.message)}`);
     }
     console.log('');

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

@@ -11,19 +11,32 @@ export type {
   IntrospectedForeignKey,
 } from './sql-driver.js';
 
-// Managed-schema drift / reconcile (#2186)
+// Managed-schema drift / reconcile (#2186), incl. the index dimension (#3728)
 export {
   diffManagedTable,
   driftKey,
   fieldHasColumn,
   BUILTIN_COLUMNS,
+  buildIndexName,
+  diffManagedIndexes,
+  expectedIndexes,
+  isIndexDriftOp,
+  isManagedIndexName,
+  legacyUniqueIndexNames,
+  legacyUniqueReplacements,
+  normalizeDeclaredIndex,
+  uniqueIndexesFromFields,
+  INDEX_DRIFT_OPS,
 } from './schema-drift.js';
 export type {
   ManagedDriftEntry,
   DriftOp,
   DriftCategory,
   SqlDialectName,
   PhysicalColumn,
+  PhysicalIndex,
+  ExpectedIndex,
+  LegacyUniqueReplacement,
   FieldDef as DriftFieldDef,
 } from './schema-drift.js';