Check migrations by statement, catch enum value renames, and never pull during a rollback

The migration check read one line at a time, so a statement wrapped across
lines got past it: ADD COLUMN "x" on one line and TEXT NOT NULL on the next.
It now reads whole statements, cutting an ALTER TABLE at its commas, with
comments dropped and strings blanked so neither sets off or hides a rule, and
DDL inside a DO block is checked too. It also catches enum value renames
(ALTER TYPE ... RENAME VALUE), and column renames and additions written
without the COLUMN keyword. Node instead of bash, for the parsing; it flags
the same 9 statements in the existing migrations as before.

The rollback now starts the previous image with --pull never, so it always
runs the image it found on the host, never whatever a floating tag points at
in the registry by then.

.github/scripts/check-migrations.mjs

@@ -0,0 +1,170 @@
+#!/usr/bin/env node
+// Flags migration statements that break the version of the app still running.
+//
+//   node check-migrations.mjs <migration.sql>...
+//
+// A deploy migrates the database while the previous version may still be
+// serving, and a rollback runs the previous version against the new schema.
+// Both work only while a migration leaves the old code working. Adding tables
+// and nullable columns, or columns with a default, is fine. Dropping, renaming,
+// retyping or tightening something the old code uses is not: stop using it in
+// one release, and change it in a later one.
+//
+// Reads whole statements, not lines, so how a statement is wrapped changes
+// nothing. Prints GitHub annotations, and exits 1 when it finds any errors.
+import { existsSync, readFileSync } from 'node:fs'
+
+const ID = String.raw`(?:"(?:[^"]|"")*"|[A-Za-z_][\w$]*)`
+const NAME = String.raw`${ID}(?:\s*\.\s*${ID})?`
+const ALTER_TABLE = new RegExp(String.raw`\bALTER\s+TABLE\s+(?:IF\s+EXISTS\s+)?(?:ONLY\s+)?${NAME}\s*\*?\s*`, 'i')
+const ALTER_TYPE = new RegExp(String.raw`\bALTER\s+TYPE\s+${NAME}\s*`, 'i')
+const ALTER_COLUMN = String.raw`^ALTER\s+(?:COLUMN\s+)?${ID}\s+`
+
+const RUNNING = 'The version still running'
+const TABLE_RULES = [
+  [/^DROP\s+(?!CONSTRAINT\b)/i, 'Drops a column', `${RUNNING} selects this column; Prisma reads every column it knows, so every query on the table fails.`],
+  [/^RENAME\s+TO\b/i, 'Renames a table', `${RUNNING} uses the old name.`],
+  [/^RENAME\s+(?!CONSTRAINT\b)/i, 'Renames a column', `${RUNNING} uses the old name. Add the new column, write both, and drop the old one in a later release.`],
+  [new RegExp(`${ALTER_COLUMN}(?:SET\\s+DATA\\s+)?TYPE\\b`, 'i'), 'Changes a column type', `${RUNNING} reads and writes the old type.`],
+  [new RegExp(`${ALTER_COLUMN}SET\\s+NOT\\s+NULL\\b`, 'i'), 'Makes a column required', `${RUNNING} may insert rows without it.`],
+  [new RegExp(`${ALTER_COLUMN}DROP\\s+DEFAULT\\b`, 'i'), 'Removes a column default', `${RUNNING} leaves the column out of inserts and relies on the default.`],
+]
+const ADD_COLUMN = /^ADD\s+(?!CONSTRAINT\b|PRIMARY\s+KEY\b|UNIQUE\b|FOREIGN\s+KEY\b|CHECK\b|EXCLUDE\b)/i
+const TYPE_RULES = [
+  [/^RENAME\s+VALUE\b/i, 'Renames an enum value', `${RUNNING} reads and writes the old value.`],
+  [/^RENAME\b/i, 'Renames a type', `${RUNNING} uses the old name. For an enum, this is usually a value being removed or renamed.`],
+]
+
+/**
+ * Cuts SQL into clauses, each with the source line of every character:
+ * statements end at `;`, and an ALTER TABLE or ALTER TYPE is cut again at each
+ * top-level comma, so every action in it is one clause however it is wrapped.
+ * Comments are dropped and string literals blanked, so neither can set off or
+ * hide a rule. Dollar quotes are read through, so DDL inside a DO block is
+ * checked like any other.
+ */
+function clauses(sql) {
+  const out = []
+  let text = ''
+  let lines = []
+  let line = 1
+  let depth = 0
+  let kind = null // 'table' or 'type' once the statement is known to alter one
+  const put = (s) => {
+    text += s
+    for (let k = 0; k < s.length; k++) lines.push(line)
+  }
+  const kindSoFar = () => kind ?? (ALTER_TABLE.test(text) ? 'table' : ALTER_TYPE.test(text) ? 'type' : null)
+  const flush = (endsStatement) => {
+    if (text.trim()) {
+      const clauseKind = kindSoFar()
+      out.push({ text, lines, kind: clauseKind })
+      kind = clauseKind
+    }
+    text = ''
+    lines = []
+    if (endsStatement) {
+      kind = null
+      depth = 0
+    }
+  }
+
+  for (let i = 0; i < sql.length; i++) {
+    const c = sql[i]
+    const next = sql[i + 1]
+    if (c === '\n') {
+      put(' ')
+      line++
+    } else if (c === '-' && next === '-') {
+      while (i + 1 < sql.length && sql[i + 1] !== '\n') i++
+      put(' ')
+    } else if (c === '/' && next === '*') {
+      i += 2
+      while (i < sql.length && !(sql[i] === '*' && sql[i + 1] === '/')) {
+        if (sql[i] === '\n') line++
+        i++
+      }
+      i++
+      put(' ')
+    } else if (c === "'") {
+      const startLine = line
+      i++
+      while (i < sql.length && !(sql[i] === "'" && sql[i + 1] !== "'")) {
+        if (sql[i] === '\n') line++
+        i += sql[i] === "'" ? 2 : 1
+      }
+      const endLine = line
+      line = startLine
+      put("''")
+      line = endLine
+    } else if (c === '"') {
+      let j = i + 1
+      while (j < sql.length && !(sql[j] === '"' && sql[j + 1] !== '"')) j += sql[j] === '"' ? 2 : 1
+      put(sql.slice(i, j + 1))
+      i = j
+    } else if (c === '$' && /^\$(?:[A-Za-z_]\w*)?\$/.test(sql.slice(i, i + 64))) {
+      i += /^\$(?:[A-Za-z_]\w*)?\$/.exec(sql.slice(i, i + 64))[0].length - 1
+      put(' ')
+    } else if (c === ';') {
+      flush(true)
+    } else if (c === ',' && depth === 0 && kindSoFar()) {
+      flush(false)
+    } else {
+      if (c === '(') depth++
+      if (c === ')') depth--
+      put(c)
+    }
+  }
+  flush(true)
+  return out
+}
+
+let errors = 0
+let warnings = 0
+function flag(level, file, line, title, message) {
+  console.log(`::${level} file=${file},line=${line},title=${title}::${message}`)
+  if (level === 'error') errors++
+  else warnings++
+}
+
+function check(file) {
+  for (const clause of clauses(readFileSync(file, 'utf8'))) {
+    const { text, lines } = clause
+    const lineAt = (index) => lines[index] ?? lines[0]
+
+    const dropTable = /\bDROP\s+TABLE\b/i.exec(text)
+    if (dropTable) flag('error', file, lineAt(dropTable.index), 'Drops a table', `${RUNNING} may read or write this table.`)
+    if (!clause.kind) continue
+
+    // The action: what follows ALTER TABLE <name>, or the whole clause after a comma.
+    const head = (clause.kind === 'table' ? ALTER_TABLE : ALTER_TYPE).exec(text)
+    const from = head ? head.index + head[0].length : text.search(/\S/)
+    const action = text.slice(from)
+    const rules = clause.kind === 'table' ? TABLE_RULES : TYPE_RULES
+    const rule = rules.find(([re]) => re.test(action))
+    if (rule) {
+      flag('error', file, lineAt(from), rule[1], rule[2])
+    } else if (clause.kind === 'table' && ADD_COLUMN.test(action)) {
+      const required = /\bNOT\s+NULL\b|\bPRIMARY\s+KEY\b/i.test(action)
+      const filled = /\bDEFAULT\b|\bGENERATED\b/i.test(action)
+      if (required && !filled) {
+        flag('error', file, lineAt(from), 'Adds a required column with no default', `${RUNNING} inserts rows without it. Give it a default, or add it nullable and require it in a later release.`)
+      }
+    } else if (clause.kind === 'type' && /^ADD\s+VALUE\b/i.test(action)) {
+      flag('warning', file, lineAt(from), 'Adds an enum value', `${RUNNING} cannot read rows that hold the new value. Fine as long as nothing writes it before the deploy is done, and nothing needs a rollback after.`)
+    }
+  }
+}
+
+const files = process.argv.slice(2)
+for (const file of files) if (existsSync(file)) check(file)
+
+if (errors > 0) {
+  console.log()
+  console.log(`${errors} statement(s) would break the version still running during a deploy, and the one a rollback puts back.`)
+  console.log('Split the change across releases. If it is safe anyway (nothing uses what is dropped, say), add the')
+  console.log('allow-breaking-migration label to the pull request and say why in its description.')
+  process.exit(1)
+}
+const note = warnings > 0 ? ` (${warnings} warning(s) above)` : ''
+console.log(`Checked ${files.length} migration file(s): nothing that breaks the running version${note}.`)

