fix(db): resolve dev schema push column ambiguity

Author: icecrasher321

.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.

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
 }

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()
+    }
+  })
+})

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<boolean> {
+  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()
+  }
+}

scripts/migrations-workflow.test.ts

@@ -25,7 +25,7 @@ function runMigration(env: Record<string, string> = {}) {
           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<string, string> = {}) {
           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)