From bdd713c20911cf21254b9a8d99b37743f3841c99 Mon Sep 17 00:00:00 2001 From: cultron Date: Mon, 26 Oct 2026 16:32:57 -0500 Subject: [PATCH 01/14] feat(migrate): review generated migrations for data loss; verify --seed checks rows survive TypeORM's migration:generate changes a column type by DROP + ADD, casts enums without remapping removed values, backfills NOT NULL columns with their default, and builds a renamed entity as a new empty table. migrate generate and verify now flag those statements, and verify --seed runs the migration against seeded rows and fails if rows or values go missing (following RENAME TO / RENAME COLUMN). Fixtures are TypeORM's real output for five changes. --- CLAUDE.md | 2 +- src/commands/migrate/verify.ts | 15 +- src/lib/verify/index.ts | 100 +++++++++++++- src/lib/verify/review.ts | 128 ++++++++++++++++++ .../typeorm-migrations/enum-replace.ts | 24 ++++ .../typeorm-migrations/rename-entity.ts | 24 ++++ .../typeorm-migrations/rename-field.ts | 14 ++ .../typeorm-migrations/required-field.ts | 16 +++ .../typeorm-migrations/type-change.ts | 16 +++ test/lib/verify/review.test.ts | 115 ++++++++++++++++ 10 files changed, 447 insertions(+), 7 deletions(-) create mode 100644 src/lib/verify/review.ts create mode 100644 test/fixtures/typeorm-migrations/enum-replace.ts create mode 100644 test/fixtures/typeorm-migrations/rename-entity.ts create mode 100644 test/fixtures/typeorm-migrations/rename-field.ts create mode 100644 test/fixtures/typeorm-migrations/required-field.ts create mode 100644 test/fixtures/typeorm-migrations/type-change.ts create mode 100644 test/lib/verify/review.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 03cfd36..e35e67f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -27,7 +27,7 @@ apso dev # Start local dev server via Docker Compose # Schema apso migrate # Test schema migrations locally (PGlite sandbox) apso migrate generate # Migration from a deployed .apsorc to this project's entities (TypeScript, PGlite) -apso migrate verify # Run a migration on the deployed schema, then check for drift +apso migrate verify # Run a migration on the deployed schema (optionally seeded with --seed rows), then check data loss and drift; generate and verify both warn on statements that lose data apso test smoke # Start the service; call /health, list, and create/read/delete each entity (TS on PGlite, Python/Go on DATABASE_URL) apso schema validate # Validate local .apsorc (includes lint) apso schema lint # Lint .apsorc with @apso/schema-tools (--fix, --json) diff --git a/src/commands/migrate/verify.ts b/src/commands/migrate/verify.ts index 39a62b4..ddbec7e 100644 --- a/src/commands/migrate/verify.ts +++ b/src/commands/migrate/verify.ts @@ -15,6 +15,7 @@ export default class MigrateVerify extends BaseCommand { static examples = [ `$ apso migrate verify --baseline deployed.apsorc --migration src/migrations/1700000000000-SchemaUpdate-migration.ts`, + `$ apso migrate verify --baseline deployed.apsorc --migration src/migrations/1700000000000-SchemaUpdate-migration.ts --seed seed.sql`, ]; static flags = { @@ -26,6 +27,14 @@ export default class MigrateVerify extends BaseCommand { description: "The migration file to run", required: true, }), + seed: Flags.string({ + description: + "SQL file of rows to insert into the deployed schema before the migration runs; the check then fails if the migration loses any of them", + }), + "accept-loss": Flags.boolean({ + description: "Report seeded rows or values the migration loses without failing", + default: false, + }), "skip-build": Flags.boolean({ description: "Use the existing dist/ build instead of building first", default: false, @@ -45,7 +54,11 @@ export default class MigrateVerify extends BaseCommand { path.resolve(flags.baseline), flags.migration, cliBinPath(this.config.root), - flags["skip-build"] + { + skipBuild: flags["skip-build"], + seedFile: flags.seed, + acceptLoss: flags["accept-loss"], + } ); report( (s) => this.log(s), diff --git a/src/lib/verify/index.ts b/src/lib/verify/index.ts index 7da48cd..5638aea 100644 --- a/src/lib/verify/index.ts +++ b/src/lib/verify/index.ts @@ -17,6 +17,7 @@ import { TargetLanguage } from "../types/generator"; import { RelationshipMap } from "../types/relationship"; import { kebabCase } from "../utils/casing"; import { planCrud } from "./crud-plan"; +import { Finding, reviewMigration } from "./review"; export interface Step { name: string; @@ -144,6 +145,8 @@ export interface GenerateResult { needed: boolean; file?: string; steps: Step[]; + /** Statements in the generated migration that lose data or fail on rows. */ + findings?: Finding[]; } /** Generate a migration from the deployed schema (baselineApsorc) to the project's entities. */ @@ -190,6 +193,9 @@ export async function generateMigration( needed: true, file: file && path.join("src", "migrations", file), steps, + findings: file + ? reviewMigration(fs.readFileSync(path.join(migrationsDir, file), "utf8")) + : undefined, }; } finally { fs.rmSync(dataDir, { recursive: true, force: true }); @@ -212,6 +218,61 @@ ds.initialize() export interface CheckResult { ok: boolean; steps: Step[]; + findings?: Finding[]; +} + +// Row and non-null value counts per table and column of the PGlite database +// at PGLITE_DATA_DIR, after running the SQL file in argv[1] if given. +const COUNT_ROWS = ` +const fs = require("fs"); +const { PGlite } = require("@electric-sql/pglite"); +(async () => { + const db = new PGlite(process.env.PGLITE_DATA_DIR); + if (process.argv[1]) await db.exec(fs.readFileSync(process.argv[1], "utf8")); + const cols = (await db.query("SELECT table_name, column_name FROM information_schema.columns WHERE table_schema = 'public' AND table_name <> 'migrations'")).rows; + const out = {}; + for (const { table_name: t, column_name: c } of cols) { + const r = (await db.query('SELECT count(*)::int AS rows, count("' + c + '")::int AS vals FROM "' + t + '"')).rows[0]; + out[t] = out[t] || { rows: r.rows, values: {} }; + out[t].values[c] = r.vals; + } + console.log(JSON.stringify(out)); + await db.close(); +})().catch((e) => { console.error(e.message); process.exit(1); }); +`; + +type Counts = Record }>; + +/** + * Rows and values present before the migration and missing after it, + * following the tables and columns the migration renames. + */ +export function dataLoss(before: Counts, after: Counts, source: string): string[] { + const tables = new Map( + [...source.matchAll(/ALTER TABLE "([^"]+)" RENAME TO "([^"]+)"/g)].map((m) => [m[1], m[2]]) + ); + const columns = new Map( + [...source.matchAll(/ALTER TABLE "([^"]+)" RENAME COLUMN "([^"]+)" TO "([^"]+)"/g)].map( + (m) => [`${m[1]}.${m[2]}`, m[3]] + ) + ); + const lost: string[] = []; + for (const [t, { rows, values }] of Object.entries(before)) { + if (!rows) continue; + const t2 = tables.get(t) ?? t; + const now = after[t2]; + if (!now) { + lost.push(`table ${t}: all ${rows} rows`); + continue; + } + if (now.rows < rows) lost.push(`table ${t}: ${rows - now.rows} of ${rows} rows`); + for (const [c, n] of Object.entries(values)) { + const c2 = columns.get(`${t}.${c}`) ?? columns.get(`${t2}.${c}`) ?? c; + const kept = now.values[c2] ?? 0; + if (n > kept) lost.push(`${t}.${c}: ${n - kept} of ${n} values`); + } + } + return lost; } /** @@ -250,9 +311,15 @@ export async function verifyMigration( baselineApsorc: string, migrationFile: string, cliBin: string, - skipBuild = false + { + skipBuild = false, + seedFile, + acceptLoss = false, + }: { skipBuild?: boolean; seedFile?: string; acceptLoss?: boolean } = {} ): Promise { if (!supportsPglite(projectDir)) return NO_PGLITE; + const source = fs.readFileSync(path.resolve(projectDir, migrationFile), "utf8"); + const findings = reviewMigration(source); const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "apso-pglite-")); const steps: Step[] = []; const done = (s: Step) => { @@ -261,7 +328,15 @@ export async function verifyMigration( }; try { if (!done(await buildBaseline(projectDir, baselineApsorc, dataDir, cliBin))) - return { ok: false, steps }; + return { ok: false, steps, findings }; + const count = async (name: string, sql?: string) => { + const r = await run(process.execPath, ["-e", COUNT_ROWS, ...(sql ? [sql] : [])], projectDir, pgliteEnv(dataDir)); + done(step(name, r)); + return r.ok ? (JSON.parse(r.output.trim().split("\n").pop()!) as Counts) : undefined; + }; + // Seed rows make the migration face the data it will meet in production. + const before = seedFile ? await count("seed", path.resolve(seedFile)) : undefined; + if (seedFile && !before) return { ok: false, steps, findings }; const compiled = path .join("dist", path.relative("src", migrationFile)) .replace(/\.ts$/, ".js"); @@ -269,7 +344,7 @@ export async function verifyMigration( const needsBuild = !skipBuild || !fs.existsSync(path.join(projectDir, compiled)); if (needsBuild && !done(step("build", await build(projectDir)))) - return { ok: false, steps }; + return { ok: false, steps, findings }; const migrate = await run( process.execPath, ["-e", RUN_ONE_MIGRATION, compiled], @@ -277,14 +352,28 @@ export async function verifyMigration( pgliteEnv(dataDir) ); if (!done({ ...step("migrate", migrate), output: tail(migrate.output) })) - return { ok: false, steps }; + return { ok: false, steps, findings }; + if (before) { + const after = await count("count"); + if (!after) return { ok: false, steps, findings }; + const lost = dataLoss(before, after, source); + const ok = lost.length === 0 || acceptLoss; + done({ + name: "data", + ok, + output: lost.length > 0 + ? `The migration lost seeded data:\n${lost.join("\n")}${ok ? "" : "\nFix the migration, or rerun with --accept-loss if the loss is intended."}` + : undefined, + }); + if (!ok) return { ok: false, steps, findings }; + } const drift = await typeorm( projectDir, ["migration:generate", "./src/migrations/drift", "--check"], pgliteEnv(dataDir) ); - return { ok: done(step("drift", drift)), steps }; + return { ok: done(step("drift", drift)), steps, findings }; } finally { fs.rmSync(dataDir, { recursive: true, force: true }); } @@ -669,5 +758,6 @@ export function report( log(`${s.ok ? "ok " : "FAIL"} ${s.name}`); if (s.output) log(s.output.replace(/^/gm, " ")); } + for (const f of result.findings ?? []) log(`WARN ${f.code}: ${f.message}`); if (summary) log(summary); } diff --git a/src/lib/verify/review.ts b/src/lib/verify/review.ts new file mode 100644 index 0000000..43da123 --- /dev/null +++ b/src/lib/verify/review.ts @@ -0,0 +1,128 @@ +/** + * Review a TypeORM-generated migration for statements that lose data or fail + * on a populated database. TypeORM changes a column type by DROP + ADD, casts + * enums without remapping removed values, and adds NOT NULL columns as if the + * table were empty. The generated file is kept; each finding names a + * statement to rewrite. + */ + +export interface Finding { + code: string; + table: string; + column?: string; + message: string; +} + +/** The SQL of each queryRunner.query call in a method body. */ +const queries = (body: string) => + [...body.matchAll(/queryRunner\.query\(\s*`([\S\s]*?)`/g)].map((m) => m[1]); + +const enumValues = (sql?: string) => + new Set( + [...(sql?.match(/ENUM\(([^)]*)\)/)?.[1] ?? "").matchAll(/'([^']*)'/g)].map( + (v) => v[1] + ) + ); + +const typeSql = (sqls: string[], type: string) => + sqls.find((s) => s.startsWith(`CREATE TYPE "public"."${type}" AS ENUM`)); + +const list = (values: string[]) => values.map((v) => `"${v}"`).join(", "); + +export function reviewMigration(source: string): Finding[] { + const [upBody, downBody = ""] = source.split(/public\s+async\s+down\s*\(/); + const up = queries(upBody); + const down = queries(downBody); + const findings: Finding[] = []; + const add = (code: string, table: string, column: string | undefined, message: string) => + findings.push({ code, table, column, message }); + + // Statements on tables this migration creates touch no existing rows. + const created = new Set( + up.map((q) => q.match(/^CREATE TABLE "([^"]+)"/)?.[1]).filter(Boolean) + ); + const existing = (table: string) => !created.has(table); + + const dropped = new Map(); + const added = new Map(); + for (const q of up) { + const d = q.match(/^ALTER TABLE "([^"]+)" DROP COLUMN "([^"]+)"/); + if (d) dropped.set(d[1], [...(dropped.get(d[1]) ?? []), d[2]]); + const a = q.match(/^ALTER TABLE "([^"]+)" ADD "([^"]+)"/); + if (a) added.set(a[1], [...(added.get(a[1]) ?? []), a[2]]); + } + for (const [table, cols] of dropped) { + const adds = added.get(table) ?? []; + const newCols = adds.filter((a) => !cols.includes(a)); + for (const col of cols) { + if (adds.includes(col)) + add("TYPE_CHANGE_DROPS_DATA", table, col, + `${table}.${col}: TypeORM drops and re-adds the column to change its type, deleting every value. Replace the DROP and ADD with ALTER COLUMN "${col}" TYPE ... USING a cast that converts the existing values.`); + else if (newCols.length > 0) + add("POSSIBLE_RENAME", table, col, + `${table}.${col} is dropped while ${list(newCols)} is added. If that is a rename, replace the DROP and ADD with RENAME COLUMN to keep the data; otherwise every value in "${col}" is deleted.`); + else + add("DROPS_COLUMN", table, col, + `${table}.${col} is dropped with all its values. Confirm the loss, or copy the data first.`); + } + } + + for (const q of up) { + let m = q.match(/^DROP TABLE "([^"]+)"/); + if (m) + add("DROPS_TABLE", m[1], undefined, + `Table ${m[1]} is dropped with all its rows. Confirm the loss, or rename the table if an entity was renamed.`); + + m = q.match(/^ALTER TABLE "([^"]+)" ADD "([^"]+)" [^,]*\bNOT NULL\b(.*)$/); + if (m && existing(m[1])) { + const def = m[3].match(/DEFAULT (.+)$/)?.[1].trim(); + if (def) + add("DEFAULT_BACKFILLS_ROWS", m[1], m[2], + `${m[1]}.${m[2]} is added NOT NULL DEFAULT ${def}, so every existing row gets ${def}. Confirm that is right for existing rows, or add the column nullable, backfill it, then SET NOT NULL.`); + else + add("NOT_NULL_WITHOUT_BACKFILL", m[1], m[2], + `${m[1]}.${m[2]} is added NOT NULL with no default, which fails if ${m[1]} has any rows. Add it nullable, backfill it, then SET NOT NULL.`); + } + + m = q.match(/^ALTER TABLE "([^"]+)" ALTER COLUMN "([^"]+)" SET NOT NULL/); + if (m && existing(m[1])) + add("SET_NOT_NULL", m[1], m[2], + `${m[1]}.${m[2]} becomes NOT NULL, which fails on any row where it is null. Backfill those rows first.`); + + m = q.match(/^ALTER TABLE "([^"]+)" ALTER COLUMN "([^"]+)" TYPE (?!"public"\."[^"]+_enum")/); + if (m && existing(m[1])) + add("TYPE_CAST", m[1], m[2], + `${m[1]}.${m[2]} changes type in place; a value the cast cannot convert fails the migration. Check the USING clause handles every existing value.`); + + m = + q.match(/^CREATE UNIQUE INDEX "[^"]+" ON "([^"]+)" \(([^)]+)\)/) ?? + q.match(/^ALTER TABLE "([^"]+)" ADD CONSTRAINT "[^"]+" UNIQUE \(([^)]+)\)/); + if (m && existing(m[1])) + add("UNIQUE_ON_EXISTING_ROWS", m[1], m[2].replace(/"/g, ""), + `A unique constraint on ${m[1]} (${m[2].replace(/"/g, "")}) fails if existing rows hold duplicates. Check for duplicates first and decide which row wins.`); + + m = q.match(/^ALTER TABLE "([^"]+)" ADD CONSTRAINT "[^"]+" FOREIGN KEY \("([^"]+)"\) REFERENCES "([^"]+)"/); + if (m && existing(m[1]) && created.has(m[3])) + add("FOREIGN_KEY_TO_NEW_TABLE", m[1], m[2], + `Existing ${m[1]} rows must point at ${m[3]}, which this migration creates empty, so the foreign key fails on any row with ${m[2]} set. If ${m[3]} replaces an existing entity, replace the CREATE TABLE with ALTER TABLE "" RENAME TO "${m[3]}" and ALTER SEQUENCE "_id_seq" RENAME TO "${m[3]}_id_seq"; TypeORM leaves the old table in place.`); + } + + // TypeORM swaps an enum by renaming the old type to *_old, creating the new + // one and casting through text; down() recreates *_old with the old values. + for (const q of up) { + const m = q.match(/^ALTER TYPE "public"\."([^"]+)" RENAME TO "([^"]+_old)"/); + if (!m) continue; + const now = enumValues(typeSql(up, m[1])); + const cast = up.find((s) => s.includes(`TYPE "public"."${m[1]}" USING`)); + // A USING clause that names a removed value maps it; the rest still fail. + const removed = [...enumValues(typeSql(down, m[2]))].filter( + (v) => !now.has(v) && !cast?.includes(`'${v}'`) + ); + const col = cast?.match(/^ALTER TABLE "([^"]+)" ALTER COLUMN "([^"]+)"/); + if (removed.length > 0 && col) + add("ENUM_VALUES_REMOVED", col[1], col[2], + `${col[1]}.${col[2]} drops enum value ${removed.map((v) => `'${v}'`).join(", ")}; the cast fails on any row that still holds it. Map those rows to a remaining value in the USING clause, e.g. CASE "${col[2]}"::text WHEN '${removed[0]}' THEN '...' ELSE "${col[2]}"::text END.`); + } + + return findings; +} diff --git a/test/fixtures/typeorm-migrations/enum-replace.ts b/test/fixtures/typeorm-migrations/enum-replace.ts new file mode 100644 index 0000000..a13558c --- /dev/null +++ b/test/fixtures/typeorm-migrations/enum-replace.ts @@ -0,0 +1,24 @@ +import { MigrationInterface, QueryRunner } from "typeorm"; + +export class SchemaUpdateMigration1793049700161 implements MigrationInterface { + name = 'SchemaUpdateMigration1793049700161' + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TYPE "public"."contact_status_enum" RENAME TO "contact_status_enum_old"`); + await queryRunner.query(`CREATE TYPE "public"."contact_status_enum" AS ENUM('lead', 'active', 'closed')`); + await queryRunner.query(`ALTER TABLE "contact" ALTER COLUMN "status" DROP DEFAULT`); + await queryRunner.query(`ALTER TABLE "contact" ALTER COLUMN "status" TYPE "public"."contact_status_enum" USING "status"::"text"::"public"."contact_status_enum"`); + await queryRunner.query(`ALTER TABLE "contact" ALTER COLUMN "status" SET DEFAULT 'lead'`); + await queryRunner.query(`DROP TYPE "public"."contact_status_enum_old"`); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query(`CREATE TYPE "public"."contact_status_enum_old" AS ENUM('lead', 'active', 'archived')`); + await queryRunner.query(`ALTER TABLE "contact" ALTER COLUMN "status" DROP DEFAULT`); + await queryRunner.query(`ALTER TABLE "contact" ALTER COLUMN "status" TYPE "public"."contact_status_enum_old" USING "status"::"text"::"public"."contact_status_enum_old"`); + await queryRunner.query(`ALTER TABLE "contact" ALTER COLUMN "status" SET DEFAULT 'lead'`); + await queryRunner.query(`DROP TYPE "public"."contact_status_enum"`); + await queryRunner.query(`ALTER TYPE "public"."contact_status_enum_old" RENAME TO "contact_status_enum"`); + } + +} diff --git a/test/fixtures/typeorm-migrations/rename-entity.ts b/test/fixtures/typeorm-migrations/rename-entity.ts new file mode 100644 index 0000000..55eb007 --- /dev/null +++ b/test/fixtures/typeorm-migrations/rename-entity.ts @@ -0,0 +1,24 @@ +import { MigrationInterface, QueryRunner } from "typeorm"; + +export class SchemaUpdateMigration1793049700178 implements MigrationInterface { + name = 'SchemaUpdateMigration1793049700178' + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "contact" DROP CONSTRAINT "FK_196374b312744cf2c400c962383"`); + await queryRunner.query(`DROP INDEX "public"."IDX_196374b312744cf2c400c96238"`); + await queryRunner.query(`ALTER TABLE "contact" RENAME COLUMN "companyId" TO "organizationId"`); + await queryRunner.query(`CREATE TABLE "organization" ("id" SERIAL NOT NULL, "created_at" TIMESTAMP NOT NULL DEFAULT now(), "updated_at" TIMESTAMP NOT NULL DEFAULT now(), "name" text NOT NULL, CONSTRAINT "PK_472c1f99a32def1b0abb219cd67" PRIMARY KEY ("id"))`); + await queryRunner.query(`CREATE INDEX "IDX_7719d73cd16a9f57ecc6ac24b3" ON "contact" ("organizationId") `); + await queryRunner.query(`ALTER TABLE "contact" ADD CONSTRAINT "FK_7719d73cd16a9f57ecc6ac24b3d" FOREIGN KEY ("organizationId") REFERENCES "organization"("id") ON DELETE NO ACTION ON UPDATE NO ACTION`); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "contact" DROP CONSTRAINT "FK_7719d73cd16a9f57ecc6ac24b3d"`); + await queryRunner.query(`DROP INDEX "public"."IDX_7719d73cd16a9f57ecc6ac24b3"`); + await queryRunner.query(`DROP TABLE "organization"`); + await queryRunner.query(`ALTER TABLE "contact" RENAME COLUMN "organizationId" TO "companyId"`); + await queryRunner.query(`CREATE INDEX "IDX_196374b312744cf2c400c96238" ON "contact" ("companyId") `); + await queryRunner.query(`ALTER TABLE "contact" ADD CONSTRAINT "FK_196374b312744cf2c400c962383" FOREIGN KEY ("companyId") REFERENCES "company"("id") ON DELETE NO ACTION ON UPDATE NO ACTION`); + } + +} diff --git a/test/fixtures/typeorm-migrations/rename-field.ts b/test/fixtures/typeorm-migrations/rename-field.ts new file mode 100644 index 0000000..65070b8 --- /dev/null +++ b/test/fixtures/typeorm-migrations/rename-field.ts @@ -0,0 +1,14 @@ +import { MigrationInterface, QueryRunner } from "typeorm"; + +export class SchemaUpdateMigration1793049717243 implements MigrationInterface { + name = 'SchemaUpdateMigration1793049717243' + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "contact" RENAME COLUMN "phone" TO "mobilePhone"`); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "contact" RENAME COLUMN "mobilePhone" TO "phone"`); + } + +} diff --git a/test/fixtures/typeorm-migrations/required-field.ts b/test/fixtures/typeorm-migrations/required-field.ts new file mode 100644 index 0000000..2fe54f6 --- /dev/null +++ b/test/fixtures/typeorm-migrations/required-field.ts @@ -0,0 +1,16 @@ +import { MigrationInterface, QueryRunner } from "typeorm"; + +export class SchemaUpdateMigration1793049726507 implements MigrationInterface { + name = 'SchemaUpdateMigration1793049726507' + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(`CREATE TYPE "public"."company_region_enum" AS ENUM('us', 'eu', 'apac')`); + await queryRunner.query(`ALTER TABLE "company" ADD "region" "public"."company_region_enum" NOT NULL DEFAULT 'us'`); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "company" DROP COLUMN "region"`); + await queryRunner.query(`DROP TYPE "public"."company_region_enum"`); + } + +} diff --git a/test/fixtures/typeorm-migrations/type-change.ts b/test/fixtures/typeorm-migrations/type-change.ts new file mode 100644 index 0000000..b3ed039 --- /dev/null +++ b/test/fixtures/typeorm-migrations/type-change.ts @@ -0,0 +1,16 @@ +import { MigrationInterface, QueryRunner } from "typeorm"; + +export class SchemaUpdateMigration1793049700136 implements MigrationInterface { + name = 'SchemaUpdateMigration1793049700136' + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "contact" DROP COLUMN "score"`); + await queryRunner.query(`ALTER TABLE "contact" ADD "score" integer`); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "contact" DROP COLUMN "score"`); + await queryRunner.query(`ALTER TABLE "contact" ADD "score" text`); + } + +} diff --git a/test/lib/verify/review.test.ts b/test/lib/verify/review.test.ts new file mode 100644 index 0000000..1f9836b --- /dev/null +++ b/test/lib/verify/review.test.ts @@ -0,0 +1,115 @@ +import { expect, describe, test } from "@jest/globals"; +import * as fs from "fs"; +import * as path from "path"; +import { reviewMigration } from "../../../src/lib/verify/review"; +import { dataLoss } from "../../../src/lib/verify"; + +// Each fixture is what `apso migrate generate` (TypeORM migration:generate) +// wrote for one .apsorc change to a Company/Contact schema. +const fixture = (name: string) => + fs.readFileSync(path.join(__dirname, "../../fixtures/typeorm-migrations", `${name}.ts`), "utf8"); +const codes = (name: string) => + reviewMigration(fixture(name)).map((f) => `${f.code} ${f.table}.${f.column}`); + +describe("reviewMigration on TypeORM output", () => { + test("a field rename TypeORM already emits as RENAME COLUMN is clean", () => { + expect(codes("rename-field")).toEqual([]); + }); + + test("a type change done as DROP + ADD", () => { + expect(codes("type-change")).toEqual(["TYPE_CHANGE_DROPS_DATA contact.score"]); + }); + + test("an enum swap that removes a value", () => { + const [f] = reviewMigration(fixture("enum-replace")); + expect(f.code).toBe("ENUM_VALUES_REMOVED"); + expect(`${f.table}.${f.column}`).toBe("contact.status"); + expect(f.message).toContain("'archived'"); + const mapped = fixture("enum-replace").replace( + `USING "status"::"text"::"public"."contact_status_enum"`, + `USING (CASE "status"::text WHEN 'archived' THEN 'closed' ELSE "status"::text END)::"public"."contact_status_enum"` + ); + expect(reviewMigration(mapped)).toEqual([]); + }); + + test("a required enum field backfilled by its default", () => { + expect(codes("required-field")).toEqual(["DEFAULT_BACKFILLS_ROWS company.region"]); + }); + + test("an entity rename that creates an empty table", () => { + expect(codes("rename-entity")).toEqual(["FOREIGN_KEY_TO_NEW_TABLE contact.organizationId"]); + }); +}); + +const migration = (...up: string[]) => + `public async up(queryRunner) {\n${up.map((q) => `await queryRunner.query(\`${q}\`);`).join("\n")}\n}\npublic async down(queryRunner) {}`; + +describe("reviewMigration rules", () => { + test("a dropped column with another added is a possible rename", () => { + const f = reviewMigration( + migration(`ALTER TABLE "contact" DROP COLUMN "phone"`, `ALTER TABLE "contact" ADD "mobile" integer`) + ); + expect(f.map((x) => x.code)).toEqual(["POSSIBLE_RENAME"]); + }); + + test("drops, NOT NULL without default, SET NOT NULL, unique and casts on existing tables", () => { + const f = reviewMigration( + migration( + `ALTER TABLE "company" DROP COLUMN "notes"`, + `DROP TABLE "legacy"`, + `ALTER TABLE "contact" ADD "email" text NOT NULL`, + `ALTER TABLE "contact" ALTER COLUMN "phone" SET NOT NULL`, + `CREATE UNIQUE INDEX "IDX_1" ON "contact" ("email") `, + `ALTER TABLE "contact" ADD CONSTRAINT "UQ_1" UNIQUE ("phone")`, + `ALTER TABLE "contact" ALTER COLUMN "age" TYPE integer USING "age"::integer` + ) + ); + expect(f.map((x) => x.code)).toEqual([ + "DROPS_COLUMN", + "DROPS_TABLE", + "NOT_NULL_WITHOUT_BACKFILL", + "SET_NOT_NULL", + "UNIQUE_ON_EXISTING_ROWS", + "UNIQUE_ON_EXISTING_ROWS", + "TYPE_CAST", + ]); + }); + + test("statements on a table the migration creates are safe", () => { + expect( + reviewMigration( + migration( + `CREATE TABLE "note" ("id" SERIAL NOT NULL, CONSTRAINT "PK_1" PRIMARY KEY ("id"))`, + `ALTER TABLE "note" ADD "body" text NOT NULL`, + `CREATE UNIQUE INDEX "IDX_2" ON "note" ("body") ` + ) + ) + ).toEqual([]); + }); +}); + +describe("dataLoss", () => { + const before = { + company: { rows: 2, values: { id: 2, name: 2 } }, + contact: { rows: 4, values: { id: 4, phone: 3, score: 4 } }, + }; + + test("follows renamed tables and columns", () => { + const after = { + organization: { rows: 2, values: { id: 2, name: 2 } }, + contact: { rows: 4, values: { id: 4, mobilePhone: 3, score: 4 } }, + }; + const sql = `ALTER TABLE "company" RENAME TO "organization"; ALTER TABLE "contact" RENAME COLUMN "phone" TO "mobilePhone"`; + expect(dataLoss(before, after, sql)).toEqual([]); + }); + + test("reports lost rows, tables and values", () => { + const after = { contact: { rows: 3, values: { id: 3, phone: 3, score: 0 } } }; + expect(dataLoss(before, after, "")).toEqual([ + "table company: all 2 rows", + "table contact: 1 of 4 rows", + "contact.id: 1 of 4 values", + "contact.score: 4 of 4 values", + ]); + }); +}); From 0dd4434aa2022fe01f5e0ef7fef028f00a9c0d9a Mon Sep 17 00:00:00 2001 From: cultron Date: Tue, 27 Oct 2026 07:45:07 -0500 Subject: [PATCH 02/14] migrate verify: follow data moved into new tables, --check assertions, per-column --accept-loss A dropped column whose values all turn up in a non-id column of a table the migration creates counts as moved, not lost (field to its own entity, foreign key to a join table). --check runs SELECTs returning ok/label after the migration, to prove transforms the data check can't follow (a split field). --accept-loss now names the table or column whose loss is intended. Review: DROPS_COLUMN names the tables the migration creates and suggests INSERT ... SELECT; a drop or SET NOT NULL after an UPDATE/INSERT that names the column is treated as handled. --- CLAUDE.md | 2 +- src/commands/migrate/verify.ts | 14 +++++-- src/lib/verify/index.ts | 75 +++++++++++++++++++++++++++++----- src/lib/verify/review.ts | 17 +++++--- test/lib/verify/review.test.ts | 43 ++++++++++++++++++- 5 files changed, 130 insertions(+), 21 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e35e67f..e232c52 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -27,7 +27,7 @@ apso dev # Start local dev server via Docker Compose # Schema apso migrate # Test schema migrations locally (PGlite sandbox) apso migrate generate # Migration from a deployed .apsorc to this project's entities (TypeScript, PGlite) -apso migrate verify # Run a migration on the deployed schema (optionally seeded with --seed rows), then check data loss and drift; generate and verify both warn on statements that lose data +apso migrate verify # Run a migration on the deployed schema (optionally seeded with --seed rows), then check data loss (following data copied into new tables), --check assertions and drift; generate and verify both warn on statements that lose data apso test smoke # Start the service; call /health, list, and create/read/delete each entity (TS on PGlite, Python/Go on DATABASE_URL) apso schema validate # Validate local .apsorc (includes lint) apso schema lint # Lint .apsorc with @apso/schema-tools (--fix, --json) diff --git a/src/commands/migrate/verify.ts b/src/commands/migrate/verify.ts index ddbec7e..6b366e1 100644 --- a/src/commands/migrate/verify.ts +++ b/src/commands/migrate/verify.ts @@ -16,6 +16,7 @@ export default class MigrateVerify extends BaseCommand { static examples = [ `$ apso migrate verify --baseline deployed.apsorc --migration src/migrations/1700000000000-SchemaUpdate-migration.ts`, `$ apso migrate verify --baseline deployed.apsorc --migration src/migrations/1700000000000-SchemaUpdate-migration.ts --seed seed.sql`, + `$ apso migrate verify --baseline deployed.apsorc --migration src/migrations/1700000000000-SchemaUpdate-migration.ts --seed seed.sql --check checks.sql --accept-loss contact.fullName`, ]; static flags = { @@ -31,9 +32,15 @@ export default class MigrateVerify extends BaseCommand { description: "SQL file of rows to insert into the deployed schema before the migration runs; the check then fails if the migration loses any of them", }), - "accept-loss": Flags.boolean({ - description: "Report seeded rows or values the migration loses without failing", - default: false, + check: Flags.string({ + description: + "SQL file of SELECTs run after the migration; each row must have a boolean ok (and a label naming the check), e.g. SELECT bool_and(\"firstName\" <> '') AS ok, 'every contact has a first name' AS label FROM contact", + }), + "accept-loss": Flags.string({ + description: + "A table or table.column whose seeded data the migration may drop on purpose (repeatable); pair it with --check to prove where the data went", + multiple: true, + default: [], }), "skip-build": Flags.boolean({ description: "Use the existing dist/ build instead of building first", @@ -57,6 +64,7 @@ export default class MigrateVerify extends BaseCommand { { skipBuild: flags["skip-build"], seedFile: flags.seed, + checkFile: flags.check, acceptLoss: flags["accept-loss"], } ); diff --git a/src/lib/verify/index.ts b/src/lib/verify/index.ts index 5638aea..225b940 100644 --- a/src/lib/verify/index.ts +++ b/src/lib/verify/index.ts @@ -222,7 +222,8 @@ export interface CheckResult { } // Row and non-null value counts per table and column of the PGlite database -// at PGLITE_DATA_DIR, after running the SQL file in argv[1] if given. +// at PGLITE_DATA_DIR, after running the SQL file in argv[1] if given, plus each +// column's distinct values (as text) so data moved to another table is found. const COUNT_ROWS = ` const fs = require("fs"); const { PGlite } = require("@electric-sql/pglite"); @@ -232,20 +233,45 @@ const { PGlite } = require("@electric-sql/pglite"); const cols = (await db.query("SELECT table_name, column_name FROM information_schema.columns WHERE table_schema = 'public' AND table_name <> 'migrations'")).rows; const out = {}; for (const { table_name: t, column_name: c } of cols) { - const r = (await db.query('SELECT count(*)::int AS rows, count("' + c + '")::int AS vals FROM "' + t + '"')).rows[0]; - out[t] = out[t] || { rows: r.rows, values: {} }; + const q = '"' + c + '"'; + const r = (await db.query("SELECT count(*)::int AS rows, count(" + q + ")::int AS vals, (SELECT array_agg(v) FROM (SELECT DISTINCT " + q + "::text AS v FROM \\"" + t + "\\" WHERE " + q + " IS NOT NULL LIMIT 1000) d) AS dv FROM \\"" + t + "\\"")).rows[0]; + out[t] = out[t] || { rows: r.rows, values: {}, distinct: {} }; out[t].values[c] = r.vals; + out[t].distinct[c] = r.dv || []; } console.log(JSON.stringify(out)); await db.close(); })().catch((e) => { console.error(e.message); process.exit(1); }); `; -type Counts = Record }>; +// Assertions the user wrote for this migration: each SELECT returns rows with a +// boolean "ok" and a "label", the same format as the eval checks. +const RUN_CHECKS = ` +const fs = require("fs"); +const { PGlite } = require("@electric-sql/pglite"); +(async () => { + const db = new PGlite(process.env.PGLITE_DATA_DIR); + const failed = []; + for (const r of await db.exec(fs.readFileSync(process.argv[1], "utf8"))) + for (const row of r.rows) if ("ok" in row && row.ok !== true) failed.push(row.label || JSON.stringify(row)); + await db.close(); + if (failed.length) { console.error("Failed checks:\\n" + failed.join("\\n")); process.exit(1); } +})().catch((e) => { console.error(e.message); process.exit(1); }); +`; + +type Counts = Record< + string, + { rows: number; values: Record; distinct?: Record } +>; + +// Filled in by the database on insert, so a moved row gets new ones. +const GENERATED = new Set(["created_at", "updated_at"]); /** * Rows and values present before the migration and missing after it, - * following the tables and columns the migration renames. + * following the tables and columns the migration renames. A column or table + * whose values all turn up in another column after the migration was moved, + * not lost (a field moved to a new table, a foreign key moved to a join table). */ export function dataLoss(before: Counts, after: Counts, source: string): string[] { const tables = new Map( @@ -256,25 +282,46 @@ export function dataLoss(before: Counts, after: Counts, source: string): string[ (m) => [`${m[1]}.${m[2]}`, m[3]] ) ); + // Only tables the migration creates count, and not their ids: small integer + // values (foreign keys) would otherwise match any existing id column. + const afterValues = Object.entries(after) + .filter(([t]) => !before[t] && ![...tables.values()].includes(t)) + .flatMap(([, a]) => + Object.entries(a.distinct ?? {}) + .filter(([c]) => c !== "id") + .map(([, v]) => new Set(v)) + ); + const moved = (t: string, c: string) => { + const vals = before[t].distinct?.[c] ?? []; + return vals.length > 0 && afterValues.some((s) => vals.every((v) => s.has(v))); + }; const lost: string[] = []; for (const [t, { rows, values }] of Object.entries(before)) { if (!rows) continue; const t2 = tables.get(t) ?? t; const now = after[t2]; if (!now) { - lost.push(`table ${t}: all ${rows} rows`); + const gone = Object.keys(values).filter( + (c) => values[c] > 0 && !GENERATED.has(c) && !moved(t, c) + ); + if (gone.length > 0) lost.push(`table ${t}: all ${rows} rows`); continue; } if (now.rows < rows) lost.push(`table ${t}: ${rows - now.rows} of ${rows} rows`); for (const [c, n] of Object.entries(values)) { const c2 = columns.get(`${t}.${c}`) ?? columns.get(`${t2}.${c}`) ?? c; const kept = now.values[c2] ?? 0; - if (n > kept) lost.push(`${t}.${c}: ${n - kept} of ${n} values`); + if (n > kept && !(kept === 0 && moved(t, c))) + lost.push(`${t}.${c}: ${n - kept} of ${n} values`); } } return lost; } +/** Losses the user accepted by name: a table, or table.column. */ +export const unaccepted = (lost: string[], accept: string[]) => + lost.filter((l) => !accept.some((a) => l.startsWith(`${a}:`) || l.startsWith(`table ${a}:`))); + /** * Projects from the v1 template have no PGlite support in src/orm.config.ts, * so the checks cannot run locally. They pass as skipped rather than block a @@ -314,8 +361,9 @@ export async function verifyMigration( { skipBuild = false, seedFile, - acceptLoss = false, - }: { skipBuild?: boolean; seedFile?: string; acceptLoss?: boolean } = {} + checkFile, + acceptLoss = [], + }: { skipBuild?: boolean; seedFile?: string; checkFile?: string; acceptLoss?: string[] } = {} ): Promise { if (!supportsPglite(projectDir)) return NO_PGLITE; const source = fs.readFileSync(path.resolve(projectDir, migrationFile), "utf8"); @@ -357,16 +405,21 @@ export async function verifyMigration( const after = await count("count"); if (!after) return { ok: false, steps, findings }; const lost = dataLoss(before, after, source); - const ok = lost.length === 0 || acceptLoss; + const ok = unaccepted(lost, acceptLoss).length === 0; done({ name: "data", ok, output: lost.length > 0 - ? `The migration lost seeded data:\n${lost.join("\n")}${ok ? "" : "\nFix the migration, or rerun with --accept-loss if the loss is intended."}` + ? `The migration lost seeded data:\n${lost.join("\n")}${ok ? "" : "\nFix the migration. If data moved somewhere the check can't follow (e.g. a field split in two), prove it with --check and rerun with --accept-loss ."}` : undefined, }); if (!ok) return { ok: false, steps, findings }; } + if ( + checkFile && + !done(step("check", await run(process.execPath, ["-e", RUN_CHECKS, path.resolve(checkFile)], projectDir, pgliteEnv(dataDir)))) + ) + return { ok: false, steps, findings }; const drift = await typeorm( projectDir, diff --git a/src/lib/verify/review.ts b/src/lib/verify/review.ts index 43da123..27c9cb2 100644 --- a/src/lib/verify/review.ts +++ b/src/lib/verify/review.ts @@ -43,11 +43,16 @@ export function reviewMigration(source: string): Finding[] { ); const existing = (table: string) => !created.has(table); + // An UPDATE or INSERT before a statement that names the column means the + // migration already moves or backfills its data. + const handled = (i: number, col: string) => + up.slice(0, i).some((q) => /^(UPDATE|INSERT)\b/.test(q) && q.includes(`"${col}"`)); + const dropped = new Map(); const added = new Map(); - for (const q of up) { + for (const [i, q] of up.entries()) { const d = q.match(/^ALTER TABLE "([^"]+)" DROP COLUMN "([^"]+)"/); - if (d) dropped.set(d[1], [...(dropped.get(d[1]) ?? []), d[2]]); + if (d && !handled(i, d[2])) dropped.set(d[1], [...(dropped.get(d[1]) ?? []), d[2]]); const a = q.match(/^ALTER TABLE "([^"]+)" ADD "([^"]+)"/); if (a) added.set(a[1], [...(added.get(a[1]) ?? []), a[2]]); } @@ -63,11 +68,13 @@ export function reviewMigration(source: string): Finding[] { `${table}.${col} is dropped while ${list(newCols)} is added. If that is a rename, replace the DROP and ADD with RENAME COLUMN to keep the data; otherwise every value in "${col}" is deleted.`); else add("DROPS_COLUMN", table, col, - `${table}.${col} is dropped with all its values. Confirm the loss, or copy the data first.`); + created.size > 0 + ? `${table}.${col} is dropped with all its values while this migration creates ${list([...created] as string[])}. If the data moves there, copy it with INSERT ... SELECT before the DROP; otherwise confirm the loss.` + : `${table}.${col} is dropped with all its values. Confirm the loss, or copy the data first.`); } } - for (const q of up) { + for (const [i, q] of up.entries()) { let m = q.match(/^DROP TABLE "([^"]+)"/); if (m) add("DROPS_TABLE", m[1], undefined, @@ -85,7 +92,7 @@ export function reviewMigration(source: string): Finding[] { } m = q.match(/^ALTER TABLE "([^"]+)" ALTER COLUMN "([^"]+)" SET NOT NULL/); - if (m && existing(m[1])) + if (m && existing(m[1]) && !handled(i, m[2])) add("SET_NOT_NULL", m[1], m[2], `${m[1]}.${m[2]} becomes NOT NULL, which fails on any row where it is null. Backfill those rows first.`); diff --git a/test/lib/verify/review.test.ts b/test/lib/verify/review.test.ts index 1f9836b..2170b72 100644 --- a/test/lib/verify/review.test.ts +++ b/test/lib/verify/review.test.ts @@ -2,7 +2,7 @@ import { expect, describe, test } from "@jest/globals"; import * as fs from "fs"; import * as path from "path"; import { reviewMigration } from "../../../src/lib/verify/review"; -import { dataLoss } from "../../../src/lib/verify"; +import { dataLoss, unaccepted } from "../../../src/lib/verify"; // Each fixture is what `apso migrate generate` (TypeORM migration:generate) // wrote for one .apsorc change to a Company/Contact schema. @@ -75,6 +75,19 @@ describe("reviewMigration rules", () => { ]); }); + test("a drop or SET NOT NULL after the data is copied or backfilled is clean", () => { + expect( + reviewMigration( + migration( + `ALTER TABLE "contact" ADD "firstName" text`, + `UPDATE "contact" SET "firstName" = split_part("fullName", ' ', 1)`, + `ALTER TABLE "contact" ALTER COLUMN "firstName" SET NOT NULL`, + `ALTER TABLE "contact" DROP COLUMN "fullName"` + ) + ) + ).toEqual([]); + }); + test("statements on a table the migration creates are safe", () => { expect( reviewMigration( @@ -112,4 +125,32 @@ describe("dataLoss", () => { "contact.score: 4 of 4 values", ]); }); + + test("data moved to another table or a join table is not lost", () => { + const seeded = { + contact: { + rows: 3, + values: { id: 3, phone: 2, companyId: 3 }, + distinct: { id: ["1", "2", "3"], phone: ["555-01", "555-02"], companyId: ["1", "2"] }, + }, + }; + const after = { + contact: { rows: 3, values: { id: 3 }, distinct: { id: ["1", "2", "3"] } }, + "phone_number": { rows: 2, values: { number: 2 }, distinct: { number: ["555-01", "555-02"] } }, + "contact_companies_company": { rows: 3, values: { companyId: 3 }, distinct: { companyId: ["1", "2"] } }, + }; + expect(dataLoss(seeded, after, "")).toEqual([]); + const dropped = { ...after, "phone_number": { rows: 1, values: { number: 1 }, distinct: { number: ["555-01"] } } }; + expect(dataLoss(seeded, dropped, "")).toEqual(["contact.phone: 2 of 2 values"]); + const company = { rows: 2, values: { id: 2 }, distinct: { id: ["1", "2"] } }; + const emptyJoin = { contact: after.contact, company, "contact_companies_company": { rows: 0, values: { companyId: 0 }, distinct: { companyId: [] } } }; + expect(dataLoss({ ...seeded, company }, { ...emptyJoin, "phone_number": after.phone_number }, "")).toEqual([ + "contact.companyId: 3 of 3 values", + ]); + }); + + test("accepted losses are named by table or column", () => { + const lost = ["table legacy: all 2 rows", "contact.fullName: 4 of 4 values", "contact.score: 1 of 4 values"]; + expect(unaccepted(lost, ["legacy", "contact.fullName"])).toEqual(["contact.score: 1 of 4 values"]); + }); }); From 9164257b7997cc581a7126603156e3206e3793db Mon Sep 17 00:00:00 2001 From: cultron Date: Tue, 27 Oct 2026 16:37:45 -0500 Subject: [PATCH 03/14] feat(migrate): apso migrate plan writes questions for ambiguous schema changes Compares the deployed .apsorc with the current one. Each change whose intent the schema can't settle becomes a question with content-derived id and fixed options: - removed field: rename to a same-type new field, split into several, move to a field on a new related entity, or drop - removed entity: rename to an added entity with the same fields, or drop - removed enum value: which remaining value replaces it - required field on existing rows: backfill value or SQL - type change: plain cast or SQL - ManyToOne becoming ManyToMany or removed: copy or drop the links A rename is only suggested when it is the single reading. Answers live in .apso/migration-plan.json (or --answer id=option) and carry over when the plan is rebuilt. Exits 2 while questions are open, 1 on a bad answer. Field-rename detection follows fix/147-migrate-rename-detection (#147). --- CLAUDE.md | 1 + src/commands/migrate/plan.ts | 89 ++++++ src/lib/plan/index.ts | 274 ++++++++++++++++++ test/fixtures/plan/enum-replace/after.apsorc | 58 ++++ test/fixtures/plan/enum-replace/before.apsorc | 58 ++++ .../fixtures/plan/extract-entity/after.apsorc | 81 ++++++ .../plan/extract-entity/before.apsorc | 58 ++++ test/fixtures/plan/many-to-many/after.apsorc | 58 ++++ test/fixtures/plan/many-to-many/before.apsorc | 58 ++++ test/fixtures/plan/rename-entity/after.apsorc | 58 ++++ .../fixtures/plan/rename-entity/before.apsorc | 58 ++++ test/fixtures/plan/rename-field/after.apsorc | 58 ++++ test/fixtures/plan/rename-field/before.apsorc | 58 ++++ .../fixtures/plan/required-field/after.apsorc | 67 +++++ .../plan/required-field/before.apsorc | 58 ++++ test/fixtures/plan/split-field/after.apsorc | 63 ++++ test/fixtures/plan/split-field/before.apsorc | 58 ++++ test/fixtures/plan/type-change/after.apsorc | 58 ++++ test/fixtures/plan/type-change/before.apsorc | 58 ++++ test/lib/plan/plan.test.ts | 117 ++++++++ 20 files changed, 1446 insertions(+) create mode 100644 src/commands/migrate/plan.ts create mode 100644 src/lib/plan/index.ts create mode 100644 test/fixtures/plan/enum-replace/after.apsorc create mode 100644 test/fixtures/plan/enum-replace/before.apsorc create mode 100644 test/fixtures/plan/extract-entity/after.apsorc create mode 100644 test/fixtures/plan/extract-entity/before.apsorc create mode 100644 test/fixtures/plan/many-to-many/after.apsorc create mode 100644 test/fixtures/plan/many-to-many/before.apsorc create mode 100644 test/fixtures/plan/rename-entity/after.apsorc create mode 100644 test/fixtures/plan/rename-entity/before.apsorc create mode 100644 test/fixtures/plan/rename-field/after.apsorc create mode 100644 test/fixtures/plan/rename-field/before.apsorc create mode 100644 test/fixtures/plan/required-field/after.apsorc create mode 100644 test/fixtures/plan/required-field/before.apsorc create mode 100644 test/fixtures/plan/split-field/after.apsorc create mode 100644 test/fixtures/plan/split-field/before.apsorc create mode 100644 test/fixtures/plan/type-change/after.apsorc create mode 100644 test/fixtures/plan/type-change/before.apsorc create mode 100644 test/lib/plan/plan.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index e232c52..c0e7735 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,6 +26,7 @@ apso dev # Start local dev server via Docker Compose # Schema apso migrate # Test schema migrations locally (PGlite sandbox) +apso migrate plan # Compare deployed vs current .apsorc; ambiguous changes (rename, split, move, enum, backfill, cast) become questions in .apso/migration-plan.json; exits 2 while any are open apso migrate generate # Migration from a deployed .apsorc to this project's entities (TypeScript, PGlite) apso migrate verify # Run a migration on the deployed schema (optionally seeded with --seed rows), then check data loss (following data copied into new tables), --check assertions and drift; generate and verify both warn on statements that lose data apso test smoke # Start the service; call /health, list, and create/read/delete each entity (TS on PGlite, Python/Go on DATABASE_URL) diff --git a/src/commands/migrate/plan.ts b/src/commands/migrate/plan.ts new file mode 100644 index 0000000..22d1bcd --- /dev/null +++ b/src/commands/migrate/plan.ts @@ -0,0 +1,89 @@ +import * as fs from "fs"; +import * as path from "path"; +import { Flags } from "@oclif/core"; +import BaseCommand from "../../lib/base-command"; +import { readApsorcFile } from "../../lib/apsorc-parser"; +import { openQuestions, Plan, planMigration } from "../../lib/plan"; + +export default class MigratePlan extends BaseCommand { + static description = + "Compare the deployed .apsorc with this one and write a migration plan. Changes whose intent is ambiguous (rename or drop, how a field splits, which value replaces a removed enum value) become questions; answer them in the plan file or with --answer. Exits 2 while questions are open."; + + static examples = [ + "$ apso migrate plan --baseline deployed.apsorc", + "$ apso migrate plan --baseline deployed.apsorc --answer field:Contact.phone=rename:mobilePhone", + "$ apso migrate plan --baseline deployed.apsorc --json", + ]; + + static flags = { + baseline: Flags.string({ + description: "The deployed .apsorc to migrate from", + required: true, + }), + plan: Flags.string({ + description: "Plan file to read answers from and write", + default: ".apso/migration-plan.json", + }), + answer: Flags.string({ + description: "Answer a question: =