Add extra deployment phase for destructive migrations

.claude/skills/postgres-migrations/SKILL.md

@@ -0,0 +1,89 @@
+---
+name: postgres-migrations
+allowed-tools: Read, Grep, Bash
+description: How to author Postgres migrations in packages/postgres under the two-phase (additive vs removal) system — additive changes go in migrations/ and run pre-deploy; destructive changes (DROP COLUMN/TABLE, RENAME) go in migrations-removal/ and run post-deploy so they never break the previous code revision mid-rollout. Use whenever creating, editing, moving, or reviewing a migration in packages/postgres, deciding which directory a migration belongs in, or touching the boxel_index / boxel_index_working index tables. Triggers on adding a DB migration, a DROP/RENAME in a migration, or a review of one.
+---
+
+# Postgres migrations — the two-phase system
+
+`packages/postgres/` migrations are split into two directories, each with its own
+node-pg-migrate tracking table, because a destructive schema change applied
+during a rolling deploy breaks the **previous** code revision while it is still
+serving (old tasks query a column the migration just dropped).
+
+| Directory             | Tracking table       | When it runs in a deploy                                                                                                  | For                                   |
+| --------------------- | -------------------- | ------------------------------------------------------------------------------------------------------------------------- | ------------------------------------- |
+| `migrations/`         | `migrations`         | **pre**-deploy (`migrate-db` job)                                                                                         | additive, backward-compatible changes |
+| `migrations-removal/` | `migrations_removal` | **post**-deploy (`migrate-db-remove` job), gated on the realm-server rollout reaching stability so old tasks have drained | destructive changes                   |
+
+## Which directory?
+
+Ask: **does the migration's `up()` remove or rename something the currently
+deployed code still reads?** If yes → `migrations-removal/`.
+
+- **Additive → `migrations/`**: `addColumn`, `createTable`, new index, backfill,
+  dual-write. Safe for old code to run against.
+- **Destructive → `migrations-removal/`**: `dropColumn` / `dropColumns` /
+  `dropTable` / `renameColumn` / `renameTable`, and the raw-SQL equivalents
+  (`pgm.sql('ALTER TABLE ... DROP COLUMN ...')`, `RENAME`). Also anything that
+  tightens a contract the old code depends on (e.g. adding `NOT NULL` to a
+  column old code doesn't populate, narrowing a type).
+
+The standard expand/contract pattern spans **two** releases: release N adds the
+new shape and stops reading the old one (additive migration + code); release
+N+1 drops the old shape (removal migration). Never ship the drop in the same
+release that stops reading it unless you are certain no old task will run against
+the new schema.
+
+## Creating a migration
+
+```sh
+# additive (the common case) — writes to migrations/
+pnpm --filter @cardstack/postgres migrate create <name>
+
+# destructive — writes straight to migrations-removal/
+pnpm --filter @cardstack/postgres migrate:create-removal <name>
+```
+
+Filenames need a real `Date.now()` millisecond timestamp; `pnpm lint:migrations`
+rejects any prefix with 6+ consecutive zeros. `migrate create` stamps a valid
+one. Never hand-pick a round number.
+
+Migration files are CommonJS (`exports.up` / `exports.down`); both directories
+carry a `package.json` pinning `{ "type": "commonjs" }`.
+
+## Enforcement
+
+`packages/postgres/scripts/check-removal-phase.cjs` (CI: "Guard removal-phase
+migrations") AST-parses each **newly added** `migrations/` file and fails if its
+`up()` drops or renames a column/table. It is scoped to changed files (so drops
+already present in `migrations/` are grandfathered) and looks only at `up()` (an
+additive migration's `down()` legitimately calls `dropColumn` to reverse
+itself). It is heuristic — it will not catch `NOT NULL` tightening, type
+narrowing, or destructive SQL built from non-literal strings — so the decision
+rule above still applies.
+
+## Applying migrations locally / in CI
+
+`pnpm migrate up|down [count]|create` is transparent — the driver
+(`scripts/migrate-local.sh`) runs both phases as one combined sequence
+(additive first on `up`; `down [count]` reverts the N most-recent across both
+phases, default 1). You do **not** think about the split when applying; only
+when authoring a destructive change.
+
+## Gotchas
+
+- **`boxel_index` and `boxel_index_working` are twin tables that must stay
+  schema-identical.** The indexer does `SELECT * FROM boxel_index` and mirrors
+  the row shape into `boxel_index_working`, so any column add/drop must touch
+  **both** (the removal migration uses `TABLES = ['boxel_index',
+'boxel_index_working']`). Changing only one breaks index writes with
+  `column "..." of relation "boxel_index_working" does not exist`.
+- **Moving an already-applied migration between directories re-runs it** under
+  the new tracking table. Only safe if its `up()` is idempotent (`IF EXISTS` /
+  `ifNotExists`). Moving a not-yet-applied file is always clean.
+- **Schema snapshot**: after any migration change run `pnpm make-schema`; the
+  host validates the committed `packages/host/config/schema/<latest>_schema.sql`
+  against the newest migration across **both** directories.
+- `packages/realm-server/migrations/` is a **separate** migration system (not
+  this one); don't conflate them.

.github/scripts/run-gated-migration-task.sh

@@ -0,0 +1,91 @@
+#!/usr/bin/env bash
+# Run one DB-migration phase as a one-shot ECS task and gate on its exit code,
+# so the deploy never proceeds past a failed migration. Shared by the
+# migrate-db (expand) and migrate-db-remove (removal) jobs in manual-deploy.yml
+# so both phases use identical, in-lockstep orchestration.
+#
+# The in-container command to run is passed as this script's arguments, e.g.
+#   run-gated-migration-task.sh ./scripts/run-migrations.sh
+#   run-gated-migration-task.sh ./scripts/run-migrations.sh migrations-removal migrations_removal
+#
+# Everything else comes from the job environment:
+#   CLUSTER SERVICE CONTAINER IMAGE LOG_GROUP TIMEOUT_SECONDS GITHUB_SHA
+set -euo pipefail
+
+region=us-east-1
+console_logs="https://${region}.console.aws.amazon.com/cloudwatch/home?region=${region}#logsV2:log-groups/log-group/${LOG_GROUP}"
+
+# Register a new revision of the pg-migration task definition pinned to the
+# image just built. Strip the read-only fields describe returns that
+# register-task-definition rejects.
+aws ecs describe-task-definition --task-definition "$SERVICE" \
+  --query 'taskDefinition' --output json > td.json
+jq --arg IMAGE "$IMAGE" --arg CONTAINER "$CONTAINER" '
+  .containerDefinitions |= map(if .name == $CONTAINER then .image = $IMAGE else . end)
+  | del(.taskDefinitionArn, .revision, .status, .requiresAttributes,
+        .compatibilities, .registeredAt, .registeredBy, .deregisteredAt)
+' td.json > td-new.json
+task_def=$(aws ecs register-task-definition --cli-input-json file://td-new.json \
+  --query 'taskDefinition.taskDefinitionArn' --output text)
+echo "Registered migration task definition: $task_def"
+
+# Place the one-shot task in the pg-migration service's own network config —
+# its security group is the one allowed to reach the DB.
+net=$(aws ecs describe-services --cluster "$CLUSTER" --services "$SERVICE" \
+  --query 'services[0].networkConfiguration.awsvpcConfiguration' --output json)
+subnets=$(echo "$net" | jq -r '.subnets | join(",")')
+groups=$(echo "$net" | jq -r '.securityGroups | join(",")')
+public=$(echo "$net" | jq -r '.assignPublicIp // "DISABLED"')
+
+# Override the command to run the migrations WITHOUT the service's trailing
+# `sleep infinity`, so the task exits with the migration's status instead of
+# staying up. The command comes from this script's arguments.
+overrides=$(jq -cn --arg CONTAINER "$CONTAINER" \
+  '{containerOverrides: [{name: $CONTAINER, command: $ARGS.positional}]}' \
+  --args "$@")
+run_out=$(aws ecs run-task \
+  --cluster "$CLUSTER" \
+  --task-definition "$task_def" \
+  --launch-type FARGATE \
+  --count 1 \
+  --started-by "deploy-${GITHUB_SHA:0:7}" \
+  --network-configuration "awsvpcConfiguration={subnets=[$subnets],securityGroups=[$groups],assignPublicIp=$public}" \
+  --overrides "$overrides" \
+  --output json)
+task_arn=$(echo "$run_out" | jq -r '.tasks[0].taskArn // empty')
+if [ -z "$task_arn" ]; then
+  echo "::error::Failed to start the migration task."
+  echo "$run_out" | jq '.failures'
+  exit 1
+fi
+echo "Started migration task: $task_arn"
+
+# Wait for the task to stop, then gate on the container's exit code.
+deadline=$(( $(date +%s) + TIMEOUT_SECONDS ))
+while :; do
+  status=$(aws ecs describe-tasks --cluster "$CLUSTER" --tasks "$task_arn" \
+    --query 'tasks[0].lastStatus' --output text)
+  echo "migration task status: $status"
+  [ "$status" = "STOPPED" ] && break
+  if [ "$(date +%s)" -ge "$deadline" ]; then
+    echo "::error::Timed out after ${TIMEOUT_SECONDS}s waiting for the migration task to finish."
+    echo "Migration logs: ${console_logs}"
+    aws ecs stop-task --cluster "$CLUSTER" --task "$task_arn" \
+      --reason "migration gate timeout" >/dev/null 2>&1 || true
+    exit 1
+  fi
+  sleep 10
+done
+
+exit_code=$(aws ecs describe-tasks --cluster "$CLUSTER" --tasks "$task_arn" \
+  --query "tasks[0].containers[?name=='${CONTAINER}']|[0].exitCode" --output text)
+echo "migration container exit code: ${exit_code}"
+if [ "$exit_code" != "0" ]; then
+  echo "::error::DB migration failed (exit ${exit_code}) — deploy blocked so the app never runs ahead of its schema."
+  aws ecs describe-tasks --cluster "$CLUSTER" --tasks "$task_arn" \
+    --query "tasks[0].{stopCode:stopCode,taskReason:stoppedReason,containerReason:containers[?name=='${CONTAINER}']|[0].reason}" \
+    --output table || true
+  echo "Migration logs: ${console_logs}"
+  exit 1
+fi
+echo "DB migration succeeded."

.github/workflows/ci.yaml

@@ -291,6 +291,17 @@ jobs:
         shell: bash
         run: packages/postgres/scripts/determine-changed-migrations.sh
 
+      # Fail fast if a newly added additive migration drops/renames a column or
+      # table in its up() — those belong in migrations-removal/ (post-deploy) so
+      # they don't break the previous code revision mid-rollout.
+      - name: Guard removal-phase migrations
+        if: steps.migrations.outputs.count && steps.migrations.outputs.count != '0'
+        env:
+          # Newline-separated file list; the guard reads it from this env var
+          # (no shell word-splitting, no multiline value inlined into the script).
+          CHANGED_MIGRATIONS: ${{ steps.migrations.outputs.files }}
+        run: node packages/postgres/scripts/check-removal-phase.cjs
+
       - name: Apply migrations
         if: steps.migrations.outputs.count && steps.migrations.outputs.count != '0'
         working-directory: packages/postgres

.github/workflows/manual-deploy.yml

@@ -237,6 +237,9 @@ jobs:
       # migration must finish under it. Genuine failures exit immediately.
       TIMEOUT_SECONDS: "1800"
     steps:
+      - name: Check out
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+
       - name: Set up env
         run: |
           if [ "${{ inputs.environment }}" = "production" ]; then
@@ -255,84 +258,7 @@ jobs:
           aws-region: us-east-1
 
       - name: Run migrations and gate on exit code
-        run: |
-          set -euo pipefail
-          region=us-east-1
-          console_logs="https://${region}.console.aws.amazon.com/cloudwatch/home?region=${region}#logsV2:log-groups/log-group/${LOG_GROUP}"
-
-          # Register a new revision of the pg-migration task definition pinned to
-          # the image just built. Strip the read-only fields describe returns
-          # that register-task-definition rejects.
-          aws ecs describe-task-definition --task-definition "$SERVICE" \
-            --query 'taskDefinition' --output json > td.json
-          jq --arg IMAGE "$IMAGE" --arg CONTAINER "$CONTAINER" '
-            .containerDefinitions |= map(if .name == $CONTAINER then .image = $IMAGE else . end)
-            | del(.taskDefinitionArn, .revision, .status, .requiresAttributes,
-                  .compatibilities, .registeredAt, .registeredBy, .deregisteredAt)
-          ' td.json > td-new.json
-          task_def=$(aws ecs register-task-definition --cli-input-json file://td-new.json \
-            --query 'taskDefinition.taskDefinitionArn' --output text)
-          echo "Registered migration task definition: $task_def"
-
-          # Place the one-shot task in the pg-migration service's own network
-          # config — its security group is the one allowed to reach the DB.
-          net=$(aws ecs describe-services --cluster "$CLUSTER" --services "$SERVICE" \
-            --query 'services[0].networkConfiguration.awsvpcConfiguration' --output json)
-          subnets=$(echo "$net" | jq -r '.subnets | join(",")')
-          groups=$(echo "$net" | jq -r '.securityGroups | join(",")')
-          public=$(echo "$net" | jq -r '.assignPublicIp // "DISABLED"')
-
-          # Override the command to run the migrations WITHOUT the service's
-          # trailing `sleep infinity`, so the task exits with the migration's
-          # status instead of staying up.
-          overrides=$(jq -cn --arg CONTAINER "$CONTAINER" \
-            '{containerOverrides: [{name: $CONTAINER, command: ["./scripts/run-migrations.sh"]}]}')
-          run_out=$(aws ecs run-task \
-            --cluster "$CLUSTER" \
-            --task-definition "$task_def" \
-            --launch-type FARGATE \
-            --count 1 \
-            --started-by "deploy-${GITHUB_SHA:0:7}" \
-            --network-configuration "awsvpcConfiguration={subnets=[$subnets],securityGroups=[$groups],assignPublicIp=$public}" \
-            --overrides "$overrides" \
-            --output json)
-          task_arn=$(echo "$run_out" | jq -r '.tasks[0].taskArn // empty')
-          if [ -z "$task_arn" ]; then
-            echo "::error::Failed to start the migration task."
-            echo "$run_out" | jq '.failures'
-            exit 1
-          fi
-          echo "Started migration task: $task_arn"
-
-          # Wait for the task to stop, then gate on the container's exit code.
-          deadline=$(( $(date +%s) + TIMEOUT_SECONDS ))
-          while :; do
-            status=$(aws ecs describe-tasks --cluster "$CLUSTER" --tasks "$task_arn" \
-              --query 'tasks[0].lastStatus' --output text)
-            echo "migration task status: $status"
-            [ "$status" = "STOPPED" ] && break
-            if [ "$(date +%s)" -ge "$deadline" ]; then
-              echo "::error::Timed out after ${TIMEOUT_SECONDS}s waiting for the migration task to finish."
-              echo "Migration logs: ${console_logs}"
-              aws ecs stop-task --cluster "$CLUSTER" --task "$task_arn" \
-                --reason "migration gate timeout" >/dev/null 2>&1 || true
-              exit 1
-            fi
-            sleep 10
-          done
-
-          exit_code=$(aws ecs describe-tasks --cluster "$CLUSTER" --tasks "$task_arn" \
-            --query "tasks[0].containers[?name=='${CONTAINER}']|[0].exitCode" --output text)
-          echo "migration container exit code: ${exit_code}"
-          if [ "$exit_code" != "0" ]; then
-            echo "::error::DB migration failed (exit ${exit_code}) — deploy blocked so the app never runs ahead of its schema."
-            aws ecs describe-tasks --cluster "$CLUSTER" --tasks "$task_arn" \
-              --query "tasks[0].{stopCode:stopCode,taskReason:stoppedReason,containerReason:containers[?name=='${CONTAINER}']|[0].reason}" \
-              --output table || true
-            echo "Migration logs: ${console_logs}"
-            exit 1
-          fi
-          echo "DB migration succeeded."
+        run: .github/scripts/run-gated-migration-task.sh ./scripts/run-migrations.sh
 
   deploy-prerender:
     name: Deploy prerender
@@ -364,8 +290,7 @@ jobs:
 
   deploy-worker:
     name: Deploy worker
-    needs:
-      [build-worker, deploy-host, migrate-db, deploy-prerender-manager]
+    needs: [build-worker, deploy-host, migrate-db, deploy-prerender-manager]
     uses: cardstack/gh-actions/.github/workflows/ecs-deploy.yml@main
     secrets: inherit
     with:
@@ -388,8 +313,7 @@ jobs:
 
   deploy-realm-server:
     name: Deploy realm server
-    needs:
-      [post-deploy-worker, build-realm-server, deploy-host, migrate-db]
+    needs: [post-deploy-worker, build-realm-server, deploy-host, migrate-db]
     uses: cardstack/gh-actions/.github/workflows/ecs-deploy.yml@main
     secrets: inherit
     with:
@@ -432,6 +356,62 @@ jobs:
             exit 1
           fi
 
+  # Destructive ("removal") migrations — column/table drops and renames, held in
+  # packages/postgres/migrations-removal/ — run HERE, after deploy-realm-server
+  # has reached stability. `wait-for-service-stability: true` on that job means
+  # the previous realm-server task set has fully drained (and, via its
+  # post-deploy-worker dependency, the previous worker task set too), so no task
+  # on the old code revision is left to query a column this phase drops. That is
+  # the whole point: running drops before the old tasks drain is what takes the
+  # published realms down (old code 500s on `column i.<x> does not exist`).
+  #
+  # A failure here is non-fatal to serving: the new code, by definition, no
+  # longer reads the removed columns, so it is already live and healthy — the
+  # drop simply hasn't happened yet and can be retried on the next deploy.
+  #
+  # Gate on deploy-realm-server, NOT post-deploy-realm-server: the latter is a
+  # separate /_post-deployment hook that can fail independently of the only real
+  # precondition here — the old tasks being gone.
+  migrate-db-remove:
+    needs: [build-pg-migration, deploy-realm-server]
+    name: Run DB removal migrations
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      id-token: write
+    env:
+      CLUSTER: ${{ inputs.environment }}
+      SERVICE: boxel-pg-migration-${{ inputs.environment }}
+      CONTAINER: boxel-pg-migration
+      IMAGE: ${{ needs.build-pg-migration.outputs.image }}
+      LOG_GROUP: ecs-boxel-pg-migration-${{ inputs.environment }}
+      # Bounds a migration that hangs (e.g. blocked on a lock); a long backfill
+      # migration must finish under it. Genuine failures exit immediately.
+      TIMEOUT_SECONDS: "1800"
+    steps:
+      - name: Check out
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+
+      - name: Set up env
+        run: |
+          if [ "${{ inputs.environment }}" = "production" ]; then
+            echo "AWS_ROLE_ARN=arn:aws:iam::120317779495:role/github" >> "$GITHUB_ENV"
+          elif [ "${{ inputs.environment }}" = "staging" ]; then
+            echo "AWS_ROLE_ARN=arn:aws:iam::680542703984:role/github" >> "$GITHUB_ENV"
+          else
+            echo "unrecognized environment: ${{ inputs.environment }}"
+            exit 1
+          fi
+
+      - name: Configure AWS credentials
+        uses: aws-actions/configure-aws-credentials@v4
+        with:
+          role-to-assume: ${{ env.AWS_ROLE_ARN }}
+          aws-region: us-east-1
+
+      - name: Run removal migrations and gate on exit code
+        run: .github/scripts/run-gated-migration-task.sh ./scripts/run-migrations.sh migrations-removal migrations_removal
+
   recycle-prerender:
     name: Recycle prerender after host is live
     # The prerender fleet deploys before the realm server (the manager,
@@ -500,6 +480,7 @@ jobs:
         post-deploy-worker,
         deploy-realm-server,
         post-deploy-realm-server,
+        migrate-db-remove,
         recycle-prerender,
         apply-observability,
       ]