diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 71708399892..9e2d86607e9 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -256,6 +256,8 @@ If you prefer not to use Docker. **All commands run from the repository root unl For ad-hoc schema iteration during development you can also use `bun run db:push` from `packages/db`, but `db:migrate` is the canonical command for staging and production. `db:push` reconciles directly to the current schema without running versioned migration guards. For disposable local/dev databases, `bun run db:push --force` accepts Drizzle's data-loss prompts, including column drops. + `db:push` treats added and removed columns, tables, and other schema objects as separate creations and deletions. It never infers a rename. For an intentional rename during local development, run `bun run db:push --interactive-renames` in a terminal and select the old object in Drizzle's chooser. This flag does not approve data loss; `--force` controls that separately. After schema reconciliation succeeds, the wrapper reconciles credential policies and OAuth providers, then backfills search vectors. A failure stops subsequent steps. Staging and production changes still use reviewed versioned migrations with expand/contract deployment steps. + 4. **Run the Development Servers:** ```bash diff --git a/.github/workflows/migrations.yml b/.github/workflows/migrations.yml index d1d196e91b7..0afdddeff97 100644 --- a/.github/workflows/migrations.yml +++ b/.github/workflows/migrations.yml @@ -75,16 +75,7 @@ jobs: echo "Dev environment — pushing schema directly (db:push)" # Dev deliberately forces direct schema reconciliation; staging and # production use guarded versioned migrations in the other branch. - # drizzle-kit push needs a TTY to resolve ambiguous renames (--force only - # covers data-loss). In CI it throws "Interactive prompts require a TTY - # terminal" but still exits 0, so the job goes green without applying the - # change. tee keeps the output live in the log; we then fail on drizzle's - # own TTY error. pipefail also preserves a non-zero db:push exit through tee. - SIM_DEV_DB_PUSH=1 bun run db:push --force < /dev/null 2>&1 | tee /tmp/db-push.log - if grep -q "Interactive prompts require a TTY terminal" /tmp/db-push.log; then - echo "ERROR: db:push needs an interactive rename decision; land it as a versioned migration instead of relying on push." >&2 - exit 1 - fi + SIM_DEV_DB_PUSH=1 bun run db:push --force < /dev/null else echo "Applying versioned migrations (db:migrate)" bun run ./scripts/migrate.ts diff --git a/bun.lock b/bun.lock index c326d4a7db7..f7c4714baa3 100644 --- a/bun.lock +++ b/bun.lock @@ -486,8 +486,9 @@ }, "devDependencies": { "@sim/tsconfig": "workspace:*", + "@types/bun": "1.4.1", "@types/node": "24.2.1", - "drizzle-kit": "^0.31.4", + "drizzle-kit": "0.31.10", "typescript": "^7.0.2", "vitest": "^4.1.0", }, @@ -817,6 +818,7 @@ ], "patchedDependencies": { "@better-auth/oauth-provider@1.6.27": "patches/@better-auth%2Foauth-provider@1.6.27.patch", + "drizzle-kit@0.31.10": "patches/drizzle-kit@0.31.10.patch", "postgres@3.4.9": "patches/postgres@3.4.9.patch", }, "overrides": { @@ -2170,6 +2172,8 @@ "@types/buffer-from": ["@types/buffer-from@1.1.3", "", { "dependencies": { "@types/node": "*" } }, "sha512-2lq4YC9uLUMGHkl2IDtX4tCXSo2+hwMpOJcY1qiIk1kybc31rIlPyM1HCVJhkPFIo75a/pOVxqyvwuf5TpCG/w=="], + "@types/bun": ["@types/bun@1.4.1", "", { "dependencies": { "bun-types": "1.4.1" } }, "sha512-0AVGiTXGajf1rgKom3N+c5L7CBxuoyyv1i44M0nX4UDK0G/fnRAMiri93nHuVPIb429KKtAgj7HatVmmOjeQLA=="], + "@types/busboy": ["@types/busboy@1.5.4", "", { "dependencies": { "@types/node": "*" } }, "sha512-kG7WrUuAKK0NoyxfQHsVE6j1m01s6kMma64E+OZenQABMQyTJop1DumUWcLwAQ2JzpefU7PDYoRDKl8uZosFjw=="], "@types/cacheable-request": ["@types/cacheable-request@6.0.3", "", { "dependencies": { "@types/http-cache-semantics": "*", "@types/keyv": "^3.1.4", "@types/node": "*", "@types/responselike": "^1.0.0" } }, "sha512-IQ3EbTzGxIigb1I3qPZc1rWJnH0BmSKv5QYTalEwweFvyBDLSAe24zP0le/hyi7ecGfZVlIVAg4BZqb8WBwKqw=="], @@ -2586,6 +2590,8 @@ "builder-util-runtime": ["builder-util-runtime@9.7.0", "", { "dependencies": { "debug": "^4.3.4", "sax": "^1.2.4" } }, "sha512-g/kR520giAFYkSXTzcmF3kqQq7wi8F6N6SzeDgZrqTBN+VHdmgWOyTdD1yD7AATDId/yXLvuP34CxW46/BwCdw=="], + "bun-types": ["bun-types@1.4.1", "", { "dependencies": { "@types/node": "*" } }, "sha512-loKuVrAFZKfEv+JvWkHRS9GW5IqLuLRjVXN9p+vZvBN86O5hf/pBZQ5hSoyipsrMmWObZBDvWnlmKvjKTM0PdA=="], + "bundle-name": ["bundle-name@4.1.0", "", { "dependencies": { "run-applescript": "^7.0.0" } }, "sha512-tjwM5exMg6BGRI+kNmTntNsvdZS1X8BFYS6tnJ2hdH0kVxM6/eVZ2xy+FqStSWvYmtfFMDLIxurorHwDKfDz5Q=="], "busboy": ["busboy@1.6.0", "", { "dependencies": { "streamsearch": "^1.1.0" } }, "sha512-8SFQbg/0hQ9xy3UNTB0YEnsNBbWfhf7RtnzpL7TkBiTBRfrQ9Fxcnz7VJsleJpyp6rVLvXiuORqjlHi5q+PYuA=="], @@ -5160,6 +5166,8 @@ "builder-util/fs-extra": ["fs-extra@10.1.0", "", { "dependencies": { "graceful-fs": "^4.2.0", "jsonfile": "^6.0.1", "universalify": "^2.0.0" } }, "sha512-oRXApq54ETRj4eMiFzGnHWGy+zo5raudjuxN0b8H7s/RU2oW0Wvsx9O0ACRN/kRq9E8Vu/ReskGB5o3ji+FzHQ=="], + "bun-types/@types/node": ["@types/node@25.9.3", "", { "dependencies": { "undici-types": ">=7.24.0 <7.24.7" } }, "sha512-603BddQMv3pUcr4U2dhujk83N2tTDVr/34wII2B6bJy6g+8WD6yUb11jszNs0gdi4PesVWl7ABt8nYMVpnLUcg=="], + "c12/chokidar": ["chokidar@4.0.3", "", { "dependencies": { "readdirp": "^4.0.1" } }, "sha512-Qgzu8kfBvo+cA4962jnP1KkS6Dop5NS6g7R5LFYJr4b8Ub94PPQXUksCw9PvXoeXPRRddRNC5C1JQUR2SMGtnA=="], "c12/confbox": ["confbox@0.2.4", "", {}, "sha512-ysOGlgTFbN2/Y6Cg3Iye8YKulHw+R2fNXHrgSmXISQdMnomY6eNDprVdW9R5xBguEqI954+S6709UyiO7B+6OQ=="], @@ -5738,6 +5746,8 @@ "builder-util/chalk/supports-color": ["supports-color@7.2.0", "", { "dependencies": { "has-flag": "^4.0.0" } }, "sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw=="], + "bun-types/@types/node/undici-types": ["undici-types@7.24.6", "", {}, "sha512-WRNW+sJgj5OBN4/0JpHFqtqzhpbnV0GuB+OozA9gCL7a993SmU+1JBZCzLNxYsbMfIeDL+lTsphD5jN5N+n0zg=="], + "c12/chokidar/readdirp": ["readdirp@4.1.2", "", {}, "sha512-GDhwkLfywWL2s6vEjyhri+eXmfH6j1L7JE27WhqLeYzoh/A3DBaYGEj2H/HFZCn/kMfim73FXxEJTw06WtxQwg=="], "chrome-launcher/@types/node/undici-types": ["undici-types@7.24.6", "", {}, "sha512-WRNW+sJgj5OBN4/0JpHFqtqzhpbnV0GuB+OozA9gCL7a993SmU+1JBZCzLNxYsbMfIeDL+lTsphD5jN5N+n0zg=="], diff --git a/package.json b/package.json index 212f9d43eee..36dc6086ef7 100644 --- a/package.json +++ b/package.json @@ -192,6 +192,7 @@ ], "patchedDependencies": { "@better-auth/oauth-provider@1.6.27": "patches/@better-auth%2Foauth-provider@1.6.27.patch", + "drizzle-kit@0.31.10": "patches/drizzle-kit@0.31.10.patch", "postgres@3.4.9": "patches/postgres@3.4.9.patch" } } diff --git a/packages/db/package.json b/packages/db/package.json index df53ffeeaad..8e402495424 100644 --- a/packages/db/package.json +++ b/packages/db/package.json @@ -48,8 +48,9 @@ }, "devDependencies": { "@sim/tsconfig": "workspace:*", - "drizzle-kit": "^0.31.4", + "@types/bun": "1.4.1", "@types/node": "24.2.1", + "drizzle-kit": "0.31.10", "typescript": "^7.0.2", "vitest": "^4.1.0" } diff --git a/packages/db/script-migrations/database-url.ts b/packages/db/script-migrations/database-url.ts index 116b224c2d4..dea515a5814 100644 --- a/packages/db/script-migrations/database-url.ts +++ b/packages/db/script-migrations/database-url.ts @@ -1,6 +1,9 @@ /** Matches the main migrator: an empty optional direct DSN falls back to DATABASE_URL. */ export function resolveMigrationDatabaseUrl( - env: { MIGRATION_DATABASE_URL?: string; DATABASE_URL?: string } = process.env + env: { MIGRATION_DATABASE_URL?: string; DATABASE_URL?: string } = { + MIGRATION_DATABASE_URL: process.env.MIGRATION_DATABASE_URL, + DATABASE_URL: process.env.DATABASE_URL, + } ): string | undefined { return env.MIGRATION_DATABASE_URL || env.DATABASE_URL } diff --git a/packages/db/scripts/push.postgres.test.ts b/packages/db/scripts/push.postgres.test.ts new file mode 100644 index 00000000000..831b6028107 --- /dev/null +++ b/packages/db/scripts/push.postgres.test.ts @@ -0,0 +1,187 @@ +import { spawnSync } from 'node:child_process' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { generateId } from '@sim/utils/id' +import postgres, { type Sql } from 'postgres' +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest' + +const databaseUrl = process.env.DB_PUSH_TEST_DATABASE_URL + +describe.skipIf(!databaseUrl)('patched Drizzle push against PostgreSQL', () => { + const databaseName = `push_policy_${generateId().replaceAll('-', '')}` + let admin: Sql + let sql: Sql + let fixtureUrl: string + let directory: string + + beforeAll(async () => { + admin = postgres(databaseUrl!, { max: 1, onnotice: () => {} }) + await admin`CREATE DATABASE ${admin(databaseName)}` + const url = new URL(databaseUrl!) + url.pathname = `/${databaseName}` + fixtureUrl = url.toString() + sql = postgres(fixtureUrl, { max: 1, onnotice: () => {} }) + directory = await mkdtemp(join(tmpdir(), 'push-policy-')) + await writeFile( + join(directory, 'drizzle.config.ts'), + `export default { + dialect: 'postgresql', + schema: ${JSON.stringify(join(directory, 'schema.ts'))}, + schemaFilter: ['public', 'old_scope', 'new_scope'], + tablesFilter: ['!script_migrations'], + dbCredentials: { url: process.env.DATABASE_URL }, +}` + ) + }) + + beforeEach(async () => { + await sql`DROP SCHEMA IF EXISTS old_scope CASCADE` + await sql`DROP SCHEMA IF EXISTS new_scope CASCADE` + await sql`DROP SCHEMA public CASCADE` + await sql`CREATE SCHEMA public` + }) + + afterAll(async () => { + await sql?.end() + if (admin) { + await admin`DROP DATABASE IF EXISTS ${admin(databaseName)}` + await admin.end() + } + if (directory) await rm(directory, { recursive: true, force: true }) + }) + + async function schema(source: string) { + await writeFile( + join(directory, 'schema.ts'), + `import { pgTable, pgSchema, pgEnum, text, integer, boolean, check } from ${JSON.stringify(import.meta.resolve('drizzle-orm/pg-core'))} +import { sql } from ${JSON.stringify(import.meta.resolve('drizzle-orm'))} +${source}` + ) + } + + /** Exercise the patched CLI with pipes, never a terminal or canned prompt answers. */ + function push(args = ['--force'], renameMode: string | undefined = 'create') { + return spawnSync( + 'bunx', + [ + '--no-install', + 'drizzle-kit', + 'push', + '--config', + join(directory, 'drizzle.config.ts'), + ...args, + ], + { + env: { ...process.env, DATABASE_URL: fixtureUrl, SIM_DB_PUSH_RENAME_MODE: renameMode }, + encoding: 'utf8', + timeout: 30_000, + } + ) + } + + async function legacyColumns() { + await sql`CREATE TABLE records (id text PRIMARY KEY, old_label text, old_enabled boolean)` + await sql`INSERT INTO records VALUES ('existing', 'original value', true)` + await schema(`export const records = pgTable('records', { + id: text('id').primaryKey(), + newLabel: text('new_label').default('new default'), + newEnabled: boolean('new_enabled').notNull().default(false), +})`) + } + + it('initializes a fresh database', async () => { + await schema(`export const records = pgTable('records', { + id: text('id').primaryKey(), enabled: boolean('enabled').notNull().default(false), +})`) + const result = push() + expect(result.error).toBeUndefined() + expect(result.status).toBe(0) + await sql`INSERT INTO records (id) VALUES ('new-row')` + expect(await sql`SELECT * FROM records`).toEqual([{ id: 'new-row', enabled: false }]) + }, 30_000) + + it('creates independent columns across ambiguous pairs and can be rerun', async () => { + await legacyColumns() + const result = push() + expect(result.error).toBeUndefined() + expect(result.status).toBe(0) + expect(result.stdout + result.stderr).not.toContain( + 'Interactive prompts require a TTY terminal' + ) + expect(await sql`SELECT * FROM records`).toEqual([ + { id: 'existing', new_label: 'new default', new_enabled: false }, + ]) + const repeated = push() + expect(repeated.status).toBe(0) + expect(repeated.stdout).toContain('No changes detected') + }, 60_000) + + it('creates independent tables and enums while preserving the excluded script ledger', async () => { + await sql`CREATE TYPE old_status AS ENUM ('active')` + await sql`CREATE TABLE old_records (id text PRIMARY KEY, status old_status)` + await sql`INSERT INTO old_records VALUES ('old-row', 'active')` + await sql`CREATE TABLE script_migrations (name text PRIMARY KEY)` + await sql`INSERT INTO script_migrations VALUES ('completed-fixture-migration')` + await schema(`export const status = pgEnum('new_status', ['active']) +export const records = pgTable('new_records', { id: text('id').primaryKey(), status: status('status') })`) + const result = push() + expect(result.error).toBeUndefined() + expect(result.status).toBe(0) + expect(await sql`SELECT * FROM new_records`).toEqual([]) + expect( + await sql`SELECT to_regclass('old_records') AS old_table, to_regtype('old_status') AS old_type` + ).toEqual([{ old_table: null, old_type: null }]) + expect(await sql`SELECT * FROM script_migrations`).toEqual([ + { name: 'completed-fixture-migration' }, + ]) + }, 30_000) + + it('creates a new schema instead of moving a removed schema', async () => { + await sql`CREATE SCHEMA old_scope` + await sql`CREATE TABLE old_scope.records (id text PRIMARY KEY)` + await sql`INSERT INTO old_scope.records VALUES ('old-row')` + await schema(`export const scope = pgSchema('new_scope') +export const records = scope.table('records', { id: text('id').primaryKey() })`) + const result = push() + expect(result.error).toBeUndefined() + expect(result.status, result.stdout + result.stderr).toBe(0) + expect(await sql`SELECT * FROM new_scope.records`).toEqual([]) + expect(await sql`SELECT to_regnamespace('old_scope') AS old_schema`).toEqual([ + { old_schema: null }, + ]) + }, 30_000) + + it('keeps the data-loss approval independent of rename resolution', async () => { + await legacyColumns() + const result = push([]) + expect(result.error).toBeUndefined() + expect(result.status).toBe(1) + expect(result.stdout).toContain('Found data-loss statements') + expect(await sql`SELECT * FROM records`).toEqual([ + { id: 'existing', old_label: 'original value', old_enabled: true }, + ]) + }, 30_000) + + it('retains native rename prompts when the policy is not enabled', async () => { + await legacyColumns() + const result = push(['--force'], 'prompt') + expect(result.error).toBeUndefined() + expect(result.status).toBe(1) + expect(result.stdout + result.stderr).toContain('Interactive prompts require a TTY terminal') + expect(await sql`SELECT old_label FROM records`).toEqual([{ old_label: 'original value' }]) + }, 30_000) + + it('propagates a database DDL error instead of reporting success', async () => { + await sql`CREATE TABLE records (id text PRIMARY KEY, value integer)` + await sql`INSERT INTO records VALUES ('invalid-row', -1)` + await schema(`export const records = pgTable('records', { + id: text('id').primaryKey(), value: integer('value'), +}, (table) => [check('nonnegative_value', sql\`\${table.value} >= 0\`)])`) + const result = push() + expect(result.error).toBeUndefined() + expect(result.status).toBe(1) + expect(result.stderr).toContain('23514') + expect(await sql`SELECT value FROM records`).toEqual([{ value: -1 }]) + }, 30_000) +}) diff --git a/packages/db/scripts/push.test.ts b/packages/db/scripts/push.test.ts new file mode 100644 index 00000000000..f8c59bd8261 --- /dev/null +++ b/packages/db/scripts/push.test.ts @@ -0,0 +1,91 @@ +import { runPush } from '@sim/db/scripts/push' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('@sim/logger', () => ({ createLogger: () => ({ info: vi.fn(), error: vi.fn() }) })) + +interface SpawnOptions { + env?: NodeJS.ProcessEnv + stdin: string + stdout: string + stderr: string +} + +const spawn = vi.fn<(command: string[], options: SpawnOptions) => { exited: Promise }>() +const stdinTty = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY') +const stdoutTty = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY') + +function setTerminal(enabled: boolean) { + Object.defineProperty(process.stdin, 'isTTY', { configurable: true, value: enabled }) + Object.defineProperty(process.stdout, 'isTTY', { configurable: true, value: enabled }) +} + +beforeEach(() => { + spawn.mockReset().mockImplementation(() => ({ exited: Promise.resolve(0) })) + vi.stubGlobal('Bun', { spawn }) + setTerminal(false) +}) + +afterEach(() => { + vi.unstubAllGlobals() + vi.unstubAllEnvs() + if (stdinTty) Object.defineProperty(process.stdin, 'isTTY', stdinTty) + else Reflect.deleteProperty(process.stdin, 'isTTY') + if (stdoutTty) Object.defineProperty(process.stdout, 'isTTY', stdoutTty) + else Reflect.deleteProperty(process.stdout, 'isTTY') +}) + +describe('db:push policy and process boundaries', () => { + it('sets create/drop only on the Drizzle child and forwards force independently', async () => { + vi.stubEnv('SIM_DB_PUSH_RENAME_MODE', undefined) + expect(await runPush(['--force'])).toBe(0) + expect(spawn).toHaveBeenCalledTimes(4) + expect(spawn.mock.calls[0][0]).toEqual([ + 'bunx', + '--no-install', + 'drizzle-kit', + 'push', + '--config=./drizzle.config.ts', + '--force', + ]) + expect(spawn.mock.calls[0][1].env?.SIM_DB_PUSH_RENAME_MODE).toBe('create') + expect(process.env.SIM_DB_PUSH_RENAME_MODE).toBeUndefined() + for (const [, options] of spawn.mock.calls.slice(1)) expect(options.env).toBeUndefined() + }) + + it('does not implicitly approve data loss', async () => { + expect(await runPush([])).toBe(0) + expect(spawn.mock.calls[0][0]).not.toContain('--force') + }) + + it('stops reconciliation when Drizzle fails', async () => { + spawn.mockReturnValueOnce({ exited: Promise.resolve(42) }) + expect(await runPush(['--force'])).toBe(42) + expect(spawn).toHaveBeenCalledTimes(1) + }) + + it('stops after the first failed reconciliation', async () => { + spawn.mockReturnValueOnce({ exited: Promise.resolve(0) }) + spawn.mockReturnValueOnce({ exited: Promise.resolve(43) }) + expect(await runPush([])).toBe(43) + expect(spawn).toHaveBeenCalledTimes(2) + }) + + it('passes intentional renames to the native chooser in a terminal', async () => { + setTerminal(true) + vi.stubEnv('SIM_DB_PUSH_RENAME_MODE', 'create') + expect(await runPush(['--interactive-renames', '--verbose'])).toBe(0) + expect(spawn.mock.calls[0][0]).not.toContain('--interactive-renames') + expect(spawn.mock.calls[0][0]).toContain('--verbose') + expect(spawn.mock.calls[0][1].env?.SIM_DB_PUSH_RENAME_MODE).toBe('prompt') + }) + + it('rejects interactive renames without a terminal before any database commands', async () => { + expect(await runPush(['--interactive-renames', '--force'])).toBe(1) + expect(spawn).not.toHaveBeenCalled() + }) + + it('does not reconcile the database when requesting help', async () => { + expect(await runPush(['--help'])).toBe(0) + expect(spawn).toHaveBeenCalledTimes(1) + }) +}) diff --git a/packages/db/scripts/push.ts b/packages/db/scripts/push.ts index 08b93f9557e..a30107ed592 100644 --- a/packages/db/scripts/push.ts +++ b/packages/db/scripts/push.ts @@ -1,13 +1,56 @@ -/** Forward push flags to Drizzle before running the schema reconciliation steps. */ -const commands = [ - ['bunx', 'drizzle-kit', 'push', '--config=./drizzle.config.ts', ...process.argv.slice(2)], +import { createLogger } from '@sim/logger' + +const logger = createLogger('DatabasePush') +const RECONCILIATION_COMMANDS = [ ['bun', '--env-file=.env', 'run', './scripts/reconcile-credential-group-resource-policies.ts'], ['bun', '--env-file=.env', 'run', './scripts/reconcile-oauth-provider.ts'], ['bun', '--env-file=.env', 'run', './script-migrations/0016_backfill_search_vectors.ts'], ] -for (const command of commands) { - const child = Bun.spawn(command, { stdin: 'inherit', stdout: 'inherit', stderr: 'inherit' }) +/** + * Push treats additions and removals as distinct objects by default. The pinned + * Drizzle patch reads this policy only in the push subprocess; generation keeps + * its ordinary rename prompts. --interactive-renames opts back into that chooser. + * --force remains the independent approval for data-loss statements. + */ +export async function runPush(args: string[]): Promise { + const interactiveRenames = args.includes('--interactive-renames') + const help = args.includes('--help') || args.includes('-h') + if (help) { + logger.info('Use --interactive-renames in a terminal to choose intentional renames.') + } + if (interactiveRenames && !help && (!process.stdin.isTTY || !process.stdout.isTTY)) { + logger.error( + '--interactive-renames requires a terminal; use the default create/drop policy in CI.' + ) + return 1 + } + + const pushArgs = args.filter((arg) => arg !== '--interactive-renames') + const child = Bun.spawn( + ['bunx', '--no-install', 'drizzle-kit', 'push', '--config=./drizzle.config.ts', ...pushArgs], + { + env: { ...process.env, SIM_DB_PUSH_RENAME_MODE: interactiveRenames ? 'prompt' : 'create' }, + stdin: 'inherit', + stdout: 'inherit', + stderr: 'inherit', + } + ) const exitCode = await child.exited - if (exitCode !== 0) process.exit(exitCode) + if (exitCode !== 0 || help) return exitCode + + for (const command of RECONCILIATION_COMMANDS) { + const reconciliation = Bun.spawn(command, { + stdin: 'inherit', + stdout: 'inherit', + stderr: 'inherit', + }) + const reconciliationExit = await reconciliation.exited + if (reconciliationExit !== 0) return reconciliationExit + } + return 0 +} + +if (import.meta.main) { + process.exit(await runPush(process.argv.slice(2))) } diff --git a/patches/README.md b/patches/README.md index b4bacf0962a..1ee961f6519 100644 --- a/patches/README.md +++ b/patches/README.md @@ -45,3 +45,28 @@ run it manually. Remove this patch when the pinned driver includes equivalent transaction-scope closure handling. Keep the reconnect regression tests when upgrading. + +# Drizzle development push policy + +`drizzle-kit@0.31.10` prompts when it sees both additions and removals, even with +`--force`. Its PostgreSQL push path also catches errors and exits successfully, +which lets post-push work run against a schema that was not applied. + +The pinned CLI patch adds an opt-in create/drop policy to the existing rename +resolvers and makes PostgreSQL push failures and cancellations return nonzero. +`packages/db/scripts/push.ts` enables the policy only in its Drizzle subprocess +using `SIM_DB_PUSH_RENAME_MODE=create`. `--interactive-renames` selects the native +chooser and requires a terminal. Normal migration generation keeps its rename +prompts. Data-loss confirmations, table filters, introspection, and generated SQL +remain owned by Drizzle; `--force` still controls data-loss approval. + +`packages/db/scripts/push.test.ts` covers argument forwarding and stopping before +reconciliation on failure. `packages/db/scripts/push.postgres.test.ts` exercises +the installed CLI against PostgreSQL, including multiple column changes, table +and enum replacement, schema replacement, preservation of the excluded script +ledger, and database errors. Set `DB_PUSH_TEST_DATABASE_URL` to a disposable local +PostgreSQL database and run these tests from `packages/db` with `bun run test`. + +Remove the patch when Drizzle provides an explicit noninteractive create/drop +policy and propagates push failures. Keep Drizzle pinned until the replacement +passes these regression tests. diff --git a/patches/drizzle-kit@0.31.10.patch b/patches/drizzle-kit@0.31.10.patch new file mode 100644 index 00000000000..5deebcc76a3 --- /dev/null +++ b/patches/drizzle-kit@0.31.10.patch @@ -0,0 +1,75 @@ +diff --git a/bin.cjs b/bin.cjs +index 07274ea6f53fd54a129b3d338892feda6e995cff..2c4556d9f8741560581e4c1f4e06d1dfb8dda63c 100755 +--- a/bin.cjs ++++ b/bin.cjs +@@ -32692,7 +32692,7 @@ var init_migrate = __esm({ + }; + }; + promptColumnsConflicts = async (tableName, newColumns, missingColumns) => { +- if (newColumns.length === 0 || missingColumns.length === 0) { ++ if (process.env.SIM_DB_PUSH_RENAME_MODE === "create" || newColumns.length === 0 || missingColumns.length === 0) { + return { created: newColumns, renamed: [], deleted: missingColumns }; + } + const result = { +@@ -32742,7 +32742,7 @@ var init_migrate = __esm({ + return result; + }; + promptNamedConflict = async (newItems, missingItems, entity) => { +- if (missingItems.length === 0 || newItems.length === 0) { ++ if (process.env.SIM_DB_PUSH_RENAME_MODE === "create" || missingItems.length === 0 || newItems.length === 0) { + return { + created: newItems, + renamed: [], +@@ -32792,7 +32792,7 @@ var init_migrate = __esm({ + return result; + }; + promptNamedWithSchemasConflict = async (newItems, missingItems, entity) => { +- if (missingItems.length === 0 || newItems.length === 0) { ++ if (process.env.SIM_DB_PUSH_RENAME_MODE === "create" || missingItems.length === 0 || newItems.length === 0) { + return { + created: newItems, + renamed: [], +@@ -32852,7 +32852,7 @@ var init_migrate = __esm({ + return result; + }; + promptSchemasConflict = async (newSchemas, missingSchemas) => { +- if (missingSchemas.length === 0 || newSchemas.length === 0) { ++ if (process.env.SIM_DB_PUSH_RENAME_MODE === "create" || missingSchemas.length === 0 || newSchemas.length === 0) { + return { created: newSchemas, renamed: [], deleted: missingSchemas }; + } + const result = { +@@ -82739,7 +82739,7 @@ var init_push = __esm({ + ); + if ((data == null ? void 0 : data.index) === 0) { + (0, import_hanji13.render)(`[${source_default.red("x")}] All changes were aborted`); +- process.exit(0); ++ process.exit(1); + } + } + } +@@ -82761,7 +82761,7 @@ var init_push = __esm({ + ); + if ((data == null ? void 0 : data.index) === 0) { + (0, import_hanji13.render)(`[${source_default.red("x")}] All changes were aborted`); +- process.exit(0); ++ process.exit(1); + } + } + for (const dStmnt of statementsToExecute) { +@@ -82774,7 +82774,7 @@ var init_push = __esm({ + } + } + } catch (e4) { +- console.error(e4); ++ throw e4; + } + }; + sqlitePush = async (schemaPath, verbose, strict, credentials2, tablesFilter, force, casing2) => { +@@ -92279,6 +92279,7 @@ var push = command2({ + } + } catch (e4) { + console.error(e4); ++ process.exit(1); + } + process.exit(0); + } diff --git a/scripts/migrations-workflow.test.ts b/scripts/migrations-workflow.test.ts index f28ca356d3a..523bfbea909 100644 --- a/scripts/migrations-workflow.test.ts +++ b/scripts/migrations-workflow.test.ts @@ -1,7 +1,5 @@ import { spawnSync } from 'node:child_process' -import { mkdtempSync, readFileSync, rmSync } from 'node:fs' -import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { readFileSync } from 'node:fs' import { describe, expect, it } from 'vitest' const workflow = readFileSync( @@ -12,87 +10,73 @@ const step = workflow.match(/^ {8}run: \|\r?\n((?: {10}.*(?:\r?\n|$)|\r?\n)+)/m) if (!step) throw new Error('Migration workflow must contain its schema application shell step') const script = step.replace(/^ {10}/gm, '') -/** Execute the checked-in shell with fake database commands and an isolated scratch log. */ +/** Execute the checked-in shell with fake database commands. */ function runMigration(env: Record = {}) { - const directory = mkdtempSync(join(tmpdir(), 'migration-workflow-')) - try { - return spawnSync( - 'bash', - [ - '-e', - '-c', - `bun() { + return spawnSync( + 'bash', + [ + '-e', + '-c', + `bun() { printf 'COMMAND: %s\n' "$*" case "$2" in db:push) printf '%s\n' "$PUSH_OUTPUT"; return "$PUSH_EXIT" ;; - ./scripts/apply-dev-workspace-file-size-cutover.ts) return "$CUTOVER_EXIT" ;; ./scripts/migrate.ts) return "$MIGRATE_EXIT" ;; *) return 99 ;; esac } - ${script.replaceAll('/tmp/db-push.log', '"$MIGRATION_TEST_LOG"')}`, - ], - { - encoding: 'utf8', - env: { - PATH: process.env.PATH, - DATABASE_URL: 'postgresql://example.invalid/unused', - MIGRATION_DATABASE_URL: '', - ENVIRONMENT: 'dev', - MIGRATION_TEST_LOG: join(directory, 'push.log'), - PUSH_EXIT: '0', - PUSH_OUTPUT: 'Changes applied', - CUTOVER_EXIT: '0', - MIGRATE_EXIT: '0', - ...env, - }, - } - ) - } finally { - rmSync(directory, { recursive: true, force: true }) - } + ${script}`, + ], + { + encoding: 'utf8', + env: { + PATH: process.env.PATH, + DATABASE_URL: 'postgresql://example.invalid/unused', + MIGRATION_DATABASE_URL: '', + ENVIRONMENT: 'dev', + PUSH_EXIT: '0', + PUSH_OUTPUT: 'Changes applied', + MIGRATE_EXIT: '0', + ...env, + }, + } + ) } describe('migration workflow exit propagation', () => { - it('fails when post-schema initialization fails before tee succeeds', () => { + it('fails when post-schema initialization fails', () => { const result = runMigration({ PUSH_EXIT: '42', PUSH_OUTPUT: 'Changes applied\nDATABASE_URL is required to initialize search vectors', }) expect(result.status).toBe(42) expect(result.stdout).toContain('DATABASE_URL is required') - expect(result.stdout).not.toContain( - 'COMMAND: run ./scripts/apply-dev-workspace-file-size-cutover.ts' - ) }) - it('runs the dev cutover only after a successful schema push', () => { + it('runs the general push command on dev', () => { const result = runMigration() expect(result.status).toBe(0) expect(result.stdout).toContain('COMMAND: run db:push --force') - expect(result.stdout).toContain( - 'COMMAND: run ./scripts/apply-dev-workspace-file-size-cutover.ts' - ) + expect(result.stdout).not.toContain('COMMAND: run ./scripts/migrate.ts') }) - it('still rejects drizzle interactive failures that exit zero', () => { - const result = runMigration({ PUSH_OUTPUT: 'Interactive prompts require a TTY terminal' }) + it('propagates a noninteractive Drizzle failure', () => { + const result = runMigration({ + PUSH_EXIT: '1', + PUSH_OUTPUT: 'Interactive prompts require a TTY terminal', + }) expect(result.status).toBe(1) - expect(result.stdout).not.toContain( - 'COMMAND: run ./scripts/apply-dev-workspace-file-size-cutover.ts' - ) }) - it('propagates dev cutover failures', () => { - expect(runMigration({ CUTOVER_EXIT: '43' }).status).toBe(43) - }) - - it('keeps versioned migration failures fatal outside dev', () => { - const result = runMigration({ ENVIRONMENT: 'staging', MIGRATE_EXIT: '44' }) - expect(result.status).toBe(44) - expect(result.stdout).toContain('COMMAND: run ./scripts/migrate.ts') - expect(result.stdout).not.toContain('COMMAND: run db:push') - }) + it.each(['staging', 'production'])( + 'keeps versioned migration failures fatal in %s', + (environment) => { + const result = runMigration({ ENVIRONMENT: environment, MIGRATE_EXIT: '44' }) + expect(result.status).toBe(44) + expect(result.stdout).toContain('COMMAND: run ./scripts/migrate.ts') + expect(result.stdout).not.toContain('COMMAND: run db:push') + } + ) it('fails before invoking commands when no database URL is configured', () => { const result = runMigration({ DATABASE_URL: '' })