diff --git a/backend/alembic/versions/e909ec65d857_sync_remediation_connection_flag.py b/backend/alembic/versions/e909ec65d857_sync_remediation_connection_flag.py index d68280b..b77d80b 100644 --- a/backend/alembic/versions/e909ec65d857_sync_remediation_connection_flag.py +++ b/backend/alembic/versions/e909ec65d857_sync_remediation_connection_flag.py @@ -23,7 +23,11 @@ def upgrade() -> None: "send_sample_data_to_llm", sa.Boolean(), nullable=False, - server_default=sa.text("1"), + # Cross-dialect boolean default: sa.true() compiles to `true` on + # PostgreSQL and `1` on SQLite. A literal sa.text("1") is rejected by + # Postgres ("column is of type boolean but default expression is of + # type integer") and crashed the v185 web boot. + server_default=sa.true(), ), ) diff --git a/backend/app/models/connection.py b/backend/app/models/connection.py index ebc79a0..8ce68dc 100644 --- a/backend/app/models/connection.py +++ b/backend/app/models/connection.py @@ -4,7 +4,7 @@ from datetime import datetime from typing import TYPE_CHECKING -from sqlalchemy import Boolean, DateTime, ForeignKey, Integer, String, Text, func +from sqlalchemy import Boolean, DateTime, ForeignKey, Integer, String, Text, func, true from sqlalchemy.orm import Mapped, mapped_column, relationship from app.models.base import Base @@ -57,7 +57,12 @@ class Connection(Base): is_read_only: Mapped[bool] = mapped_column(Boolean, default=True) is_active: Mapped[bool] = mapped_column(Boolean, default=True) send_sample_data_to_llm: Mapped[bool] = mapped_column( - Boolean, default=True, server_default="1", nullable=False + # server_default=true() (not "1"): Postgres rejects an integer default on + # a boolean column. true() compiles to `true` on PG and `1` on SQLite. + Boolean, + default=True, + server_default=true(), + nullable=False, ) created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) diff --git a/backend/tests/unit/test_boolean_server_defaults_pg.py b/backend/tests/unit/test_boolean_server_defaults_pg.py new file mode 100644 index 0000000..91bbca8 --- /dev/null +++ b/backend/tests/unit/test_boolean_server_defaults_pg.py @@ -0,0 +1,64 @@ +"""Guard: Boolean columns must use a Postgres-valid boolean server_default. + +Regression test for the 2026-06-26 production outage (release v185): the +``connections.send_sample_data_to_llm`` migration used ``server_default=sa.text("1")`` +on a Boolean column. SQLite accepts ``1`` for booleans, so every SQLite-only test +(and the local ``alembic upgrade`` check) passed — but PostgreSQL rejects it with +``DatatypeMismatchError: column ... is of type boolean but default expression is of +type integer``, crashing ``alembic upgrade head`` on web boot. + +This test compiles every mapped Boolean column's ``server_default`` for the +PostgreSQL dialect and asserts it renders a real boolean literal (``true``/``false``), +not an integer — catching the class of bug at the source (models) without needing a +live Postgres in CI. New Boolean columns MUST use ``sa.true()`` / ``sa.false()`` +(or ``sa.text("true")`` / ``sa.text("false")``), never ``sa.text("1")`` / ``"0"``. +""" + +from __future__ import annotations + +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql +from sqlalchemy.schema import CreateColumn + +import app.models # noqa: F401 — ensure all models are imported/registered +from app.models.base import Base + +_PG = postgresql.dialect() + + +def _boolean_columns_with_server_default(): + for table in Base.metadata.tables.values(): + for col in table.columns: + if isinstance(col.type, sa.Boolean) and col.server_default is not None: + yield table.name, col + + +def test_boolean_columns_have_no_bare_integer_pg_default(): + """A Boolean column must NOT render a BARE INTEGER default on Postgres. + + PG rejects ``DEFAULT 1`` ("boolean but default expression is of type integer") + — that is the ``sa.text("1")`` form that crashed v185. Quoted string literals + (``'0'``/``'1'``) and ``true``/``false`` are PG-valid and are left as-is. + """ + offenders: list[str] = [] + for table_name, col in _boolean_columns_with_server_default(): + ddl = str(CreateColumn(col).compile(dialect=_PG)) + lowered = ddl.lower() + assert "default" in lowered, f"{table_name}.{col.name}: no DEFAULT rendered ({ddl})" + # Token right after DEFAULT, e.g. "true" | "false" | "'0'" | "1". + after = lowered.split("default", 1)[1].strip().split()[0].strip("()") + if after.isdigit(): # bare unquoted integer — the only PG-incompatible form + offenders.append( + f"{table_name}.{col.name} -> DEFAULT {after} (bare integer; use sa.true()/false())" + ) + assert not offenders, ( + "Boolean column(s) have a BARE-INTEGER Postgres server_default " + "(crashes alembic on Postgres — use sa.true()/sa.false()):\n " + "\n ".join(offenders) + ) + + +def test_send_sample_data_to_llm_renders_true_on_pg(): + """The exact column that caused the v185 outage now renders `true` on Postgres.""" + col = Base.metadata.tables["connections"].columns["send_sample_data_to_llm"] + ddl = str(CreateColumn(col).compile(dialect=_PG)).lower() + assert "default true" in ddl, ddl diff --git a/docs/superpowers/specs/2026-06-26-prod-migration-boolean-default-hotfix-design.md b/docs/superpowers/specs/2026-06-26-prod-migration-boolean-default-hotfix-design.md new file mode 100644 index 0000000..1deb81f --- /dev/null +++ b/docs/superpowers/specs/2026-06-26-prod-migration-boolean-default-hotfix-design.md @@ -0,0 +1,86 @@ +# Prod Outage Hotfix — Boolean migration default rejected by Postgres + +- **Date:** 2026-06-26 +- **Severity:** 🔴 Critical (production outage — web dyno crash-loop, `/api/health` 503) +- **Branch:** `fix/prod-migration-bool-default-2026-06-26` (off `main`) +- **Found via:** Heroku log research after the R5 deploy (release v185). + +## Incident + +After R5 (`fix(sync)`) merged to `main` and auto-deployed as **release v185**, the web dyno +crash-looped and `/api/health` returned **503**. Heroku app logs: + +``` +asyncpg.exceptions.DatatypeMismatchError: column "send_sample_data_to_llm" is of type boolean +but default expression is of type integer +... +File "/app/alembic/versions/e909ec65d857_sync_remediation_connection_flag.py", line 20, in upgrade +subprocess.CalledProcessError: Command '['alembic', 'upgrade', 'head']' returned non-zero exit status 1. +uvicorn.error: Application startup failed. Exiting. +``` + +The `Procfile` web process runs `alembic upgrade head` before uvicorn; the migration failed, so the +dyno never started → 503. + +## Root cause + +The T14 migration `e909ec65d857` added a **Boolean** column with `server_default=sa.text("1")`: + +```python +sa.Column("send_sample_data_to_llm", sa.Boolean(), nullable=False, server_default=sa.text("1")) +``` + +`sa.text("1")` renders as the **bare integer** `DEFAULT 1`. PostgreSQL rejects a bare integer as a +boolean default (`boolean but ... integer`). SQLite accepts `1` for booleans, so: + +- every unit/integration test (SQLite) passed, +- the local `alembic upgrade head` / `downgrade base` check (SQLite) passed, +- the per-task + final whole-branch reviews passed, + +…and the bug only surfaced on **production Postgres** at deploy time. **Process gap:** migrations were +validated only against SQLite; the prod dialect (Postgres) was never exercised in CI or locally. + +> Note: existing Boolean columns that use a *string* `server_default` (`"0"`/`"1"`) render as the +> **quoted** literal `DEFAULT '0'`, which Postgres *does* accept (string→boolean cast) — those are +> not affected. Only the bare-integer `text("1")` form is invalid. + +## Fix (locked) + +1. **Migration `e909ec65d857`** — `server_default=sa.text("1")` → **`server_default=sa.true()`**. + `sa.true()` compiles to `true` on PostgreSQL and `1` on SQLite (verified via the dialect compiler: + PG → `BOOLEAN DEFAULT true`, SQLite → `BOOLEAN DEFAULT 1`). +2. **Model `app/models/connection.py`** — `server_default="1"` → **`server_default=true()`** (import + `true` from `sqlalchemy`) for create_all parity and autogenerate correctness. +3. **Regression guard** — `backend/tests/unit/test_boolean_server_defaults_pg.py`: compiles every + mapped Boolean column's `server_default` for the **postgresql** dialect and fails if any renders a + **bare integer** default (the exact PG-incompatible form). Quoted-string and `true`/`false` + defaults pass. Plus a targeted assertion that `connections.send_sample_data_to_llm` renders + `DEFAULT true` on PG. This catches the class of bug in CI without needing a live Postgres. + +## Prod DB state & recovery + +`alembic upgrade head` ran under transactional DDL; the failing R5 batch rolled back, so prod remained +at the pre-R5 revision. Redeploying with the corrected migration re-applies the R5 batch +(`2317bf9d9126` → `f37386df158c` no-op → `e909ec65d857` fixed) cleanly — no manual DB surgery needed. +The migration is idempotent w.r.t. the column (fresh add), and alembic resumes from prod's recorded +revision. + +## Verification + +- SQLAlchemy compiler: `sa.true()` → `true` (PG) / `1` (SQLite); old `text("1")` → `1` (PG, rejected). +- SQLite `alembic upgrade head` + `downgrade base`: clean. +- Regression guard + `test_alembic` + connection tests: green; ruff + mypy clean. +- Docker/local Postgres was unavailable in the fix environment, so PG validation is via the dialect + compiler (definitive for this DDL) + the deploy itself running the migration on prod Postgres. + +## Process follow-up (recommended, separate) + +Add a CI job (or a `make` target) that runs `alembic upgrade head` then `downgrade base` against a +**Postgres** service container, so migration-dialect bugs fail in CI rather than at deploy. Tracked as +a follow-up; the regression guard above is the immediate, zero-infra mitigation. + +## Deploy plan + +Commit fix + guard + spec → push → PR → merge to `main` (squash) → CI + auto-deploy → watch +`heroku ps` / `/api/health` until the web dyno is `up` and health is 200. If the migration still +fails, read logs and iterate (fix-after-deploy, redeploy) until green.