From 16106acfd13ed8b74c2fcc361872b5e2843639ac Mon Sep 17 00:00:00 2001 From: ditadi Date: Mon, 12 Oct 2026 13:20:49 +0100 Subject: [PATCH 1/4] feat(appkit): apply committed database migrations on deploy start A deployed DatabasePlugin app now applies the migrations committed under config/database/migrations before its startup checks, as its own service principal and under a per-schema advisory lock. Pending migrations run in one transaction, and a table lock held for over 30s fails the migration instead of stalling the table. A 0000_baseline whose tables already exist is recorded, in its own transaction, instead of run. Any failure stops startup, and the server log names it: the tags and SQLSTATE, or the missing migration file. Local runs never migrate on start. Signed-off-by: ditadi --- .../appkit/src/database/migrations/apply.ts | 469 ++++++++++ .../src/database/migrations/ownership.ts | 18 + .../appkit/src/database/migrations/paths.ts | 72 ++ .../database/migrations/tests/apply.test.ts | 832 ++++++++++++++++++ .../migrations/tests/ownership.test.ts | 21 + .../appkit/src/plugins/database/lifecycle.ts | 8 +- .../src/plugins/database/migrate-on-start.ts | 78 ++ .../src/plugins/database/schema-check.ts | 10 +- .../plugins/database/tests/lifecycle.test.ts | 97 +- .../database/tests/migrate-on-start.test.ts | 187 ++++ 10 files changed, 1784 insertions(+), 8 deletions(-) create mode 100644 packages/appkit/src/database/migrations/apply.ts create mode 100644 packages/appkit/src/database/migrations/ownership.ts create mode 100644 packages/appkit/src/database/migrations/paths.ts create mode 100644 packages/appkit/src/database/migrations/tests/apply.test.ts create mode 100644 packages/appkit/src/database/migrations/tests/ownership.test.ts create mode 100644 packages/appkit/src/plugins/database/migrate-on-start.ts create mode 100644 packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts diff --git a/packages/appkit/src/database/migrations/apply.ts b/packages/appkit/src/database/migrations/apply.ts new file mode 100644 index 000000000..a4e6dd704 --- /dev/null +++ b/packages/appkit/src/database/migrations/apply.ts @@ -0,0 +1,469 @@ +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import path from "node:path"; + +import { readMigrationFiles } from "drizzle-orm/migrator"; +import { drizzle } from "drizzle-orm/node-postgres"; +import { migrate } from "drizzle-orm/node-postgres/migrator"; +import type { Pool, PoolClient } from "pg"; + +import { createLogger } from "../../logging/logger"; +import { ownershipHint } from "./ownership"; +import { + BASELINE_TAG, + type Journal, + type JournalEntry, + MIGRATIONS_TABLE, + MigrationFilesError, + readJournal, + readJsonFile, +} from "./paths"; + +const logger = createLogger("database"); + +interface ApplyMigrationsOptions { + /** Absolute path of the committed migrations folder. */ + readonly migrationsFolder: string; + /** Journal schema = app schema. */ + readonly schemaName: string; +} + +interface ApplyMigrationsResult { + /** Tags applied by this call, in order. */ + readonly applied: readonly string[]; + /** Tags recorded without running, because the catalog already held them (the baseline). */ + readonly adopted: readonly string[]; +} + +/** lock_not_available: a table lock was not granted within LOCK_TIMEOUT. */ +const LOCK_NOT_AVAILABLE = "55P03"; +/** + * Longest wait for a table lock while migrating. Past it the migration fails; + * waiting longer would queue every other query on that table behind it. + */ +const LOCK_TIMEOUT = "30s"; + +function failureHint(code: string | undefined): string | undefined { + if (code === LOCK_NOT_AVAILABLE) { + return `Another session held a lock on a table this migration changes for over ${LOCK_TIMEOUT}; deploy again once it finishes.`; + } + return ownershipHint(code); +} + +/** + * A statement failed while migrating. Pending migrations run in one drizzle-orm + * migrator transaction, so none of them were applied; a baseline adopted just + * before commits on its own and stays recorded. + */ +export class MigrationApplyError extends Error { + /** Tags that were being applied; empty when reading the recorded migrations failed. */ + readonly pending: readonly string[]; + /** SQLSTATE of the failing statement, when the driver reported one. */ + readonly code?: string; + /** Tags this run recorded as applied before the failure. */ + readonly adopted: readonly string[]; + + constructor( + pending: readonly string[], + code: string | undefined, + options: { cause?: unknown; adopted?: readonly string[] } = {}, + ) { + const adopted = options.adopted ?? []; + const sqlState = code ? ` (SQLSTATE ${code})` : ""; + const failure = + pending.length > 0 + ? `Applying migration${pending.length === 1 ? "" : "s"} ${pending.join(", ")} failed${sqlState}; none were applied.` + : `Reading the recorded migrations in ${MIGRATIONS_TABLE} failed${sqlState}; none were applied.`; + const kept = + adopted.length > 0 + ? ` ${adopted.join(", ")} stays recorded as applied: its tables already existed.` + : ""; + const hint = failureHint(code); + super(`${failure}${kept}${hint ? ` ${hint}` : ""}`, { + cause: options.cause, + }); + this.name = "MigrationApplyError"; + this.pending = Object.freeze([...pending]); + this.code = code; + this.adopted = Object.freeze([...adopted]); + } +} + +/** + * Nothing is recorded yet and the database holds only part of what + * 0000_baseline declares: neither running nor adopting the baseline would be safe. + */ +export class MigrationBaselineMismatchError extends Error { + readonly problems: readonly string[]; + + constructor(schemaName: string, problems: readonly string[]) { + super( + `Cannot adopt ${BASELINE_TAG}: schema "${schemaName}" has no recorded migrations and only part of the baseline: ${problems.join("; ")}. ` + + "Create the missing tables and columns (or drop the partial ones), or regenerate the baseline against this database.", + ); + this.name = "MigrationBaselineMismatchError"; + this.problems = Object.freeze([...problems]); + } +} + +const SQLSTATE = /^[0-9A-Z]{5}$/; + +/** drizzle wraps driver errors; the SQLSTATE lives somewhere on the cause chain. */ +function sqlStateOf(error: unknown): string | undefined { + let current = error; + for (let depth = 0; depth < 5 && current; depth++) { + const code = (current as { code?: unknown }).code; + // No SQLSTATE class starts with "E"; Node system errors (EPIPE, EPERM, …) all do. + if (typeof code === "string" && SQLSTATE.test(code) && code[0] !== "E") { + return code; + } + current = (current as { cause?: unknown }).cause; + } + return undefined; +} + +function quoteIdentifier(name: string): string { + return `"${name.replace(/"/g, '""')}"`; +} + +/** One lock per app schema, so concurrent instances apply in turn. */ +export function migrationLockKey(schemaName: string): string { + return createHash("sha256") + .update(`appkit:migrations:${schemaName}`) + .digest() + .readBigInt64BE(0) + .toString(); +} + +type QueryClient = Pick; + +/** + * `created_at` of the last migration the database recorded; null when none is. + * An empty journal table counts as nothing recorded: drizzle-orm creates it + * before its transaction, so a failed first deploy leaves one behind. + */ +async function lastRecordedMigration( + client: QueryClient, + schemaName: string, +): Promise { + // pg_catalog needs no privileges, so a missing grant is not mistaken for a missing table. + const table = await client.query( + `select 1 from pg_catalog.pg_class c + join pg_catalog.pg_namespace n on n.oid = c.relnamespace + where n.nspname = $1 and c.relname = $2`, + [schemaName, MIGRATIONS_TABLE], + ); + if (table.rows.length === 0) return null; + const last = await client.query<{ last: string | number | null }>( + `select max(created_at) as last from ${quoteIdentifier(schemaName)}.${quoteIdentifier(MIGRATIONS_TABLE)}`, + ); + const value = last.rows[0]?.last; + return value === null || value === undefined ? null : Number(value); +} + +/** + * Journal entries newer than the last recorded migration: drizzle-orm's own + * rule, so this agrees with what its migrator runs. + * + * Known gap, shared with drizzle-orm: an entry older than the last recorded one + * (e.g. merged from a parallel branch) is never applied, and an applied + * migration edited later goes unnoticed. Comparing the recorded hashes with the + * journal would catch both. + */ +function pendingEntries( + journal: Journal, + lastRecorded: number | null, +): JournalEntry[] { + if (lastRecorded === null) return journal.entries; + return journal.entries.filter((entry) => entry.when > lastRecorded); +} + +/** Fail before connecting, naming the file, rather than inside drizzle-orm's migrator. */ +function assertMigrationFiles(folder: string, journal: Journal): void { + for (const entry of journal.entries) { + const file = path.join(folder, `${entry.tag}.sql`); + if (!fs.existsSync(file)) { + throw new MigrationFilesError( + `Missing ${file}, listed in meta/_journal.json. Restore it from version control.`, + ); + } + } +} + +/** + * Run `work` on one dedicated connection under the per-schema advisory lock, + * with no statement timeout and at most LOCK_TIMEOUT per table lock. The + * connection is always unlocked, reset and released; a connection whose + * cleanup fails is destroyed, not reused. + */ +async function withMigrationLock( + pool: Pick, + schemaName: string, + work: (client: PoolClient) => Promise, +): Promise { + const lockKey = migrationLockKey(schemaName); + const client = await pool.connect(); + let locked = false; + try { + await client.query("SET statement_timeout = 0"); + const attempt = await client.query<{ locked: boolean }>( + "select pg_catalog.pg_try_advisory_lock($1::bigint) as locked", + [lockKey], + ); + if (!attempt.rows[0]?.locked) { + logger.info( + "Waiting for another instance to finish migrating schema %s", + schemaName, + ); + await client.query("select pg_catalog.pg_advisory_lock($1::bigint)", [ + lockKey, + ]); + } + locked = true; + // Only now: lock_timeout would also cut short the wait for the advisory lock. + await client.query(`SET lock_timeout = '${LOCK_TIMEOUT}'`); + return await work(client); + } finally { + const cleanup: Array<() => Promise> = []; + if (locked) { + cleanup.push(() => + client.query("select pg_catalog.pg_advisory_unlock($1::bigint)", [ + lockKey, + ]), + ); + } + cleanup.push(() => client.query("RESET lock_timeout")); + cleanup.push(() => client.query("RESET statement_timeout")); + + let cleanupError: Error | undefined; + for (const step of cleanup) { + try { + await step(); + } catch (error) { + cleanupError ??= + error instanceof Error ? error : new Error(String(error)); + } + } + // A connection that may still hold the lock or the session settings must not be reused. + client.release(cleanupError); + } +} + +async function inTransaction( + client: QueryClient, + work: () => Promise, +): Promise { + await client.query("begin"); + try { + await work(); + await client.query("commit"); + } catch (error) { + await client.query("rollback").catch(() => undefined); + throw error; + } +} + +/** The tables and columns a migration snapshot (meta/NNNN_snapshot.json) declares in one schema. */ +function snapshotTables( + folder: string, + idx: number, + schemaName: string, +): Map { + const file = path.join( + folder, + "meta", + `${String(idx).padStart(4, "0")}_snapshot.json`, + ); + if (!fs.existsSync(file)) { + throw new MigrationFilesError( + `Missing ${file} for ${BASELINE_TAG}. Restore it from version control; it lists the tables the baseline adopts.`, + ); + } + const snapshot = readJsonFile<{ + tables?: Record< + string, + { name: string; schema?: string; columns: Record } + >; + }>(file); + const tables = new Map(); + for (const table of Object.values(snapshot.tables ?? {})) { + if ((table.schema || "public") !== schemaName) continue; + tables.set(table.name, Object.keys(table.columns)); + } + return tables; +} + +type BaselineState = + | { readonly kind: "absent" } + | { readonly kind: "present" } + | { readonly kind: "partial"; readonly problems: readonly string[] }; + +/** Compare the baseline's tables and columns with pg_catalog. */ +async function baselineState( + client: QueryClient, + options: ApplyMigrationsOptions, + baseline: JournalEntry, +): Promise { + const declared = snapshotTables( + options.migrationsFolder, + baseline.idx, + options.schemaName, + ); + if (declared.size === 0) return { kind: "absent" }; + const { rows } = await client.query<{ + table_name: string; + column_name: string | null; + }>( + `select c.relname::text as table_name, a.attname::text as column_name + from pg_catalog.pg_class c + join pg_catalog.pg_namespace n on n.oid = c.relnamespace + left join pg_catalog.pg_attribute a + on a.attrelid = c.oid and a.attnum > 0 and not a.attisdropped + where n.nspname = $1 and c.relkind in ('r', 'p') + and c.relname = any($2::text[])`, + [options.schemaName, [...declared.keys()]], + ); + const found = new Map>(); + for (const row of rows) { + const columns = found.get(row.table_name) ?? new Set(); + if (row.column_name) columns.add(row.column_name); + found.set(row.table_name, columns); + } + if (found.size === 0) return { kind: "absent" }; + + const problems: string[] = []; + for (const [table, columns] of declared) { + const qualified = `${options.schemaName}.${table}`; + const present = found.get(table); + if (!present) { + problems.push(`table ${qualified} does not exist`); + continue; + } + const missing = columns.filter((column) => !present.has(column)); + if (missing.length > 0) { + problems.push( + `table ${qualified} is missing ${missing.length === 1 ? "column" : "columns"} ${missing.join(", ")}`, + ); + } + } + return problems.length > 0 + ? { kind: "partial", problems } + : { kind: "present" }; +} + +/** + * Record migrations as applied without running them: create the journal table + * as drizzle-orm's migrator does, then insert one row per tag with the + * migrator's own hash (sha256 of the .sql file) and `created_at` (the journal's + * `when`). Runs on the caller's connection; wrap it in a transaction. + */ +async function recordMigrationsApplied( + client: QueryClient, + options: ApplyMigrationsOptions, + journal: Journal, + tags: readonly string[], +): Promise { + // One file per journal entry, in journal order. + const files = readMigrationFiles({ + migrationsFolder: options.migrationsFolder, + }); + const schema = quoteIdentifier(options.schemaName); + const table = `${schema}.${quoteIdentifier(MIGRATIONS_TABLE)}`; + await client.query(`CREATE SCHEMA IF NOT EXISTS ${schema}`); + // drizzle-orm's own definition, so its migrator later reads and extends this table. + await client.query( + `CREATE TABLE IF NOT EXISTS ${table} ( + id SERIAL PRIMARY KEY, + hash text NOT NULL, + created_at bigint + )`, + ); + for (const tag of tags) { + const index = journal.entries.findIndex((entry) => entry.tag === tag); + const file = files[index]; + if (index < 0 || !file) { + throw new Error(`Migration ${tag} is not in the journal.`); + } + await client.query( + `insert into ${table} ("hash", "created_at") values($1, $2)`, + [file.hash, file.folderMillis], + ); + } +} + +/** + * Apply every committed migration the database has not recorded yet. + * + * Runs on one dedicated connection as the connecting identity, which owns what + * it creates (see ownership.ts), under a per-schema advisory lock; drizzle-orm's + * migrator runs only when something is pending. + * + * Baseline adoption: when nothing is recorded yet and `0000_baseline` comes + * first, the baseline's tables are looked up in pg_catalog. None present → it + * runs like any migration. All present → it is recorded as applied, in its own + * transaction, and only later migrations run. Anything in between → + * MigrationBaselineMismatchError, nothing changes. + */ +export async function applyPendingMigrations( + pool: Pick, + options: ApplyMigrationsOptions, +): Promise { + const journal = readJournal(options.migrationsFolder); + if (!journal || journal.entries.length === 0) { + return { applied: [], adopted: [] }; + } + assertMigrationFiles(options.migrationsFolder, journal); + + return withMigrationLock(pool, options.schemaName, async (client) => { + const adopted: string[] = []; + // What the failing step was applying, for the error. + let applying: readonly string[] = []; + try { + const lastRecorded = await lastRecordedMigration( + client, + options.schemaName, + ); + let pending = pendingEntries(journal, lastRecorded); + if (pending.length === 0) return { applied: [], adopted }; + + const first = journal.entries[0]; + if (lastRecorded === null && first?.tag === BASELINE_TAG) { + applying = [BASELINE_TAG]; + const state = await baselineState(client, options, first); + if (state.kind === "partial") { + throw new MigrationBaselineMismatchError( + options.schemaName, + state.problems, + ); + } + if (state.kind === "present") { + await inTransaction(client, () => + recordMigrationsApplied(client, options, journal, [BASELINE_TAG]), + ); + adopted.push(BASELINE_TAG); + pending = pending.filter((entry) => entry.tag !== BASELINE_TAG); + if (pending.length === 0) return { applied: [], adopted }; + } + } + + applying = pending.map((entry) => entry.tag); + await migrate(drizzle({ client }), { + migrationsFolder: options.migrationsFolder, + migrationsSchema: options.schemaName, + migrationsTable: MIGRATIONS_TABLE, + }); + return { applied: applying, adopted }; + } catch (error) { + if ( + error instanceof MigrationBaselineMismatchError || + error instanceof MigrationFilesError + ) { + throw error; + } + throw new MigrationApplyError(applying, sqlStateOf(error), { + cause: error, + adopted, + }); + } + }); +} diff --git a/packages/appkit/src/database/migrations/ownership.ts b/packages/appkit/src/database/migrations/ownership.ts new file mode 100644 index 000000000..606efb1d1 --- /dev/null +++ b/packages/appkit/src/database/migrations/ownership.ts @@ -0,0 +1,18 @@ +// Ownership model: each branch has one owner, the identity its migrations run +// as, and that identity owns the app's objects there. On the deploy branch it +// is the app's service principal, which creates the app schema on first start. +// There is no shared owner role and no SET ROLE. A shared owner role is not +// possible: Lakebase rejects `GRANT … TO databricks_superuser`. + +/** insufficient_privilege: "must be owner of …", "permission denied for …". */ +const INSUFFICIENT_PRIVILEGE = "42501"; + +/** How to fix a migration that failed for lack of privileges; undefined for any other SQLSTATE. */ +export function ownershipHint(code: string | undefined): string | undefined { + if (code !== INSUFFICIENT_PRIVILEGE) return undefined; + return ( + "The connecting role lacks a privilege this migration needs: CREATE on the database, or ownership of the app schema and its tables. " + + "On the deploy branch that role is the app's service principal: bind the database to the app with CAN_CONNECT_AND_CREATE, " + + "and if another role created the schema, use another schemaName in schema.ts or have that role transfer the schema and its objects." + ); +} diff --git a/packages/appkit/src/database/migrations/paths.ts b/packages/appkit/src/database/migrations/paths.ts new file mode 100644 index 000000000..e331080b0 --- /dev/null +++ b/packages/appkit/src/database/migrations/paths.ts @@ -0,0 +1,72 @@ +import fs from "node:fs"; +import path from "node:path"; + +/** Committed migration history, relative to the application root. */ +const MIGRATIONS_DIR = "config/database/migrations"; +/** Journal table drizzle-orm's migrator keeps in the app schema. */ +export const MIGRATIONS_TABLE = "__drizzle_migrations"; +/** + * First migration `appkit db introspect` writes for tables that already exist. + * The migrator adopts it (records it without running it) when the catalog + * already holds every one of its tables and columns. + */ +export const BASELINE_TAG = "0000_baseline"; + +/** One entry of drizzle-kit's v7 `meta/_journal.json`. */ +export interface JournalEntry { + idx: number; + version: "7"; + when: number; + tag: string; + breakpoints: true; +} + +/** drizzle-kit's v7 `meta/_journal.json`. */ +export interface Journal { + version: "7"; + dialect: "postgresql"; + entries: JournalEntry[]; +} + +/** Absolute migrations folder for an application root. */ +export function migrationsFolder(root: string): string { + return path.resolve(root, MIGRATIONS_DIR); +} + +/** Path of the journal inside a migrations folder. */ +function journalFile(folder: string): string { + return path.join(folder, "meta", "_journal.json"); +} + +/** A committed migration file is missing or unreadable; the message names the file. */ +export class MigrationFilesError extends Error { + constructor(message: string, options?: { cause?: unknown }) { + super(message, options); + this.name = "MigrationFilesError"; + } +} + +/** Parse a committed JSON file of the migration history. */ +export function readJsonFile(file: string): T { + try { + return JSON.parse(fs.readFileSync(file, "utf-8")) as T; + } catch (error) { + throw new MigrationFilesError( + `Cannot read ${file}: ${error instanceof Error ? error.message : String(error)}. Restore it from version control.`, + { cause: error }, + ); + } +} + +/** null when meta/_journal.json is absent (nothing committed yet). */ +export function readJournal(folder: string): Journal | null { + const file = journalFile(folder); + if (!fs.existsSync(file)) return null; + const parsed = readJsonFile>(file); + if (parsed.dialect !== "postgresql" || !Array.isArray(parsed.entries)) { + throw new MigrationFilesError( + `${file} is not a drizzle-kit PostgreSQL migration journal`, + ); + } + return parsed as Journal; +} diff --git a/packages/appkit/src/database/migrations/tests/apply.test.ts b/packages/appkit/src/database/migrations/tests/apply.test.ts new file mode 100644 index 000000000..4fc178d14 --- /dev/null +++ b/packages/appkit/src/database/migrations/tests/apply.test.ts @@ -0,0 +1,832 @@ +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, describe, expect, test, vi } from "vitest"; + +import { + applyPendingMigrations, + MigrationApplyError, + MigrationBaselineMismatchError, + migrationLockKey, +} from "../apply"; +import { type Journal, MigrationFilesError } from "../paths"; + +const folders: string[] = []; + +/** Write a v7 journal with one SQL file per entry, the way drizzle-kit lays them out. */ +function migrations( + entries: Array<{ + tag: string; + when: number; + sql: string[]; + /** Tables (name → columns) for meta/NNNN_snapshot.json. */ + snapshot?: Record; + }>, +) { + const folder = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-apply-")); + folders.push(folder); + fs.mkdirSync(path.join(folder, "meta")); + const journal: Journal = { + version: "7", + dialect: "postgresql", + entries: entries.map((entry, idx) => ({ + idx, + version: "7", + when: entry.when, + tag: entry.tag, + breakpoints: true, + })), + }; + fs.writeFileSync( + path.join(folder, "meta/_journal.json"), + JSON.stringify(journal), + ); + entries.forEach((entry, idx) => { + fs.writeFileSync( + path.join(folder, `${entry.tag}.sql`), + entry.sql.join("--> statement-breakpoint\n"), + ); + if (entry.snapshot) { + const tables = Object.fromEntries( + Object.entries(entry.snapshot).map(([name, columns]) => [ + `app.${name}`, + { + name, + schema: "app", + columns: Object.fromEntries(columns.map((c) => [c, { name: c }])), + }, + ]), + ); + fs.writeFileSync( + path.join( + folder, + "meta", + `${String(idx).padStart(4, "0")}_snapshot.json`, + ), + JSON.stringify({ + version: "7", + dialect: "postgresql", + tables, + enums: {}, + }), + ); + } + }); + return folder; +} + +afterEach(() => { + vi.restoreAllMocks(); + for (const folder of folders.splice(0)) { + fs.rmSync(folder, { recursive: true, force: true }); + } +}); + +interface FakeState { + journalTableExists: boolean; + lastCreatedAt: number | null; + failOn?: RegExp; + /** SQLSTATE of the failOn error; defaults to 42P07 (duplicate_table). */ + failCode?: string; + failCleanupOn?: RegExp; + /** pg_catalog (table, column) pairs the baseline check sees. */ + catalog?: Array<[string, string]>; + /** Another instance holds the migration lock, so the first try fails. */ + lockHeld?: boolean; +} + +/** + * A pg PoolClient that records every statement. Its class name must not contain + * "Pool": drizzle-orm would take it for a pool and call connect() on it. + */ +class RecordingClient { + readonly statements: string[] = []; + /** Parameters of each journal insert. */ + readonly recorded: unknown[][] = []; + released: { error: unknown } | undefined; + + constructor(private readonly state: FakeState) {} + + async query( + input: string | { text: string; values?: unknown[] }, + values?: unknown[], + ) { + const text = (typeof input === "string" ? input : input.text).trim(); + const params = values ?? (typeof input === "string" ? [] : input.values); + this.statements.push(text); + if (this.state.failOn?.test(text)) { + throw Object.assign(new Error("relation already exists"), { + code: this.state.failCode ?? "42P07", + }); + } + if (this.state.failCleanupOn?.test(text)) { + throw new Error("connection lost"); + } + if (text.includes("pg_try_advisory_lock(")) { + return { rows: [{ locked: !this.state.lockHeld }] }; + } + if (text.includes("pg_catalog.pg_attribute")) { + return { + rows: (this.state.catalog ?? []).map(([table_name, column_name]) => ({ + table_name, + column_name, + })), + }; + } + if (text.includes("pg_catalog.pg_class")) { + return { rows: this.state.journalTableExists ? [{ one: 1 }] : [] }; + } + if (text.startsWith(`insert into "app"."__drizzle_migrations"`)) { + this.recorded.push(params ?? []); + this.state.journalTableExists = true; + this.state.lastCreatedAt = Number(params?.[1]); + } + if (text.startsWith("select max(created_at)")) { + return { rows: [{ last: this.state.lastCreatedAt }] }; + } + if (text.includes("order by created_at desc limit 1")) { + return { + rows: + this.state.lastCreatedAt === null + ? [] + : [{ id: 1, hash: "h", created_at: this.state.lastCreatedAt }], + }; + } + return { rows: [] }; + } + + release(error?: unknown) { + this.released = { error }; + } +} + +function fakePool(state: FakeState) { + const client = new RecordingClient(state); + let connects = 0; + return { + client, + connects: () => connects, + pool: { + connect: async () => { + connects++; + return client as never; + }, + }, + }; +} + +const twoMigrations = () => + migrations([ + { + tag: "0000_migration", + when: 1000, + sql: [`CREATE TABLE "app"."notes" ("id" integer);`], + }, + { + tag: "0001_migration", + when: 2000, + sql: [ + `ALTER TABLE "app"."notes" ADD COLUMN "body" text;`, + `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, + ], + }, + ]); + +/** Collapse the recorded statements into readable steps. */ +function steps(statements: readonly string[]): string[] { + return statements.map((statement) => { + if (statement.includes("pg_try_advisory_lock(")) return "try lock"; + if (statement.includes("pg_advisory_lock(")) return "lock"; + if (statement.includes("pg_advisory_unlock(")) return "unlock"; + if (statement.includes("pg_catalog.pg_attribute")) + return "baseline tables?"; + if (statement.includes("pg_catalog.pg_class")) return "journal table?"; + if (statement.startsWith("select max(created_at)")) return "last applied?"; + return statement.replace(/\s+/g, " "); + }); +} + +describe("applyPendingMigrations", () => { + test("applies every pending migration as the connecting identity under the advisory lock, then unlocks and releases", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + }); + + const result = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + expect(result.applied).toEqual(["0000_migration", "0001_migration"]); + expect(steps(client.statements)).toEqual([ + "SET statement_timeout = 0", + "try lock", + "SET lock_timeout = '30s'", + "journal table?", + // drizzle-orm's migrator as the connecting identity (no SET ROLE), journal in the app schema: + `CREATE SCHEMA IF NOT EXISTS "app"`, + `CREATE TABLE IF NOT EXISTS "app"."__drizzle_migrations" ( id SERIAL PRIMARY KEY, hash text NOT NULL, created_at bigint )`, + `select id, hash, created_at from "app"."__drizzle_migrations" order by created_at desc limit 1`, + "begin", + `CREATE TABLE "app"."notes" ("id" integer);`, + `insert into "app"."__drizzle_migrations" ("hash", "created_at") values($1, $2)`, + `ALTER TABLE "app"."notes" ADD COLUMN "body" text;`, + `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, + `insert into "app"."__drizzle_migrations" ("hash", "created_at") values($1, $2)`, + "commit", + "unlock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + expect(client.released).toEqual({ error: undefined }); + }); + + test("applies only entries newer than the last recorded migration", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: true, + lastCreatedAt: 1000, + }); + + const result = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + expect(result.applied).toEqual(["0001_migration"]); + expect(client.statements).not.toContain( + `CREATE TABLE "app"."notes" ("id" integer);`, + ); + expect(client.statements).toContain( + `ALTER TABLE "app"."notes" ADD COLUMN "body" text;`, + ); + }); + + test("with nothing pending, issues no migration SQL", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: true, + lastCreatedAt: 2000, + }); + + const result = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + expect(result.applied).toEqual([]); + expect(steps(client.statements)).toEqual([ + "SET statement_timeout = 0", + "try lock", + "SET lock_timeout = '30s'", + "journal table?", + "last applied?", + "unlock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + expect(client.released).toEqual({ error: undefined }); + }); + + test("without a journal it never connects", async () => { + const folder = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-apply-")); + folders.push(folder); + const fake = fakePool({ journalTableExists: false, lastCreatedAt: null }); + + await expect( + applyPendingMigrations(fake.pool, { + migrationsFolder: folder, + schemaName: "app", + }), + ).resolves.toEqual({ applied: [], adopted: [] }); + expect(fake.connects()).toBe(0); + }); + + test("a failing statement rolls back all pending migrations and still unlocks and releases", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + failOn: /ADD COLUMN "title"/, + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect(error).toMatchObject({ + pending: ["0000_migration", "0001_migration"], + code: "42P07", + }); + expect((error as Error).message).toBe( + "Applying migrations 0000_migration, 0001_migration failed (SQLSTATE 42P07); none were applied.", + ); + expect(steps(client.statements).slice(-5)).toEqual([ + `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, + "rollback", + "unlock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + expect(client.released).toEqual({ error: undefined }); + }); + + test("an ownership failure (42501) names the branch-owner rule and the fix", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + failOn: /ADD COLUMN "body"/, + failCode: "42501", + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toMatchObject({ code: "42501" }); + expect((error as Error).message).toMatch( + /^Applying migrations 0000_migration, 0001_migration failed \(SQLSTATE 42501\); none were applied\. The connecting role lacks a privilege this migration needs: /, + ); + }); + + test("a failed unlock destroys the connection instead of returning it to the pool", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + failCleanupOn: /pg_advisory_unlock/, + }); + + await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + // The remaining cleanup still runs. + expect(steps(client.statements).slice(-3)).toEqual([ + "unlock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + expect(client.released?.error).toBeInstanceOf(Error); + }); + + test("a failure before the lock is held releases without unlocking", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + failOn: /pg_try_advisory_lock/, + }); + + await expect( + applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }), + ).rejects.toThrow("relation already exists"); + expect(steps(client.statements)).toEqual([ + "SET statement_timeout = 0", + "try lock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + expect(client.released).toEqual({ error: undefined }); + }); +}); + +describe("migrationLockKey", () => { + test("is a stable signed 64-bit key per schema", () => { + const key = migrationLockKey("app"); + expect(key).toBe(migrationLockKey("app")); + expect(key).not.toBe(migrationLockKey("public")); + const value = BigInt(key); + expect(value).toBeGreaterThanOrEqual(-(2n ** 63n)); + expect(value).toBeLessThan(2n ** 63n); + }); +}); + +const BASELINE_SQL = [ + `CREATE TABLE "app"."notes" ("id" integer PRIMARY KEY, "body" text);`, + `CREATE TABLE "app"."tags" ("id" integer PRIMARY KEY);`, +]; + +/** 0000_baseline (notes, tags) followed by one ordinary migration. */ +const baselineThenMigration = () => + migrations([ + { + tag: "0000_baseline", + when: 1000, + sql: BASELINE_SQL, + snapshot: { notes: ["id", "body"], tags: ["id"] }, + }, + { + tag: "0001_migration", + when: 2000, + sql: [`ALTER TABLE "app"."notes" ADD COLUMN "title" text;`], + snapshot: { notes: ["id", "body", "title"], tags: ["id"] }, + }, + ]); + +describe("applyPendingMigrations — baseline adoption", () => { + test("none of the baseline's tables exist: the baseline runs like any migration", async () => { + const folder = baselineThenMigration(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + catalog: [], + }); + + const result = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + expect(result).toEqual({ + applied: ["0000_baseline", "0001_migration"], + adopted: [], + }); + expect(client.statements).toContain(BASELINE_SQL[0]); + }); + + test("every baseline table and column exists: the baseline is recorded, not run, and later migrations apply", async () => { + const folder = baselineThenMigration(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + catalog: [ + ["notes", "id"], + ["notes", "body"], + ["tags", "id"], + ], + }); + + const result = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + expect(result).toEqual({ + applied: ["0001_migration"], + adopted: ["0000_baseline"], + }); + expect(client.statements).not.toContain(BASELINE_SQL[0]); + expect(client.statements).toContain( + `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, + ); + // The adopted row carries exactly what drizzle-orm's migrator would have written. + const baselineFile = fs.readFileSync( + path.join(folder, "0000_baseline.sql"), + "utf-8", + ); + expect(client.recorded[0]).toEqual([ + createHash("sha256").update(baselineFile).digest("hex"), + 1000, + ]); + expect(steps(client.statements).slice(0, 10)).toEqual([ + "SET statement_timeout = 0", + "try lock", + "SET lock_timeout = '30s'", + "journal table?", + "baseline tables?", + "begin", + `CREATE SCHEMA IF NOT EXISTS "app"`, + `CREATE TABLE IF NOT EXISTS "app"."__drizzle_migrations" ( id SERIAL PRIMARY KEY, hash text NOT NULL, created_at bigint )`, + `insert into "app"."__drizzle_migrations" ("hash", "created_at") values($1, $2)`, + "commit", + ]); + expect(client.released).toEqual({ error: undefined }); + }); + + test("adopting a lone baseline applies nothing else", async () => { + const folder = migrations([ + { + tag: "0000_baseline", + when: 1000, + sql: BASELINE_SQL, + snapshot: { notes: ["id", "body"], tags: ["id"] }, + }, + ]); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + catalog: [ + ["notes", "id"], + ["notes", "body"], + ["tags", "id"], + ], + }); + + await expect( + applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }), + ).resolves.toEqual({ applied: [], adopted: ["0000_baseline"] }); + expect(client.statements.slice(-3)).toEqual([ + expect.stringContaining("pg_advisory_unlock"), + "RESET lock_timeout", + "RESET statement_timeout", + ]); + }); + + test("only part of the baseline exists: MigrationBaselineMismatchError, nothing recorded or run", async () => { + const folder = baselineThenMigration(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + catalog: [["notes", "id"]], + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationBaselineMismatchError); + expect((error as MigrationBaselineMismatchError).problems).toEqual([ + "table app.notes is missing column body", + "table app.tags does not exist", + ]); + expect((error as Error).message).toMatch( + /^Cannot adopt 0000_baseline: schema "app" has no recorded migrations and only part of the baseline: /, + ); + expect(client.recorded).toEqual([]); + expect(client.statements).not.toContain("begin"); + expect(steps(client.statements).slice(-3)).toEqual([ + "unlock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + expect(client.released).toEqual({ error: undefined }); + }); + + test("recording the adopted baseline fails: MigrationApplyError with the SQLSTATE and the ownership hint", async () => { + const folder = baselineThenMigration(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + catalog: [ + ["notes", "id"], + ["notes", "body"], + ["tags", "id"], + ], + // The schema belongs to another role, so the journal table cannot be created in it. + failOn: /^CREATE TABLE IF NOT EXISTS "app"\."__drizzle_migrations"/, + failCode: "42501", + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect(error).toMatchObject({ pending: ["0000_baseline"], code: "42501" }); + expect((error as Error).message).toContain( + "On the deploy branch that role is the app's service principal", + ); + expect(client.statements).toContain("rollback"); + expect(client.statements).not.toContain( + `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, + ); + }); + + test("with a recorded migration, the baseline is never adopted", async () => { + const folder = baselineThenMigration(); + const { pool, client } = fakePool({ + journalTableExists: true, + lastCreatedAt: 1000, + catalog: [["notes", "id"]], + }); + + await expect( + applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }), + ).resolves.toEqual({ applied: ["0001_migration"], adopted: [] }); + expect( + client.statements.some((s) => s.includes("pg_catalog.pg_attribute")), + ).toBe(false); + }); + + test("an empty journal table, left by a failed first deploy, still adopts the baseline", async () => { + const folder = baselineThenMigration(); + const { pool } = fakePool({ + journalTableExists: true, + lastCreatedAt: null, + catalog: [ + ["notes", "id"], + ["notes", "body"], + ["tags", "id"], + ], + }); + + await expect( + applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }), + ).resolves.toEqual({ + applied: ["0001_migration"], + adopted: ["0000_baseline"], + }); + }); + + test("a migration failing after the adoption says the baseline stays recorded", async () => { + const folder = baselineThenMigration(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + catalog: [ + ["notes", "id"], + ["notes", "body"], + ["tags", "id"], + ], + failOn: /ADD COLUMN "title"/, + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect(error).toMatchObject({ + pending: ["0001_migration"], + adopted: ["0000_baseline"], + }); + expect((error as Error).message).toBe( + "Applying migration 0001_migration failed (SQLSTATE 42P07); none were applied. 0000_baseline stays recorded as applied: its tables already existed.", + ); + expect(client.recorded).toHaveLength(1); + }); + + test("a missing baseline snapshot names the file", async () => { + const folder = baselineThenMigration(); + fs.rmSync(path.join(folder, "meta/0000_snapshot.json")); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationFilesError); + expect((error as Error).message).toMatch( + /^Missing .*meta\/0000_snapshot\.json for 0000_baseline\. Restore it from version control/, + ); + expect(client.released).toEqual({ error: undefined }); + }); +}); + +describe("applyPendingMigrations — failures name their cause", () => { + test("reading the recorded migrations fails (42501): the hint, and nothing applied", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: true, + lastCreatedAt: 1000, + // The journal table belongs to another role. + failOn: /^select max\(created_at\)/, + failCode: "42501", + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect(error).toMatchObject({ pending: [], code: "42501" }); + expect((error as Error).message).toMatch( + /^Reading the recorded migrations in __drizzle_migrations failed \(SQLSTATE 42501\); none were applied\. The connecting role lacks a privilege/, + ); + expect(steps(client.statements).slice(-3)).toEqual([ + "unlock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + }); + + test("a table lock held past the lock timeout says to deploy again", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + journalTableExists: true, + lastCreatedAt: 1000, + failOn: /ADD COLUMN "body"/, + failCode: "55P03", + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect((error as Error).message).toBe( + "Applying migration 0001_migration failed (SQLSTATE 55P03); none were applied. Another session held a lock on a table this migration changes for over 30s; deploy again once it finishes.", + ); + }); + + test("a Node system error is not reported as a SQLSTATE", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + journalTableExists: true, + lastCreatedAt: 1000, + failOn: /ADD COLUMN "body"/, + failCode: "EPIPE", + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toMatchObject({ code: undefined }); + expect((error as Error).message).toBe( + "Applying migration 0001_migration failed; none were applied.", + ); + }); + + test("a missing .sql file fails before connecting, naming the file", async () => { + const folder = twoMigrations(); + fs.rmSync(path.join(folder, "0001_migration.sql")); + const fake = fakePool({ journalTableExists: false, lastCreatedAt: null }); + + const error = await applyPendingMigrations(fake.pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationFilesError); + expect((error as Error).message).toMatch( + /^Missing .*0001_migration\.sql, listed in meta\/_journal\.json\. Restore it from version control\.$/, + ); + expect(fake.connects()).toBe(0); + }); + + test.each([ + [ + "not JSON", + "{", + /^Cannot read .*_journal\.json: .*Restore it from version control\.$/, + ], + [ + "another dialect", + JSON.stringify({ version: "7", dialect: "mysql", entries: [] }), + /_journal\.json is not a drizzle-kit PostgreSQL migration journal$/, + ], + ])( + "a journal that is %s fails before connecting, naming the file", + async (_label, contents, message) => { + const folder = twoMigrations(); + fs.writeFileSync(path.join(folder, "meta/_journal.json"), contents); + const fake = fakePool({ journalTableExists: false, lastCreatedAt: null }); + + const error = await applyPendingMigrations(fake.pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationFilesError); + expect((error as Error).message).toMatch(message); + expect(fake.connects()).toBe(0); + }, + ); + + test("waits for another instance's lock, and says so", async () => { + const folder = twoMigrations(); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + const { pool, client } = fakePool({ + journalTableExists: true, + lastCreatedAt: 2000, + lockHeld: true, + }); + + await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + expect(steps(client.statements).slice(0, 4)).toEqual([ + "SET statement_timeout = 0", + "try lock", + "lock", + "SET lock_timeout = '30s'", + ]); + expect(log.mock.calls.flat().map(String).join(" ")).toContain( + "Waiting for another instance to finish migrating schema app", + ); + }); +}); diff --git a/packages/appkit/src/database/migrations/tests/ownership.test.ts b/packages/appkit/src/database/migrations/tests/ownership.test.ts new file mode 100644 index 000000000..56ea47022 --- /dev/null +++ b/packages/appkit/src/database/migrations/tests/ownership.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, test } from "vitest"; + +import { ownershipHint } from "../ownership"; + +describe("ownership: one owner per branch", () => { + test("insufficient_privilege gets the branch-owner rule and the fix", () => { + const hint = ownershipHint("42501"); + expect(hint).toContain( + "lacks a privilege this migration needs: CREATE on the database, or ownership of the app schema and its tables", + ); + expect(hint).toContain( + "On the deploy branch that role is the app's service principal: bind the database to the app with CAN_CONNECT_AND_CREATE", + ); + expect(hint).toContain("use another schemaName in schema.ts"); + }); + + test("any other failure gets no ownership hint", () => { + expect(ownershipHint("42P07")).toBeUndefined(); + expect(ownershipHint(undefined)).toBeUndefined(); + }); +}); diff --git a/packages/appkit/src/plugins/database/lifecycle.ts b/packages/appkit/src/plugins/database/lifecycle.ts index c50badf6f..aff2abc08 100644 --- a/packages/appkit/src/plugins/database/lifecycle.ts +++ b/packages/appkit/src/plugins/database/lifecycle.ts @@ -23,6 +23,7 @@ import type { SqlTag, TransactionClient, } from "./entity-types"; +import { migrateOnStart, shouldMigrateOnStart } from "./migrate-on-start"; import { assertSchemaMatchesDatabase } from "./schema-check"; import { createMutationScope, type MutationScope } from "./scope"; import type { DatabaseHooks } from "./types"; @@ -163,8 +164,9 @@ function buildDatabaseExports(context: ExportContext): DatabaseExports { } /** - * Validate the schema, create one pool-backed API, and verify connectivity and - * that every declared table and column exists. + * Validate the schema, create one pool-backed API, apply committed migrations + * when deployed, and verify connectivity and that every declared table and + * column exists. */ export async function createDatabaseState( schema: TSchema, @@ -188,6 +190,8 @@ export async function createDatabaseState( statement_timeout: STATEMENT_TIMEOUT_MS, idle_in_transaction_session_timeout: IDLE_IN_TRANSACTION_TIMEOUT_MS, }); + // A deployed app applies committed migrations as its own identity before the checks below. + if (shouldMigrateOnStart()) await migrateOnStart(pool, schema); const db = createDrizzleDb(pool, schema); const dataPath = createDrizzleDataPath(db, schema, { columnAccess: "trusted", diff --git a/packages/appkit/src/plugins/database/migrate-on-start.ts b/packages/appkit/src/plugins/database/migrate-on-start.ts new file mode 100644 index 000000000..8061c1124 --- /dev/null +++ b/packages/appkit/src/plugins/database/migrate-on-start.ts @@ -0,0 +1,78 @@ +import type { Pool } from "pg"; + +import { databaseSetupFailed } from "../../database/errors"; +import { + applyPendingMigrations, + MigrationApplyError, + MigrationBaselineMismatchError, +} from "../../database/migrations/apply"; +import { + MigrationFilesError, + migrationsFolder, +} from "../../database/migrations/paths"; +import type { Schema } from "../../database/schema-builder"; +import { createLogger } from "../../logging/logger"; + +const logger = createLogger("database"); + +/** + * True only inside a deployed Databricks App: not local development, and the + * runtime has set the app's name and its service principal's credentials, as + * Databricks Apps does for every app. `databricks apps run-local` sets the + * name but not the credentials. Locally, `appkit db migrate` applies + * migrations instead. + */ +export function shouldMigrateOnStart( + env: NodeJS.ProcessEnv = process.env, +): boolean { + return ( + env.NODE_ENV !== "development" && + Boolean(env.DATABRICKS_APP_NAME) && + Boolean(env.DATABRICKS_CLIENT_ID) && + Boolean(env.DATABRICKS_CLIENT_SECRET) + ); +} + +/** + * Apply pending committed migrations from /config/database/migrations as + * the app's identity (see database/migrations/ownership.ts). No journal → + * no-op. A failed or unadoptable migration, or a missing or unreadable + * migration file, becomes a setup failure that names it; any other error + * (e.g. no connection) is left to lifecycle's generic sanitizer. + */ +export async function migrateOnStart( + pool: Pick, + schema: Schema, + root: string = process.cwd(), +): Promise { + let result: Awaited>; + try { + result = await applyPendingMigrations(pool, { + migrationsFolder: migrationsFolder(root), + schemaName: schema.$schemaName, + }); + } catch (error) { + if ( + error instanceof MigrationApplyError || + error instanceof MigrationBaselineMismatchError || + error instanceof MigrationFilesError + ) { + throw databaseSetupFailed(error.message); + } + throw error; + } + if (result.adopted.length > 0) { + logger.info( + "Adopted %s in schema %s: its tables already existed", + result.adopted.join(", "), + schema.$schemaName, + ); + } + if (result.applied.length > 0) { + logger.info( + "Applied migrations %s to schema %s", + result.applied.join(", "), + schema.$schemaName, + ); + } +} diff --git a/packages/appkit/src/plugins/database/schema-check.ts b/packages/appkit/src/plugins/database/schema-check.ts index f1334ecda..9b6f7176a 100644 --- a/packages/appkit/src/plugins/database/schema-check.ts +++ b/packages/appkit/src/plugins/database/schema-check.ts @@ -10,10 +10,10 @@ interface CatalogColumn { /** * Confirm every declared table and column exists before routes are published. - * The plugin never migrates, so drift would otherwise surface on each request - * as an undefined-table or undefined-column failure. pg_catalog is read rather - * than information_schema, which hides objects the role cannot access and - * would report a privilege gap as a missing column. + * Even after a deployed app applies its migrations, drift would surface on each + * request as an undefined-table or undefined-column failure, so check up front. + * pg_catalog is read rather than information_schema, which hides objects the + * role cannot access and would report a privilege gap as a missing column. */ export async function assertSchemaMatchesDatabase( dataPath: DataPath, @@ -76,7 +76,7 @@ export async function assertSchemaMatchesDatabase( } if (problems.length > 0) { throw databaseSetupFailed( - `the declared schema does not match the database: ${problems.join("; ")}. Create or migrate these before starting the app; the plugin does not.`, + `the declared schema does not match the database: ${problems.join("; ")}. Create them, or commit migrations under config/database/migrations so the deployed app applies them on start.`, ); } } diff --git a/packages/appkit/src/plugins/database/tests/lifecycle.test.ts b/packages/appkit/src/plugins/database/tests/lifecycle.test.ts index aae7c04d2..0401912f6 100644 --- a/packages/appkit/src/plugins/database/tests/lifecycle.test.ts +++ b/packages/appkit/src/plugins/database/tests/lifecycle.test.ts @@ -9,6 +9,8 @@ const mocks = vi.hoisted(() => ({ createDrizzleDb: vi.fn(), createDrizzleDataPath: vi.fn(), assertSchemaMatchesDatabase: vi.fn(), + shouldMigrateOnStart: vi.fn(() => false), + migrateOnStart: vi.fn(async () => undefined), })); vi.mock("../../../connectors/lakebase", () => ({ initializeLakebasePool: mocks.initializeLakebasePool, @@ -21,6 +23,11 @@ vi.mock("../../../database/runtime/engine/drizzle-data-path", () => ({ vi.mock("../schema-check", () => ({ assertSchemaMatchesDatabase: mocks.assertSchemaMatchesDatabase, })); +// The migrator has its own suite; here only its place in setup matters. +vi.mock("../migrate-on-start", () => ({ + shouldMigrateOnStart: mocks.shouldMigrateOnStart, + migrateOnStart: mocks.migrateOnStart, +})); import { IDLE_IN_TRANSACTION_TIMEOUT_MS, @@ -94,7 +101,11 @@ function arrange(path = fakePath()) { return { pool, db, path, execute }; } -beforeEach(() => vi.clearAllMocks()); +beforeEach(() => { + vi.clearAllMocks(); + // clearAllMocks keeps implementations; a test that turns migration on must not leak it. + mocks.shouldMigrateOnStart.mockReturnValue(false); +}); afterEach(() => vi.restoreAllMocks()); describe("createDatabaseState", () => { @@ -216,6 +227,90 @@ describe("createDatabaseState", () => { expect(pool.end).toHaveBeenCalledTimes(1); }); + test("a deployed app applies migrations after the pool opens, before readiness and the catalog check", async () => { + const order: string[] = []; + const path = fakePath({ + raw: vi.fn(async () => { + order.push("select 1"); + return []; + }) as unknown as DataPath["raw"], + }); + const { pool, execute } = arrange(path); + mocks.initializeLakebasePool.mockImplementationOnce(async () => { + order.push("pool"); + return pool; + }); + mocks.shouldMigrateOnStart.mockReturnValue(true); + mocks.migrateOnStart.mockImplementationOnce(async () => { + order.push("migrate"); + }); + mocks.assertSchemaMatchesDatabase.mockImplementationOnce(async () => { + order.push("catalog"); + }); + + await createDatabaseState(schema, execute); + + expect(order).toEqual(["pool", "migrate", "select 1", "catalog"]); + expect(mocks.migrateOnStart).toHaveBeenCalledWith(pool, schema); + }); + + test("local runs never call the migrator", async () => { + const { execute } = arrange(); + mocks.shouldMigrateOnStart.mockReturnValue(false); + + await createDatabaseState(schema, execute); + + expect(mocks.migrateOnStart).not.toHaveBeenCalled(); + }); + + test("a migration failure keeps its reason, ends the pool and publishes nothing", async () => { + const { pool, path, execute } = arrange(); + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + mocks.shouldMigrateOnStart.mockReturnValue(true); + const failure = new DatabasePluginError( + "SETUP_FAILED", + "setup", + "Database setup failed: Applying migration 0001_migration failed (SQLSTATE 42501); none were applied.", + ); + mocks.migrateOnStart.mockRejectedValueOnce(failure); + + const error = await createDatabaseState(schema, execute).catch( + (caught) => caught, + ); + + expect(error).toBe(failure); + expect(errorLog.mock.calls.flat().map(String).join(" ")).toContain( + "0001_migration failed (SQLSTATE 42501)", + ); + expect(pool.end).toHaveBeenCalledTimes(1); + expect(path.raw).not.toHaveBeenCalled(); + expect(mocks.assertSchemaMatchesDatabase).not.toHaveBeenCalled(); + }); + + test("an unexpected migrator error is sanitized like any driver error", async () => { + const { pool, execute } = arrange(); + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + mocks.shouldMigrateOnStart.mockReturnValue(true); + mocks.migrateOnStart.mockRejectedValueOnce( + new Error("secret connection string"), + ); + + const error = await createDatabaseState(schema, execute).catch( + (caught) => caught, + ); + + expect(error).toMatchObject({ category: "SETUP_FAILED", phase: "setup" }); + expect(error.message).toBe("Database setup failed"); + expect(errorLog.mock.calls.flat().map(String).join(" ")).not.toContain( + "secret connection string", + ); + expect(pool.end).toHaveBeenCalledTimes(1); + }); + test("sanitizes connector initialization failures", async () => { const { execute } = arrange(); const errorLog = vi diff --git a/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts b/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts new file mode 100644 index 000000000..cf453a32d --- /dev/null +++ b/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts @@ -0,0 +1,187 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; + +const { applyPendingMigrations } = vi.hoisted(() => ({ + applyPendingMigrations: vi.fn(), +})); +vi.mock("../../../database/migrations/apply", async (importOriginal) => ({ + ...(await importOriginal< + typeof import("../../../database/migrations/apply") + >()), + applyPendingMigrations, +})); + +import { DatabasePluginError } from "../../../database/errors"; +import * as apply from "../../../database/migrations/apply"; +import { MigrationFilesError } from "../../../database/migrations/paths"; +import { defineSchema, id } from "../../../database/schema-builder"; +import { migrateOnStart, shouldMigrateOnStart } from "../migrate-on-start"; + +const schema = defineSchema( + ({ table }) => ({ notes: table("notes", { id: id() }) }), + { schemaName: "app" }, +); +const roots: string[] = []; +const pool = { connect: vi.fn() }; + +function appRoot(): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-migrate-start-")); + roots.push(root); + return root; +} + +beforeEach(async () => { + const actual = await vi.importActual< + typeof import("../../../database/migrations/apply") + >("../../../database/migrations/apply"); + applyPendingMigrations.mockReset(); + applyPendingMigrations.mockImplementation(actual.applyPendingMigrations); + pool.connect.mockReset(); +}); + +afterEach(() => { + vi.restoreAllMocks(); + for (const root of roots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +describe("shouldMigrateOnStart", () => { + const credentials = { + DATABRICKS_CLIENT_ID: "sp-client-id", + DATABRICKS_CLIENT_SECRET: "secret", + }; + const app = { DATABRICKS_APP_NAME: "my-app", ...credentials }; + + test.each([ + ["a deployed app", { ...app, NODE_ENV: "production" }, true], + ["a deployed app without NODE_ENV", app, true], + [ + "local development, even with app credentials", + { ...app, NODE_ENV: "development" }, + false, + ], + [ + "`databricks apps run-local`: the app name without its credentials", + { DATABRICKS_APP_NAME: "my-app", NODE_ENV: "production" }, + false, + ], + [ + "a local production start with app credentials in .env but no app name", + { ...credentials, NODE_ENV: "production" }, + false, + ], + [ + "no client secret (a developer's own credentials)", + { + DATABRICKS_APP_NAME: "my-app", + DATABRICKS_CLIENT_ID: "x", + NODE_ENV: "production", + }, + false, + ], + [ + "no client id", + { + DATABRICKS_APP_NAME: "my-app", + DATABRICKS_CLIENT_SECRET: "x", + NODE_ENV: "production", + }, + false, + ], + ["nothing set", {}, false], + ] as const)("%s → %s", (_label, env, expected) => { + expect(shouldMigrateOnStart(env)).toBe(expected); + }); +}); + +describe("migrateOnStart", () => { + test("no committed journal: a no-op that never connects", async () => { + await migrateOnStart(pool, schema, appRoot()); + + expect(pool.connect).not.toHaveBeenCalled(); + }); + + test("applies from /config/database/migrations into the schema's own journal, and logs what it did", async () => { + const root = appRoot(); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + applyPendingMigrations.mockResolvedValueOnce({ + applied: ["0001_migration"], + adopted: ["0000_baseline"], + }); + + await migrateOnStart(pool, schema, root); + + expect(applyPendingMigrations).toHaveBeenCalledWith(pool, { + migrationsFolder: path.join(root, "config/database/migrations"), + schemaName: "app", + }); + const logged = log.mock.calls.map((call) => call.map(String).join(" ")); + expect(logged).toEqual([ + expect.stringContaining( + "Adopted 0000_baseline in schema app: its tables already existed", + ), + expect.stringContaining( + "Applied migrations 0001_migration to schema app", + ), + ]); + }); + + test("nothing pending logs nothing", async () => { + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + applyPendingMigrations.mockResolvedValueOnce({ applied: [], adopted: [] }); + + await migrateOnStart(pool, schema, appRoot()); + + expect(log).not.toHaveBeenCalled(); + }); + + test.each([ + [ + "a failed migration", + new apply.MigrationApplyError(["0001_migration"], "42501"), + "Applying migration 0001_migration failed (SQLSTATE 42501); none were applied.", + ], + [ + "a baseline that only partly matches", + new apply.MigrationBaselineMismatchError("app", [ + "table app.tags does not exist", + ]), + "Cannot adopt 0000_baseline", + ], + [ + "a missing migration file", + new MigrationFilesError( + "Missing /app/config/database/migrations/0001_migration.sql, listed in meta/_journal.json. Restore it from version control.", + ), + "Missing /app/config/database/migrations/0001_migration.sql", + ], + ])( + "%s becomes a setup failure that names it", + async (_label, failure, reason) => { + applyPendingMigrations.mockRejectedValueOnce(failure); + + const error = await migrateOnStart(pool, schema, appRoot()).catch( + (caught: unknown) => caught, + ); + + expect(error).toBeInstanceOf(DatabasePluginError); + expect(error).toMatchObject({ category: "SETUP_FAILED", phase: "setup" }); + expect((error as Error).message).toContain(reason); + // Only the stable message crosses a request boundary. + expect((error as DatabasePluginError).clientMessage).toBe( + "Database setup failed", + ); + }, + ); + + test("any other error is left to lifecycle's sanitizer", async () => { + const raw = new Error("connection reset"); + applyPendingMigrations.mockRejectedValueOnce(raw); + + await expect(migrateOnStart(pool, schema, appRoot())).rejects.toBe(raw); + }); +}); From 19adf06c5f575f1917401926c7cc2733c81ac31b Mon Sep 17 00:00:00 2001 From: ditadi Date: Mon, 19 Oct 2026 14:07:23 +0200 Subject: [PATCH 2/4] fix(appkit): apply each migration in its own transaction and fail closed - Run and record each pending migration in its own transaction. An enum value added by one migration can now be used by the next (55P04), and a failure keeps the migrations before it. - Compare the committed journal with the recorded rows by hash and created_at. Fail closed on a migration older than the last recorded one; warn on edited migrations and on recorded rows with no committed file. - Read the history without the lock first. Take the lock only when something is pending, then read again. - Bound the wait for the migration lock to 5 minutes and name its holder. Set application_name and client_connection_check_interval. Lower lock_timeout to 5s with three tries per migration. - Migrate on start only with APPKIT_DB_MIGRATE_ON_START=true and service principal credentials. Skip dev- and session- branches; refuse a -pooler host. - Name the failed step and the SQLSTATE or system code. On 42501, name the schema owner and the connecting role. - Add real-Postgres tests gated on APPKIT_TEST_PG_URL; CI runs them against a postgres:17 service. Signed-off-by: ditadi --- .github/workflows/ci.yml | 14 + .../appkit/src/database/migrations/apply.ts | 723 +++++++++---- .../appkit/src/database/migrations/history.ts | 64 ++ .../src/database/migrations/ownership.ts | 58 +- .../appkit/src/database/migrations/paths.ts | 2 +- .../migrations/tests/apply.pg.test.ts | 408 ++++++++ .../database/migrations/tests/apply.test.ts | 984 +++++++++--------- .../src/database/migrations/tests/fixtures.ts | 86 ++ .../database/migrations/tests/history.test.ts | 75 ++ .../migrations/tests/ownership.test.ts | 28 +- .../src/plugins/database/migrate-on-start.ts | 79 +- .../src/plugins/database/schema-check.ts | 2 +- .../database/tests/migrate-on-start.test.ts | 160 ++- 13 files changed, 1909 insertions(+), 774 deletions(-) create mode 100644 packages/appkit/src/database/migrations/history.ts create mode 100644 packages/appkit/src/database/migrations/tests/apply.pg.test.ts create mode 100644 packages/appkit/src/database/migrations/tests/fixtures.ts create mode 100644 packages/appkit/src/database/migrations/tests/history.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 23bb21c94..8a2cea71f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -117,6 +117,18 @@ jobs: runs-on: group: databricks-ghec-untrusted-runner-group labels: linux-ubuntu-latest-untrusted-2core-8gb + services: + postgres: + image: postgres:17@sha256:2d2b8998d31037bf721cfdf764d76ba74171b4fab3431b7f72c27c56ddbdf9e3 + env: + POSTGRES_PASSWORD: postgres + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U postgres" + --health-interval 5s + --health-timeout 5s + --health-retries 10 steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 @@ -131,6 +143,8 @@ jobs: run: pnpm install --frozen-lockfile - name: Run Tests run: pnpm test + env: + APPKIT_TEST_PG_URL: postgres://postgres:postgres@localhost:5432/postgres playground-integration-test: name: Playground Integration Tests diff --git a/packages/appkit/src/database/migrations/apply.ts b/packages/appkit/src/database/migrations/apply.ts index a4e6dd704..bc9a1f99d 100644 --- a/packages/appkit/src/database/migrations/apply.ts +++ b/packages/appkit/src/database/migrations/apply.ts @@ -1,18 +1,25 @@ import { createHash } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; +import { setTimeout as sleep } from "node:timers/promises"; import { readMigrationFiles } from "drizzle-orm/migrator"; -import { drizzle } from "drizzle-orm/node-postgres"; -import { migrate } from "drizzle-orm/node-postgres/migrator"; import type { Pool, PoolClient } from "pg"; import { createLogger } from "../../logging/logger"; -import { ownershipHint } from "./ownership"; +import { + type MigrationFile, + type RecordedMigration, + reconcileMigrations, +} from "./history"; +import { + INSUFFICIENT_PRIVILEGE, + ownershipHint, + schemaOwnership, +} from "./ownership"; import { BASELINE_TAG, type Journal, - type JournalEntry, MIGRATIONS_TABLE, MigrationFilesError, readJournal, @@ -26,6 +33,10 @@ interface ApplyMigrationsOptions { readonly migrationsFolder: string; /** Journal schema = app schema. */ readonly schemaName: string; + /** Longest wait for another session's migration lock, in milliseconds. */ + readonly lockWaitMs?: number; + /** Longest wait for each table lock a migration takes, in milliseconds. */ + readonly lockTimeoutMs?: number; } interface ApplyMigrationsResult { @@ -33,59 +44,95 @@ interface ApplyMigrationsResult { readonly applied: readonly string[]; /** Tags recorded without running, because the catalog already held them (the baseline). */ readonly adopted: readonly string[]; + /** Recorded migrations whose committed file changed since; the change never runs. */ + readonly edited: readonly string[]; + /** Recorded migrations that are not committed here. */ + readonly unknown: number; + /** Time spent waiting for another session's migration lock. */ + readonly lockWaitMs: number; } -/** lock_not_available: a table lock was not granted within LOCK_TIMEOUT. */ -const LOCK_NOT_AVAILABLE = "55P03"; +/** A deploy waits this long for another instance to finish migrating, then fails. */ +const LOCK_WAIT_MS = 5 * 60_000; /** - * Longest wait for a table lock while migrating. Past it the migration fails; - * waiting longer would queue every other query on that table behind it. + * Longest wait for one table lock. Past it the migration rolls back and is + * retried: a longer wait would queue every other query on that table behind it. */ -const LOCK_TIMEOUT = "30s"; +const LOCK_TIMEOUT_MS = 5_000; +/** Tries per migration when a table lock is not granted within the lock timeout. */ +const LOCK_ATTEMPTS = 3; +const LOCK_POLL_MS = 1_000; +const LOCK_REPORT_MS = 30_000; +/** Shown in pg_stat_activity, so a waiting instance can name the lock holder. */ +const APPLICATION_NAME = "appkit-migrate"; +/** Lets the server cancel a migration whose instance died. Linux servers only. */ +const CONNECTION_CHECK_INTERVAL = "10s"; -function failureHint(code: string | undefined): string | undefined { - if (code === LOCK_NOT_AVAILABLE) { - return `Another session held a lock on a table this migration changes for over ${LOCK_TIMEOUT}; deploy again once it finishes.`; - } - return ownershipHint(code); -} +/** lock_not_available: a table lock was not granted within the lock timeout. */ +const LOCK_NOT_AVAILABLE = "55P03"; /** - * A statement failed while migrating. Pending migrations run in one drizzle-orm - * migrator transaction, so none of them were applied; a baseline adopted just - * before commits on its own and stays recorded. + * A step of applying migrations failed. Each migration commits in its own + * transaction: the failing one rolled back, the ones before it stay applied. */ export class MigrationApplyError extends Error { - /** Tags that were being applied; empty when reading the recorded migrations failed. */ + /** The migration that failed and rolled back; undefined when another step failed. */ + readonly failed?: string; + /** Committed migrations still not applied, the failed one first. */ readonly pending: readonly string[]; + /** Tags this run applied before the failure. */ + readonly applied: readonly string[]; + /** Tags this run recorded as applied (adopted) before the failure. */ + readonly adopted: readonly string[]; /** SQLSTATE of the failing statement, when the driver reported one. */ readonly code?: string; - /** Tags this run recorded as applied before the failure. */ - readonly adopted: readonly string[]; - constructor( - pending: readonly string[], - code: string | undefined, - options: { cause?: unknown; adopted?: readonly string[] } = {}, - ) { - const adopted = options.adopted ?? []; - const sqlState = code ? ` (SQLSTATE ${code})` : ""; - const failure = - pending.length > 0 - ? `Applying migration${pending.length === 1 ? "" : "s"} ${pending.join(", ")} failed${sqlState}; none were applied.` - : `Reading the recorded migrations in ${MIGRATIONS_TABLE} failed${sqlState}; none were applied.`; - const kept = - adopted.length > 0 - ? ` ${adopted.join(", ")} stays recorded as applied: its tables already existed.` + constructor(details: { + /** What failed when no migration did, e.g. "Connecting to the database". */ + readonly step: string; + readonly failed?: string; + readonly pending?: readonly string[]; + readonly applied?: readonly string[]; + readonly adopted?: readonly string[]; + readonly hint?: string; + readonly cause?: unknown; + }) { + const pending = details.pending ?? []; + const applied = details.applied ?? []; + const adopted = details.adopted ?? []; + const code = sqlStateOf(details.cause); + const systemCode = code ? undefined : systemCodeOf(details.cause); + const reason = code + ? ` (SQLSTATE ${code})` + : systemCode + ? ` (${systemCode})` : ""; - const hint = failureHint(code); - super(`${failure}${kept}${hint ? ` ${hint}` : ""}`, { - cause: options.cause, - }); + const parts = [ + details.failed + ? `Migration ${details.failed} failed${reason} and was rolled back.` + : `${details.step} failed${reason}.`, + ]; + if (adopted.length > 0) { + parts.push( + `${adopted.join(", ")} stays recorded as applied: its tables already existed.`, + ); + } + if (applied.length > 0) { + parts.push(`Applied before the failure: ${applied.join(", ")}.`); + } + parts.push( + pending.length > 0 + ? `Not applied: ${pending.join(", ")}.` + : "No migration was applied.", + ); + if (details.hint) parts.push(details.hint); + super(parts.join(" "), { cause: details.cause }); this.name = "MigrationApplyError"; + this.failed = details.failed; this.pending = Object.freeze([...pending]); - this.code = code; + this.applied = Object.freeze([...applied]); this.adopted = Object.freeze([...adopted]); + this.code = code; } } @@ -106,26 +153,67 @@ export class MigrationBaselineMismatchError extends Error { } } +/** + * Committed migrations are older than the last one the database recorded and + * were never applied, so they would never run. Nothing was applied. + */ +export class MigrationHistoryError extends Error { + readonly stranded: readonly string[]; + + constructor(schemaName: string, stranded: readonly string[], last: string) { + const many = stranded.length > 1; + super( + `Migration${many ? "s" : ""} ${stranded.join(", ")} ${many ? "are" : "is"} older than ${last}, which schema "${schemaName}" already recorded, ` + + `and ${many ? "were" : "was"} never applied, so ${many ? "they" : "it"} would never run. This happens when migrations from parallel branches merge. ` + + `Delete ${many ? "their" : "its"} .sql file, snapshot and journal entry, generate the migration again from schema.ts, and deploy again. ` + + "No migration was applied.", + ); + this.name = "MigrationHistoryError"; + this.stranded = Object.freeze([...stranded]); + } +} + +/** The migration lock stayed taken past the wait limit. */ +class MigrationLockWaitError extends Error { + constructor(readonly holder: string | undefined) { + super("migration lock wait timed out"); + } +} + const SQLSTATE = /^[0-9A-Z]{5}$/; +const SYSTEM_CODE = /^E[A-Z0-9_]+$/; -/** drizzle wraps driver errors; the SQLSTATE lives somewhere on the cause chain. */ -function sqlStateOf(error: unknown): string | undefined { +/** drizzle and the connector wrap driver errors; the code lives somewhere on the cause chain. */ +function findCode( + error: unknown, + matches: (code: string) => boolean, +): string | undefined { let current = error; for (let depth = 0; depth < 5 && current; depth++) { const code = (current as { code?: unknown }).code; - // No SQLSTATE class starts with "E"; Node system errors (EPIPE, EPERM, …) all do. - if (typeof code === "string" && SQLSTATE.test(code) && code[0] !== "E") { - return code; - } + if (typeof code === "string" && matches(code)) return code; current = (current as { cause?: unknown }).cause; } return undefined; } +// No SQLSTATE class starts with "E"; Node system errors (EPIPE, ECONNREFUSED, …) all do. +function sqlStateOf(error: unknown): string | undefined { + return findCode(error, (code) => SQLSTATE.test(code) && code[0] !== "E"); +} + +function systemCodeOf(error: unknown): string | undefined { + return findCode(error, (code) => SYSTEM_CODE.test(code)); +} + function quoteIdentifier(name: string): string { return `"${name.replace(/"/g, '""')}"`; } +function journalTable(schemaName: string): string { + return `${quoteIdentifier(schemaName)}.${quoteIdentifier(MIGRATIONS_TABLE)}`; +} + /** One lock per app schema, so concurrent instances apply in turn. */ export function migrationLockKey(schemaName: string): string { return createHash("sha256") @@ -137,15 +225,40 @@ export function migrationLockKey(schemaName: string): string { type QueryClient = Pick; -/** - * `created_at` of the last migration the database recorded; null when none is. - * An empty journal table counts as nothing recorded: drizzle-orm creates it - * before its transaction, so a failed first deploy leaves one behind. - */ -async function lastRecordedMigration( +/** Every committed migration, in journal order. Fails on a missing file before connecting. */ +function readMigrations(folder: string, journal: Journal): MigrationFile[] { + for (const entry of journal.entries) { + const file = path.join(folder, `${entry.tag}.sql`); + if (!fs.existsSync(file)) { + throw new MigrationFilesError( + `Missing ${file}, listed in meta/_journal.json. Restore it from version control.`, + ); + } + } + // drizzle-orm's reader, so `hash` matches what drizzle-kit records. + const files = readMigrationFiles({ migrationsFolder: folder }); + return journal.entries.map((entry, index) => { + const file = files[index]; + if (!file) { + throw new MigrationFilesError( + `Cannot read ${entry.tag}.sql in ${folder}. Restore it from version control.`, + ); + } + return { + tag: entry.tag, + idx: entry.idx, + when: entry.when, + hash: file.hash, + statements: file.sql, + }; + }); +} + +/** The journal table's rows; none when the table does not exist yet. */ +async function readRecordedMigrations( client: QueryClient, schemaName: string, -): Promise { +): Promise { // pg_catalog needs no privileges, so a missing grant is not mistaken for a missing table. const table = await client.query( `select 1 from pg_catalog.pg_class c @@ -153,99 +266,103 @@ async function lastRecordedMigration( where n.nspname = $1 and c.relname = $2`, [schemaName, MIGRATIONS_TABLE], ); - if (table.rows.length === 0) return null; - const last = await client.query<{ last: string | number | null }>( - `select max(created_at) as last from ${quoteIdentifier(schemaName)}.${quoteIdentifier(MIGRATIONS_TABLE)}`, - ); - const value = last.rows[0]?.last; - return value === null || value === undefined ? null : Number(value); + if (table.rows.length === 0) return []; + const { rows } = await client.query<{ + hash: string; + created_at: string | number | null; + }>(`select hash, created_at from ${journalTable(schemaName)} order by id`); + return rows.map((row) => ({ + hash: row.hash, + createdAt: row.created_at === null ? null : Number(row.created_at), + })); } -/** - * Journal entries newer than the last recorded migration: drizzle-orm's own - * rule, so this agrees with what its migrator runs. - * - * Known gap, shared with drizzle-orm: an entry older than the last recorded one - * (e.g. merged from a parallel branch) is never applied, and an applied - * migration edited later goes unnoticed. Comparing the recorded hashes with the - * journal would catch both. - */ -function pendingEntries( - journal: Journal, - lastRecorded: number | null, -): JournalEntry[] { - if (lastRecorded === null) return journal.entries; - return journal.entries.filter((entry) => entry.when > lastRecorded); +/** Session settings for migrating, each added to `reset` once set. */ +async function prepareMigrationSession( + client: QueryClient, + reset: string[], +): Promise { + await client.query("SET statement_timeout = 0"); + reset.push("statement_timeout"); + await client.query(`SET application_name = '${APPLICATION_NAME}'`); + reset.push("application_name"); + try { + await client.query( + `SET client_connection_check_interval = '${CONNECTION_CHECK_INTERVAL}'`, + ); + reset.push("client_connection_check_interval"); + } catch (error) { + // Postgres before 14, or a server that cannot poll sockets. + logger.debug( + "client_connection_check_interval not set: %s", + error instanceof Error ? error.message : String(error), + ); + } } -/** Fail before connecting, naming the file, rather than inside drizzle-orm's migrator. */ -function assertMigrationFiles(folder: string, journal: Journal): void { - for (const entry of journal.entries) { - const file = path.join(folder, `${entry.tag}.sql`); - if (!fs.existsSync(file)) { - throw new MigrationFilesError( - `Missing ${file}, listed in meta/_journal.json. Restore it from version control.`, - ); - } - } +/** pg_locks splits a bigint advisory key into classid (high 32 bits) and objid (low 32 bits). */ +function lockKeyParts(key: string): [string, string] { + const unsigned = BigInt.asUintN(64, BigInt(key)); + return [(unsigned >> 32n).toString(), (unsigned & 0xffffffffn).toString()]; +} + +/** "pid 123 (appkit-migrate, active)" for the session holding the migration lock. */ +async function migrationLockHolder( + client: QueryClient, + key: string, +): Promise { + const [classid, objid] = lockKeyParts(key); + const { rows } = await client.query<{ + pid: number; + application_name: string | null; + state: string | null; + }>( + `select a.pid, a.application_name, a.state + from pg_catalog.pg_locks l + join pg_catalog.pg_stat_activity a on a.pid = l.pid + where l.locktype = 'advisory' and l.granted + and l.database = (select d.oid from pg_catalog.pg_database d where d.datname = current_database()) + and l.classid = $1::oid and l.objid = $2::oid and l.objsubid = 1`, + [classid, objid], + ); + const holder = rows[0]; + if (!holder) return undefined; + const details = [holder.application_name, holder.state].filter(Boolean); + return `pid ${holder.pid}${details.length > 0 ? ` (${details.join(", ")})` : ""}`; } /** - * Run `work` on one dedicated connection under the per-schema advisory lock, - * with no statement timeout and at most LOCK_TIMEOUT per table lock. The - * connection is always unlocked, reset and released; a connection whose - * cleanup fails is destroyed, not reused. + * Take the per-schema advisory lock, polling so the wait has a limit and the + * log names the holder. Returns how long it waited. */ -async function withMigrationLock( - pool: Pick, +async function acquireMigrationLock( + client: QueryClient, schemaName: string, - work: (client: PoolClient) => Promise, -): Promise { - const lockKey = migrationLockKey(schemaName); - const client = await pool.connect(); - let locked = false; - try { - await client.query("SET statement_timeout = 0"); - const attempt = await client.query<{ locked: boolean }>( + waitMs: number, +): Promise { + const key = migrationLockKey(schemaName); + const started = Date.now(); + let reportAt = started; + for (;;) { + const { rows } = await client.query<{ locked: boolean }>( "select pg_catalog.pg_try_advisory_lock($1::bigint) as locked", - [lockKey], + [key], ); - if (!attempt.rows[0]?.locked) { + if (rows[0]?.locked) return Date.now() - started; + const waited = Date.now() - started; + if (waited >= waitMs || Date.now() >= reportAt) { + const holder = await migrationLockHolder(client, key).catch( + () => undefined, + ); + if (waited >= waitMs) throw new MigrationLockWaitError(holder); logger.info( - "Waiting for another instance to finish migrating schema %s", + "Waiting for another session to finish migrating schema %s (lock held by %s)", schemaName, + holder ?? "an unknown session", ); - await client.query("select pg_catalog.pg_advisory_lock($1::bigint)", [ - lockKey, - ]); + reportAt = Date.now() + LOCK_REPORT_MS; } - locked = true; - // Only now: lock_timeout would also cut short the wait for the advisory lock. - await client.query(`SET lock_timeout = '${LOCK_TIMEOUT}'`); - return await work(client); - } finally { - const cleanup: Array<() => Promise> = []; - if (locked) { - cleanup.push(() => - client.query("select pg_catalog.pg_advisory_unlock($1::bigint)", [ - lockKey, - ]), - ); - } - cleanup.push(() => client.query("RESET lock_timeout")); - cleanup.push(() => client.query("RESET statement_timeout")); - - let cleanupError: Error | undefined; - for (const step of cleanup) { - try { - await step(); - } catch (error) { - cleanupError ??= - error instanceof Error ? error : new Error(String(error)); - } - } - // A connection that may still hold the lock or the session settings must not be reused. - client.release(cleanupError); + await sleep(Math.min(LOCK_POLL_MS, waitMs - waited)); } } @@ -263,6 +380,74 @@ async function inTransaction( } } +/** drizzle-orm's own journal table, so drizzle-kit reads and extends it too. */ +async function ensureJournalTable( + client: QueryClient, + schemaName: string, +): Promise { + await client.query( + `CREATE SCHEMA IF NOT EXISTS ${quoteIdentifier(schemaName)}`, + ); + await client.query( + `CREATE TABLE IF NOT EXISTS ${journalTable(schemaName)} ( + id SERIAL PRIMARY KEY, + hash text NOT NULL, + created_at bigint + )`, + ); +} + +/** Record a migration the way drizzle-orm's migrator does: its file hash and the journal's `when`. */ +async function recordMigration( + client: QueryClient, + schemaName: string, + file: MigrationFile, +): Promise { + await client.query( + `insert into ${journalTable(schemaName)} ("hash", "created_at") values($1, $2)`, + [file.hash, file.when], + ); +} + +/** + * Run one migration and record it in one transaction. A table lock not granted + * within the lock timeout rolls it back; it is retried LOCK_ATTEMPTS times. + */ +async function applyMigration( + client: QueryClient, + schemaName: string, + file: MigrationFile, + lockTimeoutMs: number, +): Promise { + for (let attempt = 1; ; attempt++) { + try { + await inTransaction(client, async () => { + await ensureJournalTable(client, schemaName); + for (const statement of file.statements) { + if (statement.trim()) await client.query(statement); + } + await recordMigration(client, schemaName, file); + }); + return; + } catch (error) { + if ( + attempt >= LOCK_ATTEMPTS || + sqlStateOf(error) !== LOCK_NOT_AVAILABLE + ) { + throw error; + } + logger.warn( + "Migration %s waited over %dms for a table lock (try %d of %d); retrying", + file.tag, + lockTimeoutMs, + attempt, + LOCK_ATTEMPTS, + ); + await sleep(Math.min(attempt * LOCK_POLL_MS, lockTimeoutMs)); + } + } +} + /** The tables and columns a migration snapshot (meta/NNNN_snapshot.json) declares in one schema. */ function snapshotTables( folder: string, @@ -302,7 +487,7 @@ type BaselineState = async function baselineState( client: QueryClient, options: ApplyMigrationsOptions, - baseline: JournalEntry, + baseline: MigrationFile, ): Promise { const declared = snapshotTables( options.migrationsFolder, @@ -351,52 +536,60 @@ async function baselineState( : { kind: "present" }; } -/** - * Record migrations as applied without running them: create the journal table - * as drizzle-orm's migrator does, then insert one row per tag with the - * migrator's own hash (sha256 of the .sql file) and `created_at` (the journal's - * `when`). Runs on the caller's connection; wrap it in a transaction. - */ -async function recordMigrationsApplied( - client: QueryClient, - options: ApplyMigrationsOptions, - journal: Journal, - tags: readonly string[], +/** Unlock and reset the session. A connection whose cleanup fails is destroyed, not reused. */ +async function releaseMigrationClient( + client: PoolClient, + lockKey: string | undefined, + reset: readonly string[], ): Promise { - // One file per journal entry, in journal order. - const files = readMigrationFiles({ - migrationsFolder: options.migrationsFolder, - }); - const schema = quoteIdentifier(options.schemaName); - const table = `${schema}.${quoteIdentifier(MIGRATIONS_TABLE)}`; - await client.query(`CREATE SCHEMA IF NOT EXISTS ${schema}`); - // drizzle-orm's own definition, so its migrator later reads and extends this table. - await client.query( - `CREATE TABLE IF NOT EXISTS ${table} ( - id SERIAL PRIMARY KEY, - hash text NOT NULL, - created_at bigint - )`, - ); - for (const tag of tags) { - const index = journal.entries.findIndex((entry) => entry.tag === tag); - const file = files[index]; - if (index < 0 || !file) { - throw new Error(`Migration ${tag} is not in the journal.`); - } - await client.query( - `insert into ${table} ("hash", "created_at") values($1, $2)`, - [file.hash, file.folderMillis], + const cleanup: Array<() => Promise> = []; + if (lockKey) { + cleanup.push(() => + client.query("select pg_catalog.pg_advisory_unlock($1::bigint)", [ + lockKey, + ]), ); } + for (const name of [...reset].reverse()) { + cleanup.push(() => client.query(`RESET ${name}`)); + } + let cleanupError: Error | undefined; + for (const step of cleanup) { + try { + await step(); + } catch (error) { + cleanupError ??= + error instanceof Error ? error : new Error(String(error)); + } + } + client.release(cleanupError); +} + +function tags(files: readonly MigrationFile[]): string[] { + return files.map((file) => file.tag); +} + +function duration(ms: number): string { + if (ms >= 60_000) { + const minutes = Math.round(ms / 60_000); + return `${minutes} minute${minutes === 1 ? "" : "s"}`; + } + if (ms >= 1_000) return `${Math.round(ms / 1_000)}s`; + return `${ms}ms`; } /** * Apply every committed migration the database has not recorded yet. * - * Runs on one dedicated connection as the connecting identity, which owns what - * it creates (see ownership.ts), under a per-schema advisory lock; drizzle-orm's - * migrator runs only when something is pending. + * Runs on one connection as the connecting identity, which owns what it + * creates (see ownership.ts). The recorded history is read first without a + * lock; only when something is pending does it take the per-schema advisory + * lock and read again. Each migration then runs and is recorded in its own + * transaction, so a failure keeps the ones before it. + * + * Fails closed, changing nothing, when a committed migration is older than the + * last recorded one but was never applied. An applied migration whose file + * changed, and recorded migrations that are not committed, are logged. * * Baseline adoption: when nothing is recorded yet and `0000_baseline` comes * first, the baseline's tables are looked up in pg_catalog. None present → it @@ -410,60 +603,150 @@ export async function applyPendingMigrations( ): Promise { const journal = readJournal(options.migrationsFolder); if (!journal || journal.entries.length === 0) { - return { applied: [], adopted: [] }; + return { applied: [], adopted: [], edited: [], unknown: 0, lockWaitMs: 0 }; } - assertMigrationFiles(options.migrationsFolder, journal); + const files = readMigrations(options.migrationsFolder, journal); + const { schemaName } = options; + const lockWaitMs = options.lockWaitMs ?? LOCK_WAIT_MS; + const lockTimeoutMs = options.lockTimeoutMs ?? LOCK_TIMEOUT_MS; - return withMigrationLock(pool, options.schemaName, async (client) => { - const adopted: string[] = []; - // What the failing step was applying, for the error. - let applying: readonly string[] = []; - try { - const lastRecorded = await lastRecordedMigration( - client, - options.schemaName, + let client: PoolClient; + try { + client = await pool.connect(); + } catch (error) { + throw new MigrationApplyError({ + step: "Connecting to the database", + cause: error, + }); + } + + const reset: string[] = []; + let locked = false; + const applied: string[] = []; + const adopted: string[] = []; + let pending: string[] = []; + let step = "Reading the recorded migrations"; + let failed: string | undefined; + try { + let history = reconcileMigrations( + files, + await readRecordedMigrations(client, schemaName), + ); + let waitedMs = 0; + if (history.pending.length > 0 && history.stranded.length === 0) { + pending = tags(history.pending); + step = "Waiting for the migration lock"; + await prepareMigrationSession(client, reset); + waitedMs = await acquireMigrationLock(client, schemaName, lockWaitMs); + locked = true; + await client.query(`SET lock_timeout = '${lockTimeoutMs}ms'`); + reset.push("lock_timeout"); + step = "Reading the recorded migrations"; + // Another instance may have applied them while this one waited. + history = reconcileMigrations( + files, + await readRecordedMigrations(client, schemaName), ); - let pending = pendingEntries(journal, lastRecorded); - if (pending.length === 0) return { applied: [], adopted }; - - const first = journal.entries[0]; - if (lastRecorded === null && first?.tag === BASELINE_TAG) { - applying = [BASELINE_TAG]; - const state = await baselineState(client, options, first); - if (state.kind === "partial") { - throw new MigrationBaselineMismatchError( - options.schemaName, - state.problems, - ); - } - if (state.kind === "present") { - await inTransaction(client, () => - recordMigrationsApplied(client, options, journal, [BASELINE_TAG]), - ); - adopted.push(BASELINE_TAG); - pending = pending.filter((entry) => entry.tag !== BASELINE_TAG); - if (pending.length === 0) return { applied: [], adopted }; - } - } + } - applying = pending.map((entry) => entry.tag); - await migrate(drizzle({ client }), { - migrationsFolder: options.migrationsFolder, - migrationsSchema: options.schemaName, - migrationsTable: MIGRATIONS_TABLE, - }); - return { applied: applying, adopted }; - } catch (error) { - if ( - error instanceof MigrationBaselineMismatchError || - error instanceof MigrationFilesError - ) { - throw error; + if (history.stranded.length > 0) { + const last = + files.find((file) => file.when === history.lastRecorded)?.tag ?? + `the migration recorded at ${new Date(history.lastRecorded ?? 0).toISOString()}`; + throw new MigrationHistoryError(schemaName, tags(history.stranded), last); + } + if (history.edited.length > 0) { + logger.warn( + "Migrations %s changed after schema %s applied them; the changes never run. Restore the files and put the change in a new migration.", + tags(history.edited).join(", "), + schemaName, + ); + } + if (history.unknown.length > 0) { + logger.warn( + "Schema %s recorded %d migration(s) that are not committed here. A newer release applied them, or their files were deleted.", + schemaName, + history.unknown.length, + ); + } + + const toApply = [...history.pending]; + pending = tags(toApply); + const baseline = toApply[0]; + if (history.lastRecorded === null && baseline?.tag === BASELINE_TAG) { + failed = BASELINE_TAG; + const state = await baselineState(client, options, baseline); + if (state.kind === "partial") { + throw new MigrationBaselineMismatchError(schemaName, state.problems); + } + if (state.kind === "present") { + await inTransaction(client, async () => { + await ensureJournalTable(client, schemaName); + await recordMigration(client, schemaName, baseline); + }); + adopted.push(BASELINE_TAG); + toApply.shift(); + pending.shift(); } - throw new MigrationApplyError(applying, sqlStateOf(error), { - cause: error, - adopted, - }); } - }); + + for (const file of toApply) { + failed = file.tag; + const started = Date.now(); + await applyMigration(client, schemaName, file, lockTimeoutMs); + applied.push(file.tag); + pending.shift(); + logger.info( + "Applied migration %s to schema %s in %dms", + file.tag, + schemaName, + Date.now() - started, + ); + } + return { + applied, + adopted, + edited: tags(history.edited), + unknown: history.unknown.length, + lockWaitMs: waitedMs, + }; + } catch (error) { + if ( + error instanceof MigrationBaselineMismatchError || + error instanceof MigrationFilesError || + error instanceof MigrationHistoryError + ) { + throw error; + } + const code = sqlStateOf(error); + let hint: string | undefined; + if (error instanceof MigrationLockWaitError) { + const holder = error.holder + ? `Session ${error.holder}` + : "Another session"; + hint = `${holder} held the migration lock for over ${duration(lockWaitMs)}. Deploy again once it finishes; if it is stuck, end it with pg_terminate_backend.`; + } else if (code === LOCK_NOT_AVAILABLE) { + hint = `Another session held a lock on a table this migration changes for over ${duration(lockTimeoutMs)}, ${LOCK_ATTEMPTS} times. Deploy again once the long-running query or transaction finishes.`; + } else if (code === INSUFFICIENT_PRIVILEGE) { + const ownership = await schemaOwnership(client, schemaName).catch( + () => undefined, + ); + hint = ownershipHint(code, ownership); + } + throw new MigrationApplyError({ + step, + failed, + pending, + applied, + adopted, + hint, + cause: error, + }); + } finally { + await releaseMigrationClient( + client, + locked ? migrationLockKey(schemaName) : undefined, + reset, + ); + } } diff --git a/packages/appkit/src/database/migrations/history.ts b/packages/appkit/src/database/migrations/history.ts new file mode 100644 index 000000000..c7ca35cda --- /dev/null +++ b/packages/appkit/src/database/migrations/history.ts @@ -0,0 +1,64 @@ +/** A committed migration: its journal entry plus what drizzle-orm reads from its .sql file. */ +export interface MigrationFile { + readonly tag: string; + readonly idx: number; + /** The journal's `when`, recorded as `created_at`. */ + readonly when: number; + /** sha256 of the .sql file, recorded as `hash`. */ + readonly hash: string; + /** The file split on `--> statement-breakpoint`. */ + readonly statements: readonly string[]; +} + +/** One row of the journal table. */ +export interface RecordedMigration { + readonly hash: string; + readonly createdAt: number | null; +} + +/** Committed migrations compared with what the database recorded. */ +interface MigrationHistory { + /** Not recorded and newer than every recorded migration: applied next, oldest first. */ + readonly pending: readonly MigrationFile[]; + /** Not recorded but older than the last recorded migration, so a later one ran first. */ + readonly stranded: readonly MigrationFile[]; + /** Recorded, but the committed file changed since; the change never runs. */ + readonly edited: readonly MigrationFile[]; + /** Recorded rows no committed migration matches: a newer release ran, or files were deleted. */ + readonly unknown: readonly RecordedMigration[]; + /** `created_at` of the newest recorded migration; null when none is. */ + readonly lastRecorded: number | null; +} + +/** + * Match each committed migration with a recorded row by `when` = `created_at`, + * the pair drizzle-orm's migrator writes, then compare hashes. + */ +export function reconcileMigrations( + files: readonly MigrationFile[], + recorded: readonly RecordedMigration[], +): MigrationHistory { + const hashAt = new Map(); + for (const row of recorded) { + if (row.createdAt !== null) hashAt.set(row.createdAt, row.hash); + } + const lastRecorded = hashAt.size > 0 ? Math.max(...hashAt.keys()) : null; + + const pending: MigrationFile[] = []; + const stranded: MigrationFile[] = []; + const edited: MigrationFile[] = []; + for (const file of files) { + const hash = hashAt.get(file.when); + if (hash === undefined) { + if (lastRecorded === null || file.when > lastRecorded) pending.push(file); + else stranded.push(file); + } else if (hash !== file.hash) { + edited.push(file); + } + } + const committed = new Set(files.map((file) => file.when)); + const unknown = recorded.filter( + (row) => row.createdAt === null || !committed.has(row.createdAt), + ); + return { pending, stranded, edited, unknown, lastRecorded }; +} diff --git a/packages/appkit/src/database/migrations/ownership.ts b/packages/appkit/src/database/migrations/ownership.ts index 606efb1d1..ec2dd75ac 100644 --- a/packages/appkit/src/database/migrations/ownership.ts +++ b/packages/appkit/src/database/migrations/ownership.ts @@ -1,18 +1,66 @@ // Ownership model: each branch has one owner, the identity its migrations run // as, and that identity owns the app's objects there. On the deploy branch it // is the app's service principal, which creates the app schema on first start. -// There is no shared owner role and no SET ROLE. A shared owner role is not -// possible: Lakebase rejects `GRANT … TO databricks_superuser`. +// There is no shared owner role and no SET ROLE: a shared role needs someone +// holding ADMIN OPTION on it to grant it to every new app, which a deployed +// app cannot do for itself. + +import type { PoolClient } from "pg"; /** insufficient_privilege: "must be owner of …", "permission denied for …". */ -const INSUFFICIENT_PRIVILEGE = "42501"; +export const INSUFFICIENT_PRIVILEGE = "42501"; + +/** Who owns the app schema, next to who is connected; owner is null when the schema does not exist. */ +interface SchemaOwnership { + readonly schemaName: string; + readonly owner: string | null; + readonly currentUser: string; +} + +/** Read the app schema's owner. pg_catalog needs no privileges. */ +export async function schemaOwnership( + client: Pick, + schemaName: string, +): Promise { + const { rows } = await client.query<{ + owner: string | null; + current_user: string; + }>( + `select (select pg_catalog.pg_get_userbyid(n.nspowner) + from pg_catalog.pg_namespace n where n.nspname = $1) as owner, + current_user::text as current_user`, + [schemaName], + ); + return { + schemaName, + owner: rows[0]?.owner ?? null, + currentUser: rows[0]?.current_user ?? "", + }; +} /** How to fix a migration that failed for lack of privileges; undefined for any other SQLSTATE. */ -export function ownershipHint(code: string | undefined): string | undefined { +export function ownershipHint( + code: string | undefined, + ownership?: SchemaOwnership, +): string | undefined { if (code !== INSUFFICIENT_PRIVILEGE) return undefined; + if (ownership && ownership.owner === null) { + return ( + `Role "${ownership.currentUser}" cannot create schema "${ownership.schemaName}". ` + + "On the deploy branch that role is the app's service principal: bind the database to the app with CAN_CONNECT_AND_CREATE." + ); + } + if (ownership && ownership.owner !== ownership.currentUser) { + return ( + `Schema "${ownership.schemaName}" is owned by "${ownership.owner}", but the app connects as "${ownership.currentUser}". ` + + "This happens when the app's database resource was removed and added again, or the app was recreated with a new service principal: " + + `Lakebase hands the objects to another role. Have "${ownership.owner}" transfer the schema and its tables to "${ownership.currentUser}" ` + + "(ALTER … OWNER TO), or set another schemaName in schema.ts." + ); + } return ( "The connecting role lacks a privilege this migration needs: CREATE on the database, or ownership of the app schema and its tables. " + "On the deploy branch that role is the app's service principal: bind the database to the app with CAN_CONNECT_AND_CREATE, " + - "and if another role created the schema, use another schemaName in schema.ts or have that role transfer the schema and its objects." + "and if another role created an object the migration changes, have that role transfer it to the app's service principal." ); } diff --git a/packages/appkit/src/database/migrations/paths.ts b/packages/appkit/src/database/migrations/paths.ts index e331080b0..8d5126076 100644 --- a/packages/appkit/src/database/migrations/paths.ts +++ b/packages/appkit/src/database/migrations/paths.ts @@ -34,7 +34,7 @@ export function migrationsFolder(root: string): string { } /** Path of the journal inside a migrations folder. */ -function journalFile(folder: string): string { +export function journalFile(folder: string): string { return path.join(folder, "meta", "_journal.json"); } diff --git a/packages/appkit/src/database/migrations/tests/apply.pg.test.ts b/packages/appkit/src/database/migrations/tests/apply.pg.test.ts new file mode 100644 index 000000000..7e4875576 --- /dev/null +++ b/packages/appkit/src/database/migrations/tests/apply.pg.test.ts @@ -0,0 +1,408 @@ +import { randomUUID } from "node:crypto"; + +import { Pool, type PoolClient } from "pg"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + test, + vi, +} from "vitest"; + +import { + applyPendingMigrations, + MigrationApplyError, + MigrationHistoryError, + migrationLockKey, +} from "../apply"; +import { hashOf, removeMigrationFolders, writeMigrations } from "./fixtures"; + +// A real server, for what a fake cannot show: transactions, enum rules, locks +// and catalogs. Point it at a scratch database, e.g. +// APPKIT_TEST_PG_URL=postgres://localhost:5432/postgres +const url = process.env.APPKIT_TEST_PG_URL; +const ROLE_PASSWORD = "appkit-test"; + +describe.skipIf(!url)("applyPendingMigrations against Postgres", () => { + let admin: Pool; + let schema: string; + const pools: Pool[] = []; + const held: PoolClient[] = []; + + const newPool = (config: { user?: string; max?: number } = {}) => { + const target = new URL(url ?? ""); + if (config.user) { + target.username = config.user; + target.password = ROLE_PASSWORD; + } + const pool = new Pool({ + connectionString: target.toString(), + max: config.max ?? 2, + }); + pools.push(pool); + return pool; + }; + const q = (name: string) => `"${schema}"."${name}"`; + const options = (folder: string) => ({ + migrationsFolder: folder, + schemaName: schema, + }); + const recorded = async () => + ( + await admin.query<{ hash: string; created_at: string }>( + `select hash, created_at from ${q("__drizzle_migrations")} order by id`, + ) + ).rows.map((row) => ({ + hash: row.hash, + createdAt: Number(row.created_at), + })); + const columns = async (table: string) => + ( + await admin.query<{ column_name: string }>( + `select column_name from information_schema.columns + where table_schema = $1 and table_name = $2 order by ordinal_position`, + [schema, table], + ) + ).rows.map((row) => row.column_name); + + beforeAll(() => { + admin = new Pool({ connectionString: url, max: 4 }); + }); + afterAll(async () => { + await admin.end(); + }); + beforeEach(() => { + schema = `appkit_test_${randomUUID().slice(0, 8)}`; + vi.spyOn(console, "log").mockImplementation(() => undefined); + vi.spyOn(console, "warn").mockImplementation(() => undefined); + }); + afterEach(async () => { + for (const client of held.splice(0)) client.release(true); + await Promise.all(pools.splice(0).map((pool) => pool.end())); + await admin.query(`drop schema if exists "${schema}" cascade`); + removeMigrationFolders(); + vi.restoreAllMocks(); + }); + + const notes = () => + writeMigrations([ + { + tag: "0000_notes", + when: 1000, + sql: [`CREATE TABLE ${q("notes")} ("id" integer PRIMARY KEY);`], + }, + { + tag: "0001_body", + when: 2000, + sql: [ + `ALTER TABLE ${q("notes")} ADD COLUMN "body" text;`, + `ALTER TABLE ${q("notes")} ADD COLUMN "title" text;`, + ], + }, + ]); + + test("applies each migration once and records what drizzle-kit records", async () => { + const folder = notes(); + const pool = newPool(); + + const first = await applyPendingMigrations(pool, options(folder)); + const second = await applyPendingMigrations(pool, options(folder)); + + expect(first.applied).toEqual(["0000_notes", "0001_body"]); + expect(second.applied).toEqual([]); + expect(await columns("notes")).toEqual(["id", "body", "title"]); + expect(await recorded()).toEqual([ + { hash: hashOf(folder, "0000_notes"), createdAt: 1000 }, + { hash: hashOf(folder, "0001_body"), createdAt: 2000 }, + ]); + }); + + test("an enum value one migration adds can be used by the next one", async () => { + // In one transaction for all, Postgres rejects this with 55P04. + const folder = writeMigrations([ + { + tag: "0000_mood", + when: 1000, + sql: [ + `CREATE TYPE ${q("mood")} AS ENUM ('ok');`, + `CREATE TABLE ${q("notes")} ("id" integer, "mood" ${q("mood")});`, + ], + }, + { + tag: "0001_great", + when: 2000, + sql: [`ALTER TYPE ${q("mood")} ADD VALUE 'great';`], + }, + { + tag: "0002_default", + when: 3000, + sql: [ + `ALTER TABLE ${q("notes")} ALTER COLUMN "mood" SET DEFAULT 'great';`, + ], + }, + ]); + + const result = await applyPendingMigrations(newPool(), options(folder)); + + expect(result.applied).toEqual(["0000_mood", "0001_great", "0002_default"]); + }); + + test("a failing migration keeps the ones before it, and runs once it is fixed", async () => { + const folder = writeMigrations([ + { + tag: "0000_notes", + when: 1000, + sql: [`CREATE TABLE ${q("notes")} ("id" integer);`], + }, + { + tag: "0001_broken", + when: 2000, + sql: [`ALTER TABLE ${q("missing")} ADD COLUMN "x" integer;`], + }, + ]); + const pool = newPool(); + + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect(error).toMatchObject({ + failed: "0001_broken", + applied: ["0000_notes"], + code: "42P01", + }); + expect(await columns("notes")).toEqual(["id"]); + expect(await recorded()).toHaveLength(1); + + const fixed = writeMigrations([ + { + tag: "0000_notes", + when: 1000, + sql: [`CREATE TABLE ${q("notes")} ("id" integer);`], + }, + { + tag: "0001_broken", + when: 2000, + sql: [`ALTER TABLE ${q("notes")} ADD COLUMN "x" integer;`], + }, + ]); + const result = await applyPendingMigrations(pool, options(fixed)); + expect(result.applied).toEqual(["0001_broken"]); + }); + + test("a migration merged behind the last recorded one fails closed and changes nothing", async () => { + const a = { + tag: "0000_a", + when: 1000, + sql: [`CREATE TABLE ${q("a")} ("id" integer);`], + }; + const c = { + tag: "0002_c", + when: 3000, + sql: [`CREATE TABLE ${q("c")} ("id" integer);`], + }; + const pool = newPool(); + await applyPendingMigrations(pool, options(writeMigrations([a, c]))); + + const merged = writeMigrations([ + a, + { + tag: "0001_b", + when: 2000, + sql: [`CREATE TABLE ${q("b")} ("id" integer);`], + }, + c, + { + tag: "0003_d", + when: 4000, + sql: [`CREATE TABLE ${q("d")} ("id" integer);`], + }, + ]); + const error = await applyPendingMigrations(pool, options(merged)).catch( + (caught: unknown) => caught, + ); + + expect(error).toBeInstanceOf(MigrationHistoryError); + expect(error).toMatchObject({ stranded: ["0001_b"] }); + expect(await recorded()).toHaveLength(2); + expect(await columns("d")).toEqual([]); + }); + + test("instances starting together apply each migration once", async () => { + const folder = notes(); + + const results = await Promise.all([ + applyPendingMigrations(newPool(), options(folder)), + applyPendingMigrations(newPool(), options(folder)), + applyPendingMigrations(newPool(), options(folder)), + ]); + + expect(results.flatMap((result) => result.applied).sort()).toEqual([ + "0000_notes", + "0001_body", + ]); + expect(await recorded()).toHaveLength(2); + }); + + test("the wait for another session's migration lock has a limit, and the error names the holder", async () => { + const holder = await admin.connect(); + held.push(holder); + await holder.query("set application_name = 'another-deploy'"); + await holder.query("select pg_advisory_lock($1::bigint)", [ + migrationLockKey(schema), + ]); + const pid = ( + await holder.query<{ pid: number }>("select pg_backend_pid() as pid") + ).rows[0]?.pid; + + const error = await applyPendingMigrations(newPool(), { + ...options(notes()), + lockWaitMs: 300, + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect((error as Error).message).toContain( + `Session pid ${pid} (another-deploy, idle) held the migration lock for over 300ms.`, + ); + }); + + test("a table lock held elsewhere is retried, and the migration applies once it is released", async () => { + const folder = notes(); + const pool = newPool(); + await applyPendingMigrations(pool, { + ...options( + writeMigrations([ + { + tag: "0000_notes", + when: 1000, + sql: [`CREATE TABLE ${q("notes")} ("id" integer PRIMARY KEY);`], + }, + ]), + ), + }); + const holder = await admin.connect(); + held.push(holder); + await holder.query("begin"); + await holder.query(`lock table ${q("notes")} in access share mode`); + setTimeout(() => void holder.query("commit"), 300); + + const result = await applyPendingMigrations(pool, { + ...options(folder), + lockTimeoutMs: 200, + }); + + expect(result.applied).toEqual(["0001_body"]); + expect(console.warn).toHaveBeenCalled(); + }); + + test("a table lock held through every try fails with 55P03 and leaves the migration unapplied", async () => { + const pool = newPool(); + await applyPendingMigrations(pool, { + ...options( + writeMigrations([ + { + tag: "0000_notes", + when: 1000, + sql: [`CREATE TABLE ${q("notes")} ("id" integer PRIMARY KEY);`], + }, + ]), + ), + }); + const holder = await admin.connect(); + held.push(holder); + await holder.query("begin"); + await holder.query(`lock table ${q("notes")} in access share mode`); + + const error = await applyPendingMigrations(pool, { + ...options(notes()), + lockTimeoutMs: 100, + }).catch((caught: unknown) => caught); + await holder.query("rollback"); + + expect(error).toMatchObject({ failed: "0001_body", code: "55P03" }); + expect(await columns("notes")).toEqual(["id"]); + }); + + test("the session goes back to the pool reset and unlocked", async () => { + const pool = newPool({ max: 1 }); + const before = await pool.query<{ application_name: string }>( + "select current_setting('application_name') as application_name", + ); + + await applyPendingMigrations(pool, options(notes())); + + const after = await pool.query<{ + application_name: string; + lock_timeout: string; + locks: string; + }>( + `select current_setting('application_name') as application_name, + current_setting('lock_timeout') as lock_timeout, + (select count(*) from pg_locks where locktype = 'advisory' and pid = pg_backend_pid())::text as locks`, + ); + expect(after.rows[0]).toEqual({ + application_name: before.rows[0]?.application_name, + lock_timeout: "0", + locks: "0", + }); + }); + + test("an existing schema owned by another role: the error names both roles", async () => { + const suffix = schema.slice(-8); + const owner = `appkit_owner_${suffix}`; + const app = `appkit_app_${suffix}`; + await admin.query(`create role "${owner}" nologin`); + await admin.query(`create role "${app}" login password '${ROLE_PASSWORD}'`); + await admin.query(`create schema "${schema}" authorization "${owner}"`); + try { + const error = await applyPendingMigrations( + newPool({ user: app }), + options(notes()), + ).catch((caught: unknown) => caught); + + expect(error).toMatchObject({ code: "42501" }); + expect((error as Error).message).toContain( + `Schema "${schema}" is owned by "${owner}", but the app connects as "${app}".`, + ); + } finally { + await Promise.all(pools.splice(0).map((pool) => pool.end())); + await admin.query(`drop schema if exists "${schema}" cascade`); + await admin.query(`drop owned by "${app}"`); + await admin.query(`drop role "${app}"`); + await admin.query(`drop role "${owner}"`); + } + }); + + test("a baseline whose tables already exist is adopted, and later migrations apply", async () => { + await admin.query(`create schema "${schema}"`); + await admin.query(`create table ${q("notes")} ("id" integer primary key)`); + const folder = writeMigrations( + [ + { + tag: "0000_baseline", + when: 1000, + sql: [`CREATE TABLE ${q("notes")} ("id" integer PRIMARY KEY);`], + snapshot: { notes: ["id"] }, + }, + { + tag: "0001_body", + when: 2000, + sql: [`ALTER TABLE ${q("notes")} ADD COLUMN "body" text;`], + }, + ], + schema, + ); + + const result = await applyPendingMigrations(newPool(), options(folder)); + + expect(result).toMatchObject({ + adopted: ["0000_baseline"], + applied: ["0001_body"], + }); + expect(await columns("notes")).toEqual(["id", "body"]); + }); +}); diff --git a/packages/appkit/src/database/migrations/tests/apply.test.ts b/packages/appkit/src/database/migrations/tests/apply.test.ts index 4fc178d14..6892e5df0 100644 --- a/packages/appkit/src/database/migrations/tests/apply.test.ts +++ b/packages/appkit/src/database/migrations/tests/apply.test.ts @@ -1,6 +1,4 @@ -import { createHash } from "node:crypto"; import fs from "node:fs"; -import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, test, vi } from "vitest"; @@ -9,103 +7,52 @@ import { applyPendingMigrations, MigrationApplyError, MigrationBaselineMismatchError, + MigrationHistoryError, migrationLockKey, } from "../apply"; -import { type Journal, MigrationFilesError } from "../paths"; - -const folders: string[] = []; - -/** Write a v7 journal with one SQL file per entry, the way drizzle-kit lays them out. */ -function migrations( - entries: Array<{ - tag: string; - when: number; - sql: string[]; - /** Tables (name → columns) for meta/NNNN_snapshot.json. */ - snapshot?: Record; - }>, -) { - const folder = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-apply-")); - folders.push(folder); - fs.mkdirSync(path.join(folder, "meta")); - const journal: Journal = { - version: "7", - dialect: "postgresql", - entries: entries.map((entry, idx) => ({ - idx, - version: "7", - when: entry.when, - tag: entry.tag, - breakpoints: true, - })), - }; - fs.writeFileSync( - path.join(folder, "meta/_journal.json"), - JSON.stringify(journal), - ); - entries.forEach((entry, idx) => { - fs.writeFileSync( - path.join(folder, `${entry.tag}.sql`), - entry.sql.join("--> statement-breakpoint\n"), - ); - if (entry.snapshot) { - const tables = Object.fromEntries( - Object.entries(entry.snapshot).map(([name, columns]) => [ - `app.${name}`, - { - name, - schema: "app", - columns: Object.fromEntries(columns.map((c) => [c, { name: c }])), - }, - ]), - ); - fs.writeFileSync( - path.join( - folder, - "meta", - `${String(idx).padStart(4, "0")}_snapshot.json`, - ), - JSON.stringify({ - version: "7", - dialect: "postgresql", - tables, - enums: {}, - }), - ); - } - }); - return folder; -} +import { MigrationFilesError } from "../paths"; +import { + hashOf, + removeMigrationFolders, + writeMigrations as migrations, +} from "./fixtures"; afterEach(() => { vi.restoreAllMocks(); - for (const folder of folders.splice(0)) { - fs.rmSync(folder, { recursive: true, force: true }); - } + removeMigrationFolders(); }); +interface JournalRow { + hash: string; + created_at: number | null; +} + interface FakeState { - journalTableExists: boolean; - lastCreatedAt: number | null; + /** Rows of the journal table; null while the table does not exist. */ + recorded: JournalRow[] | null; failOn?: RegExp; /** SQLSTATE of the failOn error; defaults to 42P07 (duplicate_table). */ failCode?: string; + /** How many failOn matches fail before one succeeds; all of them when unset. */ + failTimes?: number; failCleanupOn?: RegExp; /** pg_catalog (table, column) pairs the baseline check sees. */ catalog?: Array<[string, string]>; - /** Another instance holds the migration lock, so the first try fails. */ + /** Another session holds the migration lock. */ lockHeld?: boolean; + lockHolder?: { pid: number; application_name: string; state: string }; + ownership?: { owner: string | null; current_user: string }; } /** - * A pg PoolClient that records every statement. Its class name must not contain - * "Pool": drizzle-orm would take it for a pool and call connect() on it. + * A pg PoolClient that records every statement and keeps journal inserts + * only when their transaction commits. */ class RecordingClient { readonly statements: string[] = []; - /** Parameters of each journal insert. */ - readonly recorded: unknown[][] = []; released: { error: unknown } | undefined; + private uncommitted: JournalRow[] | undefined; + private failures = 0; constructor(private readonly state: FakeState) {} @@ -116,17 +63,43 @@ class RecordingClient { const text = (typeof input === "string" ? input : input.text).trim(); const params = values ?? (typeof input === "string" ? [] : input.values); this.statements.push(text); - if (this.state.failOn?.test(text)) { - throw Object.assign(new Error("relation already exists"), { + if ( + this.state.failOn?.test(text) && + (this.state.failTimes === undefined || + this.failures < this.state.failTimes) + ) { + this.failures++; + throw Object.assign(new Error("statement failed"), { code: this.state.failCode ?? "42P07", }); } if (this.state.failCleanupOn?.test(text)) { throw new Error("connection lost"); } + if (text === "begin") this.uncommitted = []; + if (text === "commit") { + if (this.uncommitted?.length) { + this.state.recorded = [ + ...(this.state.recorded ?? []), + ...this.uncommitted, + ]; + } + this.uncommitted = undefined; + } + if (text === "rollback") this.uncommitted = undefined; if (text.includes("pg_try_advisory_lock(")) { return { rows: [{ locked: !this.state.lockHeld }] }; } + if (text.includes("pg_catalog.pg_locks")) { + return { rows: this.state.lockHolder ? [this.state.lockHolder] : [] }; + } + if (text.includes("pg_get_userbyid")) { + return { + rows: [ + this.state.ownership ?? { owner: "app-sp", current_user: "app-sp" }, + ], + }; + } if (text.includes("pg_catalog.pg_attribute")) { return { rows: (this.state.catalog ?? []).map(([table_name, column_name]) => ({ @@ -136,23 +109,16 @@ class RecordingClient { }; } if (text.includes("pg_catalog.pg_class")) { - return { rows: this.state.journalTableExists ? [{ one: 1 }] : [] }; - } - if (text.startsWith(`insert into "app"."__drizzle_migrations"`)) { - this.recorded.push(params ?? []); - this.state.journalTableExists = true; - this.state.lastCreatedAt = Number(params?.[1]); + return { rows: this.state.recorded ? [{ one: 1 }] : [] }; } - if (text.startsWith("select max(created_at)")) { - return { rows: [{ last: this.state.lastCreatedAt }] }; + if (text.startsWith("select hash, created_at")) { + return { rows: this.state.recorded ?? [] }; } - if (text.includes("order by created_at desc limit 1")) { - return { - rows: - this.state.lastCreatedAt === null - ? [] - : [{ id: 1, hash: "h", created_at: this.state.lastCreatedAt }], - }; + if (text.startsWith(`insert into "app"."__drizzle_migrations"`)) { + this.uncommitted?.push({ + hash: String(params?.[0]), + created_at: Number(params?.[1]), + }); } return { rows: [] }; } @@ -167,6 +133,7 @@ function fakePool(state: FakeState) { let connects = 0; return { client, + state, connects: () => connects, pool: { connect: async () => { @@ -194,213 +161,490 @@ const twoMigrations = () => }, ]); +const options = (folder: string) => ({ + migrationsFolder: folder, + schemaName: "app", +}); + /** Collapse the recorded statements into readable steps. */ function steps(statements: readonly string[]): string[] { return statements.map((statement) => { if (statement.includes("pg_try_advisory_lock(")) return "try lock"; - if (statement.includes("pg_advisory_lock(")) return "lock"; if (statement.includes("pg_advisory_unlock(")) return "unlock"; + if (statement.includes("pg_catalog.pg_locks")) return "lock holder?"; + if (statement.includes("pg_get_userbyid")) return "schema owner?"; if (statement.includes("pg_catalog.pg_attribute")) return "baseline tables?"; if (statement.includes("pg_catalog.pg_class")) return "journal table?"; - if (statement.startsWith("select max(created_at)")) return "last applied?"; + if (statement.startsWith("select hash, created_at")) return "recorded?"; + if (statement.startsWith(`CREATE SCHEMA IF NOT EXISTS`)) + return "create schema"; + if (statement.startsWith(`CREATE TABLE IF NOT EXISTS "app"."__drizzle`)) + return "create journal"; + if (statement.startsWith(`insert into "app"."__drizzle_migrations"`)) + return "record"; return statement.replace(/\s+/g, " "); }); } +const SESSION_SETUP = [ + "SET statement_timeout = 0", + "SET application_name = 'appkit-migrate'", + "SET client_connection_check_interval = '10s'", + "try lock", + "SET lock_timeout = '5000ms'", +]; + +const SESSION_CLEANUP = [ + "unlock", + "RESET lock_timeout", + "RESET client_connection_check_interval", + "RESET application_name", + "RESET statement_timeout", +]; + describe("applyPendingMigrations", () => { - test("applies every pending migration as the connecting identity under the advisory lock, then unlocks and releases", async () => { + test("reads without the lock, then applies each pending migration in its own transaction under it", async () => { const folder = twoMigrations(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - }); + const { pool, client, state } = fakePool({ recorded: null }); - const result = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); + const result = await applyPendingMigrations(pool, options(folder)); - expect(result.applied).toEqual(["0000_migration", "0001_migration"]); + expect(result).toEqual({ + applied: ["0000_migration", "0001_migration"], + adopted: [], + edited: [], + unknown: 0, + lockWaitMs: expect.any(Number), + }); expect(steps(client.statements)).toEqual([ - "SET statement_timeout = 0", - "try lock", - "SET lock_timeout = '30s'", "journal table?", - // drizzle-orm's migrator as the connecting identity (no SET ROLE), journal in the app schema: - `CREATE SCHEMA IF NOT EXISTS "app"`, - `CREATE TABLE IF NOT EXISTS "app"."__drizzle_migrations" ( id SERIAL PRIMARY KEY, hash text NOT NULL, created_at bigint )`, - `select id, hash, created_at from "app"."__drizzle_migrations" order by created_at desc limit 1`, + ...SESSION_SETUP, + "journal table?", "begin", + "create schema", + "create journal", `CREATE TABLE "app"."notes" ("id" integer);`, - `insert into "app"."__drizzle_migrations" ("hash", "created_at") values($1, $2)`, + "record", + "commit", + "begin", + "create schema", + "create journal", `ALTER TABLE "app"."notes" ADD COLUMN "body" text;`, `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, - `insert into "app"."__drizzle_migrations" ("hash", "created_at") values($1, $2)`, + "record", "commit", - "unlock", - "RESET lock_timeout", - "RESET statement_timeout", + ...SESSION_CLEANUP, + ]); + // The rows drizzle-orm's migrator would have written. + expect(state.recorded).toEqual([ + { hash: hashOf(folder, "0000_migration"), created_at: 1000 }, + { hash: hashOf(folder, "0001_migration"), created_at: 2000 }, ]); expect(client.released).toEqual({ error: undefined }); }); - test("applies only entries newer than the last recorded migration", async () => { + test("applies only migrations newer than the last recorded one", async () => { const folder = twoMigrations(); const { pool, client } = fakePool({ - journalTableExists: true, - lastCreatedAt: 1000, + recorded: [{ hash: hashOf(folder, "0000_migration"), created_at: 1000 }], }); - const result = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); + const result = await applyPendingMigrations(pool, options(folder)); expect(result.applied).toEqual(["0001_migration"]); expect(client.statements).not.toContain( `CREATE TABLE "app"."notes" ("id" integer);`, ); - expect(client.statements).toContain( - `ALTER TABLE "app"."notes" ADD COLUMN "body" text;`, - ); }); - test("with nothing pending, issues no migration SQL", async () => { + test("with nothing pending, it only reads: no lock, no session settings", async () => { const folder = twoMigrations(); const { pool, client } = fakePool({ - journalTableExists: true, - lastCreatedAt: 2000, + recorded: [ + { hash: hashOf(folder, "0000_migration"), created_at: 1000 }, + { hash: hashOf(folder, "0001_migration"), created_at: 2000 }, + ], }); - const result = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); + const result = await applyPendingMigrations(pool, options(folder)); expect(result.applied).toEqual([]); - expect(steps(client.statements)).toEqual([ - "SET statement_timeout = 0", - "try lock", - "SET lock_timeout = '30s'", - "journal table?", - "last applied?", - "unlock", - "RESET lock_timeout", - "RESET statement_timeout", - ]); + expect(steps(client.statements)).toEqual(["journal table?", "recorded?"]); expect(client.released).toEqual({ error: undefined }); }); + test("another instance applied everything while this one waited: nothing runs twice", async () => { + const folder = twoMigrations(); + const fake = fakePool({ recorded: null }); + const query = fake.client.query.bind(fake.client); + vi.spyOn(fake.client, "query").mockImplementation(async (input, values) => { + const result = await query(input, values); + if (String(input).includes("pg_try_advisory_lock(")) { + fake.state.recorded = [ + { hash: hashOf(folder, "0000_migration"), created_at: 1000 }, + { hash: hashOf(folder, "0001_migration"), created_at: 2000 }, + ]; + } + return result; + }); + + const result = await applyPendingMigrations(fake.pool, options(folder)); + + expect(result.applied).toEqual([]); + expect(fake.client.statements).not.toContain("begin"); + }); + test("without a journal it never connects", async () => { - const folder = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-apply-")); - folders.push(folder); - const fake = fakePool({ journalTableExists: false, lastCreatedAt: null }); + const folder = migrations([]); + fs.rmSync(path.join(folder, "meta"), { recursive: true }); + const fake = fakePool({ recorded: null }); await expect( - applyPendingMigrations(fake.pool, { - migrationsFolder: folder, - schemaName: "app", - }), - ).resolves.toEqual({ applied: [], adopted: [] }); + applyPendingMigrations(fake.pool, options(folder)), + ).resolves.toEqual({ + applied: [], + adopted: [], + edited: [], + unknown: 0, + lockWaitMs: 0, + }); expect(fake.connects()).toBe(0); }); - test("a failing statement rolls back all pending migrations and still unlocks and releases", async () => { + test("a failing migration rolls back alone: the ones before it stay applied", async () => { const folder = twoMigrations(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, + const { pool, client, state } = fakePool({ + recorded: null, failOn: /ADD COLUMN "title"/, }); - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); expect(error).toBeInstanceOf(MigrationApplyError); expect(error).toMatchObject({ - pending: ["0000_migration", "0001_migration"], + failed: "0001_migration", + applied: ["0000_migration"], + pending: ["0001_migration"], code: "42P07", }); expect((error as Error).message).toBe( - "Applying migrations 0000_migration, 0001_migration failed (SQLSTATE 42P07); none were applied.", + "Migration 0001_migration failed (SQLSTATE 42P07) and was rolled back. Applied before the failure: 0000_migration. Not applied: 0001_migration.", ); - expect(steps(client.statements).slice(-5)).toEqual([ - `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, + expect(state.recorded).toEqual([ + { hash: hashOf(folder, "0000_migration"), created_at: 1000 }, + ]); + expect(steps(client.statements).slice(-6)).toEqual([ "rollback", - "unlock", - "RESET lock_timeout", - "RESET statement_timeout", + ...SESSION_CLEANUP, ]); expect(client.released).toEqual({ error: undefined }); }); - test("an ownership failure (42501) names the branch-owner rule and the fix", async () => { + test("a migration older than the last recorded one, never applied, fails closed before taking the lock", async () => { + const folder = migrations([ + { tag: "0000_a", when: 1000, sql: [`SELECT 1;`] }, + { tag: "0001_parallel", when: 1500, sql: [`SELECT 2;`] }, + { tag: "0002_b", when: 2000, sql: [`SELECT 3;`] }, + ]); + const { pool, client } = fakePool({ + recorded: [ + { hash: hashOf(folder, "0000_a"), created_at: 1000 }, + { hash: hashOf(folder, "0002_b"), created_at: 2000 }, + ], + }); + + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); + + expect(error).toBeInstanceOf(MigrationHistoryError); + expect(error).toMatchObject({ stranded: ["0001_parallel"] }); + expect((error as Error).message).toMatch( + /^Migration 0001_parallel is older than 0002_b, which schema "app" already recorded, and was never applied/, + ); + expect(steps(client.statements)).toEqual(["journal table?", "recorded?"]); + }); + + test("a stranded migration also stops the pending ones after it", async () => { + const folder = migrations([ + { tag: "0000_a", when: 1000, sql: [`SELECT 1;`] }, + { tag: "0001_parallel", when: 1500, sql: [`SELECT 2;`] }, + { tag: "0002_b", when: 2000, sql: [`SELECT 3;`] }, + { tag: "0003_c", when: 3000, sql: [`SELECT 4;`] }, + ]); + const { pool, client } = fakePool({ + recorded: [ + { hash: hashOf(folder, "0000_a"), created_at: 1000 }, + { hash: hashOf(folder, "0002_b"), created_at: 2000 }, + ], + }); + + await expect( + applyPendingMigrations(pool, options(folder)), + ).rejects.toBeInstanceOf(MigrationHistoryError); + expect(client.statements).not.toContain("SELECT 4;"); + }); + + test("an applied migration whose file changed is logged, and its change does not run", async () => { + const folder = twoMigrations(); + const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined); + const { pool } = fakePool({ + recorded: [ + { hash: "the-hash-before-the-edit", created_at: 1000 }, + { hash: hashOf(folder, "0001_migration"), created_at: 2000 }, + ], + }); + + const result = await applyPendingMigrations(pool, options(folder)); + + expect(result).toMatchObject({ applied: [], edited: ["0000_migration"] }); + expect(warn.mock.calls.flat().map(String).join(" ")).toContain( + "Migrations 0000_migration changed after schema app applied them", + ); + }); + + test("recorded migrations that are not committed here are logged", async () => { const folder = twoMigrations(); + const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined); const { pool } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, + recorded: [ + { hash: hashOf(folder, "0000_migration"), created_at: 1000 }, + { hash: hashOf(folder, "0001_migration"), created_at: 2000 }, + { hash: "from-a-newer-release", created_at: 3000 }, + ], + }); + + const result = await applyPendingMigrations(pool, options(folder)); + + expect(result).toMatchObject({ applied: [], unknown: 1 }); + expect(warn.mock.calls.flat().map(String).join(" ")).toContain( + "Schema app recorded 1 migration(s) that are not committed here", + ); + }); + + test("a table lock not granted in time is retried, then fails with the hint", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + recorded: [{ hash: hashOf(folder, "0000_migration"), created_at: 1000 }], failOn: /ADD COLUMN "body"/, - failCode: "42501", + failCode: "55P03", }); const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", + ...options(folder), + lockTimeoutMs: 1, }).catch((caught: unknown) => caught); + expect(client.statements.filter((s) => s === "rollback")).toHaveLength(3); + expect((error as Error).message).toBe( + "Migration 0001_migration failed (SQLSTATE 55P03) and was rolled back. Not applied: 0001_migration. " + + "Another session held a lock on a table this migration changes for over 1ms, 3 times. Deploy again once the long-running query or transaction finishes.", + ); + }); + + test("a table lock granted on the retry applies the migration", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + recorded: [{ hash: hashOf(folder, "0000_migration"), created_at: 1000 }], + failOn: /ADD COLUMN "body"/, + failCode: "55P03", + failTimes: 1, + }); + vi.spyOn(console, "warn").mockImplementation(() => undefined); + + const result = await applyPendingMigrations(pool, { + ...options(folder), + lockTimeoutMs: 1, + }); + + expect(result.applied).toEqual(["0001_migration"]); + }); + + test("the wait for another session's migration lock has a limit, and the error names the holder", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + recorded: null, + lockHeld: true, + lockHolder: { + pid: 42, + application_name: "appkit-migrate", + state: "active", + }, + }); + + const error = await applyPendingMigrations(pool, { + ...options(folder), + lockWaitMs: 0, + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect((error as Error).message).toBe( + "Waiting for the migration lock failed. Not applied: 0000_migration, 0001_migration. " + + "Session pid 42 (appkit-migrate, active) held the migration lock for over 0ms. Deploy again once it finishes; if it is stuck, end it with pg_terminate_backend.", + ); + // Never locked, so never unlocked; the session settings are still reset. + expect(steps(client.statements).slice(-3)).toEqual([ + "RESET client_connection_check_interval", + "RESET application_name", + "RESET statement_timeout", + ]); + }); + + test("a server that cannot check client connections still migrates", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + recorded: null, + failOn: /client_connection_check_interval/, + failCode: "22023", + }); + + const result = await applyPendingMigrations(pool, options(folder)); + + expect(result.applied).toHaveLength(2); + expect(client.statements).not.toContain( + "RESET client_connection_check_interval", + ); + }); + + test("an ownership failure (42501) names the schema owner and the connecting role", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + recorded: null, + failOn: /CREATE TABLE "app"."notes"/, + failCode: "42501", + ownership: { owner: "old-app-sp", current_user: "new-app-sp" }, + }); + + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); + expect(error).toMatchObject({ code: "42501" }); + expect((error as Error).message).toContain( + `Schema "app" is owned by "old-app-sp", but the app connects as "new-app-sp".`, + ); + }); + + test("reading the recorded migrations fails (42501): the hint, and nothing applied", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + recorded: [], + failOn: /^select hash, created_at/, + failCode: "42501", + ownership: { owner: "someone-else", current_user: "app-sp" }, + }); + + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); + + expect(error).toMatchObject({ pending: [], code: "42501" }); expect((error as Error).message).toMatch( - /^Applying migrations 0000_migration, 0001_migration failed \(SQLSTATE 42501\); none were applied\. The connecting role lacks a privilege this migration needs: /, + /^Reading the recorded migrations failed \(SQLSTATE 42501\)\. No migration was applied\. Schema "app" is owned by "someone-else"/, + ); + }); + + test("a connection failure carries the system error code", async () => { + const folder = twoMigrations(); + const pool = { + connect: async () => { + throw Object.assign(new Error("connect ECONNREFUSED"), { + code: "ECONNREFUSED", + }); + }, + }; + + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect(error).toMatchObject({ code: undefined }); + expect((error as Error).message).toBe( + "Connecting to the database failed (ECONNREFUSED). No migration was applied.", + ); + }); + + test("a Node system error is not reported as a SQLSTATE", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + recorded: [{ hash: hashOf(folder, "0000_migration"), created_at: 1000 }], + failOn: /ADD COLUMN "body"/, + failCode: "EPIPE", + }); + + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); + + expect(error).toMatchObject({ code: undefined }); + expect((error as Error).message).toBe( + "Migration 0001_migration failed (EPIPE) and was rolled back. Not applied: 0001_migration.", ); }); test("a failed unlock destroys the connection instead of returning it to the pool", async () => { const folder = twoMigrations(); const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, + recorded: null, failCleanupOn: /pg_advisory_unlock/, }); - await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); + await applyPendingMigrations(pool, options(folder)); // The remaining cleanup still runs. - expect(steps(client.statements).slice(-3)).toEqual([ - "unlock", - "RESET lock_timeout", - "RESET statement_timeout", - ]); + expect(steps(client.statements).slice(-5)).toEqual(SESSION_CLEANUP); expect(client.released?.error).toBeInstanceOf(Error); }); - test("a failure before the lock is held releases without unlocking", async () => { + test("a missing .sql file fails before connecting, naming the file", async () => { const folder = twoMigrations(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - failOn: /pg_try_advisory_lock/, - }); + fs.rmSync(path.join(folder, "0001_migration.sql")); + const fake = fakePool({ recorded: null }); - await expect( - applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }), - ).rejects.toThrow("relation already exists"); - expect(steps(client.statements)).toEqual([ - "SET statement_timeout = 0", - "try lock", - "RESET lock_timeout", - "RESET statement_timeout", - ]); - expect(client.released).toEqual({ error: undefined }); + const error = await applyPendingMigrations( + fake.pool, + options(folder), + ).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationFilesError); + expect((error as Error).message).toMatch( + /^Missing .*0001_migration\.sql, listed in meta\/_journal\.json\. Restore it from version control\.$/, + ); + expect(fake.connects()).toBe(0); }); + + test.each([ + [ + "not JSON", + "{", + /^Cannot read .*_journal\.json: .*Restore it from version control\.$/, + ], + [ + "another dialect", + JSON.stringify({ version: "7", dialect: "mysql", entries: [] }), + /_journal\.json is not a drizzle-kit PostgreSQL migration journal$/, + ], + ])( + "a journal that is %s fails before connecting, naming the file", + async (_label, contents, message) => { + const folder = twoMigrations(); + fs.writeFileSync(path.join(folder, "meta/_journal.json"), contents); + const fake = fakePool({ recorded: null }); + + const error = await applyPendingMigrations( + fake.pool, + options(folder), + ).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationFilesError); + expect((error as Error).message).toMatch(message); + expect(fake.connects()).toBe(0); + }, + ); }); describe("migrationLockKey", () => { @@ -436,21 +680,20 @@ const baselineThenMigration = () => }, ]); +const BASELINE_CATALOG: Array<[string, string]> = [ + ["notes", "id"], + ["notes", "body"], + ["tags", "id"], +]; + describe("applyPendingMigrations — baseline adoption", () => { test("none of the baseline's tables exist: the baseline runs like any migration", async () => { const folder = baselineThenMigration(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - catalog: [], - }); + const { pool, client } = fakePool({ recorded: null, catalog: [] }); - const result = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); + const result = await applyPendingMigrations(pool, options(folder)); - expect(result).toEqual({ + expect(result).toMatchObject({ applied: ["0000_baseline", "0001_migration"], adopted: [], }); @@ -459,22 +702,14 @@ describe("applyPendingMigrations — baseline adoption", () => { test("every baseline table and column exists: the baseline is recorded, not run, and later migrations apply", async () => { const folder = baselineThenMigration(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - catalog: [ - ["notes", "id"], - ["notes", "body"], - ["tags", "id"], - ], + const { pool, client, state } = fakePool({ + recorded: null, + catalog: BASELINE_CATALOG, }); - const result = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); + const result = await applyPendingMigrations(pool, options(folder)); - expect(result).toEqual({ + expect(result).toMatchObject({ applied: ["0001_migration"], adopted: ["0000_baseline"], }); @@ -483,27 +718,10 @@ describe("applyPendingMigrations — baseline adoption", () => { `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, ); // The adopted row carries exactly what drizzle-orm's migrator would have written. - const baselineFile = fs.readFileSync( - path.join(folder, "0000_baseline.sql"), - "utf-8", - ); - expect(client.recorded[0]).toEqual([ - createHash("sha256").update(baselineFile).digest("hex"), - 1000, - ]); - expect(steps(client.statements).slice(0, 10)).toEqual([ - "SET statement_timeout = 0", - "try lock", - "SET lock_timeout = '30s'", - "journal table?", - "baseline tables?", - "begin", - `CREATE SCHEMA IF NOT EXISTS "app"`, - `CREATE TABLE IF NOT EXISTS "app"."__drizzle_migrations" ( id SERIAL PRIMARY KEY, hash text NOT NULL, created_at bigint )`, - `insert into "app"."__drizzle_migrations" ("hash", "created_at") values($1, $2)`, - "commit", - ]); - expect(client.released).toEqual({ error: undefined }); + expect(state.recorded?.[0]).toEqual({ + hash: hashOf(folder, "0000_baseline"), + created_at: 1000, + }); }); test("adopting a lone baseline applies nothing else", async () => { @@ -515,41 +733,23 @@ describe("applyPendingMigrations — baseline adoption", () => { snapshot: { notes: ["id", "body"], tags: ["id"] }, }, ]); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - catalog: [ - ["notes", "id"], - ["notes", "body"], - ["tags", "id"], - ], - }); + const { pool } = fakePool({ recorded: null, catalog: BASELINE_CATALOG }); await expect( - applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }), - ).resolves.toEqual({ applied: [], adopted: ["0000_baseline"] }); - expect(client.statements.slice(-3)).toEqual([ - expect.stringContaining("pg_advisory_unlock"), - "RESET lock_timeout", - "RESET statement_timeout", - ]); + applyPendingMigrations(pool, options(folder)), + ).resolves.toMatchObject({ applied: [], adopted: ["0000_baseline"] }); }); test("only part of the baseline exists: MigrationBaselineMismatchError, nothing recorded or run", async () => { const folder = baselineThenMigration(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, + const { pool, client, state } = fakePool({ + recorded: null, catalog: [["notes", "id"]], }); - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); expect(error).toBeInstanceOf(MigrationBaselineMismatchError); expect((error as MigrationBaselineMismatchError).problems).toEqual([ @@ -559,40 +759,34 @@ describe("applyPendingMigrations — baseline adoption", () => { expect((error as Error).message).toMatch( /^Cannot adopt 0000_baseline: schema "app" has no recorded migrations and only part of the baseline: /, ); - expect(client.recorded).toEqual([]); + expect(state.recorded).toBeNull(); expect(client.statements).not.toContain("begin"); - expect(steps(client.statements).slice(-3)).toEqual([ - "unlock", - "RESET lock_timeout", - "RESET statement_timeout", - ]); expect(client.released).toEqual({ error: undefined }); }); test("recording the adopted baseline fails: MigrationApplyError with the SQLSTATE and the ownership hint", async () => { const folder = baselineThenMigration(); const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - catalog: [ - ["notes", "id"], - ["notes", "body"], - ["tags", "id"], - ], + recorded: null, + catalog: BASELINE_CATALOG, // The schema belongs to another role, so the journal table cannot be created in it. failOn: /^CREATE TABLE IF NOT EXISTS "app"\."__drizzle_migrations"/, failCode: "42501", + ownership: { owner: "someone-else", current_user: "app-sp" }, }); - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); expect(error).toBeInstanceOf(MigrationApplyError); - expect(error).toMatchObject({ pending: ["0000_baseline"], code: "42501" }); + expect(error).toMatchObject({ + failed: "0000_baseline", + pending: ["0000_baseline", "0001_migration"], + code: "42501", + }); expect((error as Error).message).toContain( - "On the deploy branch that role is the app's service principal", + `Schema "app" is owned by "someone-else"`, ); expect(client.statements).toContain("rollback"); expect(client.statements).not.toContain( @@ -603,40 +797,25 @@ describe("applyPendingMigrations — baseline adoption", () => { test("with a recorded migration, the baseline is never adopted", async () => { const folder = baselineThenMigration(); const { pool, client } = fakePool({ - journalTableExists: true, - lastCreatedAt: 1000, + recorded: [{ hash: hashOf(folder, "0000_baseline"), created_at: 1000 }], catalog: [["notes", "id"]], }); await expect( - applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }), - ).resolves.toEqual({ applied: ["0001_migration"], adopted: [] }); + applyPendingMigrations(pool, options(folder)), + ).resolves.toMatchObject({ applied: ["0001_migration"], adopted: [] }); expect( client.statements.some((s) => s.includes("pg_catalog.pg_attribute")), ).toBe(false); }); - test("an empty journal table, left by a failed first deploy, still adopts the baseline", async () => { + test("an empty journal table still adopts the baseline", async () => { const folder = baselineThenMigration(); - const { pool } = fakePool({ - journalTableExists: true, - lastCreatedAt: null, - catalog: [ - ["notes", "id"], - ["notes", "body"], - ["tags", "id"], - ], - }); + const { pool } = fakePool({ recorded: [], catalog: BASELINE_CATALOG }); await expect( - applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }), - ).resolves.toEqual({ + applyPendingMigrations(pool, options(folder)), + ).resolves.toMatchObject({ applied: ["0001_migration"], adopted: ["0000_baseline"], }); @@ -644,21 +823,15 @@ describe("applyPendingMigrations — baseline adoption", () => { test("a migration failing after the adoption says the baseline stays recorded", async () => { const folder = baselineThenMigration(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - catalog: [ - ["notes", "id"], - ["notes", "body"], - ["tags", "id"], - ], + const { pool, state } = fakePool({ + recorded: null, + catalog: BASELINE_CATALOG, failOn: /ADD COLUMN "title"/, }); - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); expect(error).toBeInstanceOf(MigrationApplyError); expect(error).toMatchObject({ @@ -666,23 +839,19 @@ describe("applyPendingMigrations — baseline adoption", () => { adopted: ["0000_baseline"], }); expect((error as Error).message).toBe( - "Applying migration 0001_migration failed (SQLSTATE 42P07); none were applied. 0000_baseline stays recorded as applied: its tables already existed.", + "Migration 0001_migration failed (SQLSTATE 42P07) and was rolled back. 0000_baseline stays recorded as applied: its tables already existed. Not applied: 0001_migration.", ); - expect(client.recorded).toHaveLength(1); + expect(state.recorded).toHaveLength(1); }); test("a missing baseline snapshot names the file", async () => { const folder = baselineThenMigration(); fs.rmSync(path.join(folder, "meta/0000_snapshot.json")); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - }); + const { pool, client } = fakePool({ recorded: null }); - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); expect(error).toBeInstanceOf(MigrationFilesError); expect((error as Error).message).toMatch( @@ -691,142 +860,3 @@ describe("applyPendingMigrations — baseline adoption", () => { expect(client.released).toEqual({ error: undefined }); }); }); - -describe("applyPendingMigrations — failures name their cause", () => { - test("reading the recorded migrations fails (42501): the hint, and nothing applied", async () => { - const folder = twoMigrations(); - const { pool, client } = fakePool({ - journalTableExists: true, - lastCreatedAt: 1000, - // The journal table belongs to another role. - failOn: /^select max\(created_at\)/, - failCode: "42501", - }); - - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); - - expect(error).toBeInstanceOf(MigrationApplyError); - expect(error).toMatchObject({ pending: [], code: "42501" }); - expect((error as Error).message).toMatch( - /^Reading the recorded migrations in __drizzle_migrations failed \(SQLSTATE 42501\); none were applied\. The connecting role lacks a privilege/, - ); - expect(steps(client.statements).slice(-3)).toEqual([ - "unlock", - "RESET lock_timeout", - "RESET statement_timeout", - ]); - }); - - test("a table lock held past the lock timeout says to deploy again", async () => { - const folder = twoMigrations(); - const { pool } = fakePool({ - journalTableExists: true, - lastCreatedAt: 1000, - failOn: /ADD COLUMN "body"/, - failCode: "55P03", - }); - - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); - - expect((error as Error).message).toBe( - "Applying migration 0001_migration failed (SQLSTATE 55P03); none were applied. Another session held a lock on a table this migration changes for over 30s; deploy again once it finishes.", - ); - }); - - test("a Node system error is not reported as a SQLSTATE", async () => { - const folder = twoMigrations(); - const { pool } = fakePool({ - journalTableExists: true, - lastCreatedAt: 1000, - failOn: /ADD COLUMN "body"/, - failCode: "EPIPE", - }); - - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); - - expect(error).toMatchObject({ code: undefined }); - expect((error as Error).message).toBe( - "Applying migration 0001_migration failed; none were applied.", - ); - }); - - test("a missing .sql file fails before connecting, naming the file", async () => { - const folder = twoMigrations(); - fs.rmSync(path.join(folder, "0001_migration.sql")); - const fake = fakePool({ journalTableExists: false, lastCreatedAt: null }); - - const error = await applyPendingMigrations(fake.pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); - - expect(error).toBeInstanceOf(MigrationFilesError); - expect((error as Error).message).toMatch( - /^Missing .*0001_migration\.sql, listed in meta\/_journal\.json\. Restore it from version control\.$/, - ); - expect(fake.connects()).toBe(0); - }); - - test.each([ - [ - "not JSON", - "{", - /^Cannot read .*_journal\.json: .*Restore it from version control\.$/, - ], - [ - "another dialect", - JSON.stringify({ version: "7", dialect: "mysql", entries: [] }), - /_journal\.json is not a drizzle-kit PostgreSQL migration journal$/, - ], - ])( - "a journal that is %s fails before connecting, naming the file", - async (_label, contents, message) => { - const folder = twoMigrations(); - fs.writeFileSync(path.join(folder, "meta/_journal.json"), contents); - const fake = fakePool({ journalTableExists: false, lastCreatedAt: null }); - - const error = await applyPendingMigrations(fake.pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); - - expect(error).toBeInstanceOf(MigrationFilesError); - expect((error as Error).message).toMatch(message); - expect(fake.connects()).toBe(0); - }, - ); - - test("waits for another instance's lock, and says so", async () => { - const folder = twoMigrations(); - const log = vi.spyOn(console, "log").mockImplementation(() => undefined); - const { pool, client } = fakePool({ - journalTableExists: true, - lastCreatedAt: 2000, - lockHeld: true, - }); - - await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); - - expect(steps(client.statements).slice(0, 4)).toEqual([ - "SET statement_timeout = 0", - "try lock", - "lock", - "SET lock_timeout = '30s'", - ]); - expect(log.mock.calls.flat().map(String).join(" ")).toContain( - "Waiting for another instance to finish migrating schema app", - ); - }); -}); diff --git a/packages/appkit/src/database/migrations/tests/fixtures.ts b/packages/appkit/src/database/migrations/tests/fixtures.ts new file mode 100644 index 000000000..33d5d1baa --- /dev/null +++ b/packages/appkit/src/database/migrations/tests/fixtures.ts @@ -0,0 +1,86 @@ +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import type { Journal } from "../paths"; + +interface MigrationFixture { + tag: string; + when: number; + sql: string[]; + /** Tables (name → columns) for meta/NNNN_snapshot.json. */ + snapshot?: Record; +} + +const folders: string[] = []; + +/** Write a v7 journal with one SQL file per entry, the way drizzle-kit lays them out. */ +export function writeMigrations( + entries: MigrationFixture[], + schemaName = "app", +): string { + const folder = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-apply-")); + folders.push(folder); + fs.mkdirSync(path.join(folder, "meta")); + const journal: Journal = { + version: "7", + dialect: "postgresql", + entries: entries.map((entry, idx) => ({ + idx, + version: "7", + when: entry.when, + tag: entry.tag, + breakpoints: true, + })), + }; + fs.writeFileSync( + path.join(folder, "meta/_journal.json"), + JSON.stringify(journal), + ); + entries.forEach((entry, idx) => { + fs.writeFileSync( + path.join(folder, `${entry.tag}.sql`), + entry.sql.join("--> statement-breakpoint\n"), + ); + if (entry.snapshot) { + const tables = Object.fromEntries( + Object.entries(entry.snapshot).map(([name, columns]) => [ + `${schemaName}.${name}`, + { + name, + schema: schemaName, + columns: Object.fromEntries(columns.map((c) => [c, { name: c }])), + }, + ]), + ); + fs.writeFileSync( + path.join( + folder, + "meta", + `${String(idx).padStart(4, "0")}_snapshot.json`, + ), + JSON.stringify({ + version: "7", + dialect: "postgresql", + tables, + enums: {}, + }), + ); + } + }); + return folder; +} + +/** The hash drizzle-orm's migrator records for a migration file. */ +export function hashOf(folder: string, tag: string): string { + return createHash("sha256") + .update(fs.readFileSync(path.join(folder, `${tag}.sql`), "utf-8")) + .digest("hex"); +} + +export function removeMigrationFolders(): void { + for (const folder of folders.splice(0)) { + fs.rmSync(folder, { recursive: true, force: true }); + } +} diff --git a/packages/appkit/src/database/migrations/tests/history.test.ts b/packages/appkit/src/database/migrations/tests/history.test.ts new file mode 100644 index 000000000..04da4ac6e --- /dev/null +++ b/packages/appkit/src/database/migrations/tests/history.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, test } from "vitest"; + +import { type MigrationFile, reconcileMigrations } from "../history"; + +const file = ( + tag: string, + when: number, + hash = `hash-${tag}`, +): MigrationFile => ({ + tag, + idx: 0, + when, + hash, + statements: [], +}); + +const tags = (files: readonly MigrationFile[]) => files.map((f) => f.tag); + +describe("reconcileMigrations", () => { + const files = [ + file("0000_a", 1000), + file("0001_b", 2000), + file("0002_c", 3000), + ]; + + test("nothing recorded: every migration is pending", () => { + const history = reconcileMigrations(files, []); + expect(tags(history.pending)).toEqual(["0000_a", "0001_b", "0002_c"]); + expect(history.lastRecorded).toBeNull(); + }); + + test("newer than the last recorded migration: pending", () => { + const history = reconcileMigrations(files, [ + { hash: "hash-0000_a", createdAt: 1000 }, + ]); + expect(tags(history.pending)).toEqual(["0001_b", "0002_c"]); + expect(history.stranded).toEqual([]); + expect(history.lastRecorded).toBe(1000); + }); + + test("older than the last recorded migration and never recorded: stranded", () => { + const history = reconcileMigrations(files, [ + { hash: "hash-0000_a", createdAt: 1000 }, + { hash: "hash-0002_c", createdAt: 3000 }, + ]); + expect(tags(history.stranded)).toEqual(["0001_b"]); + expect(history.pending).toEqual([]); + }); + + test("recorded with another hash: edited", () => { + const history = reconcileMigrations(files, [ + { hash: "before-the-edit", createdAt: 1000 }, + { hash: "hash-0001_b", createdAt: 2000 }, + { hash: "hash-0002_c", createdAt: 3000 }, + ]); + expect(tags(history.edited)).toEqual(["0000_a"]); + expect(history.pending).toEqual([]); + }); + + test("recorded rows no committed migration matches: unknown, and they still set the last recorded", () => { + const unknownRow = { hash: "newer-release", createdAt: 4000 }; + const nullRow = { hash: "no-timestamp", createdAt: null }; + const history = reconcileMigrations(files, [ + { hash: "hash-0000_a", createdAt: 1000 }, + { hash: "hash-0001_b", createdAt: 2000 }, + { hash: "hash-0002_c", createdAt: 3000 }, + unknownRow, + nullRow, + ]); + expect(history.unknown).toEqual([unknownRow, nullRow]); + expect(history.lastRecorded).toBe(4000); + expect(history.pending).toEqual([]); + expect(history.stranded).toEqual([]); + }); +}); diff --git a/packages/appkit/src/database/migrations/tests/ownership.test.ts b/packages/appkit/src/database/migrations/tests/ownership.test.ts index 56ea47022..4f93e59d7 100644 --- a/packages/appkit/src/database/migrations/tests/ownership.test.ts +++ b/packages/appkit/src/database/migrations/tests/ownership.test.ts @@ -11,7 +11,33 @@ describe("ownership: one owner per branch", () => { expect(hint).toContain( "On the deploy branch that role is the app's service principal: bind the database to the app with CAN_CONNECT_AND_CREATE", ); - expect(hint).toContain("use another schemaName in schema.ts"); + }); + + test("a schema owned by another role names both roles and how that happens", () => { + const hint = ownershipHint("42501", { + schemaName: "app", + owner: "old-app-sp", + currentUser: "new-app-sp", + }); + expect(hint).toContain( + `Schema "app" is owned by "old-app-sp", but the app connects as "new-app-sp".`, + ); + expect(hint).toContain( + "the app's database resource was removed and added again, or the app was recreated", + ); + expect(hint).toContain("set another schemaName in schema.ts"); + }); + + test("a schema that does not exist yet points at CREATE on the database", () => { + expect( + ownershipHint("42501", { + schemaName: "app", + owner: null, + currentUser: "app-sp", + }), + ).toBe( + `Role "app-sp" cannot create schema "app". On the deploy branch that role is the app's service principal: bind the database to the app with CAN_CONNECT_AND_CREATE.`, + ); }); test("any other failure gets no ownership hint", () => { diff --git a/packages/appkit/src/plugins/database/migrate-on-start.ts b/packages/appkit/src/plugins/database/migrate-on-start.ts index 8061c1124..a350b3537 100644 --- a/packages/appkit/src/plugins/database/migrate-on-start.ts +++ b/packages/appkit/src/plugins/database/migrate-on-start.ts @@ -1,3 +1,5 @@ +import fs from "node:fs"; + import type { Pool } from "pg"; import { databaseSetupFailed } from "../../database/errors"; @@ -5,8 +7,10 @@ import { applyPendingMigrations, MigrationApplyError, MigrationBaselineMismatchError, + MigrationHistoryError, } from "../../database/migrations/apply"; import { + journalFile, MigrationFilesError, migrationsFolder, } from "../../database/migrations/paths"; @@ -15,64 +19,89 @@ import { createLogger } from "../../logging/logger"; const logger = createLogger("database"); +/** Branches `appkit db` creates for one developer or one agent session; only they migrate them. */ +const DEVELOPER_BRANCH = /^(dev-[a-z0-9-]+-[0-9a-f]{6}|session-[a-z0-9-]+)$/; + /** - * True only inside a deployed Databricks App: not local development, and the - * runtime has set the app's name and its service principal's credentials, as - * Databricks Apps does for every app. `databricks apps run-local` sets the - * name but not the credentials. Locally, `appkit db migrate` applies - * migrations instead. + * True when the app opts in with APPKIT_DB_MIGRATE_ON_START=true (app.yaml) + * and runs with service principal credentials, as Databricks Apps sets for + * every app. `.env` never carries the flag, so a local start does not migrate; + * `databricks apps run-local` reads app.yaml but sets no credentials. */ export function shouldMigrateOnStart( env: NodeJS.ProcessEnv = process.env, ): boolean { return ( - env.NODE_ENV !== "development" && - Boolean(env.DATABRICKS_APP_NAME) && + env.APPKIT_DB_MIGRATE_ON_START === "true" && Boolean(env.DATABRICKS_CLIENT_ID) && Boolean(env.DATABRICKS_CLIENT_SECRET) ); } +/** The branch id in LAKEBASE_ENDPOINT (projects/

/branches//endpoints/). */ +function endpointBranch(endpoint: string | undefined): string | undefined { + return endpoint?.match(/\/branches\/([^/]+)\//)?.[1]; +} + /** * Apply pending committed migrations from /config/database/migrations as * the app's identity (see database/migrations/ownership.ts). No journal → - * no-op. A failed or unadoptable migration, or a missing or unreadable - * migration file, becomes a setup failure that names it; any other error - * (e.g. no connection) is left to lifecycle's generic sanitizer. + * no-op. Every migration failure becomes a setup failure that names it. */ export async function migrateOnStart( pool: Pick, schema: Schema, root: string = process.cwd(), + env: NodeJS.ProcessEnv = process.env, ): Promise { + const folder = migrationsFolder(root); + const branch = endpointBranch(env.LAKEBASE_ENDPOINT); + if (branch && DEVELOPER_BRANCH.test(branch)) { + logger.warn( + "Not migrating branch %s on start: it belongs to a developer or agent session, which migrates it with `appkit db migrate`.", + branch, + ); + return; + } + if (!fs.existsSync(journalFile(folder))) { + logger.info("No committed migrations at %s; nothing to apply", folder); + return; + } + // A transaction-mode pooler hands each statement to any server session, so + // the session-level migration lock would not exclude other instances. + if (env.PGHOST?.includes("-pooler.")) { + throw databaseSetupFailed( + `PGHOST ${env.PGHOST} is a pooled endpoint, and migrations need a direct connection. Use the host without "-pooler".`, + ); + } + + const started = Date.now(); let result: Awaited>; try { result = await applyPendingMigrations(pool, { - migrationsFolder: migrationsFolder(root), + migrationsFolder: folder, schemaName: schema.$schemaName, }); } catch (error) { if ( error instanceof MigrationApplyError || error instanceof MigrationBaselineMismatchError || + error instanceof MigrationHistoryError || error instanceof MigrationFilesError ) { throw databaseSetupFailed(error.message); } throw error; } - if (result.adopted.length > 0) { - logger.info( - "Adopted %s in schema %s: its tables already existed", - result.adopted.join(", "), - schema.$schemaName, - ); - } - if (result.applied.length > 0) { - logger.info( - "Applied migrations %s to schema %s", - result.applied.join(", "), - schema.$schemaName, - ); - } + logger.info( + "Migrations for schema %s from %s: applied=%d adopted=%d edited=%d unknown=%d lock_wait_ms=%d duration_ms=%d", + schema.$schemaName, + folder, + result.applied.length, + result.adopted.length, + result.edited.length, + result.unknown, + result.lockWaitMs, + Date.now() - started, + ); } diff --git a/packages/appkit/src/plugins/database/schema-check.ts b/packages/appkit/src/plugins/database/schema-check.ts index 9b6f7176a..bf206c427 100644 --- a/packages/appkit/src/plugins/database/schema-check.ts +++ b/packages/appkit/src/plugins/database/schema-check.ts @@ -76,7 +76,7 @@ export async function assertSchemaMatchesDatabase( } if (problems.length > 0) { throw databaseSetupFailed( - `the declared schema does not match the database: ${problems.join("; ")}. Create them, or commit migrations under config/database/migrations so the deployed app applies them on start.`, + `the declared schema does not match the database: ${problems.join("; ")}. Create them, or commit migrations under config/database/migrations and set APPKIT_DB_MIGRATE_ON_START=true in app.yaml so the deployed app applies them on start.`, ); } } diff --git a/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts b/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts index cf453a32d..6d53bc5c4 100644 --- a/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts +++ b/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts @@ -26,6 +26,10 @@ const schema = defineSchema( ); const roots: string[] = []; const pool = { connect: vi.fn() }; +const deployEnv = { + LAKEBASE_ENDPOINT: "projects/p/branches/production/endpoints/primary", + PGHOST: "ep-abc.database.cloud.databricks.com", +}; function appRoot(): string { const root = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-migrate-start-")); @@ -33,6 +37,26 @@ function appRoot(): string { return root; } +/** An app root with a committed (empty) journal. */ +function appRootWithMigrations(): string { + const root = appRoot(); + const meta = path.join(root, "config/database/migrations/meta"); + fs.mkdirSync(meta, { recursive: true }); + fs.writeFileSync( + path.join(meta, "_journal.json"), + JSON.stringify({ version: "7", dialect: "postgresql", entries: [] }), + ); + return root; +} + +const nothingDone = { + applied: [], + adopted: [], + edited: [], + unknown: 0, + lockWaitMs: 0, +}; + beforeEach(async () => { const actual = await vi.importActual< typeof import("../../../database/migrations/apply") @@ -54,44 +78,31 @@ describe("shouldMigrateOnStart", () => { DATABRICKS_CLIENT_ID: "sp-client-id", DATABRICKS_CLIENT_SECRET: "secret", }; - const app = { DATABRICKS_APP_NAME: "my-app", ...credentials }; + const optIn = { APPKIT_DB_MIGRATE_ON_START: "true" }; test.each([ - ["a deployed app", { ...app, NODE_ENV: "production" }, true], - ["a deployed app without NODE_ENV", app, true], + ["a deployed app that opts in", { ...optIn, ...credentials }, true], [ - "local development, even with app credentials", - { ...app, NODE_ENV: "development" }, + "a deployed app without the flag", + { DATABRICKS_APP_NAME: "my-app", ...credentials }, false, ], [ - "`databricks apps run-local`: the app name without its credentials", - { DATABRICKS_APP_NAME: "my-app", NODE_ENV: "production" }, + "a flag other than true", + { APPKIT_DB_MIGRATE_ON_START: "1", ...credentials }, false, ], [ - "a local production start with app credentials in .env but no app name", - { ...credentials, NODE_ENV: "production" }, + "`databricks apps run-local`: the flag from app.yaml, no credentials", + { ...optIn, DATABRICKS_APP_NAME: "my-app" }, false, ], [ "no client secret (a developer's own credentials)", - { - DATABRICKS_APP_NAME: "my-app", - DATABRICKS_CLIENT_ID: "x", - NODE_ENV: "production", - }, - false, - ], - [ - "no client id", - { - DATABRICKS_APP_NAME: "my-app", - DATABRICKS_CLIENT_SECRET: "x", - NODE_ENV: "production", - }, + { ...optIn, DATABRICKS_CLIENT_ID: "x" }, false, ], + ["no client id", { ...optIn, DATABRICKS_CLIENT_SECRET: "x" }, false], ["nothing set", {}, false], ] as const)("%s → %s", (_label, env, expected) => { expect(shouldMigrateOnStart(env)).toBe(expected); @@ -99,21 +110,29 @@ describe("shouldMigrateOnStart", () => { }); describe("migrateOnStart", () => { - test("no committed journal: a no-op that never connects", async () => { - await migrateOnStart(pool, schema, appRoot()); + test("no committed journal: says so and never connects", async () => { + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + const root = appRoot(); + + await migrateOnStart(pool, schema, root, deployEnv); expect(pool.connect).not.toHaveBeenCalled(); + expect(log.mock.calls.flat().map(String).join(" ")).toContain( + `No committed migrations at ${path.join(root, "config/database/migrations")}`, + ); }); - test("applies from /config/database/migrations into the schema's own journal, and logs what it did", async () => { - const root = appRoot(); + test("applies from /config/database/migrations into the schema's own journal, and logs one summary", async () => { + const root = appRootWithMigrations(); const log = vi.spyOn(console, "log").mockImplementation(() => undefined); applyPendingMigrations.mockResolvedValueOnce({ + ...nothingDone, applied: ["0001_migration"], adopted: ["0000_baseline"], + lockWaitMs: 12, }); - await migrateOnStart(pool, schema, root); + await migrateOnStart(pool, schema, root, deployEnv); expect(applyPendingMigrations).toHaveBeenCalledWith(pool, { migrationsFolder: path.join(root, "config/database/migrations"), @@ -121,29 +140,77 @@ describe("migrateOnStart", () => { }); const logged = log.mock.calls.map((call) => call.map(String).join(" ")); expect(logged).toEqual([ - expect.stringContaining( - "Adopted 0000_baseline in schema app: its tables already existed", - ), - expect.stringContaining( - "Applied migrations 0001_migration to schema app", + expect.stringMatching( + /Migrations for schema app from .*: applied=1 adopted=1 edited=0 unknown=0 lock_wait_ms=12 duration_ms=\d+/, ), ]); }); - test("nothing pending logs nothing", async () => { - const log = vi.spyOn(console, "log").mockImplementation(() => undefined); - applyPendingMigrations.mockResolvedValueOnce({ applied: [], adopted: [] }); + test.each([ + ["a developer branch", "dev-jane-a1b2c3"], + ["an agent session branch", "session-k3x9q2m7"], + ])( + "%s is never migrated on start, even with committed migrations", + async (_label, branch) => { + const warn = vi + .spyOn(console, "warn") + .mockImplementation(() => undefined); + + await migrateOnStart(pool, schema, appRootWithMigrations(), { + ...deployEnv, + LAKEBASE_ENDPOINT: `projects/p/branches/${branch}/endpoints/primary`, + }); + + expect(applyPendingMigrations).not.toHaveBeenCalled(); + expect(warn.mock.calls.flat().map(String).join(" ")).toContain( + `Not migrating branch ${branch} on start`, + ); + }, + ); - await migrateOnStart(pool, schema, appRoot()); + test.each([ + ["a branch that only starts with dev-", "dev-environment"], + ["the production branch", "production"], + ])("%s is migrated", async (_label, branch) => { + vi.spyOn(console, "log").mockImplementation(() => undefined); + applyPendingMigrations.mockResolvedValueOnce(nothingDone); - expect(log).not.toHaveBeenCalled(); + await migrateOnStart(pool, schema, appRootWithMigrations(), { + ...deployEnv, + LAKEBASE_ENDPOINT: `projects/p/branches/${branch}/endpoints/primary`, + }); + + expect(applyPendingMigrations).toHaveBeenCalled(); + }); + + test("a pooled endpoint is refused: the migration lock would not hold", async () => { + const error = await migrateOnStart(pool, schema, appRootWithMigrations(), { + ...deployEnv, + PGHOST: "ep-abc-pooler.database.cloud.databricks.com", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(DatabasePluginError); + expect((error as Error).message).toContain( + "is a pooled endpoint, and migrations need a direct connection", + ); + expect(applyPendingMigrations).not.toHaveBeenCalled(); }); test.each([ [ "a failed migration", - new apply.MigrationApplyError(["0001_migration"], "42501"), - "Applying migration 0001_migration failed (SQLSTATE 42501); none were applied.", + new apply.MigrationApplyError({ + step: "Applying migrations", + failed: "0001_migration", + pending: ["0001_migration"], + cause: Object.assign(new Error("denied"), { code: "42501" }), + }), + "Migration 0001_migration failed (SQLSTATE 42501) and was rolled back.", + ], + [ + "a stranded migration", + new apply.MigrationHistoryError("app", ["0001_parallel"], "0002_b"), + "Migration 0001_parallel is older than 0002_b", ], [ "a baseline that only partly matches", @@ -164,9 +231,12 @@ describe("migrateOnStart", () => { async (_label, failure, reason) => { applyPendingMigrations.mockRejectedValueOnce(failure); - const error = await migrateOnStart(pool, schema, appRoot()).catch( - (caught: unknown) => caught, - ); + const error = await migrateOnStart( + pool, + schema, + appRootWithMigrations(), + deployEnv, + ).catch((caught: unknown) => caught); expect(error).toBeInstanceOf(DatabasePluginError); expect(error).toMatchObject({ category: "SETUP_FAILED", phase: "setup" }); @@ -182,6 +252,8 @@ describe("migrateOnStart", () => { const raw = new Error("connection reset"); applyPendingMigrations.mockRejectedValueOnce(raw); - await expect(migrateOnStart(pool, schema, appRoot())).rejects.toBe(raw); + await expect( + migrateOnStart(pool, schema, appRootWithMigrations(), deployEnv), + ).rejects.toBe(raw); }); }); From c517b833f6b85958e281d976692d8098aa1843a3 Mon Sep 17 00:00:00 2001 From: ditadi Date: Fri, 30 Oct 2026 15:10:02 +0000 Subject: [PATCH 3/4] chore: declare tools and the appkit bin as knip entry points knip.json ignores tools/**, so knip reported esbuild, ora and zod as unused even though tools/bundle-size.ts, tools/generate-json-schema.ts and tools/playground/deploy-playground.ts import them. It also reported packages/appkit/bin/appkit.js as an unused file. Declare these as entry points so the pre-commit knip check passes without suppressing any finding. Signed-off-by: ditadi --- knip.json | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/knip.json b/knip.json index 8e2d0f8a1..566a07df1 100644 --- a/knip.json +++ b/knip.json @@ -7,7 +7,15 @@ "docs" ], "workspaces": { + ".": { + "entry": [ + "tools/bundle-size.ts", + "tools/generate-json-schema.ts", + "tools/playground/deploy-playground.ts" + ] + }, "packages/appkit": { + "entry": ["bin/appkit.js"], "ignoreDependencies": [ "vitest", "@databricks/sdk-apps", From 966a51a7e1c7820897553db53ffbf9f85ae25338 Mon Sep 17 00:00:00 2001 From: ditadi Date: Fri, 30 Oct 2026 15:10:16 +0000 Subject: [PATCH 4/4] fix(appkit): strand unapplied migrations that share a recorded timestamp reconcileMigrations kept one recorded hash per created_at, so a committed migration whose journal entry shares a when with an applied sibling was reported as edited and never ran. Keep every recorded hash per when, and classify an entry whose sibling owns the recorded row as pending or stranded. Signed-off-by: ditadi --- .../appkit/src/database/migrations/history.ts | 31 ++++++++++++------- .../database/migrations/tests/history.test.ts | 15 +++++++++ 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/packages/appkit/src/database/migrations/history.ts b/packages/appkit/src/database/migrations/history.ts index c7ca35cda..bf3583c57 100644 --- a/packages/appkit/src/database/migrations/history.ts +++ b/packages/appkit/src/database/migrations/history.ts @@ -32,29 +32,38 @@ interface MigrationHistory { /** * Match each committed migration with a recorded row by `when` = `created_at`, - * the pair drizzle-orm's migrator writes, then compare hashes. + * the pair drizzle-orm's migrator writes, then compare hashes. Two entries can + * share a `when` after a hand edit or a merge, so each `when` keeps every hash + * recorded at it. */ export function reconcileMigrations( files: readonly MigrationFile[], recorded: readonly RecordedMigration[], ): MigrationHistory { - const hashAt = new Map(); + const hashesAt = new Map>(); for (const row of recorded) { - if (row.createdAt !== null) hashAt.set(row.createdAt, row.hash); + if (row.createdAt === null) continue; + const hashes = hashesAt.get(row.createdAt) ?? new Set(); + hashes.add(row.hash); + hashesAt.set(row.createdAt, hashes); } - const lastRecorded = hashAt.size > 0 ? Math.max(...hashAt.keys()) : null; + const lastRecorded = hashesAt.size > 0 ? Math.max(...hashesAt.keys()) : null; const pending: MigrationFile[] = []; const stranded: MigrationFile[] = []; const edited: MigrationFile[] = []; for (const file of files) { - const hash = hashAt.get(file.when); - if (hash === undefined) { - if (lastRecorded === null || file.when > lastRecorded) pending.push(file); - else stranded.push(file); - } else if (hash !== file.hash) { - edited.push(file); - } + const hashes = hashesAt.get(file.when); + if (hashes?.has(file.hash)) continue; + // A row at this `when` that is another entry's: this one never ran. + const sibling = files.some( + (other) => + other !== file && other.when === file.when && hashes?.has(other.hash), + ); + if (hashes && !sibling) edited.push(file); + else if (lastRecorded === null || file.when > lastRecorded) { + pending.push(file); + } else stranded.push(file); } const committed = new Set(files.map((file) => file.when)); const unknown = recorded.filter( diff --git a/packages/appkit/src/database/migrations/tests/history.test.ts b/packages/appkit/src/database/migrations/tests/history.test.ts index 04da4ac6e..367dd605a 100644 --- a/packages/appkit/src/database/migrations/tests/history.test.ts +++ b/packages/appkit/src/database/migrations/tests/history.test.ts @@ -72,4 +72,19 @@ describe("reconcileMigrations", () => { expect(history.pending).toEqual([]); expect(history.stranded).toEqual([]); }); + + test("two entries with one `when` keep their own rows: both recorded is clean, one recorded strands the other", () => { + const twins = [file("0000_a", 1000), file("0001_twin", 1000)]; + const both = reconcileMigrations(twins, [ + { hash: "hash-0000_a", createdAt: 1000 }, + { hash: "hash-0001_twin", createdAt: 1000 }, + ]); + expect([...both.pending, ...both.stranded, ...both.edited]).toEqual([]); + + const one = reconcileMigrations(twins, [ + { hash: "hash-0000_a", createdAt: 1000 }, + ]); + expect(tags(one.stranded)).toEqual(["0001_twin"]); + expect(one.edited).toEqual([]); + }); });