feat(database): reserve an ordinal band for package migrations, and defend it (#2430)

Guard rails ahead of the change that needs them. Nothing stages a package's
migrations yet; these are the four places that would renumber or delete one the
moment it existed, and installing them afterwards would ship a window in which
an application's own generator could delete a package's schema.

Migrations run in `readdirSync().sort()` order, so the leading ordinal IS the
run order. A package's tables carry foreign keys into the application's and
never the reverse, so a package's files must sort last. A reserved high band
rather than `max + 1`, because three ordinal computations would otherwise
invert that:

  `nextMigrationNumber` maxes over every file, so one staged file would drag
  every future application migration into the band behind it.

  `historicalBoundary` takes the maximum among unmarked files, and a staged
  file is unmarked, so `migrate:regenerate` would number the regenerated
  application corpus above the package's.

  `startAt` falls back to the maximum among preserved files, with the same
  result by a different route.

The fourth is a delete rather than a renumber. `preprocessSqliteMigrations`
prunes duplicate create-table files by keeping the earliest filename, and a
package's ordinal is high by construction, so it loses that comparison against
anything the generator emitted for the same table. `regenerateMigrationCorpus`
is the other one: under `replaceUnmarked` or `onlyExistingTables` it deletes
every file regardless of marker. A package's migrations are not this corpus's
to rewrite, and the package would not put them back, since it ships them rather
than generating them.

The band is ten digits so lexicographic and numeric order still agree, which
is the property the whole scheme rests on and which a test pins.

All four guards are no-ops on a corpus with no band files, which is every
corpus today.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: glennmichael123

storage/framework/core/database/src/index.ts

@@ -152,6 +152,7 @@ export * from './migration-ledger'
 // Model resolution for the generator: userland + framework defaults, flattened
 // because bun-query-builder's loadModels reads only the top level of a dir.
 export * from './model-sources'
+export * from './package-migrations'
 export * from './package-models'
 export * from './shadowed-models'
 

storage/framework/core/database/src/migrations.ts

@@ -8,6 +8,7 @@
 import type { UnsafeRowsResult } from './utils'
 import type { Result } from '@stacksjs/error-handling'
 import { existsSync, mkdirSync, readdirSync, readFileSync, renameSync, unlinkSync, writeFileSync } from 'node:fs'
+import { isPackageMigration } from './package-migrations'
 import { dirname, isAbsolute, join, resolve } from 'node:path'
 import { log as _log } from '@stacksjs/logging'
 
@@ -711,7 +712,11 @@ export function preprocessSqliteMigrations(): void {
     if (createTableMatch && createTableMatch[1]) {
       const tableName = createTableMatch[1]
       const earliest = createTableEarliest.get(tableName)
-      if (earliest && earliest !== file) {
+      // Never the package's copy. Its ordinal is high, so it loses an
+      // earliest-filename-wins comparison against anything the generator
+      // emitted for the same table - and deleting it would take the package's
+      // own schema with it.
+      if (earliest && earliest !== file && !isPackageMigration(file)) {
         deleteMigration(file, filePath, `duplicate create-table for "${tableName}" (kept ${earliest})`)
         continue
       }
@@ -3153,9 +3158,15 @@ export async function regenerateMigrationCorpus(options: {
       options.replaceUnmarked || options.onlyExistingTables ? [] : historicallyRootedTables(dir, existing),
     )
     const outOfScope = new Set(migrationsOutsideCorpus(dir, existing, createdTablesOf(statements)))
-    const deletable = options.replaceUnmarked || options.onlyExistingTables
+    // `replaceUnmarked` and `onlyExistingTables` delete every file regardless
+    // of marker. A package's migrations are not this corpus's to rewrite, and
+    // the package would not put them back: it ships them, it does not generate
+    // them. Excluded from the deletable set rather than rescued afterwards, so
+    // no later filter can put them back in.
+    const deletable = (options.replaceUnmarked || options.onlyExistingTables
       ? existing
-      : existing.filter(file => isGeneratedMigration(dir, file))
+      : existing.filter(file => isGeneratedMigration(dir, file)))
+      .filter(file => !isPackageMigration(file))
     const removed = deletable.filter(file => {
       return !outOfScope.has(file) && !migrationTouchesRootedTable(dir, file, rootedTables)
     })
@@ -3205,12 +3216,17 @@ export async function regenerateMigrationCorpus(options: {
     // such as SQLite table rebuilds. Skip their occupied ordinals instead of
     // renumbering them, since authored backfills may depend on those filenames'
     // relative position.
+    // Package files are unmarked and preserved, so both of these would other-
+    // wise take their maximum from the reserved band and number the whole
+    // regenerated application corpus above them.
     const historicalBoundary = existing
-      .filter(file => !isGeneratedMigration(dir, file))
+      .filter(file => !isGeneratedMigration(dir, file) && !isPackageMigration(file))
       .reduce((max, file) => Math.max(max, migrationOrdinal(file)), 0)
     const startAt = rootedTables.size > 0
       ? historicalBoundary + 1
-      : preserved.reduce((max, file) => Math.max(max, migrationOrdinal(file)), 0) + 1
+      : preserved
+          .filter(file => !isPackageMigration(file))
+          .reduce((max, file) => Math.max(max, migrationOrdinal(file)), 0) + 1
     const reservedOrdinals = new Set(preserved.map(migrationOrdinal))
     const ordinals = allocateMigrationOrdinals(writableGroups.length, startAt, reservedOrdinals)
 
@@ -3559,6 +3575,11 @@ function nextMigrationNumber(migrationsDir: string): number {
   let max = 0
   try {
     for (const f of readdirSync(migrationsDir)) {
+      // A package's migrations sit in a reserved high band. Counting them here
+      // would push the application's next ordinal into that band, and every
+      // application migration after it would then run AFTER the package's.
+      if (isPackageMigration(f))
+        continue
       const m = f.match(/^(\d+)-/)
       if (m?.[1]) max = Math.max(max, Number.parseInt(m[1], 10))
     }

storage/framework/core/database/src/package-migrations.ts

@@ -0,0 +1,34 @@
+/**
+ * The ordinal band reserved for migrations a discovered package brings.
+ *
+ * Migrations run in the order `readdirSync(dir).sort()` returns, so the leading
+ * ordinal in a filename IS the run order. A package's tables carry foreign keys
+ * into the application's (`user_id`, `team_id`) and never the reverse, because
+ * the application predates whatever it installed. A `REFERENCES "users"` on a
+ * table created before `users` fails on Postgres and MySQL while SQLite
+ * tolerates it, so getting this order wrong is green locally and red on deploy.
+ *
+ * A reserved high band rather than `max + 1`, because three separate ordinal
+ * computations would otherwise invert the order: `migrate:regenerate`
+ * renumbers the application corpus from 1 while preserving unmarked files,
+ * `historicalBoundary` takes the maximum ordinal among unmarked files, and
+ * `nextMigrationNumber` maxes over every file on disk. Each of them would
+ * either number an application migration above a package's or drag the whole
+ * application corpus up into the band.
+ *
+ * Still ten digits, so lexicographic order and numeric order agree.
+ */
+export const PACKAGE_MIGRATION_BAND = 9_000_000_000
+
+/**
+ * Whether a migration filename belongs to a discovered package.
+ *
+ * Read from the ordinal rather than from the file's contents: every guard that
+ * needs this answer is deciding whether to renumber or delete the file, and
+ * those run in loops over a directory listing where opening each file would be
+ * the expensive part.
+ */
+export function isPackageMigration(file: string): boolean {
+  const ordinal = /^(\d+)-/.exec(file)?.[1]
+  return ordinal !== undefined && Number.parseInt(ordinal, 10) >= PACKAGE_MIGRATION_BAND
+}

storage/framework/core/database/tests/package-migrations.test.ts

@@ -0,0 +1,50 @@
+/**
+ * The ordinal band reserved for a discovered package's migrations.
+ *
+ * Nothing stages package migrations yet. These guards land first on purpose:
+ * every one of them is a place that would renumber or delete a staged file the
+ * moment one existed, and installing them afterwards would mean shipping a
+ * window in which a package's schema could be deleted by the application's own
+ * generator.
+ *
+ * All four are no-ops on a corpus with no band files, which is every corpus
+ * today, so this is inert until the staging change arrives.
+ */
+import { describe, expect, test } from 'bun:test'
+import { isPackageMigration, PACKAGE_MIGRATION_BAND } from '../src/package-migrations'
+
+describe('the reserved package migration band', () => {
+  test('is ten digits, so lexicographic order still equals numeric order', () => {
+    // Run order is `readdirSync().sort()` on the basename. An eleven-digit
+    // ordinal would sort before a ten-digit one and the band would invert.
+    expect(String(PACKAGE_MIGRATION_BAND)).toHaveLength(10)
+    expect(String(PACKAGE_MIGRATION_BAND).padStart(10, '0')).toBe('9000000000')
+  })
+
+  test('sorts after every plausible application ordinal', () => {
+    const app = String(999_999).padStart(10, '0')
+    const pkg = String(PACKAGE_MIGRATION_BAND + 1).padStart(10, '0')
+
+    expect([`${pkg}-loghq.sql`, `${app}-create-users-table.sql`].sort())
+      .toEqual([`${app}-create-users-table.sql`, `${pkg}-loghq.sql`])
+  })
+
+  test('recognises a file in the band', () => {
+    expect(isPackageMigration('9000000001-loghq__create-log-entries-table.sql')).toBe(true)
+    expect(isPackageMigration('9999999999-bughq__create-issues-table.sql')).toBe(true)
+  })
+
+  test('leaves every application migration alone', () => {
+    expect(isPackageMigration('0000000001-create-users-table.sql')).toBe(false)
+    expect(isPackageMigration('0000000133-add-orthomosaic-to-missions.sql')).toBe(false)
+    expect(isPackageMigration('8999999999-still-the-application.sql')).toBe(false)
+  })
+
+  test('is not fooled by a filename carrying no ordinal', () => {
+    // The corpus has always been ordinal-prefixed, but a hand-written file or
+    // a stray `.sql` must not be mistaken for a package's and made undeletable.
+    expect(isPackageMigration('create-users-table.sql')).toBe(false)
+    expect(isPackageMigration('README.md')).toBe(false)
+    expect(isPackageMigration('')).toBe(false)
+  })
+})