.github/scripts/check-migrations.sh

@@ -1,72 +0,0 @@
-#!/usr/bin/env bash
-# Flags migration statements that break the version of the app still running.
-#
-#   check-migrations.sh <migration.sql>...
-#
-# A deploy migrates the database while the previous version may still be
-# serving, and a rollback runs the previous version against the new schema.
-# Both work only while a migration leaves the old code working. Adding tables
-# and nullable columns, or columns with a default, is fine. Dropping, renaming,
-# retyping or tightening something the old code uses is not: stop using it in
-# one release, and change it in a later one.
-#
-# Prints GitHub annotations, and exits 1 when it finds any of the errors.
-set -uo pipefail
-
-S='[[:space:]]'
-errors=0
-warnings=0
-
-flag() { # level file line title message
-  echo "::$1 file=$2,line=$3,title=$4::$5"
-  if [ "$1" = "error" ]; then errors=$((errors + 1)); else warnings=$((warnings + 1)); fi
-}
-
-for file in "$@"; do
-  [ -f "$file" ] || continue
-  n=0
-  while IFS= read -r raw || [ -n "$raw" ]; do
-    n=$((n + 1))
-    line="${raw%%--*}" # drop comments
-    up="${line^^}"
-
-    if [[ $up =~ DROP$S+TABLE ]]; then
-      flag error "$file" "$n" "Drops a table" "The version still running may read or write this table."
-    fi
-    if [[ $up =~ DROP$S+COLUMN ]]; then
-      flag error "$file" "$n" "Drops a column" "The version still running selects this column; Prisma reads every column it knows, so every query on the table fails."
-    fi
-    if [[ $up =~ RENAME$S+COLUMN ]]; then
-      flag error "$file" "$n" "Renames a column" "The version still running uses the old name. Add the new column, write both, and drop the old one in a later release."
-    fi
-    if [[ $up =~ ALTER$S+(TABLE|TYPE)$S.*RENAME$S+TO ]]; then
-      flag error "$file" "$n" "Renames a table or type" "The version still running uses the old name. For an enum, this is usually a value being removed or renamed."
-    fi
-    if [[ $up =~ ALTER$S+COLUMN$S+[^[:space:]]+$S+(SET$S+DATA$S+)?TYPE ]]; then
-      flag error "$file" "$n" "Changes a column type" "The version still running reads and writes the old type."
-    fi
-    if [[ $up =~ SET$S+NOT$S+NULL ]]; then
-      flag error "$file" "$n" "Makes a column required" "The version still running may insert rows without it."
-    fi
-    if [[ $up =~ ADD$S+COLUMN ]] && [[ $up =~ NOT$S+NULL ]] && ! [[ $up =~ DEFAULT ]]; then
-      flag error "$file" "$n" "Adds a required column with no default" "The version still running inserts rows without it. Give it a default, or add it nullable and require it in a later release."
-    fi
-    if [[ $up =~ DROP$S+DEFAULT ]]; then
-      flag error "$file" "$n" "Removes a column default" "The version still running leaves the column out of inserts and relies on the default."
-    fi
-    if [[ $up =~ ALTER$S+TYPE$S.*ADD$S+VALUE ]]; then
-      flag warning "$file" "$n" "Adds an enum value" "The version still running cannot read rows that hold the new value. Fine as long as nothing writes it before the deploy is done, and nothing needs a rollback after."
-    fi
-  done < "$file"
-done
-
-if [ "$errors" -gt 0 ]; then
-  echo
-  echo "$errors statement(s) would break the version still running during a deploy, and the one a rollback puts back."
-  echo "Split the change across releases. If it is safe anyway (nothing uses what is dropped, say), add the"
-  echo "allow-breaking-migration label to the pull request and say why in its description."
-  exit 1
-fi
-note=""
-[ "$warnings" -gt 0 ] && note=" ($warnings warning(s) above)"
-echo "Checked $# migration file(s): nothing that breaks the running version$note."

