fix(ci): stop the migration safety audit from passing on a branch it never read

.github/workflows/test-build.yml

@@ -104,11 +104,22 @@ jobs:
 
           echo "✅ All env flags are properly configured"
 
+      # One fetch for both base-ref audits, and no `|| true`: a swallowed fetch leaves
+      # the base ref absent, which neither audit can tell apart from a branch that
+      # changed nothing. The block-registry check at least degrades to a visible
+      # `⚠ … skipping` line; the migration audit printed `✓ No new migrations to
+      # check` and exited 0, clearing the only guard on production DDL.
+      #
+      # Depth stays at 1 — without a merge-base the migration audit diffs the two
+      # tips, which under `--diff-filter=AM` is exactly the migrations new here.
+      - name: Fetch base ref for diff-based audits
+        if: github.event_name == 'pull_request'
+        run: git fetch --depth=1 origin "${{ github.base_ref }}"
+
       - name: Check block registry invariants
         run: |
           if [ "${{ github.event_name }}" = "pull_request" ]; then
             BASE_REF="origin/${{ github.base_ref }}"
-            git fetch --depth=1 origin "${{ github.base_ref }}" 2>/dev/null || true
           else
             BASE_REF="HEAD~1"
           fi
@@ -130,7 +141,6 @@ jobs:
         run: |
           if [ "${{ github.event_name }}" = "pull_request" ]; then
             BASE_REF="origin/${{ github.base_ref }}"
-            git fetch --depth=1 origin "${{ github.base_ref }}" 2>/dev/null || true
           else
             BASE_REF="HEAD~1"
           fi

scripts/check-migrations-safety.test.ts

@@ -0,0 +1,43 @@
+import { execFile } from 'node:child_process'
+import path from 'node:path'
+import { promisify } from 'node:util'
+import { describe, expect, it } from 'vitest'
+
+const execFileAsync = promisify(execFile)
+const ROOT = path.resolve(import.meta.dirname, '..')
+const SCRIPT = path.join(ROOT, 'scripts/check-migrations-safety.ts')
+
+async function runAudit(
+  baseRef: string
+): Promise<{ code: number; stdout: string; stderr: string }> {
+  try {
+    const { stdout, stderr } = await execFileAsync('bun', ['run', SCRIPT, baseRef], { cwd: ROOT })
+    return { code: 0, stdout, stderr }
+  } catch (error) {
+    const failure = error as { code?: number; stdout?: string; stderr?: string }
+    return { code: failure.code ?? 1, stdout: failure.stdout ?? '', stderr: failure.stderr ?? '' }
+  }
+}
+
+describe('migration safety audit', () => {
+  /**
+   * The regression this guards: an unresolvable base ref made `git diff` fail, the
+   * failure was read as an empty file list, and the audit printed
+   * `✓ No new migrations to check` and exited 0 — green on a branch it never read.
+   * CI reached that state whenever its `git fetch ... || true` swallowed a failure.
+   */
+  it('fails loudly when the base ref cannot be diffed', async () => {
+    const { code, stderr } = await runAudit('origin/branch-that-does-not-exist')
+
+    expect(code).toBe(1)
+    expect(stderr).toContain('could not run')
+    expect(stderr).not.toContain('No new migrations to check')
+  }, 30_000)
+
+  it('passes against a real base ref with no new migrations', async () => {
+    const { code, stdout } = await runAudit('HEAD')
+
+    expect(code).toBe(0)
+    expect(stdout).toContain('No new migrations to check')
+  }, 30_000)
+})

scripts/check-migrations-safety.ts

@@ -390,6 +390,24 @@ function git(args: string[]): string | null {
   }
 }
 
+/**
+ * Raised when the base ref cannot be compared against `HEAD`.
+ *
+ * Distinct from "no migrations changed", which is the same empty list. Conflating
+ * the two is how this check came to pass on a branch it had never read: an
+ * unresolvable base made `git diff` fail, the failure became `[]`, and `[]`
+ * printed as `✓ No new migrations to check`.
+ */
+class BaseRefUnusableError extends Error {
+  constructor(readonly baseRef: string) {
+    super(
+      `Cannot diff against '${baseRef}'. The ref is missing, or was fetched without enough ` +
+        `history for a merge-base. Fetch it with full history before running this check.`
+    )
+    this.name = 'BaseRefUnusableError'
+  }
+}
+
 /** New migration files on this branch vs base, plus uncommitted ones locally. */
 function changedMigrationFiles(baseRef: string): string[] {
   const files = new Set<string>()
@@ -405,7 +423,9 @@ function changedMigrationFiles(baseRef: string): string[] {
     '--',
     MIGRATIONS_DIR,
   ])
-  if (committed === null) return [] // git unavailable → fail open (handled by caller)
+  /* Only a missing git binary is a legitimate skip, and `resolveFiles` detects that
+     separately. A diff that fails with git present means the ref is unusable. */
+  if (committed === null) throw new BaseRefUnusableError(baseRef)
   for (const f of committed.split('\n')) if (inDir(f)) files.add(f)
 
   const status = git(['status', '--porcelain', '--', MIGRATIONS_DIR])
@@ -442,16 +462,26 @@ async function resolveFiles(argv: string[]): Promise<string[] | null> {
     return (await listSqlFiles(path.resolve(dir))).map((f) => path.relative(ROOT, f))
   }
   const baseRef = argv.find((a) => !a.startsWith('--')) ?? 'origin/staging'
-  const files = changedMigrationFiles(baseRef)
-  if (files.length === 0 && git(['rev-parse', 'HEAD']) === null) {
+  /* Checked before the diff: without git there is nothing to compare, and that is the
+     one case where skipping is right. Every other failure must be loud. */
+  if (git(['rev-parse', 'HEAD']) === null) {
     console.warn('⚠ git unavailable — skipping migration safety check.')
     return null
   }
-  return files
+  return changedMigrationFiles(baseRef)
 }
 
 async function main() {
-  const files = await resolveFiles(process.argv.slice(2))
+  let files: string[] | null
+  try {
+    files = await resolveFiles(process.argv.slice(2))
+  } catch (error) {
+    if (error instanceof BaseRefUnusableError) {
+      console.error(`✗ Migration safety check could not run.\n  ${error.message}`)
+      process.exit(1)
+    }
+    throw error
+  }
   if (files === null) process.exit(0)
 
   if (files.length === 0) {