fix(doctor): report every migration history serve would refuse

CHANGELOG.md

@@ -40,6 +40,13 @@ not a production-readiness statement.
   and passwords are byte-for-byte compatible, and a test pins the released
   migration checksums and the export container's bytes.
 
+### Fixed
+
+- `doctor` reports a migration history `serve` would refuse (a changed
+  checksum or a failed migration), instead of `ok`. Pending migrations,
+  which the service applies on its next start, now warn instead of failing
+  ([#138](https://github.com/FieldmouseWorks/consolebook/issues/138)).
+
 ## [0.2.0] - 2026-10-15
 
 Safer upgrades and recovery for a pilot: a backup before every migration,

README.md

@@ -110,8 +110,11 @@ authentication, and [ADR 0005](docs/decisions/0005-embedded-web-interface.md)
 for the embedded interface.
 
 `doctor` opens the database read-only and reports journal mismatches without
-repairing them. SQLite may create WAL sidecars or update shared-memory
-coordination; read-only storage without usable sidecars can prevent diagnosis.
+repairing them. It also reports every recorded migration that `serve` would
+refuse to start on: a version this build does not know, a changed checksum,
+or a migration recorded as unsuccessful. SQLite may create WAL sidecars or
+update shared-memory coordination; read-only storage without usable sidecars
+can prevent diagnosis.
 Its connection-local PRAGMA checks describe the diagnostic connection.
 See [ADR 0016](docs/decisions/0016-read-only-diagnostics.md) for the precise contract.
 

crates/consolebook-server/src/doctor.rs

@@ -165,49 +165,52 @@ async fn check_invariants(pool: &SqlitePool, findings: &mut Vec<Finding>) {
     }
 }
 
+/// Reports each recorded migration `serve` would refuse to start on, using
+/// the same judgment as startup's own check ([`storage::migration_history`]).
+/// What the service does by itself on its next start (create the history,
+/// apply pending migrations) is a warning, never a failure: `doctor` fails
+/// only where `serve` would refuse.
 async fn check_migrations(pool: &SqlitePool, findings: &mut Vec<Finding>) {
-    let applied: Result<Vec<i64>, sqlx::Error> =
-        sqlx::query_scalar("SELECT version FROM _sqlx_migrations ORDER BY version")
-            .fetch_all(pool)
-            .await;
-    match applied {
-        Err(err) => findings.push(Finding::fail(
-            "migrations",
-            format!("cannot read migration history: {err}"),
-        )),
-        Ok(applied) => {
-            let expected: Vec<i64> = storage::MIGRATOR.iter().map(|m| m.version).collect();
-            let missing: Vec<String> = expected
-                .iter()
-                .filter(|v| !applied.contains(v))
-                .map(ToString::to_string)
-                .collect();
-            let unknown: Vec<String> = applied
-                .iter()
-                .filter(|v| !expected.contains(v))
-                .map(ToString::to_string)
-                .collect();
-            if missing.is_empty() && unknown.is_empty() {
-                findings.push(Finding::ok(
-                    "migrations",
-                    format!("{} applied, up to date", applied.len()),
-                ));
-            } else if !missing.is_empty() {
-                findings.push(Finding::fail(
-                    "migrations",
-                    format!("pending: {}", missing.join(", ")),
-                ));
-            } else {
-                findings.push(Finding::fail(
-                    "migrations",
-                    format!(
-                        "database has migrations this build does not know: {}",
-                        unknown.join(", ")
-                    ),
-                ));
-            }
+    const CHECK: &str = "migrations";
+    const NO_HISTORY: &str = "no migration history yet; the service creates it and applies every migration on its first start";
+    let history = match storage::migration_history(pool).await {
+        Err(err) => {
+            findings.push(Finding::fail(
+                CHECK,
+                format!("cannot read migration history: {err:#}"),
+            ));
+            return;
+        }
+        Ok(history) => history,
+    };
+    if !history.refusals.is_empty() {
+        // The service will not start, so nothing pending would be applied.
+        for refusal in &history.refusals {
+            findings.push(Finding::fail(CHECK, refusal.to_string()));
         }
+        return;
+    }
+    if history.recorded == 0 {
+        // Pristine: anything else is a `MissingHistory` refusal above.
+        findings.push(Finding::warn(CHECK, NO_HISTORY));
+        return;
+    }
+    if history.unapplied.is_empty() {
+        findings.push(Finding::ok(
+            CHECK,
+            format!("{} applied, up to date", history.recorded),
+        ));
+        return;
     }
+    let pending: Vec<String> = history.unapplied.iter().map(ToString::to_string).collect();
+    findings.push(Finding::warn(
+        CHECK,
+        format!(
+            "{} migration(s) pending ({}); the service applies them on its next start, after taking a validated backup",
+            pending.len(),
+            pending.join(", ")
+        ),
+    ));
 }
 
 async fn check_integrity(pool: &SqlitePool, findings: &mut Vec<Finding>) {

crates/consolebook-server/src/storage.rs

@@ -85,11 +85,16 @@ async fn connect(db_path: &Path, create: bool) -> Result<SqlitePool> {
 pub async fn open(db_path: &Path) -> Result<SqlitePool> {
     let pool = connect(db_path, true).await?;
 
-    let newer = newer_migrations(&pool).await?;
-    if let Some(version) = newer.first() {
-        bail!(
-            "this database was upgraded by a newer Consolebook (it has migration {version}, which this version does not know); install that release again, or restore the snapshot taken before the upgrade (docs/install.md, \"Upgrading\")"
-        );
+    // Refusals `SQLx` would not word plainly, or would reach only after
+    // trying to migrate. It words the others itself.
+    let history = migration_history(&pool).await?;
+    if let Some(refusal) = history.refusals.iter().find(|refusal| {
+        matches!(
+            refusal,
+            RefusedHistory::Newer(_) | RefusedHistory::MissingHistory
+        )
+    }) {
+        bail!("{refusal}");
     }
 
     MIGRATOR
@@ -119,7 +124,14 @@ pub async fn open(db_path: &Path) -> Result<SqlitePool> {
 /// Empty for a database that is up to date, and for one that has never been
 /// migrated (a new installation), which has nothing to lose. Reads only.
 pub async fn pending_migrations(pool: &SqlitePool) -> Result<Vec<i64>> {
-    let applied = applied_migrations(pool).await?;
+    let Some(recorded) = recorded_history(pool).await? else {
+        return Ok(Vec::new());
+    };
+    let applied: Vec<i64> = recorded
+        .iter()
+        .filter(|row| row.success)
+        .map(|row| row.version)
+        .collect();
     if applied.is_empty() {
         return Ok(Vec::new());
     }
@@ -131,32 +143,158 @@ pub async fn pending_migrations(pool: &SqlitePool) -> Result<Vec<i64>> {
         .collect())
 }
 
-/// Applied migrations this executable does not know: the database was
-/// upgraded by a newer release. Reads only.
-pub async fn newer_migrations(pool: &SqlitePool) -> Result<Vec<i64>> {
-    Ok(applied_migrations(pool)
-        .await?
-        .into_iter()
-        .filter(|version| MIGRATOR.iter().all(|known| known.version != *version))
-        .collect())
+/// A migration history this executable refuses to start on. [`open`]
+/// refuses each of these, through its own check or `SQLx`'s migrator
+/// (`sqlx-core` 0.9 `Migrator::run_direct`): a version it does not embed
+/// (`ignore_missing` is off), a row recorded as unsuccessful, a stored
+/// checksum that differs from the embedded migration's, or no recorded
+/// history in a database that already holds a schema (migration 1 cannot
+/// run over existing tables).
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum RefusedHistory {
+    /// No migration is recorded (the table is missing or empty), yet the
+    /// database already holds schema objects of its own.
+    MissingHistory,
+    /// A newer release applied a migration this build does not embed.
+    Newer(i64),
+    /// The row is recorded as not successful. `SQLx` 0.9 records only
+    /// successes, so this comes from a hand edit or another tool.
+    Unfinished(i64),
+    /// The stored checksum differs from this build's migration.
+    Changed(i64),
+}
+
+impl std::fmt::Display for RefusedHistory {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::MissingHistory => write!(
+                f,
+                "this database's migration history is missing (no migration is recorded) while it already holds data, and the service refuses to start on it: migration 1 cannot run over the existing tables; restore the snapshot taken before the upgrade, or the newest backup of this installation (docs/install.md, \"Upgrading\")"
+            ),
+            Self::Newer(version) => write!(
+                f,
+                "this database was upgraded by a newer Consolebook (it has migration {version}, which this version does not know); install that release again, or restore the snapshot taken before the upgrade (docs/install.md, \"Upgrading\")"
+            ),
+            Self::Unfinished(version) => write!(
+                f,
+                "migration {version} is recorded as unsuccessful, and the service refuses to start on it; restore the snapshot taken before the upgrade (docs/install.md, \"Upgrading\")"
+            ),
+            Self::Changed(version) => write!(
+                f,
+                "migration {version} was applied by a different build's version of it (its recorded checksum differs from this build's), and the service refuses to start on it; restore the snapshot taken before the upgrade, or a backup taken while this version was running the service, or run the build that applied it (docs/install.md, \"Upgrading\")"
+            ),
+        }
+    }
+}
+
+/// The recorded migration history, judged against this build. Reads only.
+#[derive(Debug)]
+pub struct MigrationHistory {
+    /// Rows recorded in `_sqlx_migrations`, successful or not; zero when the
+    /// table is missing.
+    pub recorded: usize,
+    /// Every recorded migration startup would refuse, in version order.
+    pub refusals: Vec<RefusedHistory>,
+    /// Embedded migrations with no recorded row, which startup applies.
+    pub unapplied: Vec<i64>,
 }
 
-/// Successfully applied migration versions, in order; empty when the
-/// database has never been migrated.
-async fn applied_migrations(pool: &SqlitePool) -> Result<Vec<i64>> {
+/// Judges the recorded history against this build. Startup ([`open`])
+/// refuses on its refusals and `doctor` reports them. Reads only, so
+/// `doctor` can use it (ADR 0016).
+///
+/// With no recorded migration, a database is pristine only when it holds no
+/// schema objects besides `SQLite`'s own and the migrations table; anything
+/// else is [`RefusedHistory::MissingHistory`].
+pub async fn migration_history(pool: &SqlitePool) -> Result<MigrationHistory> {
+    let recorded = recorded_history(pool).await?.unwrap_or_default();
+    let refusals = if recorded.is_empty() {
+        let objects: i64 = sqlx::query_scalar(
+            "SELECT COUNT(*) FROM sqlite_master
+             WHERE name NOT LIKE 'sqlite\\_%' ESCAPE '\\' AND name != '_sqlx_migrations'",
+        )
+        .fetch_one(pool)
+        .await
+        .context("looking for an existing schema")?;
+        if objects == 0 {
+            Vec::new()
+        } else {
+            vec![RefusedHistory::MissingHistory]
+        }
+    } else {
+        refusals(&recorded)
+    };
+    let unapplied = MIGRATOR
+        .iter()
+        .filter(|migration| migration.migration_type.is_up_migration())
+        .map(|migration| migration.version)
+        .filter(|version| recorded.iter().all(|row| row.version != *version))
+        .collect();
+    Ok(MigrationHistory {
+        recorded: recorded.len(),
+        refusals,
+        unapplied,
+    })
+}
+
+/// One row of `_sqlx_migrations`.
+struct RecordedMigration {
+    version: i64,
+    checksum: Vec<u8>,
+    success: bool,
+}
+
+/// Every row startup would refuse, in version order. Each row gets the
+/// refusal `SQLx` would give it (unsuccessful first, then a version this
+/// build does not embed, then an up migration's checksum). Startup stops
+/// at its first refusal, the lowest unsuccessful row before any unknown
+/// version before any changed checksum; `doctor` reports them all.
+fn refusals(recorded: &[RecordedMigration]) -> Vec<RefusedHistory> {
+    recorded
+        .iter()
+        .filter_map(|row| {
+            if !row.success {
+                return Some(RefusedHistory::Unfinished(row.version));
+            }
+            if !MIGRATOR.version_exists(row.version) {
+                return Some(RefusedHistory::Newer(row.version));
+            }
+            MIGRATOR
+                .iter()
+                .filter(|migration| migration.migration_type.is_up_migration())
+                .find(|migration| migration.version == row.version)
+                .filter(|migration| *migration.checksum != *row.checksum)
+                .map(|_| RefusedHistory::Changed(row.version))
+        })
+        .collect()
+}
+
+/// Every recorded migration row, in version order; `None` when the database
+/// has never been migrated.
+async fn recorded_history(pool: &SqlitePool) -> Result<Option<Vec<RecordedMigration>>> {
     let tracked: Option<String> = sqlx::query_scalar(
         "SELECT name FROM sqlite_master WHERE type = 'table' AND name = '_sqlx_migrations'",
     )
     .fetch_optional(pool)
     .await
     .context("looking for the migrations table")?;
     if tracked.is_none() {
-        return Ok(Vec::new());
+        return Ok(None);
     }
-    sqlx::query_scalar("SELECT version FROM _sqlx_migrations WHERE success = 1 ORDER BY version")
-        .fetch_all(pool)
-        .await
-        .context("reading applied migrations")
+    let rows: Vec<(i64, Vec<u8>, bool)> =
+        sqlx::query_as("SELECT version, checksum, success FROM _sqlx_migrations ORDER BY version")
+            .fetch_all(pool)
+            .await
+            .context("reading applied migrations")?;
+    Ok(Some(
+        rows.into_iter()
+            .map(|(version, checksum, success)| RecordedMigration {
+                version,
+                checksum,
+                success,
+            })
+            .collect(),
+    ))
 }
 
 /// Opens an existing database without creating one and without migrating.