From 48e542c6dc29c0fbc8562a387cb4b866733a781a Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Wed, 16 Sep 2026 18:20:17 -0700 Subject: [PATCH 1/3] fix(db): resolve dev schema push column ambiguity --- .github/workflows/migrations.yml | 1 + packages/db/script-migrations/database-url.ts | 5 +- .../prepare-dev-schema.postgres.test.ts | 155 ++++++++++++++++++ packages/db/scripts/prepare-dev-schema.ts | 60 +++++++ scripts/migrations-workflow.test.ts | 39 ++--- 5 files changed, 240 insertions(+), 20 deletions(-) create mode 100644 packages/db/scripts/prepare-dev-schema.postgres.test.ts create mode 100644 packages/db/scripts/prepare-dev-schema.ts diff --git a/.github/workflows/migrations.yml b/.github/workflows/migrations.yml index d1d196e91b7..cb28340db5c 100644 --- a/.github/workflows/migrations.yml +++ b/.github/workflows/migrations.yml @@ -72,6 +72,7 @@ jobs: fi if [ "${ENVIRONMENT}" = "dev" ]; then + SIM_DEV_DB_PUSH=1 bun run ./scripts/prepare-dev-schema.ts 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. 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/prepare-dev-schema.postgres.test.ts b/packages/db/scripts/prepare-dev-schema.postgres.test.ts new file mode 100644 index 00000000000..edc75f7e11a --- /dev/null +++ b/packages/db/scripts/prepare-dev-schema.postgres.test.ts @@ -0,0 +1,155 @@ +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 { fileURLToPath } from 'node:url' +import { prepareDevSchema } from '@sim/db/scripts/prepare-dev-schema' +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.DEV_SCHEMA_TEST_DATABASE_URL + +describe.skipIf(!databaseUrl)('dev schema preparation', () => { + const databaseName = `dev_schema_${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(), 'dev-schema-')) + await writeFile( + join(directory, 'schema.ts'), + `import { pgTable, text, boolean } from ${JSON.stringify(import.meta.resolve('drizzle-orm/pg-core'))} +export const organization = pgTable('organization', { + id: text('id').primaryKey(), + requireSso: boolean('require_sso').notNull().default(false), +})` + ) + await writeFile( + join(directory, 'drizzle.config.ts'), + `export default { + dialect: 'postgresql', + schema: ${JSON.stringify(join(directory, 'schema.ts'))}, + dbCredentials: { url: process.env.DATABASE_URL }, +}` + ) + }) + + beforeEach(async () => { + await sql`DROP TABLE IF EXISTS organization` + }) + + 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 createLegacyOrganization() { + await sql`CREATE TABLE organization (id text PRIMARY KEY, departed_member_usage numeric NOT NULL DEFAULT 0)` + await sql`INSERT INTO organization (id, departed_member_usage) VALUES ('existing-org', 12.5)` + } + + /** Run the actual CLI without a TTY, matching the deployment failure. */ + function push() { + return spawnSync( + 'bunx', + [ + '--no-install', + 'drizzle-kit', + 'push', + '--config', + join(directory, 'drizzle.config.ts'), + '--force', + ], + { + env: { ...process.env, DATABASE_URL: fixtureUrl }, + encoding: 'utf8', + timeout: 30_000, + } + ) + } + + it('resolves the real noninteractive rename failure without renaming existing data', async () => { + await createLegacyOrganization() + const before = push() + expect(before.error).toBeUndefined() + expect(before.stdout + before.stderr).toContain('Interactive prompts require a TTY terminal') + + expect(await prepareDevSchema(sql)).toBe(true) + expect(await sql`SELECT * FROM organization`).toEqual([ + { id: 'existing-org', departed_member_usage: '12.5', require_sso: false }, + ]) + + const after = push() + expect(after.error).toBeUndefined() + expect(after.status).toBe(0) + expect(after.stdout + after.stderr).not.toContain('Interactive prompts require a TTY terminal') + expect(await sql`SELECT * FROM organization`).toEqual([ + { id: 'existing-org', require_sso: false }, + ]) + }, 60_000) + + it('preserves an enabled SSO policy when rerun', async () => { + await createLegacyOrganization() + await prepareDevSchema(sql) + await sql`UPDATE organization SET require_sso = true` + expect(await prepareDevSchema(sql)).toBe(false) + expect(await sql`SELECT require_sso, departed_member_usage FROM organization`).toEqual([ + { require_sso: true, departed_member_usage: '12.5' }, + ]) + }) + + it('runs the CI entry point with an empty optional direct URL', async () => { + await createLegacyOrganization() + const result = spawnSync( + 'bun', + ['run', fileURLToPath(new URL('./prepare-dev-schema.ts', import.meta.url))], + { + env: { + ...process.env, + SIM_DEV_DB_PUSH: '1', + DATABASE_URL: fixtureUrl, + MIGRATION_DATABASE_URL: '', + }, + encoding: 'utf8', + timeout: 30_000, + } + ) + expect(result.error).toBeUndefined() + expect(result.status).toBe(0) + expect(await sql`SELECT require_sso FROM organization`).toEqual([{ require_sso: false }]) + }, 30_000) + + it('leaves fresh databases for push to initialize', async () => { + expect(await prepareDevSchema(sql)).toBe(false) + const result = push() + expect(result.error).toBeUndefined() + expect(result.status).toBe(0) + await sql`INSERT INTO organization (id) VALUES ('new-org')` + expect(await sql`SELECT require_sso FROM organization`).toEqual([{ require_sso: false }]) + }, 30_000) + + it('serializes overlapping preparation attempts', async () => { + await createLegacyOrganization() + const other = postgres(fixtureUrl, { max: 1, onnotice: () => {} }) + try { + const results = await Promise.all([prepareDevSchema(sql), prepareDevSchema(other)]) + expect(results.sort()).toEqual([false, true]) + expect(await sql`SELECT require_sso FROM organization`).toEqual([{ require_sso: false }]) + } finally { + await other.end() + } + }) +}) diff --git a/packages/db/scripts/prepare-dev-schema.ts b/packages/db/scripts/prepare-dev-schema.ts new file mode 100644 index 00000000000..c3167aa7d7b --- /dev/null +++ b/packages/db/scripts/prepare-dev-schema.ts @@ -0,0 +1,60 @@ +import { readFile } from 'node:fs/promises' +import { resolveMigrationDatabaseUrl } from '@sim/db/script-migrations/database-url' +import { createLogger } from '@sim/logger' +import { getPostgresErrorCode } from '@sim/utils/errors' +import postgres, { type Sql } from 'postgres' + +const logger = createLogger('DevSchemaPreparation') +const REQUIRE_SSO_MIGRATION = new URL( + '../migrations/0350_organization_require_sso.sql', + import.meta.url +) + +/** + * Apply the existing additive migration before push compares require_sso with + * departed_member_usage. Otherwise Drizzle asks whether the latter was renamed. + * Fresh databases still get the entire organization table from push. + */ +export async function prepareDevSchema(sql: Sql): Promise { + const migration = await readFile(REQUIRE_SSO_MIGRATION, 'utf8') + return sql.begin(async (tx) => { + await tx`SET LOCAL lock_timeout = '5s'` + await tx`SET LOCAL statement_timeout = '30s'` + await tx`SET LOCAL search_path = public` + const [table] = await tx<{ exists: boolean }[]>` + SELECT to_regclass('public.organization') IS NOT NULL AS exists + ` + if (!table.exists) return false + + await tx`LOCK TABLE public.organization IN ACCESS EXCLUSIVE MODE` + const [column] = await tx<{ exists: boolean }[]>` + SELECT EXISTS ( + SELECT 1 FROM pg_attribute + WHERE attrelid = 'public.organization'::regclass + AND attname = 'require_sso' AND NOT attisdropped + ) AS exists + ` + if (column.exists) return false + + await tx.unsafe(migration) + return true + }) +} + +if (import.meta.main) { + if (process.env.SIM_DEV_DB_PUSH !== '1') { + throw new Error('Dev schema preparation requires SIM_DEV_DB_PUSH=1') + } + const url = resolveMigrationDatabaseUrl() + if (!url) throw new Error('Missing database URL for dev schema preparation') + + const sql = postgres(url, { max: 1, connect_timeout: 10 }) + try { + logger.info('Dev schema preparation completed', { applied: await prepareDevSchema(sql) }) + } catch (error) { + logger.error('Dev schema preparation failed', { code: getPostgresErrorCode(error) }) + process.exitCode = 1 + } finally { + await sql.end() + } +} diff --git a/scripts/migrations-workflow.test.ts b/scripts/migrations-workflow.test.ts index f28ca356d3a..d226838c726 100644 --- a/scripts/migrations-workflow.test.ts +++ b/scripts/migrations-workflow.test.ts @@ -25,7 +25,7 @@ function runMigration(env: Record = {}) { 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/prepare-dev-schema.ts) return "$PREPARE_EXIT" ;; ./scripts/migrate.ts) return "$MIGRATE_EXIT" ;; *) return 99 ;; esac @@ -42,7 +42,7 @@ function runMigration(env: Record = {}) { MIGRATION_TEST_LOG: join(directory, 'push.log'), PUSH_EXIT: '0', PUSH_OUTPUT: 'Changes applied', - CUTOVER_EXIT: '0', + PREPARE_EXIT: '0', MIGRATE_EXIT: '0', ...env, }, @@ -61,39 +61,40 @@ describe('migration workflow exit propagation', () => { }) 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('prepares the dev schema before pushing it', () => { 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.indexOf('COMMAND: run ./scripts/prepare-dev-schema.ts')).toBeLessThan( + result.stdout.indexOf('COMMAND: run db:push --force') ) + expect(result.stdout).toContain('COMMAND: run ./scripts/prepare-dev-schema.ts') }) it('still rejects drizzle interactive failures that exit zero', () => { const result = runMigration({ 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') + it('stops before push when preparation fails', () => { + const result = runMigration({ PREPARE_EXIT: '43' }) + expect(result.status).toBe(43) 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') + expect(result.stdout).not.toContain('COMMAND: run ./scripts/prepare-dev-schema.ts') + } + ) + it('fails before invoking commands when no database URL is configured', () => { const result = runMigration({ DATABASE_URL: '' }) expect(result.status).toBe(1) From e28acbe2006e74178c1b0c0030673fe7a7b5fe32 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Wed, 16 Sep 2026 18:38:34 -0700 Subject: [PATCH 2/3] refactor(db): generalize push rename handling --- .github/CONTRIBUTING.md | 2 + .github/workflows/migrations.yml | 12 +- bun.lock | 12 +- package.json | 1 + packages/db/package.json | 3 +- .../prepare-dev-schema.postgres.test.ts | 155 --------------- packages/db/scripts/prepare-dev-schema.ts | 60 ------ packages/db/scripts/push.postgres.test.ts | 187 ++++++++++++++++++ packages/db/scripts/push.test.ts | 91 +++++++++ packages/db/scripts/push.ts | 55 +++++- patches/README.md | 25 +++ patches/drizzle-kit@0.31.10.patch | 75 +++++++ scripts/migrations-workflow.test.ts | 81 +++----- 13 files changed, 476 insertions(+), 283 deletions(-) delete mode 100644 packages/db/scripts/prepare-dev-schema.postgres.test.ts delete mode 100644 packages/db/scripts/prepare-dev-schema.ts create mode 100644 packages/db/scripts/push.postgres.test.ts create mode 100644 packages/db/scripts/push.test.ts create mode 100644 patches/drizzle-kit@0.31.10.patch diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 71708399892..16dcdb8de77 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. 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 cb28340db5c..0afdddeff97 100644 --- a/.github/workflows/migrations.yml +++ b/.github/workflows/migrations.yml @@ -72,20 +72,10 @@ jobs: fi if [ "${ENVIRONMENT}" = "dev" ]; then - SIM_DEV_DB_PUSH=1 bun run ./scripts/prepare-dev-schema.ts 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/scripts/prepare-dev-schema.postgres.test.ts b/packages/db/scripts/prepare-dev-schema.postgres.test.ts deleted file mode 100644 index edc75f7e11a..00000000000 --- a/packages/db/scripts/prepare-dev-schema.postgres.test.ts +++ /dev/null @@ -1,155 +0,0 @@ -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 { fileURLToPath } from 'node:url' -import { prepareDevSchema } from '@sim/db/scripts/prepare-dev-schema' -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.DEV_SCHEMA_TEST_DATABASE_URL - -describe.skipIf(!databaseUrl)('dev schema preparation', () => { - const databaseName = `dev_schema_${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(), 'dev-schema-')) - await writeFile( - join(directory, 'schema.ts'), - `import { pgTable, text, boolean } from ${JSON.stringify(import.meta.resolve('drizzle-orm/pg-core'))} -export const organization = pgTable('organization', { - id: text('id').primaryKey(), - requireSso: boolean('require_sso').notNull().default(false), -})` - ) - await writeFile( - join(directory, 'drizzle.config.ts'), - `export default { - dialect: 'postgresql', - schema: ${JSON.stringify(join(directory, 'schema.ts'))}, - dbCredentials: { url: process.env.DATABASE_URL }, -}` - ) - }) - - beforeEach(async () => { - await sql`DROP TABLE IF EXISTS organization` - }) - - 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 createLegacyOrganization() { - await sql`CREATE TABLE organization (id text PRIMARY KEY, departed_member_usage numeric NOT NULL DEFAULT 0)` - await sql`INSERT INTO organization (id, departed_member_usage) VALUES ('existing-org', 12.5)` - } - - /** Run the actual CLI without a TTY, matching the deployment failure. */ - function push() { - return spawnSync( - 'bunx', - [ - '--no-install', - 'drizzle-kit', - 'push', - '--config', - join(directory, 'drizzle.config.ts'), - '--force', - ], - { - env: { ...process.env, DATABASE_URL: fixtureUrl }, - encoding: 'utf8', - timeout: 30_000, - } - ) - } - - it('resolves the real noninteractive rename failure without renaming existing data', async () => { - await createLegacyOrganization() - const before = push() - expect(before.error).toBeUndefined() - expect(before.stdout + before.stderr).toContain('Interactive prompts require a TTY terminal') - - expect(await prepareDevSchema(sql)).toBe(true) - expect(await sql`SELECT * FROM organization`).toEqual([ - { id: 'existing-org', departed_member_usage: '12.5', require_sso: false }, - ]) - - const after = push() - expect(after.error).toBeUndefined() - expect(after.status).toBe(0) - expect(after.stdout + after.stderr).not.toContain('Interactive prompts require a TTY terminal') - expect(await sql`SELECT * FROM organization`).toEqual([ - { id: 'existing-org', require_sso: false }, - ]) - }, 60_000) - - it('preserves an enabled SSO policy when rerun', async () => { - await createLegacyOrganization() - await prepareDevSchema(sql) - await sql`UPDATE organization SET require_sso = true` - expect(await prepareDevSchema(sql)).toBe(false) - expect(await sql`SELECT require_sso, departed_member_usage FROM organization`).toEqual([ - { require_sso: true, departed_member_usage: '12.5' }, - ]) - }) - - it('runs the CI entry point with an empty optional direct URL', async () => { - await createLegacyOrganization() - const result = spawnSync( - 'bun', - ['run', fileURLToPath(new URL('./prepare-dev-schema.ts', import.meta.url))], - { - env: { - ...process.env, - SIM_DEV_DB_PUSH: '1', - DATABASE_URL: fixtureUrl, - MIGRATION_DATABASE_URL: '', - }, - encoding: 'utf8', - timeout: 30_000, - } - ) - expect(result.error).toBeUndefined() - expect(result.status).toBe(0) - expect(await sql`SELECT require_sso FROM organization`).toEqual([{ require_sso: false }]) - }, 30_000) - - it('leaves fresh databases for push to initialize', async () => { - expect(await prepareDevSchema(sql)).toBe(false) - const result = push() - expect(result.error).toBeUndefined() - expect(result.status).toBe(0) - await sql`INSERT INTO organization (id) VALUES ('new-org')` - expect(await sql`SELECT require_sso FROM organization`).toEqual([{ require_sso: false }]) - }, 30_000) - - it('serializes overlapping preparation attempts', async () => { - await createLegacyOrganization() - const other = postgres(fixtureUrl, { max: 1, onnotice: () => {} }) - try { - const results = await Promise.all([prepareDevSchema(sql), prepareDevSchema(other)]) - expect(results.sort()).toEqual([false, true]) - expect(await sql`SELECT require_sso FROM organization`).toEqual([{ require_sso: false }]) - } finally { - await other.end() - } - }) -}) diff --git a/packages/db/scripts/prepare-dev-schema.ts b/packages/db/scripts/prepare-dev-schema.ts deleted file mode 100644 index c3167aa7d7b..00000000000 --- a/packages/db/scripts/prepare-dev-schema.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { readFile } from 'node:fs/promises' -import { resolveMigrationDatabaseUrl } from '@sim/db/script-migrations/database-url' -import { createLogger } from '@sim/logger' -import { getPostgresErrorCode } from '@sim/utils/errors' -import postgres, { type Sql } from 'postgres' - -const logger = createLogger('DevSchemaPreparation') -const REQUIRE_SSO_MIGRATION = new URL( - '../migrations/0350_organization_require_sso.sql', - import.meta.url -) - -/** - * Apply the existing additive migration before push compares require_sso with - * departed_member_usage. Otherwise Drizzle asks whether the latter was renamed. - * Fresh databases still get the entire organization table from push. - */ -export async function prepareDevSchema(sql: Sql): Promise { - const migration = await readFile(REQUIRE_SSO_MIGRATION, 'utf8') - return sql.begin(async (tx) => { - await tx`SET LOCAL lock_timeout = '5s'` - await tx`SET LOCAL statement_timeout = '30s'` - await tx`SET LOCAL search_path = public` - const [table] = await tx<{ exists: boolean }[]>` - SELECT to_regclass('public.organization') IS NOT NULL AS exists - ` - if (!table.exists) return false - - await tx`LOCK TABLE public.organization IN ACCESS EXCLUSIVE MODE` - const [column] = await tx<{ exists: boolean }[]>` - SELECT EXISTS ( - SELECT 1 FROM pg_attribute - WHERE attrelid = 'public.organization'::regclass - AND attname = 'require_sso' AND NOT attisdropped - ) AS exists - ` - if (column.exists) return false - - await tx.unsafe(migration) - return true - }) -} - -if (import.meta.main) { - if (process.env.SIM_DEV_DB_PUSH !== '1') { - throw new Error('Dev schema preparation requires SIM_DEV_DB_PUSH=1') - } - const url = resolveMigrationDatabaseUrl() - if (!url) throw new Error('Missing database URL for dev schema preparation') - - const sql = postgres(url, { max: 1, connect_timeout: 10 }) - try { - logger.info('Dev schema preparation completed', { applied: await prepareDevSchema(sql) }) - } catch (error) { - logger.error('Dev schema preparation failed', { code: getPostgresErrorCode(error) }) - process.exitCode = 1 - } finally { - await sql.end() - } -} 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 d226838c726..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,49 +10,41 @@ 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/prepare-dev-schema.ts) return "$PREPARE_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', - PREPARE_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', @@ -63,27 +53,21 @@ describe('migration workflow exit propagation', () => { expect(result.stdout).toContain('DATABASE_URL is required') }) - it('prepares the dev schema before pushing it', () => { + 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.indexOf('COMMAND: run ./scripts/prepare-dev-schema.ts')).toBeLessThan( - result.stdout.indexOf('COMMAND: run db:push --force') - ) - expect(result.stdout).toContain('COMMAND: run ./scripts/prepare-dev-schema.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) }) - it('stops before push when preparation fails', () => { - const result = runMigration({ PREPARE_EXIT: '43' }) - expect(result.status).toBe(43) - expect(result.stdout).not.toContain('COMMAND: run db:push') - }) - it.each(['staging', 'production'])( 'keeps versioned migration failures fatal in %s', (environment) => { @@ -91,7 +75,6 @@ describe('migration workflow exit propagation', () => { expect(result.status).toBe(44) expect(result.stdout).toContain('COMMAND: run ./scripts/migrate.ts') expect(result.stdout).not.toContain('COMMAND: run db:push') - expect(result.stdout).not.toContain('COMMAND: run ./scripts/prepare-dev-schema.ts') } ) From d99e68fdb7df5df7ab811a0c551066367e77cdd1 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Wed, 16 Sep 2026 18:51:52 -0700 Subject: [PATCH 3/3] docs(db): describe schema push reconciliation steps --- .github/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 16dcdb8de77..9e2d86607e9 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -256,7 +256,7 @@ 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. Staging and production changes still use reviewed versioned migrations with expand/contract deployment steps. + `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:**