ci: prevent migration-number collisions (closes #2815)

.changeset/ci-migration-collision-guard.md

@@ -0,0 +1,12 @@
+---
+---
+
+ci: prevent migration-number collisions from slipping past the merge queue (closes #2815)
+
+Addresses the class of incident from #2813: #2793 and #2800 both reserved migration 419, passed CI individually, and merged minutes apart — leaving every fresh DB boot broken until a renumber hotfix. Two gaps closed:
+
+**`check-migration-numbers.yml`** — the PR check now takes the union of migration filenames from the PR branch *and* `origin/main`, then hunts duplicates in that union. Previously it only inspected GitHub's speculative merge commit, which goes stale when main moves on in a way that doesn't conflict with the PR's diff. Two PRs reserving the same number would both see a clean local state and pass. With the union check, the second one to run (after the first lands on main) fails immediately. Also dropped the `paths: server/src/db/migrations/**` filter so a rebase after main adds a migration triggers a re-check even if the PR itself doesn't touch migrations.
+
+**`deploy.yml`** — added a `preflight` job that runs the duplicate-number check on the merged main tree. `deploy` now `needs: preflight`, so if somehow a collision does land, the Fly deploy is blocked before we ship a container that crashloops on boot instead of after. Cheap sanity check, ~10s runtime.
+
+Together: pre-merge catches the race, post-merge catches anything pre-merge misses. Both are deterministic shell, no hidden dependencies.

.github/workflows/check-migration-numbers.yml

@@ -3,8 +3,9 @@ name: Check Migration Numbers
 on:
   pull_request:
     branches: [main]
-    paths:
-      - 'server/src/db/migrations/**'
+    # Intentionally no `paths` filter — a PR that doesn't touch the
+    # migrations dir can still collide with one that's been merged since
+    # it branched, if main added a number the PR reserved upstream.
   push:
     branches: [main]
     paths:
@@ -16,21 +17,48 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v6
+        with:
+          fetch-depth: 0
 
-      - name: Check for duplicate migration number prefixes
+      - name: Check for duplicate migration numbers against current main
         run: |
-          dupes=$(ls server/src/db/migrations/*.sql \
-            | xargs -n1 basename \
-            | sed 's/_.*//' \
-            | sort \
-            | uniq -d)
+          # On pull_request, GitHub checks out the speculative merge of the
+          # PR into main as computed at PR-open (or at last conflict). That
+          # merge commit can go stale when main moves forward without
+          # triggering a re-compute, which is exactly how #2793 and #2800
+          # both passed this check with colliding migration 419s.
+          #
+          # Guard against the stale-merge case by taking the union of
+          # migration filenames on the PR branch AND on origin/main, then
+          # hunting dupes in that union. Same check on push-to-main has no
+          # merge step — it just sees what's already merged.
+          git fetch origin main --depth=1 2>/dev/null || true
+
+          # Files on this ref (PR's speculative merge, or main).
+          local_files=$(ls server/src/db/migrations/*.sql 2>/dev/null | xargs -n1 basename || true)
+
+          # Files on main right now (empty if we're already on main).
+          main_files=""
+          if [ "${{ github.event_name }}" = "pull_request" ]; then
+            main_files=$(git ls-tree --name-only -r origin/main -- server/src/db/migrations/ \
+              | sed 's|.*/||' \
+              | grep '\.sql$' || true)
+          fi
+
+          # Union of both file lists, unique by full filename.
+          all_files=$(printf '%s\n%s\n' "$local_files" "$main_files" | sort -u | sed '/^$/d')
+
+          # Extract the leading number prefix and look for duplicates.
+          dupes=$(echo "$all_files" | sed 's/_.*//' | sort | uniq -d)
+
           if [ -n "$dupes" ]; then
             echo "::error::Duplicate migration number prefixes found: $dupes"
             echo ""
-            echo "These migration files share a number prefix:"
+            echo "These migration files share a number prefix (one may be on main,"
+            echo "the other in this PR — rebase + renumber yours if so):"
             for prefix in $dupes; do
-              ls server/src/db/migrations/${prefix}_*.sql
+              echo "$all_files" | awk -v p="$prefix" '$0 ~ "^"p"_" { print "  " $0 }'
             done
             exit 1
           fi
-          echo "No duplicate migration numbers found."
+          echo "No duplicate migration numbers found (checked PR + current main)."

.github/workflows/deploy.yml

@@ -10,8 +10,37 @@ concurrency:
   cancel-in-progress: false
 
 jobs:
+  # Preflight: cheap sanity checks that must pass before we spend the
+  # Fly build/deploy budget. If main has a broken state (e.g. two PRs
+  # merged minutes apart reserved the same migration number), catch it
+  # here instead of shipping a container that crashloops on boot.
+  preflight:
+    name: Preflight
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v6
+      - name: No duplicate migration numbers
+        run: |
+          dupes=$(ls server/src/db/migrations/*.sql \
+            | xargs -n1 basename \
+            | sed 's/_.*//' \
+            | sort \
+            | uniq -d)
+          if [ -n "$dupes" ]; then
+            echo "::error::Duplicate migration number prefixes on main: $dupes"
+            echo ""
+            echo "Deploy blocked — main would crashloop on boot. Land a renumber"
+            echo "hotfix before the next push can deploy. Duplicate files:"
+            for prefix in $dupes; do
+              ls server/src/db/migrations/${prefix}_*.sql
+            done
+            exit 1
+          fi
+          echo "No duplicate migration numbers on main."
+
   deploy:
     name: Deploy to Fly.io
+    needs: preflight
     runs-on: ubuntu-latest
     steps:
       - name: Checkout