|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#11722] On SQLite, `applied` must mean "this op happened" — not "a rebuild ran". |
| 5 | + * |
| 6 | + * ## The defect |
| 7 | + * |
| 8 | + * `SqlDriver.applyMigrationEntries` splits by dialect and the two arms |
| 9 | + * disagreed about what `applied` means. The in-place arm asks per entry and |
| 10 | + * believes the answer (`applyDriftOpInPlace` returns `false` for an op its |
| 11 | + * dialect cannot do, and the entry goes to `skipped`). The SQLite arm did not |
| 12 | + * ask at all: |
| 13 | + * |
| 14 | + * ```ts |
| 15 | + * await this.rebuildSqliteTablePatched(table, ents); |
| 16 | + * applied.push(...ents); // every entry, unconditionally |
| 17 | + * ``` |
| 18 | + * |
| 19 | + * `rebuildSqliteTablePatched` honours exactly four op types — `relax_not_null`, |
| 20 | + * `tighten_not_null`, `drop_column`, `drop_column_default` — and silently |
| 21 | + * ignores everything else; its own docblock says so for the varchar ops. An |
| 22 | + * ignored op was still pushed into `applied`. |
| 23 | + * |
| 24 | + * ## Why the failure mode is a FALSE GREEN, not an error |
| 25 | + * |
| 26 | + * Nothing throws and nothing is skipped, so every consumer announces work that |
| 27 | + * never happened: `reconcileAndWarnDrift` logs `auto-reconciled <op> on |
| 28 | + * <table>`, and the artifact boot gate prints `↪ migrated <op>`. The finding is |
| 29 | + * still physically present, so the NEXT boot detects it again, reports drift |
| 30 | + * again, and "migrates" it again — a loop with no failing signal anywhere in |
| 31 | + * it. §4 pins that consumer-visible symptom directly. |
| 32 | + * |
| 33 | + * ## Why a test that passes on `main` would prove nothing here |
| 34 | + * |
| 35 | + * The gap is LATENT: it is held closed from two independent directions, neither |
| 36 | + * aware it is holding it. `enforcesVarcharLength` excludes SQLite, so the differ |
| 37 | + * never emits `widen_varchar`/`narrow_varchar` there; and |
| 38 | + * `multiValueColumnTypeIsLoadBearing` excludes SQLite for an unrelated MEASURED |
| 39 | + * reason (a stale textual column does not corrupt the value there), so #11535's |
| 40 | + * `manual_column_type_change` is never emitted on SQLite either. |
| 41 | + * |
| 42 | + * So this suite CONSTRUCTS the reachability instead of waiting for it. It |
| 43 | + * substitutes exactly one thing and nothing else — the differ's dialect guard — |
| 44 | + * by handing the entries straight to `applyMigrationEntries`, which is the |
| 45 | + * public seam `os migrate apply` (`packages/cli/src/commands/migrate/apply.ts`) |
| 46 | + * and the artifact boot gate (`packages/cli/src/utils/artifact-boot-migration.ts`) |
| 47 | + * both call with differ output. Driver, dialect and database are real |
| 48 | + * throughout. Every assertion below FAILS on the pre-fix tree, where the |
| 49 | + * ignored entries arrive in `applied`. |
| 50 | + * |
| 51 | + * ## What this suite deliberately does NOT assert |
| 52 | + * |
| 53 | + * That the rebuild is skipped when it honours nothing. The rebuild |
| 54 | + * re-materializes every kept column's default and the whole declared index set |
| 55 | + * from metadata, so it is not a no-op — suppressing it would change what the |
| 56 | + * reconciler DOES rather than what it REPORTS, which is a different question |
| 57 | + * from this one. |
| 58 | + */ |
| 59 | + |
| 60 | +import { describe, it, expect, afterEach, vi } from 'vitest'; |
| 61 | +import { SqlDriver } from './sql-driver.js'; |
| 62 | +import type { DriftOp, ManagedDriftEntry } from './schema-drift.js'; |
| 63 | + |
| 64 | +describe('SqlDriver SQLite reconcile reports only what the rebuild honoured (#11722)', () => { |
| 65 | + let knexInstance: any; |
| 66 | + |
| 67 | + const makeDriver = (opts: any = {}, Ctor: typeof SqlDriver = SqlDriver) => { |
| 68 | + const d = new Ctor({ |
| 69 | + client: 'better-sqlite3', |
| 70 | + connection: { filename: ':memory:' }, |
| 71 | + useNullAsDefault: true, |
| 72 | + ...opts, |
| 73 | + }); |
| 74 | + knexInstance = (d as any).knex; |
| 75 | + (d as any).logger = { warn: vi.fn(), info: vi.fn() }; |
| 76 | + return d; |
| 77 | + }; |
| 78 | + |
| 79 | + afterEach(async () => { |
| 80 | + await knexInstance?.destroy(); |
| 81 | + knexInstance = undefined; |
| 82 | + }); |
| 83 | + |
| 84 | + /** A drift entry carrying `op`, exactly as the differ would hand one over. */ |
| 85 | + const entryFor = (op: DriftOp, category: ManagedDriftEntry['category']): ManagedDriftEntry => |
| 86 | + ({ |
| 87 | + kind: 'type_mismatch', |
| 88 | + severity: 'error', |
| 89 | + table: op.table, |
| 90 | + column: (op as { column?: string }).column, |
| 91 | + category, |
| 92 | + op, |
| 93 | + message: `${op.type} on ${op.table}.${(op as { column?: string }).column}`, |
| 94 | + }) as ManagedDriftEntry; |
| 95 | + |
| 96 | + const logLines = (driver: SqlDriver): string[] => { |
| 97 | + const l = (driver as any).logger; |
| 98 | + return [...l.warn.mock.calls, ...l.info.mock.calls].map((c: any[]) => String(c[0])); |
| 99 | + }; |
| 100 | + |
| 101 | + // ──────────────────────────────────────────────────────────────────── |
| 102 | + // §1 — the op that already documents its own answer: no reconciler arm |
| 103 | + // ──────────────────────────────────────────────────────────────────── |
| 104 | + describe('§1 an op the rebuild does not honour is reported skipped, not applied', () => { |
| 105 | + it('`manual_column_type_change` — the op whose docblock says "skipped, never applied" — is skipped on SQLite too', async () => { |
| 106 | + const driver = makeDriver(); |
| 107 | + await driver.initObjects([{ name: 'proj_task', fields: { tags: { type: 'string' } } }]); |
| 108 | + await knexInstance('proj_task').insert({ id: '1', tags: '["a","b"]' }); |
| 109 | + |
| 110 | + const before = await knexInstance('proj_task').columnInfo(); |
| 111 | + |
| 112 | + const op: DriftOp = { |
| 113 | + type: 'manual_column_type_change', |
| 114 | + table: 'proj_task', |
| 115 | + column: 'tags', |
| 116 | + from: 'varchar', |
| 117 | + to: 'json', |
| 118 | + }; |
| 119 | + const { applied, skipped } = await driver.applyMigrationEntries([entryFor(op, 'needs_confirm')], { |
| 120 | + allowDestructive: false, |
| 121 | + }); |
| 122 | + |
| 123 | + // The whole point: the entry is reported for what actually happened to it. |
| 124 | + expect(applied).toHaveLength(0); |
| 125 | + expect(skipped.map((d) => d.op.type)).toEqual(['manual_column_type_change']); |
| 126 | + |
| 127 | + // …and "what actually happened to it" is: nothing. The physical column is |
| 128 | + // byte-for-byte the one it was, which is why reporting it applied was a |
| 129 | + // lie rather than a naming quibble. |
| 130 | + expect(await knexInstance('proj_task').columnInfo()).toEqual(before); |
| 131 | + |
| 132 | + // Reported in the SAME words the in-place arm uses for an op its dialect |
| 133 | + // cannot perform — one greppable sentence across all three dialects. |
| 134 | + expect(logLines(driver).some((m) => /manual_column_type_change on proj_task\.tags is unsupported on dialect 'sqlite' — skipped/.test(m))).toBe(true); |
| 135 | + }); |
| 136 | + |
| 137 | + it('`widen_varchar` — a SAFE op the rebuild ignores by design — is skipped, so dev auto-reconcile cannot announce it', async () => { |
| 138 | + const driver = makeDriver(); |
| 139 | + await driver.initObjects([{ name: 'proj_task', fields: { title: { type: 'string', maxLength: 60 } } }]); |
| 140 | + |
| 141 | + const op: DriftOp = { type: 'widen_varchar', table: 'proj_task', column: 'title', from: 30, to: 60 }; |
| 142 | + const { applied, skipped } = await driver.applyMigrationEntries([entryFor(op, 'safe')], { |
| 143 | + allowDestructive: false, |
| 144 | + }); |
| 145 | + |
| 146 | + expect(applied).toHaveLength(0); |
| 147 | + expect(skipped.map((d) => d.op.type)).toEqual(['widen_varchar']); |
| 148 | + }); |
| 149 | + }); |
| 150 | + |
| 151 | + // ──────────────────────────────────────────────────────────────────── |
| 152 | + // §2 — a MIXED batch: the rebuild still runs and still reports honestly |
| 153 | + // ──────────────────────────────────────────────────────────────────── |
| 154 | + describe('§2 a mixed batch splits — the rebuild runs, and only its own work is reported applied', () => { |
| 155 | + it('relaxes NOT NULL for real while reporting the ignored sibling as skipped', async () => { |
| 156 | + const driver = makeDriver(); |
| 157 | + await knexInstance.schema.createTable('biz_unit', (t: any) => { |
| 158 | + t.string('id').primary(); |
| 159 | + t.timestamp('created_at'); |
| 160 | + t.timestamp('updated_at'); |
| 161 | + t.string('name'); |
| 162 | + t.string('organization_id').notNullable(); |
| 163 | + }); |
| 164 | + await knexInstance('biz_unit').insert({ id: '1', name: 'Acme', organization_id: 'org1' }); |
| 165 | + await driver.initObjects([ |
| 166 | + { |
| 167 | + name: 'biz_unit', |
| 168 | + fields: { |
| 169 | + name: { type: 'string' }, |
| 170 | + organization_id: { type: 'string', required: false }, |
| 171 | + }, |
| 172 | + }, |
| 173 | + ]); |
| 174 | + |
| 175 | + // The relax entry is REAL differ output — only the ignored sibling is |
| 176 | + // constructed, and only because the differ's SQLite guard withholds it. |
| 177 | + const drift = await driver.detectManagedDrift(); |
| 178 | + const relax = drift.find((d) => d.op.type === 'relax_not_null'); |
| 179 | + expect(relax, 'fixture must produce a real relax_not_null entry').toBeDefined(); |
| 180 | + |
| 181 | + const ignored = entryFor( |
| 182 | + { type: 'narrow_varchar', table: 'biz_unit', column: 'name', from: 255, to: 40 }, |
| 183 | + 'destructive', |
| 184 | + ); |
| 185 | + |
| 186 | + const { applied, skipped } = await driver.applyMigrationEntries([relax!, ignored], { |
| 187 | + allowDestructive: true, |
| 188 | + }); |
| 189 | + |
| 190 | + expect(applied.map((d) => d.op.type)).toEqual(['relax_not_null']); |
| 191 | + expect(skipped.map((d) => d.op.type)).toEqual(['narrow_varchar']); |
| 192 | + |
| 193 | + // The rebuild genuinely ran: the constraint is gone and the row survived. |
| 194 | + const info = await knexInstance('biz_unit').columnInfo(); |
| 195 | + expect(info.organization_id.nullable).toBe(true); |
| 196 | + expect(info).toHaveProperty('name'); |
| 197 | + expect(await knexInstance('biz_unit').select('*')).toMatchObject([ |
| 198 | + { id: '1', name: 'Acme', organization_id: 'org1' }, |
| 199 | + ]); |
| 200 | + }); |
| 201 | + }); |
| 202 | + |
| 203 | + // ──────────────────────────────────────────────────────────────────── |
| 204 | + // §3 — the ratchet: the honoured set is EXACTLY the four the rebuild acts on |
| 205 | + // ──────────────────────────────────────────────────────────────────── |
| 206 | + describe('§3 every column op type is partitioned by what the rebuild actually does', () => { |
| 207 | + it('honours exactly relax/tighten NOT NULL, drop_column, drop_column_default — and skips the rest', async () => { |
| 208 | + const driver = makeDriver(); |
| 209 | + await knexInstance.schema.createTable('ratchet_t', (t: any) => { |
| 210 | + t.string('id').primary(); |
| 211 | + t.string('a').notNullable(); |
| 212 | + t.string('b'); |
| 213 | + t.string('c'); |
| 214 | + t.string('d').defaultTo('current_user'); |
| 215 | + t.string('e'); |
| 216 | + t.string('f'); |
| 217 | + t.string('g'); |
| 218 | + }); |
| 219 | + await driver.initObjects([ |
| 220 | + { |
| 221 | + name: 'ratchet_t', |
| 222 | + fields: { |
| 223 | + a: { type: 'string' }, |
| 224 | + b: { type: 'string' }, |
| 225 | + d: { type: 'string' }, |
| 226 | + e: { type: 'string' }, |
| 227 | + f: { type: 'string' }, |
| 228 | + g: { type: 'string' }, |
| 229 | + }, |
| 230 | + }, |
| 231 | + ]); |
| 232 | + |
| 233 | + // One entry per COLUMN op type in `DriftOp`, each on its own column so no |
| 234 | + // two can interfere. Index ops take a different path entirely and are not |
| 235 | + // this partition's business. |
| 236 | + const batch: Array<[DriftOp, ManagedDriftEntry['category']]> = [ |
| 237 | + [{ type: 'relax_not_null', table: 'ratchet_t', column: 'a' }, 'safe'], |
| 238 | + [{ type: 'tighten_not_null', table: 'ratchet_t', column: 'b' }, 'destructive'], |
| 239 | + [{ type: 'drop_column', table: 'ratchet_t', column: 'c' }, 'destructive'], |
| 240 | + [{ type: 'drop_column_default', table: 'ratchet_t', column: 'd' }, 'safe'], |
| 241 | + [{ type: 'widen_varchar', table: 'ratchet_t', column: 'e', from: 30, to: 60 }, 'safe'], |
| 242 | + [{ type: 'narrow_varchar', table: 'ratchet_t', column: 'f', from: 60, to: 30 }, 'destructive'], |
| 243 | + [ |
| 244 | + { type: 'manual_column_type_change', table: 'ratchet_t', column: 'g', from: 'varchar', to: 'json' }, |
| 245 | + 'needs_confirm', |
| 246 | + ], |
| 247 | + ]; |
| 248 | + |
| 249 | + const { applied, skipped } = await driver.applyMigrationEntries( |
| 250 | + batch.map(([op, cat]) => entryFor(op, cat)), |
| 251 | + { allowDestructive: true }, |
| 252 | + ); |
| 253 | + |
| 254 | + expect(applied.map((d) => d.op.type).sort()).toEqual( |
| 255 | + ['drop_column', 'drop_column_default', 'relax_not_null', 'tighten_not_null'].sort(), |
| 256 | + ); |
| 257 | + expect(skipped.map((d) => d.op.type).sort()).toEqual( |
| 258 | + ['manual_column_type_change', 'narrow_varchar', 'widen_varchar'].sort(), |
| 259 | + ); |
| 260 | + |
| 261 | + // Positive control on the applied half — a partition that reported |
| 262 | + // "applied" for work that did not happen is the defect itself, so the |
| 263 | + // four honoured ops are checked against the physical table. |
| 264 | + const info = await knexInstance('ratchet_t').columnInfo(); |
| 265 | + expect(info).not.toHaveProperty('c'); |
| 266 | + expect(info.a.nullable).toBe(true); |
| 267 | + expect(info.b.nullable).toBe(false); |
| 268 | + expect(info.d.defaultValue ?? null).toBeNull(); |
| 269 | + }); |
| 270 | + }); |
| 271 | + |
| 272 | + // ──────────────────────────────────────────────────────────────────── |
| 273 | + // §4 — the consumer-visible symptom the card is actually about |
| 274 | + // ──────────────────────────────────────────────────────────────────── |
| 275 | + describe('§4 dev auto-reconcile no longer announces an op that never happened', () => { |
| 276 | + /** |
| 277 | + * The differ cannot emit a non-rebuildable column op on SQLite today, so |
| 278 | + * the future in which it can is simulated at the ONE seam that holds it |
| 279 | + * closed: `detectTableDrift`. Everything downstream of it — the reconcile, |
| 280 | + * the rebuild, the logging, the re-detect — is the real code path. |
| 281 | + */ |
| 282 | + class FutureOpDriver extends SqlDriver { |
| 283 | + public nextDrift: ManagedDriftEntry[] = []; |
| 284 | + protected async detectTableDrift(): Promise<ManagedDriftEntry[]> { |
| 285 | + return this.nextDrift; |
| 286 | + } |
| 287 | + } |
| 288 | + |
| 289 | + it('logs "unsupported … skipped" instead of "auto-reconciled", and the finding is still warned about', async () => { |
| 290 | + const driver = makeDriver({ autoMigrate: 'safe', schemaMode: 'managed' }, FutureOpDriver) as FutureOpDriver; |
| 291 | + await driver.initObjects([{ name: 'biz_unit', fields: { name: { type: 'string', maxLength: 60 } } }]); |
| 292 | + |
| 293 | + driver.nextDrift = [ |
| 294 | + entryFor({ type: 'widen_varchar', table: 'biz_unit', column: 'name', from: 30, to: 60 }, 'safe'), |
| 295 | + ]; |
| 296 | + // Re-detect after the reconcile returns the SAME entry, because the op |
| 297 | + // was never performed — which is precisely the loop this card describes. |
| 298 | + await (driver as any).reconcileAndWarnDrift('biz_unit', { name: { type: 'string', maxLength: 60 } }); |
| 299 | + |
| 300 | + const lines = logLines(driver); |
| 301 | + expect(lines.some((m) => m.includes('auto-reconciled'))).toBe(false); |
| 302 | + expect(lines.some((m) => /widen_varchar on biz_unit\.name is unsupported on dialect 'sqlite' — skipped/.test(m))).toBe(true); |
| 303 | + // Still surfaced as drift — reporting it skipped does not hide it. |
| 304 | + expect(lines.some((m) => m.includes('widen_varchar on biz_unit.name'))).toBe(true); |
| 305 | + }); |
| 306 | + }); |
| 307 | +}); |
0 commit comments