diff --git a/apps/server/src/persistence/Migrations.test.ts b/apps/server/src/persistence/Migrations.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..21c8e7a308f542993123d7391f68dcf052bfc220 --- /dev/null +++ b/apps/server/src/persistence/Migrations.test.ts @@ -0,0 +1,133 @@ +import { assert, describe, it } from "@effect/vitest"; +import * as NodeSqliteClient from "@t3tools/shared/nodeSqliteClient"; +import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; +import * as SqlClient from "effect/sql/SqlClient"; + +import { migrationManifest, runMigrations } from "./Migrations.ts"; + +const v2Migrations = migrationManifest.filter(([id]) => id >= 55); +const newerMigrationId = Math.max(...migrationManifest.map(([id]) => id)) + 1; + +const history = Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + const rows = yield* sql<{ readonly migration_id: number; readonly name: string }>` + SELECT migration_id, name FROM effect_sql_migrations ORDER BY migration_id + `; + return rows.map((row) => [row.migration_id, row.name] as const); +}); + +const v2TableCount = Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + const rows = yield* sql` + SELECT name FROM sqlite_master WHERE type = 'table' AND name LIKE 'orchestration_v2_%' + `; + return rows.length; +}); + +describe("runMigrations", () => { + it.effect("migrates a fresh database through the whole manifest", () => + Effect.gen(function* () { + yield* runMigrations(); + assert.deepStrictEqual(yield* history, migrationManifest); + assert.isAbove(yield* v2TableCount, 0); + }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))), + ); + + it.effect("refuses to start when another migration took a V2 id", () => + Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + yield* runMigrations({ toMigrationInclusive: 54 }); + yield* sql` + INSERT INTO effect_sql_migrations (migration_id, name) VALUES (55, 'FutureMainMigration') + `; + const before = yield* history; + + assert.deepStrictEqual(yield* runMigrations({ toMigrationInclusive: 54 }), []); + const exit = yield* Effect.exit(runMigrations()); + + assert.isTrue(Exit.isFailure(exit)); + assert.include(String(exit), "55_OrchestrationV2 (recorded: FutureMainMigration)"); + assert.deepStrictEqual(yield* history, before); + assert.strictEqual(yield* v2TableCount, 0); + }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))), + ); + + it.effect("refuses to start when a later id would skip V2 migrations", () => + Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + yield* runMigrations({ toMigrationInclusive: 54 }); + yield* sql` + INSERT INTO effect_sql_migrations (migration_id, name) + VALUES (${newerMigrationId}, 'FutureMainMigration') + `; + const before = yield* history; + + const exit = yield* Effect.exit(runMigrations()); + + assert.isTrue(Exit.isFailure(exit)); + for (const [id, name] of v2Migrations) { + assert.include(String(exit), `${id}_${name} (recorded: nothing)`); + } + assert.deepStrictEqual(yield* history, before); + assert.strictEqual(yield* v2TableCount, 0); + }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))), + ); + + it.effect("upgrades a database at exactly 54", () => + Effect.gen(function* () { + yield* runMigrations({ toMigrationInclusive: 54 }); + assert.deepStrictEqual(yield* runMigrations(), v2Migrations); + assert.deepStrictEqual(yield* history, migrationManifest); + assert.isAbove(yield* v2TableCount, 0); + }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))), + ); + + it.effect.each([30, 54])("still starts with a site-local migration at id %s", (id) => + Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + yield* runMigrations({ toMigrationInclusive: 54 }); + yield* sql` + UPDATE effect_sql_migrations SET name = 'SiteLocalMigration' WHERE migration_id = ${id} + `; + + assert.deepStrictEqual(yield* runMigrations(), v2Migrations); + assert.isAbove(yield* v2TableCount, 0); + }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))), + ); + + it.effect("only checks V2 migrations within the requested limit", () => + Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + yield* runMigrations({ toMigrationInclusive: 55 }); + yield* sql` + INSERT INTO effect_sql_migrations (migration_id, name) + VALUES (56, 'SiteLocalMigration'), (${newerMigrationId}, 'NewerV2Migration') + `; + const before = yield* history; + + assert.deepStrictEqual(yield* runMigrations({ toMigrationInclusive: 55 }), []); + const exit = yield* Effect.exit(runMigrations({ toMigrationInclusive: 56 })); + assert.isTrue(Exit.isFailure(exit)); + assert.include( + String(exit), + "56_RemoveRedundantProjectionIndexes (recorded: SiteLocalMigration)", + ); + assert.notInclude(String(exit), "57_ScheduledTaskWebhooks"); + assert.deepStrictEqual(yield* history, before); + }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))), + ); + + it.effect("still starts after a newer build recorded a later V2 migration", () => + Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + yield* runMigrations(); + yield* sql` + INSERT INTO effect_sql_migrations (migration_id, name) + VALUES (${newerMigrationId}, 'NewerV2Migration') + `; + + assert.deepStrictEqual(yield* runMigrations(), []); + }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))), + ); +}); diff --git a/apps/server/src/persistence/Migrations.ts b/apps/server/src/persistence/Migrations.ts index 7990c53ac454b4e10ec7cb43267424761200e718..69c057cae666b06719744386f2a5298bd68d8e4e 100644 --- a/apps/server/src/persistence/Migrations.ts +++ b/apps/server/src/persistence/Migrations.ts @@ -163,6 +163,48 @@ const makeMigrationLoader = (throughId?: number) => ), ); +// V2's schema migrations start at 55 after the legacy schema migrations. +const FIRST_V2_MIGRATION_ID = 55; + +/** + * Fail before migrating when the migrator would skip one of V2's own + * migrations. It skips every id up to the highest recorded one, whatever name + * was recorded, so a database that took a V2 id for another migration would + * start without V2's schema and fail on first use. Earlier ids keep the + * warning below: databases with a site-local migration there still work. + */ +const assertV2MigrationsCanRun = Effect.fn("assertV2MigrationsCanRun")(function* ( + throughId?: number, +) { + const sql = yield* SqlClient.SqlClient; + const tables = yield* sql` + SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'effect_sql_migrations' + `; + if (tables.length === 0) return; + const recorded = yield* sql<{ + readonly migration_id: number; + readonly name: string; + }>`SELECT migration_id, name FROM effect_sql_migrations`; + const latestRecordedId = Math.max(0, ...recorded.map((row) => row.migration_id)); + const recordedNames = new Map(recorded.map((row) => [row.migration_id, row.name])); + const skipped = migrationEntries.flatMap(([id, name]) => { + if ( + id < FIRST_V2_MIGRATION_ID || + id > latestRecordedId || + (throughId !== undefined && id > throughId) + ) + return []; + const recordedName = recordedNames.get(id); + if (recordedName === name) return []; + return [`${id}_${name} (recorded: ${recordedName ?? "nothing"})`]; + }); + if (skipped.length === 0) return; + return yield* new Migrator.MigrationError({ + kind: "BadState", + message: `This database's migration history would skip required schema migrations: ${skipped.join(", ")}. The migrator skips every id up to the highest recorded id (${latestRecordedId}), regardless of name. It was probably written by a different build. Start the build that wrote it, or report this migration history so an upgrade path can be added.`, + }); +}); + /** * Migrator run function - no schema dumping needed * Uses the base Migrator.make without platform dependencies @@ -186,10 +228,12 @@ export interface RunMigrationsOptions { export const runMigrations = Effect.fn("runMigrations")(function* ({ toMigrationInclusive, }: RunMigrationsOptions = {}) { - const previewMigrations = - toMigrationInclusive === undefined || toMigrationInclusive >= 55 - ? yield* reconcileV2PreviewMigration() - : []; + const includesV2 = + toMigrationInclusive === undefined || toMigrationInclusive >= FIRST_V2_MIGRATION_ID; + const previewMigrations = includesV2 ? yield* reconcileV2PreviewMigration() : []; + if (includesV2) { + yield* assertV2MigrationsCanRun(toMigrationInclusive); + } const executedMigrations = [ ...previewMigrations, ...(yield* run({ loader: makeMigrationLoader(toMigrationInclusive) })), diff --git a/docs/internals/legacy-orchestration-migration.md b/docs/internals/legacy-orchestration-migration.md index 2f496065a6cf31e638ae196b4a1e896ca2436e30..150b1fde8880e4c6ce1cb7aff8ae92c67059da40 100644 --- a/docs/internals/legacy-orchestration-migration.md +++ b/docs/internals/legacy-orchestration-migration.md @@ -46,10 +46,12 @@ running half-upgraded. See `packages/client-runtime/src/connection/compatibility `effect_sql_migrations` records `migration_id` and `name`, but the migrator compares ids only: rows at or below the recorded maximum are skipped without checking names. A database that ran a local or fork migration under an id this build later assigns to a different migration therefore -never receives this build's migration at that id. `runMigrations` logs each recorded id whose name -differs from the manifest so the skipped schema change is diagnosable. There is no safe id range -for a fork inside this ledger: any id at or below a future upstream id masks it forever, so fork -schema changes belong in a separate migration table or outside the migrator entirely. +never receives this build's migration at that id. After repairing supported V2 preview ledgers, +[`runMigrations`](../../apps/server/src/persistence/Migrations.ts) rejects history that would skip a +requested migration at id 55 or later without its expected name. Earlier name clashes remain +warnings for compatibility with site-local migrations. There is no safe id range for a fork inside +this ledger: any id at or below a future upstream id masks it forever, so fork schema changes belong +in a separate migration table or outside the migrator entirely. ## Recovery