From 7d7b4337ada1e5e4ecf614c4cf63eb7942c67a83 Mon Sep 17 00:00:00 2001 From: Asen Lekov Date: Tue, 25 Aug 2026 09:50:55 +0300 Subject: [PATCH] fix(cli): refuse a newer database schema with an actionable error, not a bare version number MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Store::open keeps strict migration validation — an older binary must never run against a schema written by a newer one (undefined behavior on the billing-critical event log beats no startup exactly never). But the refusal now says what happened and how to recover: Error::SchemaNewer names the db path, the unknown migration, and both fixes (dira update, or pinning back to the version that wrote it), instead of sqlx VersionMissing respawn-looping under launchd with a cryptic number. Records DIRASH-0035 (ignore_missing rejected; strict is the correct half, the message was the broken half). Co-Authored-By: Claude Fable 5 Signed-off-by: Asen Lekov --- ...ema-is-refused-loudly-never-run-against.md | 51 ++++++++++++++ cli/core/src/lib.rs | 7 ++ cli/core/src/store.rs | 67 ++++++++++++++++++- 3 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 .zavet/decisions/DIRASH-0035-a-newer-schema-is-refused-loudly-never-run-against.md diff --git a/.zavet/decisions/DIRASH-0035-a-newer-schema-is-refused-loudly-never-run-against.md b/.zavet/decisions/DIRASH-0035-a-newer-schema-is-refused-loudly-never-run-against.md new file mode 100644 index 0000000..af1fb43 --- /dev/null +++ b/.zavet/decisions/DIRASH-0035-a-newer-schema-is-refused-loudly-never-run-against.md @@ -0,0 +1,51 @@ +--- +id: DIRASH-0035 +title: A newer schema is refused loudly, never run against +status: active +guards: + - cli/core/src/store.rs +checks: + - the refusal names the cause and the recovery :: cargo test -p dira-core --lib opening_a_newer_schema +origin: session +verified: true +--- + +## Decision + +`Store::open` keeps sqlx's **strict** migration validation: a database whose +`_sqlx_migrations` records a version this binary does not know refuses to +open. What changes is the error — sqlx's bare `VersionMissing()` is +replaced by `Error::SchemaNewer`, which names the database path, the unknown +migration, the cause ("last written by a newer dira/dirad"), and both +recovery paths (`dira update`, or `dira update --version `). `ignore_missing` is never set. + +## Why + +The downgrade scenario is real: `dira update --version ` is a +documented recovery flow, and a daemon that then meets a newer `dira.db` +respawn-loops under launchd with only sqlx's cryptic number in the log. +The tempting fix — `Migrator::set_ignore_missing(true)` — would make the +older binary run anyway, and that is the wrong trade. A newer migration may +add required columns, reshape a table, or change semantics the older +binary's SQL silently violates; on the event log that feeds billing +(engaged-time accounting) and identity (the keychain-fallback secret), a +clean, diagnosable startup refusal is strictly better than undefined +runtime behavior with green process state. The same doctrine already runs +through the codebase: `dira doctor` reports and never repairs +(DIRASH-0022), and a replacement daemon is never started on optimism +(D-0019). + +The respawn loop itself is not the bug — launchd restarting a daemon that +refuses to start is supervision working as configured. The bug was that +each attempt left no line a human could act on. The friendly refusal fixes +the actionable half without touching the correctness half. + +## Rejected + +- **`ignore_missing`** — trades the one failure mode that is safe (refusing + to start) for the one that is not (running against a schema written by a + future version). +- **A version-number gate in `meta`** — duplicates what `_sqlx_migrations` + already records; a second source of truth for schema state would need its + own repair story. diff --git a/cli/core/src/lib.rs b/cli/core/src/lib.rs index ff2a556..ed9c357 100644 --- a/cli/core/src/lib.rs +++ b/cli/core/src/lib.rs @@ -36,6 +36,13 @@ pub enum Error { Sqlx(Box), #[error("migration error: {0}")] Migrate(Box), + #[error( + "{db} was last written by a newer dira/dirad (it records schema migration {version}, \ + which this binary does not know). Refusing to run against a newer schema. \ + Update this binary (`dira update`), or restore the version that wrote the database \ + (`dira update --version `)." + )] + SchemaNewer { version: i64, db: String }, #[error("time formatting error: {0}")] Time(String), #[error("time parse error: {0}")] diff --git a/cli/core/src/store.rs b/cli/core/src/store.rs index 24f7de1..3992c40 100644 --- a/cli/core/src/store.rs +++ b/cli/core/src/store.rs @@ -65,7 +65,23 @@ impl Store { #[cfg(unix)] restrict_to_owner_0600(path); - sqlx::migrate!("./migrations").run(&pool).await?; + // Strict migration validation is deliberate (DIRASH-0035): a database + // last written by a NEWER binary records migrations this one has never + // heard of, and running against that schema anyway trades a clean + // startup refusal for undefined behavior on the billing-critical event + // log. We keep the refusal but make it actionable — the raw sqlx + // `VersionMissing` names a bare number and nothing else, and under + // launchd that cryptic line is all a respawn-looping daemon leaves + // behind. + if let Err(e) = sqlx::migrate!("./migrations").run(&pool).await { + if let sqlx::migrate::MigrateError::VersionMissing(v) = e { + return Err(Error::SchemaNewer { + version: v, + db: path.display().to_string(), + }); + } + return Err(e.into()); + } Ok(Self { pool }) } @@ -2721,6 +2737,55 @@ mod tests { ); } + /// A database last written by a NEWER binary records a migration this one + /// has never heard of. `Store::open` must refuse — strict validation is + /// deliberate (DIRASH-0035) — but with an error that names the recovery + /// path, not sqlx's bare `VersionMissing()`. + #[tokio::test] + async fn opening_a_newer_schema_refuses_with_an_actionable_error() { + let dir = std::env::temp_dir().join(format!( + "dira-newer-schema-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("test.db"); + + // Migrate a real store, then stamp a future migration version into + // `_sqlx_migrations` the way a newer binary would have. + let store = Store::open(&path).await.unwrap(); + sqlx::query( + "INSERT INTO _sqlx_migrations \ + (version, description, installed_on, success, checksum, execution_time) \ + VALUES (99999999999999, 'from the future', CURRENT_TIMESTAMP, 1, x'00', 0)", + ) + .execute(&store.pool) + .await + .unwrap(); + store.wal_checkpoint_truncate().await.unwrap(); + drop(store); + + let err = match Store::open(&path).await { + Ok(_) => panic!("must refuse to open a newer schema"), + Err(e) => e, + }; + match &err { + Error::SchemaNewer { version, db } => { + assert_eq!(*version, 99_999_999_999_999); + assert!(db.contains("test.db")); + } + other => panic!("expected SchemaNewer, got {other:?}"), + } + let msg = err.to_string(); + assert!(msg.contains("newer dira"), "names the cause: {msg}"); + assert!(msg.contains("dira update"), "names the recovery: {msg}"); + + std::fs::remove_dir_all(&dir).ok(); + } + /// A missing file must fail, not be created — `open` creates on demand, /// `open_readonly` is exclusively for probing something that may or may /// not already exist.