[EPIC][CHORE]: Alembic migration hygiene — reject placeholder hashes, verify chain linearity, rehash 36 affected migrations

[jonpspri]

🔧 Epic Summary

Alembic migrations have been quietly accumulating placeholder-style revision IDs (a1b2c3d4e5f6, w7x8y9z0a1b2, aa1b2c3d4e5f, …) instead of alembic revision's actual random-hex output (bb43712cae28, 43c07ed25a24, 191a2def08d7, …). The repo has paid for this with repeated rebase/merge conflicts (most recently in #3202, also in #4429 / #4548 / #4555 / #2154 / #4138). We have an existing pre-commit hook and CI harness that checks format but neither rejects placeholder content nor verifies the full chain is linear.

This epic groups the existing-but-unmerged hygiene work under one umbrella, adds the missing guardrails, and re-hashes the 36 affected revisions in a single sweep so this stops happening.

This is an epic / tracking issue. Each bullet in "Children" gets its own PR.

---

🧱 Area Affected

• GitHub Actions / CI Pipelines (.github/workflows/alembic-upgrade-validation.yml, .github/workflows/pre-commit.yml)
• Pre-commit hooks / linters (.pre-commit-config.yaml, .pre-commit-lite.yaml, scripts/pre-commit/check_migration_patterns.py)
• Database / migrations (mcpgateway/alembic/versions/*)
• Build system (Makefile db-*, migration-test-*, upgrade-validate targets)
• Docs (tests/migration/README.md, ADR for migration hash policy)

---

⚙️ Context / Why now

Three concrete symptoms motivate this epic:

1. 36 of 96 migrations on main use placeholder-style revision IDs. The pattern is "letter + digit alternating" ([a-z]\d[a-z]\d…) — 12 alphanumeric chars that pass the existing pre-commit regex but are clearly hand-picked, not alembic revision-generated.

2. Stacks of pre-allocated prefixes. Two long manual chains exist:

• k5e6… → l6f7… → m7g8… → n8h9… → o9i0… → p0a1… → q1b2… → r2b3… → s3c4… (9-prefix sequence across PRs Correlation ID for Unified Request Tracking #1443, Move LLM settings to UI instead of ENV #1597, Performance tab and locust #1637, Database Indexing Optimization (Issue #1353) #1650, Cache global config #1717, Metrics aggregation #1736, Metrics rollup cl #1782, [BUG]: Prompt Namespacing + Name/ID Resolution (Tool-Parity) #1762 #1876, 1892 get member count n plus one #1905)
• t4d5… → u5f6… → v1a2… → w6g7… → x7h8… → y8i9… → z1a2… (7-prefix sequence across PRs feature/password-enforcement-1843 #2066, feat: Add EntraID role mapping for SSO authentication #2129, Enforce Default RBAC Role Assignment and Migration for Existing Users #2699, feat(auth): implement password reset recovery and lockout fixes #2951, fix(auth): add jwks_uri column to SSOProvider and harden create_provider #3026, fix: batch 6 hardening for oauth secrets, masking, auth timing, and outbound URL validation (h-batch-6) #3115, 1282 force to user to change password #1494)

When two PRs each pre-allocate the next letter and merge close in time, both pick the same letter and we get multiple-head merge migrations (bb43712cae28_merge_token_blocklist_with_audit_identity_heads.py is the most recent evidence).

3. Header drift. Six migrations have Revises: docstrings that disagree with the actual down_revision value in code:

File	Docstring claims	Code says
20a0e0538ac5_add_composite_indexes_for_metrics_time_.py	64acf94cb7f2	abf8ac3b6008
356a2d4eed6f_uuid_change_for_prompt_and_resources.py	z1a2b3c4d5e6	9e028ecf59c4
add_toolops_test_cases_table.py	add_oauth_tokens_table	z1a2b3c4d5e6
ba202ac1665f_migrate_user_roles_to_configurable_.py	a31c6ffc2239	f9a8b7c6d5e4
bb43712cae28_merge_token_blocklist_with_audit_identity_heads.py	cae28b15a507, b2c3d4e5f6g7	only b2c3d4e5f6g7
r2b3c4d5e6f7_add_prompt_namespacing_fields.py	k5e6f7g8h9i0, 4f07c116f917, z1a2b3c4d5e6	only 4f07c116f917

These are silent bugs — alembic uses the code value, but anyone reading the docstring (humans, AI, future maintainers debugging chain shape) gets misled.

4. The CI gap that keeps letting this in. Per audit of current state:

• scripts/pre-commit/check_migration_patterns.py validates filename hash format and revision/filename match, plus duplicate revision IDs. It does not reject placeholder-style content.
• scripts/ci/run_upgrade_validation.sh enforces "exactly one alembic head". It does not verify the chain is linear (i.e., every revision has exactly one parent except the merge points, and merges are intentional).
• .pre-commit-lite.yaml — the config used by make pre-commit in CI — does not include the check-migration-patterns hook at all. So the local hook runs only for developers with pre-commit install'd, not in CI.

---

📐 Proposed Architecture

Guardrail layer (do this first; prevents new placeholder hashes from landing)

1. Extend check_migration_patterns.py to reject the placeholder pattern. Concrete regex (rejects "alternating letter+digit, ≥11 chars"):

   import re
   _PLACEHOLDER_RE = re.compile(r"^(?:[a-z]\d){5,}[a-z]?$")
   if _PLACEHOLDER_RE.match(revision_id):
       raise SystemExit(f"{path}: revision '{revision_id}' looks placeholder-allocated. Run `alembic revision -m '...'` to get a real hash.")

   Also reject revision IDs that contain 0a1b, 1b2c, 2c3d, …, 8h9i, 9i0j substrings (the diagonal chain marker).

2. Add a down_revision <-> Revises: docstring consistency check to the same hook. Catches the 6 mismatches above and prevents new ones.

3. Move check-migration-patterns into .pre-commit-lite.yaml so CI's pre-commit job actually runs it. Currently the hook only protects developers running pre-commit install.

Chain-shape verification (do this in CI, not pre-commit; needs the full graph)

4. Add --check-linear to scripts/ci/run_upgrade_validation.sh (or a sibling script). Walks the alembic graph, asserts:

   • Exactly one head (already covered).
   • Every non-merge revision has exactly one down_revision.
   • Every merge revision is named *_merge_* AND is documented in a CHANGELOG/ADR.
   • No "orphan" revisions (must be reachable from base to head).

5. Hook the new check into alembic-upgrade-validation.yml so PRs get blocked on broken chains, not just multi-head.

Cleanup (do last; needs the guardrails first or it'll regress)

6. Re-hash the 36 placeholder revisions in a single PR per logical batch. Use python -c "import secrets; print(secrets.token_hex(6))" to generate replacements. For each replaced revision, update:

   • The filename
   • The revision = "..." line
   • The Revises: docstring of the next migration
   • The down_revision = "..." of the next migration
   • Any merge migrations that reference the old hash

7. Fix the 6 docstring/code mismatches during the rehash pass.

8. Add an ADR (docs/docs/architecture/alembic-migration-hash-policy.md) recording the policy: "All revision IDs must come from alembic revision -m '...'. Manual or pre-allocated IDs are not permitted. Merge migrations must be named *_merge_* and explained in commit/ADR."

---

🧑🏻‍💻 User Story 1 — Developer

As a: developer adding a new migration
I want: pre-commit to fail loudly if I hand-edit a revision ID into something cute
So that: I never get into a "two PRs picked m7g8… simultaneously" merge conflict again

Scenario: Placeholder pattern is rejected at commit time
  Given a new file `mcpgateway/alembic/versions/m7g8h9i0j1k2_add_my_thing.py`
  When I run `pre-commit run check-migration-patterns --files <file>`
  Then the hook fails with "revision 'm7g8h9i0j1k2' looks placeholder-allocated"

Scenario: Real alembic-generated hashes pass
  Given a new file with revision = "bb43712cae28"
  When I run the hook
  Then it passes

🧑🏻‍💻 User Story 2 — Reviewer / CI

As a: PR reviewer / CI
I want: the upgrade-validation workflow to refuse a chain that's secretly branched
So that: "exactly one head" is no longer the only graph property we enforce

Scenario: Linear-chain verification catches a stealth branch
  Given a migration tree where rev_X and rev_Y both list rev_Z as down_revision but no merge migration unifies them
  When alembic-upgrade-validation.yml runs
  Then it fails with "non-merge revisions sharing parent rev_Z: rev_X, rev_Y"

🧑🏻‍💻 User Story 3 — Cleanup

As a: maintainer
I want: the existing 36 placeholder revisions re-hashed and the 6 docstring mismatches fixed
So that: the codebase stops carrying historical hash debt forward

Scenario: Placeholder rehash PR
  Given the cleanup PR replaces every placeholder revision ID with a real random hex hash
  And updates every downstream `down_revision` and `Revises:` reference
  When `make migration-test-all` runs
  Then `alembic upgrade head` succeeds on a fresh DB
  And `alembic downgrade base` succeeds from head
  And `alembic heads` returns exactly one head
  And the new placeholder-pattern check passes

---

📋 Children (track via this epic)

Already-existing related issues that fit under this umbrella:

• [CHORE][DB]: Standardize Alembic migration revision IDs to auto-generated hex hashes #3831 — [CHORE][DB]: Standardize Alembic migration revision IDs to auto-generated hex hashes (direct match for cleanup work item 6)
• [BUG]: Unify schema creation: replace Base.metadata.create_all() with Alembic-only bootstrap #4555 — [BUG]: Unify schema creation: replace Base.metadata.create_all() with Alembic-only bootstrap (adjacent — schema ownership)
• fix(alembic): base schema tables (tools, gateways, servers) not created by migrations — causes downgrade failure on fresh DB #4548 — fix(alembic): base schema tables (tools, gateways, servers) not created by migrations (adjacent — chain-shape correctness)
• [BUG]: Alembic migration advisory lock hangs when multiple gateway pods start through PgBouncer (transaction pooling mode) #4051 — [BUG]: Alembic migration advisory lock hangs when multiple gateway pods start through PgBouncer (adjacent — startup/locking)
• [BUG][DB]: bootstrap_db crashes on alembic version mismatch when pgdata volume persists across image rebuilds #3512 — [BUG][DB]: bootstrap_db crashes on alembic version mismatch (adjacent — version drift detection, sibling of guardrail work)
• [TESTING][UPGRADE]: Version Upgrades, Database Migrations, and Rollback Procedures #2474 — [TESTING][UPGRADE]: Version Upgrades, Database Migrations, and Rollback Procedures (adjacent — broader upgrade testing)
• [PERFORMANCE]: Optimize Database Migration Performance #1694 — [PERFORMANCE]: Optimize Database Migration Performance (adjacent)

Already-closed prior art (cite for context, do not reopen):

• ✅ [CHORE]: Add CI/CD validation for Alembic migration status #2154 — [CHORE]: Add CI/CD validation for Alembic migration status (landed the existing single-head check; this epic extends it to linearity)
• ✅ [TESTING][DB]: Migration testing gaps — rollback verification, cross-DB schema validation, and CI coverage #4138 — [TESTING][DB]: Migration testing gaps — rollback verification, cross-DB schema validation, and CI coverage (landed via PR Fix/alembic migration safety #4479)
• ✅ [BUG]: Alembic downgrade is unsafe, environment-dependent, and untested across multiple migrations #4429 — [BUG]: Alembic downgrade is unsafe, environment-dependent, and untested (landed via PR Fix/alembic migration safety #4479)
• ✅ [BUG]: Incorrect Alembic migration placement and history #2096 — [BUG]: Incorrect Alembic migration placement and history (closed as duplicate, but symptom of the same problem this epic targets)

New work items to file as children of this epic (after this epic is accepted):

• A11. Extend check_migration_patterns.py with placeholder-pattern rejection + docstring-vs-code consistency check
• A12. Add check-migration-patterns to .pre-commit-lite.yaml so CI runs it
• A13. Add chain-linearity validation to scripts/ci/run_upgrade_validation.sh (or a sibling script) and wire into .github/workflows/alembic-upgrade-validation.yml
• A14. Re-hash the 36 placeholder revisions in a sequence of small batched PRs (one per logical chain to keep diffs reviewable)
• A15. Fix the 6 Revises: docstring mismatches during the rehash
• A16. ADR: document the alembic hash policy in docs/docs/architecture/

---

📦 Affected migrations (reference data for cleanup PRs)

36 placeholder-style revisions to be re-hashed:

a1b2c3d4e5f6  a4f1c7d8e9b0  a7f3c9e1b2d4  a8f3b2c1d4e5
aa1b2c3d4e5f  b1b2b3b4b5b6  b2c3d4e5f6g7  b2d9c6e4f1a7
c1c2c3c4c5c6  d3e4f5a6b7c8  d9e0f1a2b3c4  e1f2a3b4c5d6
f1a2b3c4d5e6  f8c9d3e2a1b4  f9a8b7c6d5e4  g1a2b3c4d5e6
h2b3c4d5e6f7  i3c4d5e6f7g8  j4d5e6f7g8h9  k5e6f7g8h9i0
l6f7g8h9i0j1  m7g8h9i0j1k2  n8h9i0j1k2l3  o9i0j1k2l3m4
p0a1b2c3d4e5  q1b2c3d4e5f6  r2b3c4d5e6f7  s3c4d5e6f7g8
t4d5e6f7g8h9  u5f6g7h8i9j0  v1a2b3c4d5e6  w6g7h8i9j0k1
w7x8y9z0a1b2  x7h8i9j0k1l2  y8i9j0k1l2m3  z1a2b3c4d5e6

6 Revises: docstring mismatches to fix:

• 20a0e0538ac5_add_composite_indexes_for_metrics_time_.py
• 356a2d4eed6f_uuid_change_for_prompt_and_resources.py
• add_toolops_test_cases_table.py
• ba202ac1665f_migrate_user_roles_to_configurable_.py
• bb43712cae28_merge_token_blocklist_with_audit_identity_heads.py
• r2b3c4d5e6f7_add_prompt_namespacing_fields.py

1 header anomaly to investigate:

• b77ca9d2de7e_uuid_pk_and_slug_refactor.py has no Revises: line; uses a Create Date: header instead.

---

📋 Acceptance criteria (epic-level)

• Pre-commit check-migration-patterns rejects placeholder-style revision IDs (regex test + docstring consistency)
• .pre-commit-lite.yaml includes the migration hook so CI enforces it
• CI verifies chain linearity, not just single-head
• All 36 placeholder revisions are replaced with real hex hashes
• All 6 docstring mismatches are fixed
• b77ca9d2de7e header is normalized
• ADR documenting the policy is merged
• All listed children issues are either closed or explicitly de-scoped from this epic with reasoning

---

📦 Related Make Targets

• make pre-commit — currently uses .pre-commit-lite.yaml, needs to include the migration hook
• make upgrade-validate — runs scripts/ci/run_upgrade_validation.sh, target for linearity check
• make migration-test-all — sanity gate after rehash PRs land
• make db-heads — already shows count, will be the cleanest single-head reporter post-cleanup

---

📓 Notes

• This epic is not a security issue; it's hygiene that prevents future merge-train collisions and silent bugs.
• The rehash work is inherently rebase-y. Recommend doing it during a quiet window and pinning a specific main commit so contributors with in-flight migrations can reparent cleanly.
• Both feat(transport): add gRPC methods as MCP tools #3202 (just-rebased gRPC PR) and any unmerged PR that adds a migration WILL conflict with the rehash sweep. Plan to coordinate.
• Audit data above is reproducible from the mcpgateway/alembic/versions/ tree at the time of filing; happy to drop the exact python -c invocations into a comment if reviewers want to verify.

[jonpspri]

Follow-up from PR #3202 review — child item: migration test for w7x8y9z0a1b2

PR #3202 adds mcpgateway/alembic/versions/w7x8y9z0a1b2_add_grpc_service_id_to_tools.py (one of the placeholder-style hashes this epic targets). The PR review's test-coverage analyzer flagged that there's no paired migration test under tests/migration/ for this revision.

Rather than file a one-off test for a hash that's about to be re-generated under this epic, suggest folding the test requirement into this epic's acceptance criteria:

Each re-hashed migration ships with a paired test in tests/migration/ that asserts:

1. Idempotent upgrade (running alembic upgrade head twice doesn't error)
2. Downgrade drops the column / table cleanly
3. SQLite-vs-Postgres FK behavior parity (or documented divergence)
4. The migration's down_revision resolves to a real revision in the chain

That covers the gap from PR #3202 plus prevents the same gap recurring on every future migration.

Reference: review item #11 in the PR #3202 deep-review summary.