fix(migrations): make deploy-time DDL lose the lock race, not user traffic

.github/workflows/deploy-production.yml

@@ -75,7 +75,7 @@ jobs:
         run: pnpm install --frozen-lockfile
 
       - name: Run Drizzle migrations
-        run: NODE_ENV=production pnpm run drizzle migrate
+        run: NODE_ENV=production pnpm run drizzle:migrate-safely
 
   stage-app:
     uses: ./.github/workflows/stage-vercel-deployment.yml

.github/workflows/deploy-staging.yml

@@ -71,7 +71,7 @@ jobs:
         run: pnpm install --frozen-lockfile
 
       - name: Run Drizzle migrations
-        run: NODE_ENV=production pnpm run drizzle migrate
+        run: NODE_ENV=production pnpm run drizzle:migrate-safely
 
   deploy-app:
     needs: [check-staging-db-startup, run-migrations]

.kilo/skills/database-migrations/SKILL.md

@@ -18,11 +18,44 @@ This skill covers shared PostgreSQL migrations only. It does not govern Durable
    broad, correct the schema and regenerate. Do not hand-edit generated DDL,
    snapshots, or journal entries.
 6. Append only intentional `UPDATE` or `INSERT` data backfills after generated DDL, separated with `-->  statement-breakpoint`.
-7. Apply migrations with `pnpm drizzle migrate` or run `pnpm drizzle:verify-bootstrap` when relevant.
-8. Run `pnpm format` and targeted schema or migration checks.
-9. Prefer one generated migration per unshipped feature branch. To squash
-   migrations before shipping, remove the branch-local migration SQL, snapshots,
-   and journal entries, then regenerate once from the current schema. Re-append
-   intentional backfills afterward.
+7. Check lock safety against live traffic before shipping DDL; see the section below.
+8. Apply migrations with `pnpm drizzle migrate` locally, or `pnpm drizzle:migrate-safely` to rehearse the deploy path. Run `pnpm drizzle:verify-bootstrap` when relevant.
+9. Run `pnpm format` and targeted schema or migration checks.
+10. Prefer one generated migration per unshipped feature branch. To squash
+    migrations before shipping, remove the branch-local migration SQL, snapshots,
+    and journal entries, then regenerate once from the current schema. Re-append
+    intentional backfills afterward.
 
 Keep generated artifacts generated. `packages/db/AGENTS.md` also covers shared-schema PII requirements and DB-backed timestamp serialization.
+
+## Lock safety review
+
+`packages/db/AGENTS.md` is canonical; this is the checklist.
+
+- Identify every table the DDL will lock with `ACCESS EXCLUSIVE`. That includes
+  the target table **and every table it references by foreign key**, since
+  dropping a constraint removes the referential-integrity trigger on the parent.
+  A `DROP TABLE ... CASCADE` also locks the children whose constraints it drops.
+- Treat `kilocode_users`, `organizations`, and `microdollar_usage` as always
+  busy. A statement that needs `ACCESS EXCLUSIVE` on two of them at once will
+  deadlock with ordinary traffic that touches both in the other order.
+- Split such work so each statement locks at most one busy table: drop the
+  foreign keys one statement at a time, then drop the table.
+- Confirm the new journal `when` is the newest in `meta/_journal.json`, or the
+  migrator will skip the migration silently.
+- Verify with `SHOW deadlock_timeout` before changing `MIGRATION_LOCK_TIMEOUT`;
+  the timeout must stay below it.
+
+Query the locks a statement will take before shipping anything non-trivial:
+
+```sql
+select
+  c.conname,
+  src.relname as from_table,
+  tgt.relname as to_table
+from pg_constraint c
+join pg_class src on src.oid = c.conrelid
+join pg_class tgt on tgt.oid = c.confrelid
+where c.contype = 'f'
+  and 'YOUR_TABLE' in (src.relname, tgt.relname);
+```

DEVELOPMENT.md

@@ -251,6 +251,7 @@ should pass against the local PostgreSQL database.
 | `pnpm format:changed` | Format only files changed since `main` |
 | `pnpm validate` | Run the root typecheck, lint, and test scripts |
 | `pnpm drizzle migrate` | Apply pending database migrations |
