Skip to content

feat(db): adopt Alembic for PostgreSQL migrations — Phase 1 (#1183) - #1186

Merged
vybe merged 3 commits into
devfrom
feature/1183-alembic-postgres
Jun 15, 2026
Merged

feat(db): adopt Alembic for PostgreSQL migrations — Phase 1 (#1183)#1186
vybe merged 3 commits into
devfrom
feature/1183-alembic-postgres

Conversation

@dolho

@dolho dolho commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Phase 1 of adopting Alembic for the PostgreSQL backend (#1183). The PG schema is now owned by Alembic instead of the fresh-build-from-schema.py path; SQLite keeps its bespoke db/migrations.py runner — the two coexist during the Postgres transition (SQLite is being retired).

Related to #1183 (and #746 single-source-of-truth, #1160 runner serialization, #300 the Core foundation).

What's here

  • src/backend/migrations/env.py (targets the db/tables.py MetaData, URL from DATABASE_URL), script.py.mako, and versions/0001_baseline.py. The baseline reuses the exact init_schema_postgres head DDL (tables + indexes + triggers), so a fresh PG DB built by alembic upgrade head is identical to the old fresh build.
  • src/backend/alembic.ini — for CLI/autogenerate use (runner builds its own Config programmatically).
  • db/alembic_runner.upgrade_to_head() — fresh DB → upgrade head; pre-Alembic PG DB (no alembic_version) → stamp 0001_baseline then upgrade (adopted, not rebuilt); managed DB → apply pending revisions.
  • init_database() non-SQLite branch now calls the runner (was init_schema_postgres). SQLite branch untouched.
  • alembic==1.18.4 in the backend image (the scheduler doesn't run migrations).

Verification

Ran against a throwaway postgres:16tests/integration/test_alembic_postgres.py (Postgres-gated on TEST_POSTGRES_URL, skips otherwise so SQLite CI stays green):

  • fresh upgrade head builds the full 60-table schema (incl. migration-era columns like operator_queue.cleared_at)
  • idempotent re-run
  • downgrade base tears down cleanly (and preserves Alembic's bookkeeping)
  • pre-Alembic DB is stamped at baseline, not rebuilt
  • schema parity: Alembic-built table set == legacy init_schema_postgres build

All 5 pass. Confirmed the SQLite init_database path is unchanged (still runs the bespoke migrations).

Interim workaround (accepted, per #1183)

Until SQLite is retired, a schema change lands in both db/migrations.py (SQLite) and a new Alembic revision (Postgres). Tracked for cleanup in Phase 3.

Deferred (separate work)

🤖 Generated with Claude Code

The PostgreSQL backend's schema is now owned by Alembic instead of the
fresh-build-from-schema.py path. SQLite keeps its bespoke db/migrations.py
runner — the two coexist during the Postgres transition (SQLite is being
retired).

- migrations/ (env.py targets db/tables.py MetaData), alembic.ini, and a
  0001_baseline revision that reuses the exact init_schema_postgres head DDL
  (tables + indexes + triggers) so a fresh PG DB built by `alembic upgrade
  head` is identical to the old fresh build.
- db/alembic_runner.upgrade_to_head(): fresh DB -> upgrade head; pre-Alembic
  PG DB (no alembic_version) -> stamp baseline then upgrade (not rebuilt);
  managed DB -> apply pending revisions.
- init_database() non-SQLite branch calls the runner (was init_schema_postgres).
- alembic==1.18.4 in the backend image (scheduler doesn't run migrations).
- Postgres-gated integration tests (TEST_POSTGRES_URL): fresh build, idempotency,
  downgrade-to-base, pre-Alembic stamp path, and schema parity vs the legacy
  init_schema_postgres build. All 5 pass against postgres:16.

Verified end-to-end against a throwaway postgres:16. SQLite init path unchanged.

Deferred: enterprise Alembic domain ships in the private enterprise repo
(Phase 2); autogenerate-from-metadata + SQLite retirement (Phase 3, with #746).

Related to #1183.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

Add CLAUDE.md Rule #9: every schema change needs both a SQLite entry in
db/migrations.py and a Postgres Alembic revision under migrations/versions/,
plus DDL in schema.py/tables.py. SQLite stays supported; PG-only is eventual,
not near-term.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dolho

dolho commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

PR Validation Report — /validate-pr

PR: #1186 — feat(db): adopt Alembic for PostgreSQL migrations — Phase 1 (#1183)
Branch: feature/1183-alembic-postgresdev
Files: 11 (+432 / -14) · Linked issue: #1183 (P1, type-feature, theme-reliability, complexity-high)

Summary

Category Status Notes
Commit messages Conventional (feat(db):, docs:), descriptive
Base branch Targets dev
PR size 11 files
Issue link "Related to #1183" (relate-don't-close convention)
Requirements Migration infra, not a user-facing capability — no requirements.md entry needed
Architecture Invariant #3 + POSTGRESQL_SETUP.md + CLAUDE.md Rule #9 updated
Feature flows No user-facing flow
Security check No secrets/tokens/emails/IPs/.env/cred files
Code quality Focused, well-commented; matches three-layer + schema conventions
Tests test_alembic_postgres.py covers fresh build, idempotency, downgrade, pre-Alembic stamp, schema parity

CI status

⚠️ lint (sys.modules pollution check) is red — but not caused by this PR. dev's backend-unit-test is broken: tests/unit/test_1199_agent_subscription_correlation.py has 2 un-baselined sys.modules.pop calls (#762 baseline). This PR inherits it via the PR-merged-with-dev ref. Fix is up as #1213dev. All other checks (schema-parity, prod-image-smoke, pytest matrix, CodeQL, container-security) pass.

Engineering notes (non-blocking)

  1. Two sources must not drift. 0001_baseline builds from db/schema.py (TABLES/INDEXES/POSTGRES_TRIGGERS), but migrations/env.py autogenerate targets db/tables.py MetaData. The schema-parity gate guards this today; once Collapse schema.py + migrations.py into single source of truth (follow-up to #713) #746 collapses them onto tables.py the risk goes away. Worth a comment in env.py pointing at the parity gate as the guard.
  2. Startup migration concurrency. init_database() runs alembic upgrade head on every boot. Single-instance today, but multi-replica deploys would race without an advisory lock — tracked in fix(db): migration runner — DROP-rebuild data-loss window and no cross-process serialization #1160 (runner serialization). Fine for Phase 1.
  3. Dual-write is intentional and documented — every schema change lands in both db/migrations.py (SQLite) and a new Alembic revision (PG) until SQLite retires (Collapse schema.py + migrations.py into single source of truth (follow-up to #713) #746). Captured in CLAUDE.md Rule Fix git pushing bug #9.

Recommendation

APPROVE — pending two non-code gates:

  1. Land test(#1199): fix sys.modules lint baseline failure (dev red) #1213 into dev, then merge dev → this branch to clear the inherited lint failure and the stale-branch merge conflict the nightly bot flagged.
  2. One human review approval (none on record yet).

P1 feature → /review + /cso --diff recommended before merge per the review pipeline.

🤖 Generated via /validate-pr with Claude Code

@dolho
dolho requested a review from vybe June 15, 2026 08:58
db/alembic_runner.py sets Alembic's script_location to /app/migrations at
runtime, but the Dockerfile only globbed top-level *.py plus named subdirs —
the new migrations/ dir and alembic.ini were dropped from the prod image. A
PostgreSQL deploy would crash-loop in init_database() -> upgrade_to_head()
because the script directory is absent (same #1033 packaging-gap class; CI
missed it since schema-parity/integration tests run against source and the
prod smoke boots SQLite).

COPY both into the image and include migrations/ in the readability chmod
step. alembic.ini isn't needed at runtime (the runner builds its Config
programmatically) but is copied so the alembic CLI works in-container for ops.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vybe

vybe commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Validation (/validate-pr) — one critical packaging fix pushed

Pushed fd9f8b89 to this branch.

🔴 Critical (fixed): migrations/ was missing from the backend image

db/alembic_runner.py sets Alembic's script_location to /app/migrations at runtime, but docker/backend/Dockerfile only globbed top-level *.py + named subdirs — the new src/backend/migrations/ dir and alembic.ini were dropped from the prod image. A PostgreSQL deploy would crash-loop in init_database() -> upgrade_to_head() (no script directory). Same #1033 packaging-gap class.

CI didn't catch it: schema-parity / test_alembic_postgres run against the source checkout (dir present on disk), and prod-image-smoke boots SQLite (default), so the runner is never exercised against the built image.

Fix: COPY both migrations/ and alembic.ini into the image + include migrations/ in the readability chmod step.

✅ Otherwise clean

  • Security: no keys/tokens/emails/secrets/.env
  • Config packaging: only new os.getenv is TEST_POSTGRES_URL (test-only); DATABASE_URL already wired in both compose files
  • Docs: architecture.md + CLAUDE.md + POSTGRESQL_SETUP.md updated
  • Issue link #1183 resolves (P1, type-feature, theme-reliability)

⚠️ Note (not this PR's defect)

The red lint (sys.modules pollution check) is pre-existing on dev — it fails in tests/unit/test_1199_agent_subscription_correlation.py (introduced by #1199/#1204), which is not part of this PR. Needs a separate fix (use monkeypatch.setitem/delitem or regenerate the lint baseline) before the branch can go fully green.

@vybe
vybe merged commit 4f255f4 into dev Jun 15, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants