fix(db): prevent schema push from bypassing column-drop migrations

Author: icecrasher321

.github/CONTRIBUTING.md

@@ -254,7 +254,7 @@ If you prefer not to use Docker. **All commands run from the repository root unl
    cd packages/db && bun run db:migrate && cd ../..
    ```
 
-   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 both local and CI/CD setups.
+   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 both local and CI/CD setups. `db:push` rejects column removals so it cannot bypass a versioned migration's prerequisites or backfills; use `db:migrate` for those changes.
 
 4. **Run the Development Servers:**
 

.github/workflows/migrations.yml

@@ -71,6 +71,8 @@ jobs:
 
           if [ "${ENVIRONMENT}" = "dev" ]; then
             echo "Dev environment — pushing schema directly (db:push)"
+            # db:push validates column removals before invoking drizzle-kit; --force
+            # cannot bypass the versioned migration prerequisites for a column drop.
             # 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

packages/db/package.json

@@ -28,7 +28,7 @@
   },
   "scripts": {
     "db:generate": "bunx drizzle-kit generate --config=./drizzle.config.ts",
-    "db:push": "bunx drizzle-kit push --config=./drizzle.config.ts && 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",
+    "db:push": "bun --env-file=.env run ./scripts/check-schema-push-safety.ts && bunx drizzle-kit push --config=./drizzle.config.ts && 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",
     "db:migrate": "bun --env-file=.env run ./scripts/migrate.ts",
     "db:reconcile-fork-kb-file-ownership": "bun --env-file=.env run ./scripts/reconcile-fork-kb-file-ownership.ts",
     "db:reconcile-workspace-storage": "bun --env-file=.env run ./scripts/reconcile-workspace-storage.ts",

packages/db/schema-push-safety.ts

@@ -0,0 +1,53 @@
+import * as schema from '@sim/db/schema'
+import { is } from 'drizzle-orm'
+import { getTableConfig, PgTable } from 'drizzle-orm/pg-core'
+import type { Sql } from 'postgres'
+
+/**
+ * Direct schema pushes bypass versioned migration guards and backfills. Require
+ * db:migrate for column removals, including empty tables that can receive writes
+ * between this check and the push. Fresh installs and additive pushes remain valid.
+ */
+export async function assertSafeSchemaPush(
+  sql: Sql,
+  tables: readonly PgTable[] = Object.values<unknown>(schema).filter((value): value is PgTable =>
+    is(value, PgTable)
+  )
+): Promise<void> {
+  const definitions = tables.map((table) => getTableConfig(table))
+  const expectedColumns = new Map(
+    definitions.map((table) => [
+      `${table.schema ?? 'public'}\0${table.name}`,
+      new Set(table.columns.map((column) => column.name)),
+    ])
+  )
+  const schemaNames = [...new Set(definitions.map((table) => table.schema ?? 'public'))]
+  const tableNames = [...new Set(definitions.map((table) => table.name))]
+  const existing = await sql<
+    Array<{ schema_name: string; table_name: string; column_name: string }>
+  >`
+    SELECT n.nspname AS schema_name, c.relname AS table_name, a.attname AS column_name
+    FROM pg_attribute a
+    JOIN pg_class c ON c.oid = a.attrelid
+    JOIN pg_namespace n ON n.oid = c.relnamespace
+    WHERE n.nspname = ANY(${schemaNames}::text[])
+      AND c.relname = ANY(${tableNames}::text[])
+      AND c.relkind IN ('r', 'p')
+      AND a.attnum > 0
+      AND NOT a.attisdropped
+    ORDER BY n.nspname, c.relname, a.attnum
+  `
+  const removed = existing.filter((column) => {
+    const expected = expectedColumns.get(`${column.schema_name}\0${column.table_name}`)
+    return expected && !expected.has(column.column_name)
+  })
+  if (removed.length === 0) return
+
+  throw new Error(
+    'db:push cannot remove existing columns: ' +
+      removed
+        .map((column) => `${column.schema_name}.${column.table_name}.${column.column_name}`)
+        .join(', ') +
+      '. Run db:migrate so versioned migration prerequisites and backfills are enforced.'
+  )
+}

packages/db/scripts/check-schema-push-safety.ts

@@ -0,0 +1,20 @@
+import { assertSafeSchemaPush } from '@sim/db/schema-push-safety'
+import { createLogger } from '@sim/logger'
+import postgres from 'postgres'
+
+const logger = createLogger('SchemaPushSafety')
+const url = process.env.DATABASE_URL
+if (!url) throw new Error('Missing DATABASE_URL')
+
+const sql = postgres(url, {
+  max: 1,
+  connect_timeout: 10,
+  connection: { application_name: 'sim-schema-push-safety', statement_timeout: 10_000 },
+})
+
+try {
+  await assertSafeSchemaPush(sql)
+  logger.info('Schema push does not remove existing columns')
+} finally {
+  await sql.end()
+}

packages/db/scripts/retired-columns.postgres.test.ts

@@ -1,5 +1,7 @@
 import { readFileSync } from 'node:fs'
+import { assertSafeSchemaPush } from '@sim/db/schema-push-safety'
 import { generateId } from '@sim/utils/id'
+import { bigint, numeric, pgSchema, text } from 'drizzle-orm/pg-core'
 import postgres from 'postgres'
 import { describe, expect, it } from 'vitest'
 
@@ -38,7 +40,9 @@ const receipts = [
   '0009_backfill_wel_residual_cost_total',
 ]
 
-async function fixture(run: (sql: postgres.Sql) => Promise<void>): Promise<void> {
+async function fixture(
+  run: (sql: postgres.Sql, tables: Parameters<typeof assertSafeSchemaPush>[1]) => Promise<void>
+): Promise<void> {
   const sql = postgres(databaseUrl!, { max: 1, onnotice: () => {} })
   const schemaName = `retired_columns_${generateId().replaceAll('-', '')}`
   try {
@@ -53,7 +57,20 @@ async function fixture(run: (sql: postgres.Sql) => Promise<void>): Promise<void>
     await sql`CREATE TABLE workflow_execution_logs (id text, cost jsonb, cost_total numeric)`
     await sql`CREATE TABLE workspace_files (id text, size integer NOT NULL, size_bytes bigint)`
     await sql.unsafe(bridge)
-    await run(sql)
+    const namespace = pgSchema(schemaName)
+    const tables = [
+      namespace.table('organization', { id: text('id'), creditBalance: numeric('credit_balance') }),
+      namespace.table('user_stats', { id: text('id'), creditBalance: numeric('credit_balance') }),
+      namespace.table('workflow_execution_logs', {
+        id: text('id'),
+        costTotal: numeric('cost_total'),
+      }),
+      namespace.table('workspace_files', {
+        id: text('id'),
+        sizeBytes: bigint('size_bytes', { mode: 'number' }),
+      }),
+    ]
+    await run(sql, tables)
   } finally {
     try {
       await sql`DROP SCHEMA IF EXISTS ${sql(schemaName)} CASCADE`
@@ -70,6 +87,30 @@ async function apply(sql: postgres.Sql): Promise<void> {
 }
 
 describe.skipIf(!databaseUrl)('retired-column contract migration', () => {
+  it('blocks schema push before it can discard legacy values', async () => {
+    await fixture(async (sql, tables) => {
+      await sql`INSERT INTO workspace_files (id, size) VALUES ('file', 123)`
+      await sql`INSERT INTO workflow_execution_logs (id, cost) VALUES ('log', '{"total": 1.25}')`
+      await expect(assertSafeSchemaPush(sql, tables)).rejects.toThrow(
+        'db:push cannot remove existing columns'
+      )
+      expect(await sql`SELECT size FROM workspace_files`).toEqual([{ size: 123 }])
+      expect(await sql`SELECT cost, cost_total FROM workflow_execution_logs`).toEqual([
+        { cost: { total: 1.25 }, cost_total: null },
+      ])
+    })
+  })
+
+  it('also blocks empty legacy tables and allows pushes after the guarded contract', async () => {
+    await fixture(async (sql, tables) => {
+      await expect(assertSafeSchemaPush(sql, tables)).rejects.toThrow('Run db:migrate')
+      await apply(sql)
+      await expect(assertSafeSchemaPush(sql, tables)).resolves.toBeUndefined()
+      await sql`DROP TABLE workspace_files`
+      await expect(assertSafeSchemaPush(sql, tables)).resolves.toBeUndefined()
+    })
+  })
+
   it('allows a fresh database and replays after the columns are gone', async () => {
     await fixture(async (sql) => {
       await apply(sql)