+| `pnpm drizzle:migrate-safely` | Apply pending migrations the way deploys do: `lock_timeout` below `deadlock_timeout`, retries on lock failures, and full PostgreSQL error detail on failure |
 | `pnpm drizzle generate` | Generate a new migration after schema changes |
 | `pnpm drizzle:verify-bootstrap` | Create a temporary empty database and verify `pnpm drizzle migrate` bootstraps it cleanly |
 | `pnpm dev:db:reset` | Drop all app-owned schemas in the local dev database, recreate `public`, and leave the DB truly empty before re-migrating |

REVIEW.md

@@ -25,6 +25,8 @@
 - Dropping columns/tables that may still be read by running application code
 - Large backfills or data transforms without batching
 - Missing partial index opportunities (e.g. `WHERE col IS NOT NULL`)
+- A single statement needing `ACCESS EXCLUSIVE` on more than one busy table - `DROP TABLE`/`ALTER TABLE` also locks every table the target references by foreign key, so dropping a table with FKs to both `kilocode_users` and `organizations` deadlocks against normal traffic. Flag it and ask for the foreign keys to be dropped one statement at a time before the table
+- A new migration whose journal `when` is not the newest in `meta/_journal.json` - the migrator compares against `max(created_at)` and skips older entries silently
 
 # COMMENT FORMAT
 

package.json

@@ -19,6 +19,7 @@
     "format:changed": "git diff --name-only $(git merge-base origin/main HEAD) --diff-filter=ACMR -- '**/*.js' '**/*.jsx' '**/*.ts' '**/*.tsx' '**/*.json' '**/*.css' '**/*.md' | xargs -r oxfmt --no-error-on-unmatched-pattern",
     "validate": "pnpm run typecheck && pnpm run lint && pnpm run test",
     "drizzle": "pnpm --filter @kilocode/db exec drizzle-kit",
+    "drizzle:migrate-safely": "tsx packages/db/src/migrate-cli.ts",
     "drizzle:verify-bootstrap": "bash scripts/verify-drizzle-bootstrap.sh",
     "test:e2e": "pnpm --filter web run test:e2e",
     "dependency-cycle-check": "pnpm --filter web run dependency-cycle-check",

packages/db/AGENTS.md

@@ -30,6 +30,44 @@ Prefer one generated migration per unshipped feature branch.
 Load `database-migrations` for shared PostgreSQL migration work. Load
 `git-rebase` for migration conflicts during a rebase.
 
+## Lock safety
+
+Deploys apply migrations against a database that is serving traffic, so DDL
+competes with live queries for relation-level locks.
+
+`DROP TABLE` and `ALTER TABLE` take `ACCESS EXCLUSIVE` on the target table **and
+on every table the target references by foreign key**, because dropping the
+constraint removes the referential-integrity trigger that lives on the parent.
+A migration touching a table with foreign keys to `kilocode_users` and
+`organizations` therefore locks both of those, and any concurrent query that
+holds one while waiting for the other deadlocks. PostgreSQL then aborts whichever
+party detects the cycle, which in practice is user requests rather than the
+migration.
+
+Two consequences when writing migrations against hot parent tables:
+
+- Prefer dropping each foreign key in its own statement over one `DROP TABLE
+  ... CASCADE`, so a single statement never needs `ACCESS EXCLUSIVE` on more
+  than one busy table at a time. Once the constraints are gone, the table drop
+  needs no lock on any parent.
+- Migrations are applied by `pnpm drizzle:migrate-safely`, which sets
+  `lock_timeout` below the server's `deadlock_timeout` so the migration loses
+  the lock race instead of user queries, and retries lock failures. A migration
+  that breaks out of the migrator's transaction with a bare `COMMIT;` (the
+  `CREATE INDEX CONCURRENTLY` workaround) cannot be replayed, so it runs with
+  retries disabled — keep those migrations small and expect no retry safety net.
+
+The migrator applies every pending migration in one transaction, so locks taken
+by the first statement are held until the last migration commits. Prefer one
+migration per deploy over letting a batch accumulate.
+
+The migrator decides what is pending by comparing each journal entry's `when`
+against `max(created_at)` in `drizzle.__drizzle_migrations`, not by checking each
+entry individually. A migration whose journal timestamp predates an
+already-applied migration is skipped permanently and silently, which is a real
+risk after a rebase or a long-lived branch. Confirm a new migration's `when` is
+the newest in `meta/_journal.json` before merging.
+
 When adding user or account PII to shared PostgreSQL, update `softDeleteUser` in
 `apps/web/src/lib/user/index.ts` to delete or anonymize it and add corresponding
 coverage in `apps/web/src/lib/user/index.test.ts`.

