fix(migrations): use numeric ordering to allow 3-digit migrations

storage/migration.go

@@ -5,7 +5,6 @@ import (
 	"log/slog"
 	"path/filepath"
 	"slices"
-	"sort"
 	"strconv"
 	"strings"
 
@@ -73,6 +72,49 @@ func (m *DatabaseMigration) rollbackMigration(migration MigrationFile) error {
 	return err
 }
 
+// migrationID extracts the numeric id from a migration filename of the form
+// "<id>__<description>.yaml". It is the single source of truth for both
+// ordering migrations (sortMigrationKeys) and gating them against the applied
+// high-water mark (runMigrations), so the two can never disagree.
+//
+// The "__" separator is required: descriptions are snake_case, so a single "_"
+// is a word boundary, not an id boundary. A missing separator is a malformed
+// filename and is surfaced as an error rather than silently misparsed.
+func migrationID(key string) (int, error) {
+	prefix, _, ok := strings.Cut(key, "__")
+	if !ok {
+		return 0, fmt.Errorf("migration %q is missing the %q id separator", key, "__")
+	}
+	return strconv.Atoi(prefix)
+}
+
+// sortMigrationKeys orders migration filenames by their parsed numeric id
+// rather than lexically. Filenames are not fixed-width, so a 3-digit id like
+// "100__" sorts before "10__".."99__" under a plain string sort, while the
+// apply gate compares ids numerically. That mismatch makes the runner apply
+// 100/101 first on a fresh database, advance the numeric high-water mark past
+// 99, then silently skip every migration 10..99 (their ids fall below the
+// mark). Sorting by the same id the gate uses keeps run order and gate
+// consistent regardless of digit count. Ties — and keys whose prefix does not
+// parse — fall back to a lexical compare so ordering stays deterministic; an
+// unparseable id is surfaced by runMigrations when it reaches that key.
+func sortMigrationKeys(keys []string) {
+	slices.SortStableFunc(keys, func(a, b string) int {
+		idA, errA := migrationID(a)
+		idB, errB := migrationID(b)
+		if errA != nil || errB != nil {
+			return strings.Compare(a, b)
+		}
+		if idA != idB {
+			if idA < idB {
+				return -1
+			}
+			return 1
+		}
+		return strings.Compare(a, b)
+	})
+}
+
 func (m *DatabaseMigration) runMigrations(migrations map[string]MigrationFile) {
 	slog.Info("Getting last migration applied")
 	rollback := false
@@ -86,10 +128,10 @@ func (m *DatabaseMigration) runMigrations(migrations map[string]MigrationFile) {
 	for k := range migrations {
 		keys = append(keys, k)
 	}
-	sort.Strings(keys)
+	sortMigrationKeys(keys)
 
 	for _, k := range keys {
-		migrationId, err := strconv.Atoi(strings.Split(k, "__")[0])
+		migrationId, err := migrationID(k)
 		if err != nil {
 			logger.Fatal("failed to determine migration id", slog.Any("error", err))
 		}

storage/migration_internal_test.go

@@ -2,6 +2,7 @@ package storage
 
 import (
 	"errors"
+	"slices"
 	"testing"
 )
 
@@ -75,6 +76,56 @@ func TestRollbackMigrationStopsAtFirstFailure(t *testing.T) {
 	}
 }
 
+func TestSortMigrationKeysOrdersNumericallyNotLexically(t *testing.T) {
+	// The unsorted set deliberately mixes one-, two-, and three-digit ids,
+	// and includes two keys that share id 100 to exercise the tie-break.
+	// A lexical sort would place "100__" before "10__".."99__"; the numeric
+	// sort must not.
+	keys := []string{
+		"100__add_widgets", "2__add_users", "10__add_orders",
+		"9__add_carts", "100__add_audit", "1__init", "99__add_index",
+	}
+	sortMigrationKeys(keys)
+
+	want := []string{
+		"1__init", "2__add_users", "9__add_carts", "10__add_orders",
+		"99__add_index", "100__add_audit", "100__add_widgets",
+	}
+	if len(keys) != len(want) {
+		t.Fatalf("len = %d; want %d", len(keys), len(want))
+	}
+	for i, got := range keys {
+		if got != want[i] {
+			t.Fatalf("keys[%d] = %q; want %q (full order: %v)", i, got, want[i], keys)
+		}
+	}
+}
+
+func TestSortMigrationKeysFallsBackToLexicalForUnparseablePrefix(t *testing.T) {
+	// Keys whose prefix is not a number must still sort deterministically
+	// rather than panic; runMigrations reports the bad id when it gets there.
+	keys := []string{"10__ok", "bad__name", "2__ok", "another"}
+	sortMigrationKeys(keys)
+
+	// Numeric-prefixed keys keep numeric order among themselves; the rest are
+	// ordered lexically relative to whatever they compare against.
+	if got := slices.Index(keys, "2__ok"); got > slices.Index(keys, "10__ok") {
+		t.Fatalf("2__ok should precede 10__ok, got order %v", keys)
+	}
+}
+
+func TestMigrationIDRequiresDoubleUnderscoreSeparator(t *testing.T) {
+	// A single underscore is a snake_case word boundary inside the
+	// description, not an id boundary, so a name lacking "__" is malformed
+	// and must error loudly rather than misparse its prefix as an id.
+	if _, err := migrationID("1_init.yaml"); err == nil {
+		t.Fatal("expected error for filename without __ separator")
+	}
+	if got, err := migrationID("12__add_users.yaml"); err != nil || got != 12 {
+		t.Fatalf("migrationID = %d, %v; want 12, nil", got, err)
+	}
+}
+
 func TestGetMigrationFilesReturnsEmptyForUninitializedConfigFs(t *testing.T) {
 	// ConfigFs is a zero-value embed.FS unless the embedding
 	// application has wired its own in, so ReadDir fails