fix(lagoon): warn instead of failing when the re-read after a successful migrate fails (WR-13)

modules/lagoon/README.md

@@ -256,7 +256,7 @@ storage:
 
 | Command | Flags | Description |
 |---------|-------|-------------|
-| `migrate` | `--pretend`, `--force` | Runs the framework migrations (`system_files`, the backend admin tables, `deferred_bindings` and the job queue), then each plugin's migrations in dependency order; prints `<set>: N applied` per set and `Applied N migrations`, or `Nothing to migrate`. The applied lines count what was actually applied: when a migration fails, `migrate` prints the lines for the sets applied before it, then the error, and no `Applied` line. `--pretend` prints each pending migration as `<plugin>: <id>` and `N migrations would run`, and applies nothing. In production on an interactive terminal it asks first (`--force` skips the question); off a terminal it never asks. |
+| `migrate` | `--pretend`, `--force` | Runs the framework migrations (`system_files`, the backend admin tables, `deferred_bindings` and the job queue), then each plugin's migrations in dependency order; prints `<set>: N applied` per set and `Applied N migrations`, or `Nothing to migrate`. The applied lines count what was actually applied: when a migration fails, `migrate` prints the lines for the sets applied before it, then the error, and no `Applied` line. If the history cannot be re-read after a successful run, `migrate` warns and counts every pending migration as applied (exit 0). `--pretend` prints each pending migration as `<plugin>: <id>` and `N migrations would run`, and applies nothing. In production on an interactive terminal it asks first (`--force` skips the question); off a terminal it never asks. |
 | `migrate:rollback` | `--plugin <id>`, `--pretend`, `--force` | Rolls back the last applied migration of the given plugin; without the flag, of the last activated plugin that has migrations. Prints `Rolled back <id> of <plugin>`; `--pretend` names the migration and changes nothing. Production confirmation as for `migrate`. |
 | `migrate:status` | `--json` | Prints a table of the framework's own migration sets and every plugin with migrations, with history table and applied migration IDs. `--json` prints a JSON array of every declared migration of those sets: `plugin`, `table`, `id`, `applied`. |
 | `deferred:purge` | `--days <n>`, `--pretend`, `--force` | Deletes deferred bindings older than `n` days (default `database.deferred_bindings.purge_days`, else 5), with the unattached uploads they hold (blobs after the commit) and the records created under deferral, and prints the counts. Records that were only linked are kept. Model types come from every plugin's `pact.HasModels`; the uploads bucket from the published one or `storage.uploads.bucket_url`. `--pretend` counts the bindings past the cut-off and deletes nothing; production confirmation as for `migrate`. |

modules/lagoon/commands.go

@@ -174,11 +174,22 @@ asks.`,
 					// Report what Migrate actually applied: the pending
 					// migrations before it ran that are no longer pending,
 					// also when it failed partway.
+					applied := pending
 					after, err := PendingMigrations(gdb, plugins)
-					if err != nil {
+					switch {
+					case err == nil:
+						applied = appliedSince(pending, after)
+					case migrateErr != nil:
 						return errors.Join(migrateErr, err)
+					default:
+						// Migrate succeeded, so every pending migration is
+						// applied and the schema change is in place; only
+						// the read for the report failed. Failing the run
+						// now would make a deploy roll back or alert over
+						// a database that is up to date, so warn and count
+						// the pending list instead.
+						out.Warning("Could not re-read the migration history: " + bonfire.Sanitize(err.Error()))
 					}
-					applied := appliedSince(pending, after)
 					counts := map[string]int{}
 					var order []string
 					for _, ref := range applied {

modules/lagoon/pending_test.go

@@ -391,3 +391,54 @@ func TestMigrateAppliedLines(t *testing.T) {
 		t.Errorf("third migrate:\n%s", out)
 	}
 }
+
+// TestMigrateRereadFailureWarns (WR-13): when Migrate succeeds but reading
+// the history afterwards fails, migrate warns, reports every migration that
+// was pending as applied and exits 0: the schema change is in place.
+func TestMigrateRereadFailureWarns(t *testing.T) {
+	db, dsn := dedicatedDB(t, "lagoon_reread_warns")
+	gdb, err := Use(t.Context(), db)
+	if err != nil {
+		t.Fatal(err)
+	}
+	firstHistory, err := HistoryTableName("demo.rereadfirst")
+	if err != nil {
+		t.Fatal(err)
+	}
+	first := migPlugin{id: "demo.rereadfirst", migrations: []*gormigrate.Migration{{
+		ID:       "202610220001_create_reread_first",
+		Migrate:  func(tx *gorm.DB) error { return tx.Exec(`CREATE TABLE reread_first (id BIGSERIAL PRIMARY KEY)`).Error },
+		Rollback: func(tx *gorm.DB) error { return tx.Exec(`DROP TABLE IF EXISTS reread_first`).Error },
+	}}}
+	// The last migration breaks the first plugin's history table, so the
+	// read after a successful Migrate fails.
+	last := migPlugin{id: "demo.rereadlast", migrations: []*gormigrate.Migration{{
+		ID: "202610220002_break_reread_history",
+		Migrate: func(tx *gorm.DB) error {
+			return tx.Exec(`ALTER TABLE ` + firstHistory + ` RENAME COLUMN id TO id_gone`).Error
+		},
+		Rollback: func(tx *gorm.DB) error {
+			return tx.Exec(`ALTER TABLE ` + firstHistory + ` RENAME COLUMN id_gone TO id`).Error
+		},
+	}}}
+	plugins := []party.Plugin{first, last}
+	before, err := PendingMigrations(gdb, plugins)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	out, errOut, code, _ := runLagoon(t, "local", dsn, plugins, "migrate")
+	if code != bonfire.ExitOK {
+		t.Fatalf("migrate: exit %d, want 0\n%s%s", code, out, errOut)
+	}
+	for _, want := range []string{
+		"Could not re-read the migration history",
+		"demo.rereadfirst: 1 applied",
+		"demo.rereadlast: 1 applied",
+		fmt.Sprintf("Applied %d migrations", len(before)),
+	} {
+		if !strings.Contains(out, want) {
+			t.Errorf("migrate output lacks %q:\n%s%s", want, out, errOut)
+		}
+	}
+}