fix(db): make development schema pushes noninteractive (#7906)

* fix(db): resolve dev schema push column ambiguity

* refactor(db): generalize push rename handling

* docs(db): describe schema push reconciliation steps

Author: icecrasher321

.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

.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

bun.lock

@@ -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"
   }
 }

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"
   }

packages/db/package.json

@@ -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
 }

packages/db/script-migrations/database-url.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)
+})

packages/db/scripts/push.postgres.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<number> }>()
+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)
+  })
+})