packages/db/src/migrate-cli.ts

@@ -0,0 +1,15 @@
+#!/usr/bin/env tsx
+/**
+ * Entry point for `pnpm drizzle:migrate-safely`.
+ *
+ * Kept separate from `migrate.ts` so importing the runner (in tests) never
+ * connects to a database or applies migrations as an import side effect.
+ */
+import process from 'node:process';
+
+import { reportFailure, runMigrations } from './migrate';
+
+runMigrations().catch((error: unknown) => {
+  reportFailure(error);
+  process.exitCode = 1;
+});

packages/db/src/migrate.test.ts

@@ -0,0 +1,207 @@
+import {
+  applyWithRetries,
+  findFailingQuery,
+  findPostgresError,
+  findTransactionBreakingMigrations,
+  MigrationSafetyError,
+  reportFailure,
+} from './migrate';
+
+/** Shaped like a `pg` error: the code lives on the error object itself. */
+function postgresError(code: string, message = 'boom'): Error {
+  return Object.assign(new Error(message), { code });
+}
+
+/** Shaped like DrizzleQueryError: the pg error is the `cause`. */
+function wrappedPostgresError(code: string, query: string): Error {
+  return Object.assign(new Error(`Failed query: ${query}`), {
+    query,
+    cause: postgresError(code, 'deadlock detected'),
+  });
+}
+
+const noSleep = async () => {};
+
+describe('findPostgresError', () => {
+  it('reads the code off a bare pg error', () => {
+    expect(findPostgresError(postgresError('40P01'))?.code).toBe('40P01');
+  });
+
+  it('unwraps the cause chain that drizzle wraps the pg error in', () => {
+    const error = wrappedPostgresError('40P01', 'drop table "x"');
+
+    expect(findPostgresError(error)?.code).toBe('40P01');
+    expect(findPostgresError(error)?.message).toBe('deadlock detected');
+  });
+
+  it('returns undefined when nothing in the chain carries a code', () => {
+    expect(findPostgresError(new Error('plain'))).toBeUndefined();
+    expect(findPostgresError('not an error')).toBeUndefined();
+  });
+
+  it('surfaces the fields drizzle-kit discards', () => {
+    const error = Object.assign(new Error('deadlock detected'), {
+      code: '40P01',
+      detail: 'Process 123 waits for AccessExclusiveLock on relation 456',
+      hint: 'See server log for query details.',
+      table: 'organizations',
+    });
+
+    expect(findPostgresError(error)).toMatchObject({
+      code: '40P01',
+      detail: 'Process 123 waits for AccessExclusiveLock on relation 456',
+      hint: 'See server log for query details.',
+      table: 'organizations',
+    });
+  });
+});
+
+describe('findFailingQuery', () => {
+  it('reads the query drizzle attaches to the wrapper', () => {
+    expect(findFailingQuery(wrappedPostgresError('55P03', 'drop table "y"'))).toBe(
+      'drop table "y"'
+    );
+  });
+
+  it('returns undefined when no wrapper carries a query', () => {
+    expect(findFailingQuery(postgresError('55P03'))).toBeUndefined();
+  });
+});
+
+describe('findTransactionBreakingMigrations', () => {
+  it('flags a migration that commits mid-file for CONCURRENTLY', () => {
+    const pending = [
+      {
+        tag: '0193_concurrent_index',
+        sql: [
+          'ALTER TABLE "a" DROP CONSTRAINT "b";',
+          'COMMIT;',
+          'CREATE INDEX CONCURRENTLY "idx" ON "a" ("c");',
+          'BEGIN;',
+        ].join('--> statement-breakpoint'),
+      },
+    ];
+
+    expect(findTransactionBreakingMigrations(pending)).toEqual(['0193_concurrent_index']);
+  });
+
+  it('ignores ordinary DDL', () => {
+    const pending = [
+      { tag: '0204_drop_tables', sql: 'DROP TABLE IF EXISTS "x" CASCADE;' },
+      { tag: '0205_add_column', sql: 'ALTER TABLE "y" ADD COLUMN "z" text;' },
+    ];
+
+    expect(findTransactionBreakingMigrations(pending)).toEqual([]);
+  });
+
+  it('does not mistake COMMIT inside an identifier or comment for a statement', () => {
+    const pending = [
+      { tag: '0206_commit_column', sql: 'ALTER TABLE "kiloclaw" ADD COLUMN "commit_sha" text;' },
+    ];
+
+    expect(findTransactionBreakingMigrations(pending)).toEqual([]);
+  });
+});
+
+describe('reportFailure', () => {
+  let errors: string[];
+  let spy: jest.SpyInstance;
+
+  beforeEach(() => {
+    errors = [];
+    spy = jest.spyOn(console, 'error').mockImplementation((...args: unknown[]) => {
+      errors.push(args.map(String).join(' '));
+    });
+  });
+
+  afterEach(() => spy.mockRestore());
+
+  it('reports a safety refusal without a stack trace', () => {
+    reportFailure(new MigrationSafetyError('lock_timeout is disabled'));
+
+    expect(errors).toEqual(['[migrate] refusing to run: lock_timeout is disabled']);
+  });
+
+  it('reports the pg fields drizzle-kit discards', () => {
+    reportFailure(
+      Object.assign(new Error('Failed query: drop table "x"'), {
+        query: 'drop table "x"',
+        cause: Object.assign(new Error('deadlock detected'), {
+          code: '40P01',
+          detail: 'Process 1 waits for AccessExclusiveLock',
+        }),
+      })
+    );
+
+    expect(errors.join('\n')).toContain('[migrate] migration failed: 40P01');
+    expect(errors.join('\n')).toContain('detail: Process 1 waits for AccessExclusiveLock');
+    expect(errors.join('\n')).toContain('failing statement: drop table "x"');
+  });
+});
+
+describe('applyWithRetries', () => {
+  it('returns the attempt count on first success', async () => {
+    const apply = jest.fn(async () => {});
+
+    await expect(applyWithRetries(apply, 5, noSleep)).resolves.toBe(1);
+    expect(apply).toHaveBeenCalledTimes(1);
+  });
+
+  it.each(['40P01', '55P03', '40001'])('retries %s until it succeeds', async code => {
+    let calls = 0;
+    const apply = jest.fn(async () => {
+      calls += 1;
+      if (calls < 3) throw postgresError(code);
+    });
+
+    await expect(applyWithRetries(apply, 5, noSleep)).resolves.toBe(3);
+    expect(apply).toHaveBeenCalledTimes(3);
+  });
+
+  it('retries a lock failure that drizzle wrapped', async () => {
+    let calls = 0;
+    const apply = jest.fn(async () => {
+      calls += 1;
+      if (calls < 2) throw wrappedPostgresError('55P03', 'drop table "x"');
+    });
+
+    await expect(applyWithRetries(apply, 3, noSleep)).resolves.toBe(2);
+  });
+
+  it('gives up after maxAttempts and rethrows the lock failure', async () => {
+    const apply = jest.fn(async () => {
+      throw postgresError('55P03');
+    });
+
+    await expect(applyWithRetries(apply, 3, noSleep)).rejects.toMatchObject({ code: '55P03' });
+    expect(apply).toHaveBeenCalledTimes(3);
+  });
+
+  it('does not retry a real migration bug', async () => {
+    // 42P01 undefined_table: replaying this would never help.
+    const apply = jest.fn(async () => {
+      throw postgresError('42P01', 'relation "gone" does not exist');
+    });
+
+    await expect(applyWithRetries(apply, 5, noSleep)).rejects.toMatchObject({ code: '42P01' });
+    expect(apply).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not retry an error with no pg code', async () => {
+    const apply = jest.fn(async () => {
+      throw new Error('connection terminated');
+    });
+
+    await expect(applyWithRetries(apply, 5, noSleep)).rejects.toThrow('connection terminated');
+    expect(apply).toHaveBeenCalledTimes(1);
+  });
+
+  it('honours maxAttempts of 1 for migrations that cannot be replayed', async () => {
+    const apply = jest.fn(async () => {
+      throw postgresError('40P01');
+    });
+
+    await expect(applyWithRetries(apply, 1, noSleep)).rejects.toMatchObject({ code: '40P01' });
+    expect(apply).toHaveBeenCalledTimes(1);
+  });
+});