.github/scripts/roll-back-if-unhealthy.sh

@@ -65,7 +65,9 @@ if [ "$(docker image inspect --format '{{.Id}}' "$prev_ref" 2>/dev/null)" != "$p
 fi
 
 echo "::warning title=Rolling back::$new_tag did not get healthy. Putting back $prev_name."
-APP_TAG="$prev_tag" docker compose up -d
+# Never pull: the image was found on this host above, and a pull could bring
+# the failed release back under the same tag (latest, say) instead.
+APP_TAG="$prev_tag" docker compose up -d --pull never
 
 if bash "$here/wait-healthy.sh" "$container"; then
   echo "::error title=Rolled back::$new_tag did not get healthy, so $prev_name is running again. The database was not touched."

.github/workflows/migration-check.yml

@@ -4,7 +4,7 @@ name: Migration check
 # serving, and a rollback runs the previous version against the new schema.
 # Both work only while a migration leaves the old code working, so this fails
 # a pull request whose new migrations drop, rename, retype or tighten
-# something (see .github/scripts/check-migrations.sh). When such a change is
+# something (see .github/scripts/check-migrations.mjs). When such a change is
 # safe anyway, add the allow-breaking-migration label and say why in the
 # description; the label reruns this check.
 
@@ -33,7 +33,7 @@ jobs:
         if: ${{ !contains(github.event.pull_request.labels.*.name, 'allow-breaking-migration') }}
         run: |
           mapfile -t files < <(git diff --name-only --diff-filter=AM HEAD^1 HEAD -- 'prisma/migrations/*/migration.sql')
-          bash .github/scripts/check-migrations.sh "${files[@]}"
+          node .github/scripts/check-migrations.mjs "${files[@]}"
 
       - name: Allowed by label
         if: ${{ contains(github.event.pull_request.labels.*.name, 'allow-breaking-migration') }}