ci(backend): block non-additive Alembic migrations pre-merge

[aatchison]

Draft — feedback welcome before this becomes a required check.

Blocks a PR from adding an Alembic migration that isn't expand/contract safe.

Why

For a window during every deploy, the OLD and NEW replicas run against the SAME database, and a schema change cannot be undone by aborting the deploy. So migrations must only ADD; removals ship a release later.

That window is short and incidental on today's ECS rolling deploy. It gets long and deliberate under the Argo Rollouts blue-green deploy going up on EKS tb-dev/tb-prod (thunderbird/platform-infrastructure#781) — which also adds an abort path that looks like a rollback but can't undo DDL. This is groundwork for that.

Why pre-merge, not at deploy time

We built the deploy-time version first and it can't work: backend/scripts/entry.sh runs alembic upgrade head before uvicorn binds, and Rollouts only runs pre-promotion analysis once pods are Ready. The migration has already landed by then. Pre-merge is the last point at which it hasn't run anywhere.

What's flagged

upgrade() only, following module-level helpers it calls (the repo's newest migration already hides a drop_index in one):

drop_column / drop_table / drop_constraint / drop_index / rename_table · alter_column rename, narrowing, or dropped default · add_column(nullable=False) with no server_default · unique index/constraint on an existing table · destructive batch_alter_table · .execute() with destructive raw SQL, on any receiver.

Not flagged: NOT NULL/unique on a table created in the same migration · existing_nullable= · add_column(nullable=False, server_default=...). Enum widenings print as non-blocking notes.

Calibration

67 existing migrations: 56 pass, 11 blocking. Existing migrations are untouched — CI only looks at what a branch changes. (A regex over whole files flags ~45; most are the autogenerated destructive downgrade() on an ordinary add_column. The AST scoping is what makes this usable.)

Escape hatch

# additivity-exempt: https://github.com/thunderbird/appointment/issues/1234
#   invites table; nothing has referenced it since v1.9

Thunderbird issue/PR URL + reason required, honoured only in a real comment token. Waived findings are still printed. Prefer splitting into expand/contract.

Proof it fires

This PR adds no migrations, so its own run reports "nothing to check" — the checker invocation path never executes here. #1771 (throwaway, now closed) added one deliberately non-additive migration to exercise it.

migration-additivity — red in 7s

Migrations to check:
  src/appointment/migrations/versions/..._demo_non_additive.py
FAIL   not additive
  line 29: op.add_column(nullable=False) without server_default
     why: old replicas' INSERTs omit the column, so they fail
     fix: Add server_default=... (SQLAlchemy's default= is client-side, emits no DDL default)
  line 31: execute(<destructive SQL>)
     why: raw SQL contains 'ALTER TABLE subscribers DROP'
     fix: Defer the destructive statement to a later release.
  line 22: op.drop_column
     why: drops a column the old replicas may still SELECT
     fix: Ship the removal in a LATER release (contract phase).

the migration it was run against

def _tidy_up():
    """Hidden in a helper on purpose: proves the scan follows call graphs."""
    op.drop_column('subscribers', 'ftue_level')

def upgrade() -> None:
    _tidy_up()                                              # 1. destructive, via helper
    op.add_column('subscribers',
                  sa.Column('demo_flag', sa.Boolean(), nullable=False))   # 2. NOT NULL, no default
    op.get_bind().execute(                                  # 3. raw DDL, non-`op` receiver
        sa.text('ALTER TABLE subscribers DROP COLUMN avatar_url'))

def downgrade() -> None:
    op.drop_column('subscribers', 'demo_flag')              # destructive, must NOT be flagged

The destructive downgrade() was correctly ignored. The corpus calibration test caught it independently too (1 failed, 491 passed), which shows that test isn't vacuous.

Two things for the team

1. main has no required status checks — this is advisory until migration-additivity is added to branch protection.
2. concurrency: group: validate isn't ref-scoped, so a push on one branch cancels in-flight runs on another.

Context: thunderbird/platform-infrastructure#781.

🤖 Generated with Claude Code

[aatchison]

Review fixes applied (7657be7f, ccc48eaa, f75e856a). The review found four ways to get a non-additive migration past the check — all reproduced, all fixed, each pinned by a test.

The four bypasses

1. Helper functions were invisible — and this was already live: d4e5f6a7b8c9 hides drop_index + drop_constraint in _drop_global_slug_unique(). Now follows calls transitively.
2. session.execute / op.get_bind().execute unchecked — 13 of 67 migrations use that handle. Now inspects .execute() on any receiver.
3. The script exempted itself via its own docstring example. Now comment-tokens only, with a strict Thunderbird issue-URL + reason.
4. A missing path exited 0 — any wiring slip would have made this a green no-op. Now exits 2.

Also fixed the CodeQL "incomplete URL substring sanitization" finding, which was pointing at something real: the checker accepted a bare https:// as a valid exemption URL.

Two claims I made earlier in this PR were wrong, in case you read them before the description was rewritten:

• "Zero false positives" — 4 of 15 were enum widenings, which are additive. Now non-blocking notes; the description carries the corrected counts.
• "Appointment is deployed blue-green" — it isn't yet. Today's deploy is an ECS Fargate rolling update; blue-green is what this is groundwork for.