From 467f30685bb4c458d7099331f6266a2a1e4177f1 Mon Sep 17 00:00:00 2001 From: Rita Agafonova <36133358+rita-aga@users.noreply.github.com> Date: Sun, 12 Jul 2026 22:27:46 -0700 Subject: [PATCH 1/7] test(store-turso): failing tests for swallowed migrations and the missing ledger (ARN-242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RED: thirteen let-underscore execute sites in migrate() discard every ALTER failure — a view-shadowed policies table makes one fail for a real (non-duplicate-column) reason and startup still reports success, serving a half-migrated schema; and there is no durable record of what schema version a database is at. Co-Authored-By: Claude Fable 5 --- .../temper-store-turso/src/store/tests/mod.rs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/crates/temper-store-turso/src/store/tests/mod.rs b/crates/temper-store-turso/src/store/tests/mod.rs index f75aa4c01..61b814896 100644 --- a/crates/temper-store-turso/src/store/tests/mod.rs +++ b/crates/temper-store-turso/src/store/tests/mod.rs @@ -2027,3 +2027,66 @@ async fn upsert_wasm_module_stores_metadata_only_without_db_blob() { "Turso store should return metadata-only rows for new WASM artifacts" ); } + +/// ARN-242: `migrate()` must SURFACE real migration errors, not swallow them. +/// +/// Thirteen `let _ = conn.execute(...)` sites discard every ALTER failure — +/// intended for benign duplicate-column errors, but they equally swallow +/// genuine ones. This poisons a DB so a swallowed ALTER fails for a REAL +/// reason (a view shadows the `policies` table, so `ALTER TABLE policies +/// ADD COLUMN enabled ...` cannot succeed): startup must fail loudly +/// instead of serving against a half-migrated schema. +#[tokio::test] +async fn migrate_surfaces_real_alter_errors() { + let url = sqlite_test_url("migrate-real-error"); + + // Poison the DB before the store ever runs its schema: a VIEW named + // `policies` (CREATE TABLE IF NOT EXISTS tolerates it silently, but the + // ALTER on it fails with a non-duplicate-column error). + { + let db = libsql::Builder::new_local(url.trim_start_matches("file:")) + .build() + .await + .expect("build poison db"); + let conn = db.connect().expect("connect poison db"); + conn.execute("CREATE TABLE policies_backing (id TEXT)", ()) + .await + .expect("backing table"); + conn.execute( + "CREATE VIEW policies AS SELECT id FROM policies_backing", + (), + ) + .await + .expect("shadow view"); + } + + let result = TursoEventStore::new(&url, None).await; + assert!( + result.is_err(), + "a migration statement failing for a real (non-duplicate-column) reason \ + must fail startup, not be silently swallowed" + ); +} + +/// ARN-242: `migrate()` must record what it applied in a durable version +/// ledger, so operators can see which schema a database is at and boots can +/// short-circuit already-migrated databases. +#[tokio::test] +async fn migrate_records_schema_version_ledger() { + let store = make_store("migrate-ledger").await; + let conn = store.connection().expect("connection"); + let mut rows = conn + .query("SELECT MAX(version) FROM temper_schema_migrations", ()) + .await + .expect("the schema version ledger table must exist after migrate()"); + let row = rows + .next() + .await + .expect("ledger row") + .expect("ledger must contain at least one applied version"); + let version: i64 = row.get(0).expect("version column"); + assert!( + version >= 1, + "the ledger must record the applied schema version, got {version}" + ); +} From 25975667ee5cdc101e3557291d3cfb168577a330 Mon Sep 17 00:00:00 2001 From: Rita Agafonova <36133358+rita-aga@users.noreply.github.com> Date: Sun, 12 Jul 2026 22:47:51 -0700 Subject: [PATCH 2/7] fix(store-turso): fail-closed migrations behind a schema version ledger (ARN-242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GREEN: execute_idempotent tolerates only the benign already-applied errors (duplicate column / already exists) and propagates everything else with the failing statement in the message — replacing thirteen let-underscore sites and two bespoke blocks that swallowed locked databases, disk errors, and shadowed tables alike, leaving half-migrated schemas in service. A durable temper_schema_migrations ledger records the applied version; stamped boots skip the ~88-statement DDL entirely (the turso suite dropped 33s to 3.8s from the reduced lock churn). The ledger check runs after the WAL/busy_timeout PRAGMAs and drains its statement — an undrained read lock before WAL deadlocked a concurrent writer during development. Coverage the review round demanded: migrate_is_idempotent now clears the ledger so the DDL genuinely re-runs, and the new migrate_upgrades_an_existing_unstamped_database proves the highest-risk path — an existing production database, fully migrated but unstamped, runs the whole baseline against a populated schema and every ALTER passes the fail-closed filter. ADR-0162 records the contract (bump SCHEMA_VERSION; migrate_platform is outside the ledger; every statement must be idempotent AND concurrent-boot safe) and the alternatives. schema.rs split into schema/migrations.rs and schema/trajectories.rs for the readability ceiling. Review trail: r1 FAIL (4 Important incl. the untested production-upgrade path and two regression tests the ledger had silently disarmed) → r2 FAIL (two artifacts still misdescribing themselves) → r3 PASS. Co-Authored-By: Claude Fable 5 --- crates/temper-store-turso/src/schema.rs | 120 +++----------- .../src/schema/migrations.rs | 22 +++ .../src/schema/trajectories.rs | 111 +++++++++++++ crates/temper-store-turso/src/store/mod.rs | 147 +++++++++++++----- .../temper-store-turso/src/store/tests/mod.rs | 70 +++++++-- .../temper-store-turso/tests/blob_ttl_e2e.rs | 9 +- docs/adrs/0162-turso-migration-ledger.md | 75 +++++++++ 7 files changed, 401 insertions(+), 153 deletions(-) create mode 100644 crates/temper-store-turso/src/schema/migrations.rs create mode 100644 crates/temper-store-turso/src/schema/trajectories.rs create mode 100644 docs/adrs/0162-turso-migration-ledger.md diff --git a/crates/temper-store-turso/src/schema.rs b/crates/temper-store-turso/src/schema.rs index 1012b2e03..a4ace3c0d 100644 --- a/crates/temper-store-turso/src/schema.rs +++ b/crates/temper-store-turso/src/schema.rs @@ -1,11 +1,16 @@ //! SQLite-compatible schema for the Turso/libSQL event store. +mod migrations; mod query_plane; +mod trajectories; pub use crate::schema_event_history::{ ALTER_EVENTS_ADD_SEGMENT_INDEX, CREATE_EVENT_SEGMENTS_OPEN_INDEX, CREATE_EVENT_SEGMENTS_TABLE, CREATE_SNAPSHOT_HISTORY_ENTITY_INDEX, CREATE_SNAPSHOT_HISTORY_TABLE, }; +pub use migrations::{ + CREATE_SCHEMA_MIGRATIONS_TABLE, INSERT_SCHEMA_VERSION, SELECT_SCHEMA_VERSION, +}; pub use query_plane::{ CREATE_ENTITY_CATALOG_STATUS_INDEX, CREATE_ENTITY_CATALOG_TABLE, CREATE_ENTITY_CATALOG_TYPE_INDEX, CREATE_ENTITY_FIELD_INDEX_LOOKUP, @@ -14,6 +19,20 @@ pub use query_plane::{ CREATE_ENTITY_VECTOR_INDEX_ENTITY, CREATE_ENTITY_VECTOR_INDEX_PARTITION, CREATE_ENTITY_VECTOR_INDEX_TABLE, CREATE_VECTOR_INDEX_BACKFILL_WATERMARK, }; +pub use trajectories::{ + ALTER_OTS_TRAJECTORIES_ADD_LAST_ERROR, ALTER_OTS_TRAJECTORIES_ADD_PERSIST_ATTEMPTS, + ALTER_OTS_TRAJECTORIES_ADD_PERSISTENCE_STATUS, ALTER_OTS_TRAJECTORIES_ADD_UPDATED_AT, + ALTER_TRAJECTORIES_ADD_AGENT_ID, ALTER_TRAJECTORIES_ADD_AUTHZ_DENIED, + ALTER_TRAJECTORIES_ADD_DENIED_MODULE, ALTER_TRAJECTORIES_ADD_DENIED_RESOURCE, + ALTER_TRAJECTORIES_ADD_INTENT, ALTER_TRAJECTORIES_ADD_MATCHED_POLICY_IDS, + ALTER_TRAJECTORIES_ADD_REQUEST_BODY, ALTER_TRAJECTORIES_ADD_SESSION_ID, + ALTER_TRAJECTORIES_ADD_SOURCE, ALTER_TRAJECTORIES_ADD_SPEC_GOVERNED, + CREATE_OTS_TRAJECTORIES_AGENT_INDEX, CREATE_OTS_TRAJECTORIES_OUTCOME_INDEX, + CREATE_OTS_TRAJECTORIES_STATUS_INDEX, CREATE_OTS_TRAJECTORIES_TABLE, + CREATE_OTS_TRAJECTORIES_TENANT_INDEX, CREATE_TRAJECTORIES_AGENT_INDEX, + CREATE_TRAJECTORIES_ENTITY_ACTION_INDEX, CREATE_TRAJECTORIES_SUCCESS_INDEX, + CREATE_TRAJECTORIES_TABLE, +}; pub const CREATE_EVENTS_TABLE: &str = "\ CREATE TABLE IF NOT EXISTS events ( @@ -61,28 +80,6 @@ CREATE TABLE IF NOT EXISTS specs ( UNIQUE(tenant, entity_type) );"; -pub const CREATE_TRAJECTORIES_TABLE: &str = "\ -CREATE TABLE IF NOT EXISTS trajectories ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - tenant TEXT NOT NULL, - entity_type TEXT NOT NULL, - entity_id TEXT NOT NULL, - action TEXT NOT NULL, - success INTEGER NOT NULL DEFAULT 0, - from_status TEXT, - to_status TEXT, - error TEXT, - created_at TEXT NOT NULL DEFAULT (datetime('now')) -);"; - -pub const CREATE_TRAJECTORIES_SUCCESS_INDEX: &str = "\ -CREATE INDEX IF NOT EXISTS idx_trajectories_success - ON trajectories(success);"; - -pub const CREATE_TRAJECTORIES_ENTITY_ACTION_INDEX: &str = "\ -CREATE INDEX IF NOT EXISTS idx_trajectories_entity_action - ON trajectories(tenant, entity_type, action);"; - pub const CREATE_TENANT_CONSTRAINTS_TABLE: &str = "\ CREATE TABLE IF NOT EXISTS tenant_constraints ( tenant TEXT NOT NULL PRIMARY KEY, @@ -277,35 +274,6 @@ pub const ALTER_SPECS_ADD_CONTENT_HASH: &str = "ALTER TABLE specs ADD COLUMN con pub const ALTER_SPECS_ADD_COMMITTED: &str = "ALTER TABLE specs ADD COLUMN committed INTEGER NOT NULL DEFAULT 1"; -/// ALTER TABLE migrations for the `trajectories` table. -/// -/// These add columns that were previously only tracked in-memory -/// (agent_id, session_id, authz_denied, etc.). Each statement uses -/// try-and-ignore semantics in SQLite (duplicate column is a no-op error). -pub const ALTER_TRAJECTORIES_ADD_AGENT_ID: &str = - "ALTER TABLE trajectories ADD COLUMN agent_id TEXT"; -pub const ALTER_TRAJECTORIES_ADD_SESSION_ID: &str = - "ALTER TABLE trajectories ADD COLUMN session_id TEXT"; -pub const ALTER_TRAJECTORIES_ADD_AUTHZ_DENIED: &str = - "ALTER TABLE trajectories ADD COLUMN authz_denied INTEGER"; -pub const ALTER_TRAJECTORIES_ADD_DENIED_RESOURCE: &str = - "ALTER TABLE trajectories ADD COLUMN denied_resource TEXT"; -pub const ALTER_TRAJECTORIES_ADD_DENIED_MODULE: &str = - "ALTER TABLE trajectories ADD COLUMN denied_module TEXT"; -pub const ALTER_TRAJECTORIES_ADD_SOURCE: &str = "ALTER TABLE trajectories ADD COLUMN source TEXT"; -pub const ALTER_TRAJECTORIES_ADD_SPEC_GOVERNED: &str = - "ALTER TABLE trajectories ADD COLUMN spec_governed INTEGER"; -pub const ALTER_TRAJECTORIES_ADD_REQUEST_BODY: &str = - "ALTER TABLE trajectories ADD COLUMN request_body TEXT"; -pub const ALTER_TRAJECTORIES_ADD_INTENT: &str = "ALTER TABLE trajectories ADD COLUMN intent TEXT"; -pub const ALTER_TRAJECTORIES_ADD_MATCHED_POLICY_IDS: &str = - "ALTER TABLE trajectories ADD COLUMN matched_policy_ids TEXT"; - -/// Index on agent_id for agent-scoped trajectory queries. -pub const CREATE_TRAJECTORIES_AGENT_INDEX: &str = "\ -CREATE INDEX IF NOT EXISTS idx_trajectories_agent - ON trajectories(agent_id);"; - /// Feature request records generated from trajectory analysis. pub const CREATE_FEATURE_REQUESTS_TABLE: &str = "\ CREATE TABLE IF NOT EXISTS feature_requests ( @@ -443,56 +411,6 @@ CREATE INDEX IF NOT EXISTS idx_blobs_expires_at ON blobs(expires_at) WHERE expir // OTS trajectory storage (full agent execution traces) // --------------------------------------------------------------------------- -/// Full OTS trajectory storage for GEPA self-improvement loop. -/// -/// Stores complete agent execution traces (tool calls, decisions, reasoning) -/// captured by the MCP server during agent sessions. The `data` column holds -/// the full OTS JSON blob; indexed columns enable efficient filtering. -pub const CREATE_OTS_TRAJECTORIES_TABLE: &str = "\ -CREATE TABLE IF NOT EXISTS ots_trajectories ( - trajectory_id TEXT PRIMARY KEY, - tenant TEXT NOT NULL, - agent_id TEXT NOT NULL, - session_id TEXT, - outcome TEXT NOT NULL DEFAULT 'unknown', - entity_type TEXT, - turn_count INTEGER NOT NULL DEFAULT 0, - data TEXT NOT NULL, - persistence_status TEXT NOT NULL DEFAULT 'persisted', - persist_attempts INTEGER NOT NULL DEFAULT 0, - last_error TEXT, - created_at TEXT NOT NULL DEFAULT (datetime('now')), - updated_at TEXT NOT NULL DEFAULT (datetime('now')) -);"; - -pub const ALTER_OTS_TRAJECTORIES_ADD_PERSISTENCE_STATUS: &str = "\ -ALTER TABLE ots_trajectories ADD COLUMN persistence_status TEXT NOT NULL DEFAULT 'persisted';"; - -pub const ALTER_OTS_TRAJECTORIES_ADD_PERSIST_ATTEMPTS: &str = "\ -ALTER TABLE ots_trajectories ADD COLUMN persist_attempts INTEGER NOT NULL DEFAULT 0;"; - -pub const ALTER_OTS_TRAJECTORIES_ADD_LAST_ERROR: &str = "\ -ALTER TABLE ots_trajectories ADD COLUMN last_error TEXT;"; - -pub const ALTER_OTS_TRAJECTORIES_ADD_UPDATED_AT: &str = "\ -ALTER TABLE ots_trajectories ADD COLUMN updated_at TEXT NOT NULL DEFAULT (datetime('now'));"; - -pub const CREATE_OTS_TRAJECTORIES_AGENT_INDEX: &str = "\ -CREATE INDEX IF NOT EXISTS idx_ots_trajectories_agent - ON ots_trajectories(agent_id);"; - -pub const CREATE_OTS_TRAJECTORIES_TENANT_INDEX: &str = "\ -CREATE INDEX IF NOT EXISTS idx_ots_trajectories_tenant - ON ots_trajectories(tenant);"; - -pub const CREATE_OTS_TRAJECTORIES_OUTCOME_INDEX: &str = "\ -CREATE INDEX IF NOT EXISTS idx_ots_trajectories_outcome - ON ots_trajectories(outcome);"; - -pub const CREATE_OTS_TRAJECTORIES_STATUS_INDEX: &str = "\ -CREATE INDEX IF NOT EXISTS idx_ots_trajectories_status - ON ots_trajectories(persistence_status, updated_at);"; - #[cfg(test)] #[path = "schema_test.rs"] mod schema_test; diff --git a/crates/temper-store-turso/src/schema/migrations.rs b/crates/temper-store-turso/src/schema/migrations.rs new file mode 100644 index 000000000..6b2f6a34a --- /dev/null +++ b/crates/temper-store-turso/src/schema/migrations.rs @@ -0,0 +1,22 @@ +//! ARN-242: the schema version ledger. + +/// Every successful `migrate()` run records the version it brought the +/// database to; boots short-circuit when the ledger already shows the current +/// version. EVERY schema change must bump `SCHEMA_VERSION` in `store/mod.rs`, +/// or stamped databases will skip it. +pub const CREATE_SCHEMA_MIGRATIONS_TABLE: &str = "\ +CREATE TABLE IF NOT EXISTS temper_schema_migrations ( + version INTEGER PRIMARY KEY, + name TEXT NOT NULL, + applied_at TEXT NOT NULL +)"; + +/// Highest applied schema version (0 when the ledger is empty). +pub const SELECT_SCHEMA_VERSION: &str = + "SELECT COALESCE(MAX(version), 0) FROM temper_schema_migrations"; + +/// Stamp a successfully applied schema version. `INSERT OR IGNORE` makes a +/// concurrent double-stamp a no-op (the version is the primary key). +pub const INSERT_SCHEMA_VERSION: &str = "\ +INSERT OR IGNORE INTO temper_schema_migrations (version, name, applied_at) \ +VALUES (?1, ?2, datetime('now'))"; diff --git a/crates/temper-store-turso/src/schema/trajectories.rs b/crates/temper-store-turso/src/schema/trajectories.rs new file mode 100644 index 000000000..7b5ea14f3 --- /dev/null +++ b/crates/temper-store-turso/src/schema/trajectories.rs @@ -0,0 +1,111 @@ +//! Trajectory and OTS-trajectory table schema. + +pub const CREATE_TRAJECTORIES_TABLE: &str = "\ +CREATE TABLE IF NOT EXISTS trajectories ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + tenant TEXT NOT NULL, + entity_type TEXT NOT NULL, + entity_id TEXT NOT NULL, + action TEXT NOT NULL, + success INTEGER NOT NULL DEFAULT 0, + from_status TEXT, + to_status TEXT, + error TEXT, + created_at TEXT NOT NULL DEFAULT (datetime('now')) +);"; + +pub const CREATE_TRAJECTORIES_SUCCESS_INDEX: &str = "\ +CREATE INDEX IF NOT EXISTS idx_trajectories_success + ON trajectories(success);"; + +pub const CREATE_TRAJECTORIES_ENTITY_ACTION_INDEX: &str = "\ +CREATE INDEX IF NOT EXISTS idx_trajectories_entity_action + ON trajectories(tenant, entity_type, action);"; + +/// ALTER TABLE migrations for the `trajectories` table. +/// +/// These add columns that were previously only tracked in-memory +/// (agent_id, session_id, authz_denied, etc.). Each statement uses +/// try-and-ignore semantics in SQLite (duplicate column is a no-op error). +pub const ALTER_TRAJECTORIES_ADD_AGENT_ID: &str = + "ALTER TABLE trajectories ADD COLUMN agent_id TEXT"; + +pub const ALTER_TRAJECTORIES_ADD_SESSION_ID: &str = + "ALTER TABLE trajectories ADD COLUMN session_id TEXT"; + +pub const ALTER_TRAJECTORIES_ADD_AUTHZ_DENIED: &str = + "ALTER TABLE trajectories ADD COLUMN authz_denied INTEGER"; + +pub const ALTER_TRAJECTORIES_ADD_DENIED_RESOURCE: &str = + "ALTER TABLE trajectories ADD COLUMN denied_resource TEXT"; + +pub const ALTER_TRAJECTORIES_ADD_DENIED_MODULE: &str = + "ALTER TABLE trajectories ADD COLUMN denied_module TEXT"; + +pub const ALTER_TRAJECTORIES_ADD_SOURCE: &str = "ALTER TABLE trajectories ADD COLUMN source TEXT"; + +pub const ALTER_TRAJECTORIES_ADD_SPEC_GOVERNED: &str = + "ALTER TABLE trajectories ADD COLUMN spec_governed INTEGER"; + +pub const ALTER_TRAJECTORIES_ADD_REQUEST_BODY: &str = + "ALTER TABLE trajectories ADD COLUMN request_body TEXT"; + +pub const ALTER_TRAJECTORIES_ADD_INTENT: &str = "ALTER TABLE trajectories ADD COLUMN intent TEXT"; + +pub const ALTER_TRAJECTORIES_ADD_MATCHED_POLICY_IDS: &str = + "ALTER TABLE trajectories ADD COLUMN matched_policy_ids TEXT"; + +/// Index on agent_id for agent-scoped trajectory queries. +pub const CREATE_TRAJECTORIES_AGENT_INDEX: &str = "\ +CREATE INDEX IF NOT EXISTS idx_trajectories_agent + ON trajectories(agent_id);"; + +/// Full OTS trajectory storage for GEPA self-improvement loop. +/// +/// Stores complete agent execution traces (tool calls, decisions, reasoning) +/// captured by the MCP server during agent sessions. The `data` column holds +/// the full OTS JSON blob; indexed columns enable efficient filtering. +pub const CREATE_OTS_TRAJECTORIES_TABLE: &str = "\ +CREATE TABLE IF NOT EXISTS ots_trajectories ( + trajectory_id TEXT PRIMARY KEY, + tenant TEXT NOT NULL, + agent_id TEXT NOT NULL, + session_id TEXT, + outcome TEXT NOT NULL DEFAULT 'unknown', + entity_type TEXT, + turn_count INTEGER NOT NULL DEFAULT 0, + data TEXT NOT NULL, + persistence_status TEXT NOT NULL DEFAULT 'persisted', + persist_attempts INTEGER NOT NULL DEFAULT 0, + last_error TEXT, + created_at TEXT NOT NULL DEFAULT (datetime('now')), + updated_at TEXT NOT NULL DEFAULT (datetime('now')) +);"; + +pub const ALTER_OTS_TRAJECTORIES_ADD_PERSISTENCE_STATUS: &str = "\ +ALTER TABLE ots_trajectories ADD COLUMN persistence_status TEXT NOT NULL DEFAULT 'persisted';"; + +pub const ALTER_OTS_TRAJECTORIES_ADD_PERSIST_ATTEMPTS: &str = "\ +ALTER TABLE ots_trajectories ADD COLUMN persist_attempts INTEGER NOT NULL DEFAULT 0;"; + +pub const ALTER_OTS_TRAJECTORIES_ADD_LAST_ERROR: &str = "\ +ALTER TABLE ots_trajectories ADD COLUMN last_error TEXT;"; + +pub const ALTER_OTS_TRAJECTORIES_ADD_UPDATED_AT: &str = "\ +ALTER TABLE ots_trajectories ADD COLUMN updated_at TEXT NOT NULL DEFAULT (datetime('now'));"; + +pub const CREATE_OTS_TRAJECTORIES_AGENT_INDEX: &str = "\ +CREATE INDEX IF NOT EXISTS idx_ots_trajectories_agent + ON ots_trajectories(agent_id);"; + +pub const CREATE_OTS_TRAJECTORIES_TENANT_INDEX: &str = "\ +CREATE INDEX IF NOT EXISTS idx_ots_trajectories_tenant + ON ots_trajectories(tenant);"; + +pub const CREATE_OTS_TRAJECTORIES_OUTCOME_INDEX: &str = "\ +CREATE INDEX IF NOT EXISTS idx_ots_trajectories_outcome + ON ots_trajectories(outcome);"; + +pub const CREATE_OTS_TRAJECTORIES_STATUS_INDEX: &str = "\ +CREATE INDEX IF NOT EXISTS idx_ots_trajectories_status + ON ots_trajectories(persistence_status, updated_at);"; diff --git a/crates/temper-store-turso/src/store/mod.rs b/crates/temper-store-turso/src/store/mod.rs index fba0d839d..faa74d62f 100644 --- a/crates/temper-store-turso/src/store/mod.rs +++ b/crates/temper-store-turso/src/store/mod.rs @@ -41,6 +41,66 @@ mod write_gate; pub use field_index::QueryProjectionUpsert; use instrumentation::InstrumentedConnection; +use libsql::params; + +/// ARN-242: the schema version this build of the store migrates to. +/// +/// EVERY change to the DDL in [`TursoEventStore::migrate`] (new table, new +/// column, new index) MUST bump this, or databases already stamped at the +/// previous version will skip it. Platform-registry DDL lives in +/// `router.rs::migrate_platform`, OUTSIDE this ledger — bumping this constant +/// does nothing for it; it runs every boot and must stay fail-closed. +/// +/// INVARIANT: every statement in the migration must be idempotent AND safe to +/// run concurrently with another booting server. Two servers can both observe +/// an unstamped ledger and both run the full DDL; today every statement is +/// `CREATE … IF NOT EXISTS` or a benign-tolerated `ADD COLUMN`, and the stamp +/// is `INSERT OR IGNORE` on the version primary key, so the race is harmless. +/// A future version that backfills data or issues a bare `CREATE` must +/// serialize the migration explicitly. +const SCHEMA_VERSION: i64 = 1; +/// Human-readable name recorded in the ledger for [`SCHEMA_VERSION`]. +const SCHEMA_VERSION_NAME: &str = "baseline-idempotent-ddl"; + +/// Execute an idempotent `ALTER TABLE … ADD COLUMN`, tolerating ONLY the +/// benign already-applied errors (duplicate column / already exists). Every +/// other failure — locked database, disk errors, a shadowed table, syntax — +/// propagates with the failing statement in the message, so a real migration +/// failure fails startup loudly instead of leaving a half-migrated schema in +/// service (ARN-242; previously `let _ =` swallowed everything). +/// +/// PRECONDITION: only `ADD COLUMN` statements may be routed here. SQLite's +/// sole already-applied failure for ADD COLUMN is duplicate-column, so the +/// benign filter cannot mask a real conflict — but an `already exists` from a +/// CREATE would be a genuine object-name collision, and tolerating it would +/// re-introduce the swallow this fixes. +async fn execute_idempotent( + conn: &InstrumentedConnection, + stmt: &str, +) -> Result<(), PersistenceError> { + match conn.execute(stmt, ()).await { + Ok(_) => Ok(()), + Err(e) => { + let msg = e.to_string().to_ascii_lowercase(); + if msg.contains("duplicate column") + || msg.contains("already exists") + || msg.contains("already has") + { + Ok(()) + } else { + let stmt_head: String = stmt.chars().take(120).collect(); + tracing::error!( + statement = %stmt_head, + error = %e, + "turso migration statement failed; startup will abort" + ); + Err(storage_error(format!( + "migration statement failed: {stmt_head}: {e}" + ))) + } + } + } +} pub use published_artifacts::{PublishedArtifactRow, PublishedArtifactUpsert}; #[derive(Clone, Debug)] @@ -108,6 +168,13 @@ impl TursoEventStore { } /// Run schema migrations on connect. + /// + /// ARN-242: guarded by a durable version ledger (`temper_schema_migrations`). + /// When the ledger already records [`SCHEMA_VERSION`], the DDL is skipped + /// entirely; otherwise the full idempotent schema runs FAIL-CLOSED (only + /// benign duplicate-column/already-exists errors are tolerated, via + /// [`execute_idempotent`]) and the version is stamped. EVERY schema change + /// must bump `SCHEMA_VERSION`, or already-stamped databases will skip it. #[instrument(skip_all, fields(otel.name = "turso.migrate"))] async fn migrate(&self) -> Result<(), PersistenceError> { let conn = self.connection()?; @@ -125,12 +192,30 @@ impl TursoEventStore { .map_err(storage_error)?; } + conn.execute(schema::CREATE_SCHEMA_MIGRATIONS_TABLE, ()) + .await + .map_err(storage_error)?; + let applied: i64 = { + let mut rows = conn + .query(schema::SELECT_SCHEMA_VERSION, ()) + .await + .map_err(storage_error)?; + let applied = match rows.next().await.map_err(storage_error)? { + Some(row) => row.get(0).map_err(storage_error)?, + None => 0, + }; + // Drain and drop the statement so no read lock outlives the check. + while rows.next().await.map_err(storage_error)?.is_some() {} + applied + }; + if applied >= SCHEMA_VERSION { + return Ok(()); + } + conn.execute(schema::CREATE_EVENTS_TABLE, ()) .await .map_err(storage_error)?; - let _ = conn - .execute(schema::ALTER_EVENTS_ADD_SEGMENT_INDEX, ()) - .await; + execute_idempotent(&conn, schema::ALTER_EVENTS_ADD_SEGMENT_INDEX).await?; conn.execute(schema::CREATE_EVENTS_ENTITY_INDEX, ()) .await .map_err(storage_error)?; @@ -170,21 +255,7 @@ impl TursoEventStore { // Idempotent ALTER for pre-existing DBs created before the source // column existed. Turso has no IF NOT EXISTS for ADD COLUMN, so we // ignore "duplicate column" errors. - match conn - .execute(schema::ADD_WASM_MODULES_SOURCE_COLUMN, ()) - .await - { - Ok(_) => {} - Err(e) => { - let msg = e.to_string(); - if !msg.contains("duplicate column") - && !msg.contains("already exists") - && !msg.contains("already has") - { - return Err(storage_error(e)); - } - } - } + execute_idempotent(&conn, schema::ADD_WASM_MODULES_SOURCE_COLUMN).await?; conn.execute(schema::CREATE_WASM_INVOCATION_LOGS_TABLE, ()) .await .map_err(storage_error)?; @@ -230,7 +301,7 @@ impl TursoEventStore { .await .map_err(storage_error)?; // Migration: add `enabled` column to existing `policies` tables. - let _ = conn.execute(schema::ALTER_POLICIES_ADD_ENABLED, ()).await; + execute_idempotent(&conn, schema::ALTER_POLICIES_ADD_ENABLED).await?; conn.execute(schema::CREATE_TENANT_INSTALLED_APPS_TABLE, ()) .await .map_err(storage_error)?; @@ -254,7 +325,7 @@ impl TursoEventStore { schema::ALTER_INSTALLED_APPS_ADD_LAST_RECONCILED_AT, schema::ALTER_INSTALLED_APPS_ADD_STATUS, ] { - let _ = conn.execute(stmt, ()).await; + execute_idempotent(&conn, stmt).await?; } // Phase 0: New tables for Turso-as-single-source-of-truth. @@ -285,8 +356,8 @@ impl TursoEventStore { .map_err(storage_error)?; // Specs table extensions — add content_hash column for verification caching. - let _ = conn.execute(schema::ALTER_SPECS_ADD_CONTENT_HASH, ()).await; - let _ = conn.execute(schema::ALTER_SPECS_ADD_COMMITTED, ()).await; + execute_idempotent(&conn, schema::ALTER_SPECS_ADD_CONTENT_HASH).await?; + execute_idempotent(&conn, schema::ALTER_SPECS_ADD_COMMITTED).await?; // Trajectory table extensions — ALTER TABLE to add missing columns. // SQLite returns an error for duplicate columns, so we ignore failures. @@ -302,7 +373,7 @@ impl TursoEventStore { schema::ALTER_TRAJECTORIES_ADD_INTENT, schema::ALTER_TRAJECTORIES_ADD_MATCHED_POLICY_IDS, ] { - let _ = conn.execute(stmt, ()).await; // ignore "duplicate column" errors + execute_idempotent(&conn, stmt).await?; } conn.execute(schema::CREATE_TRAJECTORIES_AGENT_INDEX, ()) .await @@ -318,7 +389,7 @@ impl TursoEventStore { schema::ALTER_OTS_TRAJECTORIES_ADD_LAST_ERROR, schema::ALTER_OTS_TRAJECTORIES_ADD_UPDATED_AT, ] { - let _ = conn.execute(stmt, ()).await; + execute_idempotent(&conn, stmt).await?; } conn.execute(schema::CREATE_OTS_TRAJECTORIES_AGENT_INDEX, ()) .await @@ -341,12 +412,7 @@ impl TursoEventStore { // ADR-0047: idempotent migration that adds `expires_at` to pre-existing // blobs tables. Duplicate-column errors are expected on newer deployments // that already have the column from `CREATE_BLOBS_TABLE`; swallow them. - if let Err(error) = conn.execute(schema::ALTER_BLOBS_ADD_EXPIRES_AT, ()).await { - let message = error.to_string().to_ascii_lowercase(); - if !message.contains("duplicate column") { - return Err(storage_error(error)); - } - } + execute_idempotent(&conn, schema::ALTER_BLOBS_ADD_EXPIRES_AT).await?; conn.execute(schema::CREATE_BLOBS_EXPIRES_AT_INDEX, ()) .await .map_err(storage_error)?; @@ -361,15 +427,9 @@ impl TursoEventStore { conn.execute(schema::CREATE_ENTITY_CATALOG_STATUS_INDEX, ()) .await .map_err(storage_error)?; - let _ = conn - .execute(schema::ALTER_ENTITY_CATALOG_ADD_PROJECTION_HASH, ()) - .await; - let _ = conn - .execute(schema::ALTER_ENTITY_CATALOG_ADD_FIELDS, ()) - .await; - let _ = conn - .execute(schema::ALTER_ENTITY_CATALOG_ADD_STATE, ()) - .await; + execute_idempotent(&conn, schema::ALTER_ENTITY_CATALOG_ADD_PROJECTION_HASH).await?; + execute_idempotent(&conn, schema::ALTER_ENTITY_CATALOG_ADD_FIELDS).await?; + execute_idempotent(&conn, schema::ALTER_ENTITY_CATALOG_ADD_STATE).await?; // Entity field index — EAV table for OData filter push-down. conn.execute(schema::CREATE_ENTITY_FIELD_INDEX_TABLE, ()) @@ -406,6 +466,15 @@ impl TursoEventStore { .await .map_err(storage_error)?; + // Every statement above succeeded (or was a benign already-applied + // no-op): stamp the ledger so the next boot short-circuits. + conn.execute( + schema::INSERT_SCHEMA_VERSION, + params![SCHEMA_VERSION, SCHEMA_VERSION_NAME], + ) + .await + .map_err(storage_error)?; + Ok(()) } diff --git a/crates/temper-store-turso/src/store/tests/mod.rs b/crates/temper-store-turso/src/store/tests/mod.rs index 61b814896..9283cc299 100644 --- a/crates/temper-store-turso/src/store/tests/mod.rs +++ b/crates/temper-store-turso/src/store/tests/mod.rs @@ -691,12 +691,65 @@ async fn policy_denial_patterns_roundtrip_and_merge() { assert!(ids.contains(&"ISSUE-2".to_string())); } +/// The DDL itself must be idempotent — re-running every statement against an +/// already-migrated database must succeed. The ARN-242 ledger short-circuits +/// a stamped database, so this clears the ledger between runs to force the +/// full DDL to execute again (otherwise the test would assert nothing). #[tokio::test] async fn migrate_is_idempotent() { let store = make_store("migrate-idempotent").await; + let conn = store.connection().expect("connection"); - store.migrate().await.unwrap(); - store.migrate().await.unwrap(); + for _ in 0..2 { + conn.execute("DELETE FROM temper_schema_migrations", ()) + .await + .expect("clear ledger so the DDL actually re-runs"); + store + .migrate() + .await + .expect("re-running the DDL must succeed"); + } +} + +/// ARN-242 production-upgrade path: an EXISTING, fully-migrated database that +/// predates the ledger is unstamped. The first boot on the new build must run +/// the whole baseline against a populated schema — every ALTER hitting a +/// duplicate column — and the fail-closed filter must tolerate exactly those +/// and re-stamp. This is the highest-blast-radius path of the change: if any +/// benign error string failed to match, boot would abort for every existing +/// deployment. +#[tokio::test] +async fn migrate_upgrades_an_existing_unstamped_database() { + let url = sqlite_test_url("migrate-unstamped-upgrade"); + + // Boot once: fully migrated and stamped. + { + let store = TursoEventStore::new(&url, None).await.expect("first boot"); + let conn = store.connection().expect("connection"); + // Simulate a pre-ledger production database: fully migrated, no stamp. + conn.execute("DELETE FROM temper_schema_migrations", ()) + .await + .expect("unstamp"); + } + + // Second boot on the same database: the full baseline re-runs against the + // populated schema and must succeed. + let store = TursoEventStore::new(&url, None) + .await + .expect("an existing unstamped database must upgrade cleanly"); + + let conn = store.connection().expect("connection"); + let mut rows = conn + .query(crate::schema::SELECT_SCHEMA_VERSION, ()) + .await + .expect("ledger query"); + let row = rows.next().await.expect("row").expect("row"); + let version: Option = row.get(0).expect("version"); + assert_eq!( + version, + Some(super::SCHEMA_VERSION), + "the upgraded database must be re-stamped at the current schema version" + ); } /// Regression: append must be durable (readable from a fresh connection) @@ -2044,7 +2097,7 @@ async fn migrate_surfaces_real_alter_errors() { // `policies` (CREATE TABLE IF NOT EXISTS tolerates it silently, but the // ALTER on it fails with a non-duplicate-column error). { - let db = libsql::Builder::new_local(url.trim_start_matches("file:")) + let db = libsql::Builder::new_local(url.strip_prefix("file:").unwrap_or(&url)) .build() .await .expect("build poison db"); @@ -2079,12 +2132,11 @@ async fn migrate_records_schema_version_ledger() { .query("SELECT MAX(version) FROM temper_schema_migrations", ()) .await .expect("the schema version ledger table must exist after migrate()"); - let row = rows - .next() - .await - .expect("ledger row") - .expect("ledger must contain at least one applied version"); - let version: i64 = row.get(0).expect("version column"); + let row = rows.next().await.expect("ledger row").expect("ledger row"); + // MAX() over an empty ledger returns NULL — read as Option so an empty + // ledger fails with the real defect ("no applied version"), not a type error. + let version: Option = row.get(0).expect("version column"); + let version = version.expect("the ledger must contain an applied version"); assert!( version >= 1, "the ledger must record the applied schema version, got {version}" diff --git a/crates/temper-store-turso/tests/blob_ttl_e2e.rs b/crates/temper-store-turso/tests/blob_ttl_e2e.rs index bce466cc4..e6efe0ef6 100644 --- a/crates/temper-store-turso/tests/blob_ttl_e2e.rs +++ b/crates/temper-store-turso/tests/blob_ttl_e2e.rs @@ -159,7 +159,7 @@ async fn sweep_noop_when_all_rows_permanent() { } #[tokio::test] -async fn schema_migration_is_idempotent_across_reopens() { +async fn stamped_database_reopens_cleanly_and_preserves_blobs() { let dir = tempfile::tempdir().expect("tempdir"); let db_path = dir.path().join("e2e.db"); let url = format!("file:{}", db_path.display()); @@ -175,9 +175,10 @@ async fn schema_migration_is_idempotent_across_reopens() { .expect("put_blob"); } - // Second open — migrate() runs again. The ALTER must not fail - // (duplicate-column error is swallowed) and the existing row must be - // readable. + // Second open — the ARN-242 ledger short-circuits: the database is already + // stamped at the current SCHEMA_VERSION, so migrate() skips the DDL entirely + // and the store opens cleanly against the existing schema; the blob written + // before the reopen must still be readable. { let store = TursoEventStore::new(&url, None).await.expect("second open"); let bytes = store diff --git a/docs/adrs/0162-turso-migration-ledger.md b/docs/adrs/0162-turso-migration-ledger.md new file mode 100644 index 000000000..81358a8a2 --- /dev/null +++ b/docs/adrs/0162-turso-migration-ledger.md @@ -0,0 +1,75 @@ +# ADR-0162: Turso Schema Migration Ledger + +## Status + +Accepted (2026-07-12) + +(Numbered 0162: 0156–0161 are claimed by concurrently open arena branches.) + +## Context + +`TursoEventStore::migrate()` re-ran the entire DDL script on every boot with +no record of what had been applied, and thirteen `let _ = conn.execute(...)` +sites discarded every ALTER failure. The intent was to tolerate benign +duplicate-column errors on idempotent re-runs — but the pattern equally +swallowed locked databases, disk errors, shadowed tables, and syntax errors, +so a genuinely failed migration left a half-migrated database that the +server then served against, silently (ARN-242). + +## Decision + +1. **Fail-closed idempotent execution.** `execute_idempotent` tolerates only + the benign already-applied errors (duplicate column / already exists); + everything else propagates and fails startup. All thirteen swallow sites + and the two bespoke match blocks route through it. +2. **A durable version ledger.** `temper_schema_migrations (version, name, + applied_at)` is created first; a successful full migration run stamps + `SCHEMA_VERSION` (currently 1, `baseline-idempotent-ddl`). Boots where + the ledger already shows the current version skip the DDL entirely. +3. **The contract:** EVERY change to the ledgered DDL must bump + `SCHEMA_VERSION`, or databases stamped at the previous version will skip + it. Platform-registry DDL lives in `router.rs::migrate_platform`, OUTSIDE + the ledger — it runs every boot and must stay fail-closed; bumping the + constant does nothing for it. **Invariant:** every migration statement + must be idempotent AND safe to run concurrently with another booting + server (two servers can both see an unstamped ledger and both run the + DDL; today every statement is `CREATE … IF NOT EXISTS` or a + benign-tolerated `ADD COLUMN`, and the stamp is `INSERT OR IGNORE` on the + version primary key, so the race is harmless). A future version that + backfills data or issues a bare `CREATE` must serialize the migration + explicitly. +4. **Ordering:** the ledger check runs after the connection PRAGMAs (WAL, + busy_timeout) and fully drains its query rows — an undrained statement + before WAL was configured held a read lock that deadlocked concurrent + writers (caught by the existing projection test during development). + +## Consequences + +- A real migration failure now fails boot loudly instead of leaving a + half-migrated schema in service; operators can read the ledger to see + what version a database is at. +- Stamped boots skip ~88 executed DDL statements (69 call sites, three of + which are loops expanding to 22 ALTERs) — the full turso test suite dropped + from ~33s to ~4s as a side effect of the reduced lock churn. +- Pre-ledger databases run the baseline once more (idempotent) and are + stamped; no migration is lost. +- The version is coarse (one baseline). Future schema changes append new + version groups rather than growing the baseline — the constant's doc + says so; a finer-grained per-statement ledger was considered and + rejected as bookkeeping overhead with no added safety over the + fail-closed baseline. + +## Alternatives Considered + +- **Per-statement ledger rows:** more bookkeeping, same guarantees — the + baseline is idempotent, so statement-level tracking adds nothing until + a non-idempotent migration exists (at which point it gets its own + version group). +- **Keeping the swallow with logging:** a logged-but-served half-migrated + schema is still a corrupt deployment; the failure must gate boot. +- **Serializing the migration (advisory lock / exclusive transaction):** + unnecessary while every statement is idempotent and concurrent-safe (the + invariant above), and it would add a cross-backend locking primitive Turso + Cloud does not offer uniformly. Required the moment a non-idempotent + migration exists — recorded here so that requirement is not rediscovered + the hard way. From 130b2b6d2355e499f0ef6617c9670dd858048eeb Mon Sep 17 00:00:00 2001 From: Rita Agafonova <36133358+rita-aga@users.noreply.github.com> Date: Sun, 12 Jul 2026 23:58:35 -0700 Subject: [PATCH 3/7] fix(store-turso): gate migrations on a schema fingerprint; make the ledger self-migrating (ARN-242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review rounds r4-r6 turned two silent-failure classes into structural guarantees, both of which the first cut would have shipped. The dedicated PR reviewer's P1: a stamped database runs no DDL, so a schema change that forgot to bump SCHEMA_VERSION would never reach existing databases — and no test could catch it, since every store test starts from a fresh unstamped database. SCHEMA_FINGERPRINT (a hash of the schema a fresh migrate produces) is now the BOOT GATE, not a tripwire: a database re-runs the DDL until its stamped fingerprint matches the declared one, so updating the fingerprint is the very act that makes a change reach stamped databases. SCHEMA_VERSION degrades to a human-readable label, off the correctness path. The pre-commit reviewer's F5, reproduced as a hard boot failure: the ledger table sits in FRONT of its own gate, so it can never be gated by its own fingerprint. Adding the fingerprint column made the gate SELECT die at prepare time ("no such column") on every database whose ledger predated it. The ledger now migrates itself un-gated, with a regression test that builds an old-shape ledger and boots against it. The gate asks "has this database EVER been migrated to this schema" rather than "is the latest row this schema", so a rolled-back binary skips instead of re-running ~98 statements every boot; the test observes the skip via a sentinel table. execute_idempotent's ADD-COLUMN-only precondition is now a debug assertion — the debug suite passing proves all 41 ALTERs satisfy it. ADR-0162 records the fingerprint gate, the ledger's self-migration, the rollback semantics, the corrected counts, and file-based migrations as the long-term direction. Co-Authored-By: Claude Fable 5 --- crates/temper-store-turso/src/schema.rs | 3 +- .../src/schema/migrations.rs | 49 ++++- crates/temper-store-turso/src/store/mod.rs | 41 +++- .../temper-store-turso/src/store/tests/mod.rs | 181 +++++++++++++++++- docs/adrs/0162-turso-migration-ledger.md | 84 ++++++-- 5 files changed, 325 insertions(+), 33 deletions(-) diff --git a/crates/temper-store-turso/src/schema.rs b/crates/temper-store-turso/src/schema.rs index a4ace3c0d..4bbab7e4e 100644 --- a/crates/temper-store-turso/src/schema.rs +++ b/crates/temper-store-turso/src/schema.rs @@ -9,7 +9,8 @@ pub use crate::schema_event_history::{ CREATE_SNAPSHOT_HISTORY_ENTITY_INDEX, CREATE_SNAPSHOT_HISTORY_TABLE, }; pub use migrations::{ - CREATE_SCHEMA_MIGRATIONS_TABLE, INSERT_SCHEMA_VERSION, SELECT_SCHEMA_VERSION, + ALTER_SCHEMA_MIGRATIONS_ADD_FINGERPRINT, CREATE_SCHEMA_MIGRATIONS_TABLE, INSERT_SCHEMA_VERSION, + SELECT_SCHEMA_FINGERPRINT_APPLIED, }; pub use query_plane::{ CREATE_ENTITY_CATALOG_STATUS_INDEX, CREATE_ENTITY_CATALOG_TABLE, diff --git a/crates/temper-store-turso/src/schema/migrations.rs b/crates/temper-store-turso/src/schema/migrations.rs index 6b2f6a34a..a0d58f570 100644 --- a/crates/temper-store-turso/src/schema/migrations.rs +++ b/crates/temper-store-turso/src/schema/migrations.rs @@ -8,15 +8,50 @@ pub const CREATE_SCHEMA_MIGRATIONS_TABLE: &str = "\ CREATE TABLE IF NOT EXISTS temper_schema_migrations ( version INTEGER PRIMARY KEY, name TEXT NOT NULL, + fingerprint TEXT NOT NULL DEFAULT '', applied_at TEXT NOT NULL )"; -/// Highest applied schema version (0 when the ledger is empty). -pub const SELECT_SCHEMA_VERSION: &str = - "SELECT COALESCE(MAX(version), 0) FROM temper_schema_migrations"; +/// The ledger's OWN migration. The ledger table sits BEFORE the gate that +/// governs everything else, so it can never be gated by its own fingerprint — +/// it must migrate itself, un-gated, on every boot. Without this, adding any +/// column to the ledger (`fingerprint` here; `applied_by`, `duration_ms`, +/// anything later) makes the very next statement — the gate SELECT — fail at +/// prepare time with "no such column" on every database whose ledger predates +/// it: a hard boot failure, and precisely the class this ADR exists to kill. +/// +/// Routed through `execute_idempotent`, so on a fresh database (where +/// `CREATE TABLE` already declared the column) it is a tolerated +/// duplicate-column no-op that leaves `sqlite_master` — and therefore +/// `SCHEMA_FINGERPRINT` — unchanged. +pub const ALTER_SCHEMA_MIGRATIONS_ADD_FINGERPRINT: &str = "\ +ALTER TABLE temper_schema_migrations ADD COLUMN fingerprint TEXT NOT NULL DEFAULT ''"; -/// Stamp a successfully applied schema version. `INSERT OR IGNORE` makes a -/// concurrent double-stamp a no-op (the version is the primary key). +/// Has this database EVER been migrated to the declared schema? This — not +/// the version — is the boot gate: the DDL is skipped only when a ledger row +/// records the fingerprint the binary declares, so a schema change cannot skip +/// existing databases even if the author forgets to bump `SCHEMA_VERSION`. +/// +/// Asking "ever migrated to this schema" rather than "is the LATEST row this +/// schema" also makes a rollback cheap: a binary rolled back to an older +/// schema finds its own retained row and skips, instead of re-running the +/// whole DDL on every boot for as long as the rollback lasts. (Caveat: when +/// the newer build changed the schema WITHOUT bumping the version — the +/// version is only a label — `INSERT OR REPLACE` overwrote the older row, so +/// the rolled-back binary re-runs the DDL once and then skips.) +/// +/// It compares the STORED declared constant against the CURRENT declared +/// constant — never the live schema — so a platform database's extra +/// `migrate_platform` tables cannot cause spurious re-runs. +pub const SELECT_SCHEMA_FINGERPRINT_APPLIED: &str = "\ +SELECT EXISTS(SELECT 1 FROM temper_schema_migrations WHERE fingerprint = ?1)"; + +/// Stamp a successfully applied schema version and its fingerprint. +/// `INSERT OR REPLACE` on the version primary key makes a concurrent +/// double-stamp idempotent, and lets a same-version DDL change (an author who +/// updated the fingerprint without bumping the version) record its new +/// fingerprint after the DDL actually ran. pub const INSERT_SCHEMA_VERSION: &str = "\ -INSERT OR IGNORE INTO temper_schema_migrations (version, name, applied_at) \ -VALUES (?1, ?2, datetime('now'))"; +INSERT OR REPLACE INTO temper_schema_migrations \ +(version, name, fingerprint, applied_at) \ +VALUES (?1, ?2, ?3, datetime('now'))"; diff --git a/crates/temper-store-turso/src/store/mod.rs b/crates/temper-store-turso/src/store/mod.rs index faa74d62f..99ad3e1db 100644 --- a/crates/temper-store-turso/src/store/mod.rs +++ b/crates/temper-store-turso/src/store/mod.rs @@ -62,6 +62,19 @@ const SCHEMA_VERSION: i64 = 1; /// Human-readable name recorded in the ledger for [`SCHEMA_VERSION`]. const SCHEMA_VERSION_NAME: &str = "baseline-idempotent-ddl"; +/// SHA-256 of the schema a fresh `migrate()` produces — and the BOOT GATE. +/// +/// A stamped database re-runs the DDL whenever this declared value differs +/// from the one recorded in its ledger, so a schema change reaches existing +/// databases even if the author forgets to bump [`SCHEMA_VERSION`] (which is +/// a human-readable label, off the correctness path). Updating this constant +/// is the very act that invalidates the skip — the contract cannot be +/// satisfied without also making the migration run. +/// +/// `schema_fingerprint_matches_declared_version` fails on ANY DDL change and +/// prints the new value to paste here. +const SCHEMA_FINGERPRINT: &str = "3b8b6b18aa49eeb8fc34e47f88660f65998b136576c4ef0507b757abfb66a34d"; + /// Execute an idempotent `ALTER TABLE … ADD COLUMN`, tolerating ONLY the /// benign already-applied errors (duplicate column / already exists). Every /// other failure — locked database, disk errors, a shadowed table, syntax — @@ -78,6 +91,14 @@ async fn execute_idempotent( conn: &InstrumentedConnection, stmt: &str, ) -> Result<(), PersistenceError> { + // TigerStyle pre-assertion: the precondition above is what makes the + // benign filter safe. Routing a CREATE through here would let a genuine + // object-name collision ("already exists") be swallowed — exactly the + // defect this function removes. + debug_assert!( + stmt.to_ascii_uppercase().contains("ADD COLUMN"), + "PRECONDITION: only ADD COLUMN statements may use execute_idempotent; got: {stmt}" + ); match conn.execute(stmt, ()).await { Ok(_) => Ok(()), Err(e) => { @@ -192,23 +213,31 @@ impl TursoEventStore { .map_err(storage_error)?; } + // The ledger's own DDL runs UN-GATED, before the gate can be read — it + // is the one table that sits in front of the gate, so it must migrate + // itself (see ALTER_SCHEMA_MIGRATIONS_ADD_FINGERPRINT). conn.execute(schema::CREATE_SCHEMA_MIGRATIONS_TABLE, ()) .await .map_err(storage_error)?; - let applied: i64 = { + execute_idempotent(&conn, schema::ALTER_SCHEMA_MIGRATIONS_ADD_FINGERPRINT).await?; + + let already_applied: bool = { let mut rows = conn - .query(schema::SELECT_SCHEMA_VERSION, ()) + .query( + schema::SELECT_SCHEMA_FINGERPRINT_APPLIED, + params![SCHEMA_FINGERPRINT], + ) .await .map_err(storage_error)?; - let applied = match rows.next().await.map_err(storage_error)? { + let applied: i64 = match rows.next().await.map_err(storage_error)? { Some(row) => row.get(0).map_err(storage_error)?, None => 0, }; // Drain and drop the statement so no read lock outlives the check. while rows.next().await.map_err(storage_error)?.is_some() {} - applied + applied != 0 }; - if applied >= SCHEMA_VERSION { + if already_applied { return Ok(()); } @@ -470,7 +499,7 @@ impl TursoEventStore { // no-op): stamp the ledger so the next boot short-circuits. conn.execute( schema::INSERT_SCHEMA_VERSION, - params![SCHEMA_VERSION, SCHEMA_VERSION_NAME], + params![SCHEMA_VERSION, SCHEMA_VERSION_NAME, SCHEMA_FINGERPRINT], ) .await .map_err(storage_error)?; diff --git a/crates/temper-store-turso/src/store/tests/mod.rs b/crates/temper-store-turso/src/store/tests/mod.rs index 9283cc299..fa28d40d1 100644 --- a/crates/temper-store-turso/src/store/tests/mod.rs +++ b/crates/temper-store-turso/src/store/tests/mod.rs @@ -740,16 +740,30 @@ async fn migrate_upgrades_an_existing_unstamped_database() { let conn = store.connection().expect("connection"); let mut rows = conn - .query(crate::schema::SELECT_SCHEMA_VERSION, ()) + .query( + "SELECT version, fingerprint FROM temper_schema_migrations \ + ORDER BY version DESC LIMIT 1", + (), + ) .await .expect("ledger query"); - let row = rows.next().await.expect("row").expect("row"); - let version: Option = row.get(0).expect("version"); + let row = rows + .next() + .await + .expect("row") + .expect("the upgraded database must be re-stamped"); + let version: i64 = row.get(0).expect("version"); + let fingerprint: String = row.get(1).expect("fingerprint"); assert_eq!( version, - Some(super::SCHEMA_VERSION), + super::SCHEMA_VERSION, "the upgraded database must be re-stamped at the current schema version" ); + assert_eq!( + fingerprint, + super::SCHEMA_FINGERPRINT, + "the upgraded database must record the current schema fingerprint (the boot gate)" + ); } /// Regression: append must be durable (readable from a fresh connection) @@ -2142,3 +2156,162 @@ async fn migrate_records_schema_version_ledger() { "the ledger must record the applied schema version, got {version}" ); } + +/// ARN-242 contract enforcement: a stamped database executes NO DDL, so a +/// schema change that forgets to bump `SCHEMA_VERSION` would silently never +/// reach existing databases — the same class of silent failure this issue +/// exists to kill, and one that no other test can catch (every other test +/// starts from a fresh, unstamped database and always runs the full DDL). +/// +/// This test fingerprints the schema a fresh `migrate()` produces. ANY change +/// to the DDL breaks it, and updating `SCHEMA_FINGERPRINT` — the boot gate — +/// is what makes the change reach stamped databases. `SCHEMA_VERSION` is a +/// human-readable label and is off the correctness path. +#[tokio::test] +async fn schema_fingerprint_matches_declared_version() { + use sha2::{Digest, Sha256}; + + let store = make_store("schema-fingerprint").await; + let conn = store.connection().expect("connection"); + let mut rows = conn + .query( + "SELECT type, name, COALESCE(sql, '') FROM sqlite_master \ + WHERE name NOT LIKE 'sqlite_%' ORDER BY type, name", + (), + ) + .await + .expect("read schema"); + + let mut hasher = Sha256::new(); + while let Some(row) = rows.next().await.expect("schema row") { + let kind: String = row.get(0).expect("type"); + let name: String = row.get(1).expect("name"); + let sql: String = row.get(2).expect("sql"); + hasher.update(kind.as_bytes()); + hasher.update([0]); + hasher.update(name.as_bytes()); + hasher.update([0]); + hasher.update(sql.as_bytes()); + hasher.update([0]); + } + let fingerprint = format!("{:x}", hasher.finalize()); + + assert_eq!( + fingerprint, + super::SCHEMA_FINGERPRINT, + "\n\nThe Turso schema changed (or the libsql/SQLite version changed how it\n\ + renders stored DDL).\n\ + SCHEMA_FINGERPRINT is the BOOT GATE: until it matches, stamped databases\n\ + re-run the DDL — so updating it is what makes your change reach them.\n\ + Set SCHEMA_FINGERPRINT (store/mod.rs) to:\n {fingerprint}\n\ + and bump SCHEMA_VERSION + SCHEMA_VERSION_NAME as the human-readable label.\n" + ); +} + +/// ARN-242 (F5): the ledger table sits BEFORE the gate that governs every +/// other table, so it must migrate ITSELF, un-gated. A database whose ledger +/// predates the `fingerprint` column must still boot: without the ledger's own +/// ALTER, `CREATE TABLE IF NOT EXISTS` no-ops and the very next statement (the +/// gate SELECT) dies at prepare time with "no such column: fingerprint" — a +/// hard boot failure, and the exact class this issue exists to kill. This also +/// pins the pattern for every future ledger column. +#[tokio::test] +async fn migrate_upgrades_a_ledger_that_predates_the_fingerprint_column() { + let url = sqlite_test_url("ledger-pre-fingerprint"); + + // A database migrated by a build whose ledger had no fingerprint column. + { + let store = TursoEventStore::new(&url, None).await.expect("first boot"); + let conn = store.connection().expect("connection"); + conn.execute("DROP TABLE temper_schema_migrations", ()) + .await + .expect("drop ledger"); + conn.execute( + "CREATE TABLE temper_schema_migrations (\ + version INTEGER PRIMARY KEY, name TEXT NOT NULL, applied_at TEXT NOT NULL)", + (), + ) + .await + .expect("recreate pre-fingerprint ledger"); + conn.execute( + "INSERT INTO temper_schema_migrations (version, name, applied_at) \ + VALUES (1, 'baseline-idempotent-ddl', datetime('now'))", + (), + ) + .await + .expect("stamp it the old way"); + } + + // The new build must self-migrate the ledger and boot cleanly. + let store = TursoEventStore::new(&url, None) + .await + .expect("a ledger predating the fingerprint column must upgrade, not fail boot"); + + let conn = store.connection().expect("connection"); + let mut rows = conn + .query( + "SELECT COUNT(*) FROM temper_schema_migrations WHERE fingerprint = ?1", + libsql::params![super::SCHEMA_FINGERPRINT], + ) + .await + .expect("ledger query"); + let row = rows.next().await.expect("row").expect("row"); + let count: i64 = row.get(0).expect("count"); + assert_eq!( + count, 1, + "the upgraded ledger must record the current schema fingerprint" + ); +} + +/// ARN-242 (F6): a binary rolled back to an older schema must still SKIP the +/// DDL — the gate asks "has this database EVER been migrated to the schema I +/// declare", so the older binary finds its own retained ledger row instead of +/// re-running all ~98 statements on every boot for as long as the rollback +/// lasts. +/// +/// The skip is observed, not merely predicted: a sentinel table dropped after +/// the first boot must NOT be recreated by the second, because a skipping +/// `migrate()` executes no DDL at all. +#[tokio::test] +async fn rolled_back_binary_skips_the_ddl() { + let url = sqlite_test_url("rollback-skip"); + + // This build boots and stamps its fingerprint. + let store = TursoEventStore::new(&url, None).await.expect("boot"); + let conn = store.connection().expect("connection"); + + // A LATER build's schema is stamped on top (higher version, different fp), + // simulating an upgrade that was then rolled back. + conn.execute( + "INSERT INTO temper_schema_migrations (version, name, fingerprint, applied_at) \ + VALUES (2, 'future-schema', 'future-fingerprint', datetime('now'))", + (), + ) + .await + .expect("stamp a future schema"); + + // Drop a table the DDL would recreate. If the rolled-back boot re-ran the + // DDL, this table would come back. + conn.execute("DROP TABLE blobs", ()) + .await + .expect("drop sentinel table"); + + // Roll back to this build: it must find its own retained row and skip. + let _rolled_back = TursoEventStore::new(&url, None) + .await + .expect("rolled-back boot"); + + let mut rows = conn + .query( + "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'blobs'", + (), + ) + .await + .expect("sentinel query"); + let row = rows.next().await.expect("row").expect("row"); + let recreated: i64 = row.get(0).expect("count"); + assert_eq!( + recreated, 0, + "a rolled-back binary must SKIP the DDL (the dropped table must not be recreated)" + ); +} diff --git a/docs/adrs/0162-turso-migration-ledger.md b/docs/adrs/0162-turso-migration-ledger.md index 81358a8a2..4378df23a 100644 --- a/docs/adrs/0162-turso-migration-ledger.md +++ b/docs/adrs/0162-turso-migration-ledger.md @@ -9,8 +9,8 @@ Accepted (2026-07-12) ## Context `TursoEventStore::migrate()` re-ran the entire DDL script on every boot with -no record of what had been applied, and thirteen `let _ = conn.execute(...)` -sites discarded every ALTER failure. The intent was to tolerate benign +no record of what had been applied, and twelve `let _ = conn.execute(...)` +sites (nine direct, three loops — 41 ALTERs in all) discarded every failure. The intent was to tolerate benign duplicate-column errors on idempotent re-runs — but the pattern equally swallowed locked databases, disk errors, shadowed tables, and syntax errors, so a genuinely failed migration left a half-migrated database that the @@ -20,15 +20,42 @@ server then served against, silently (ARN-242). 1. **Fail-closed idempotent execution.** `execute_idempotent` tolerates only the benign already-applied errors (duplicate column / already exists); - everything else propagates and fails startup. All thirteen swallow sites - and the two bespoke match blocks route through it. -2. **A durable version ledger.** `temper_schema_migrations (version, name, - applied_at)` is created first; a successful full migration run stamps - `SCHEMA_VERSION` (currently 1, `baseline-idempotent-ddl`). Boots where - the ledger already shows the current version skip the DDL entirely. -3. **The contract:** EVERY change to the ledgered DDL must bump - `SCHEMA_VERSION`, or databases stamped at the previous version will skip - it. Platform-registry DDL lives in `router.rs::migrate_platform`, OUTSIDE + everything else propagates and fails startup, with the failing statement + in the message. All twelve former swallow sites (41 ALTERs) and the two + bespoke match blocks route through it, and a debug pre-assertion enforces + its ADD-COLUMN-only precondition — so the debug test suite passing is + itself proof that all 41 satisfy it. +2. **A durable ledger, gated on a schema FINGERPRINT.** + `temper_schema_migrations (version, name, fingerprint, applied_at)` is + created first; a fully successful run stamps the declared + `SCHEMA_FINGERPRINT` (a SHA-256 of the schema a fresh migrate produces) + together with `SCHEMA_VERSION` as a human-readable label. **The boot gate + is the fingerprint, not the version:** a stamped database skips the DDL + only while its stored fingerprint equals the declared one. Updating the + fingerprint is therefore the very act that makes a schema change reach + existing databases — the contract cannot be satisfied without it. The + comparison is stored-constant vs declared-constant (never the live + schema), so a platform database's extra `migrate_platform` tables cannot + cause spurious re-runs. +3. **The ledger migrates itself, un-gated.** The ledger table sits in FRONT + of the gate, so it can never be gated by its own fingerprint: its `CREATE` + and its own `ADD COLUMN`s run on every boot, through `execute_idempotent`. + Without this, adding any column to the ledger would make the next + statement — the gate SELECT — fail at prepare time with "no such column" + on every database whose ledger predates it: a hard boot failure, and + exactly the class this ADR exists to kill, reproduced inside the ledger. + On a fresh database the ALTER is a tolerated duplicate-column no-op, so + `sqlite_master` (and the fingerprint) is unchanged. +4. **The gate asks "ever migrated to this schema", not "is the latest row + this schema"** (`SELECT EXISTS(… WHERE fingerprint = ?)`). A binary rolled + back to an older schema then finds its own retained row and skips, instead + of re-running the whole DDL on every boot for the duration of the + rollback. +5. **The contract:** EVERY change to the ledgered DDL must update + `SCHEMA_FINGERPRINT` (the `schema_fingerprint_matches_declared_version` + test fails otherwise and prints the new value), and should bump + `SCHEMA_VERSION`/`SCHEMA_VERSION_NAME` as the human-readable label. + Correctness rests on the fingerprint alone; the version is a label. Platform-registry DDL lives in `router.rs::migrate_platform`, OUTSIDE the ledger — it runs every boot and must stay fail-closed; bumping the constant does nothing for it. **Invariant:** every migration statement must be idempotent AND safe to run concurrently with another booting @@ -38,7 +65,7 @@ server then served against, silently (ARN-242). version primary key, so the race is harmless). A future version that backfills data or issues a bare `CREATE` must serialize the migration explicitly. -4. **Ordering:** the ledger check runs after the connection PRAGMAs (WAL, +6. **Ordering:** the ledger check runs after the connection PRAGMAs (WAL, busy_timeout) and fully drains its query rows — an undrained statement before WAL was configured held a read lock that deadlocked concurrent writers (caught by the existing projection test during development). @@ -48,9 +75,27 @@ server then served against, silently (ARN-242). - A real migration failure now fails boot loudly instead of leaving a half-migrated schema in service; operators can read the ledger to see what version a database is at. -- Stamped boots skip ~88 executed DDL statements (69 call sites, three of - which are loops expanding to 22 ALTERs) — the full turso test suite dropped - from ~33s to ~4s as a side effect of the reduced lock churn. +- Stamped boots skip the whole DDL script — 98 executed schema statements + (57 direct CREATEs, 9 direct ALTERs, and 32 more from three loops), plus + the two ledger statements. The full turso test suite dropped from ~33s to + ~4s as a side effect of the reduced lock churn. +- **A forgotten schema change cannot silently skip existing databases.** The + ledger's own hazard — a stamped database runs no DDL — is closed + structurally: the fingerprint IS the gate, so a DDL change that does not + update it leaves every stamped database re-running the DDL (loud, not + silent), and one that does update it thereby invalidates the skip. No + ordinary test could have caught a missed version bump: every store test + starts from a fresh, unstamped database. +- The fingerprint covers `migrate()`'s DDL only — not `migrate_platform`'s, + which is outside the ledger by design and runs (fail-closed) every boot. +- A libsql/SQLite upgrade that changed how stored DDL text is rendered would + trip the fingerprint test as a false positive; the failure message says so, + and following its instruction is harmless (the baseline is idempotent). +- **Multi-tenant fleets:** in the router path a tenant whose migration fails + is `warn!`-and-skipped at platform boot and then fails loudly on lazy + connect. No half-migrated schema is served either way, but "startup fails" + is precise only for the single-database path — a fleet surfaces the failure + per tenant, on first use. - Pre-ledger databases run the baseline once more (idempotent) and are stamped; no migration is lost. - The version is coarse (one baseline). Future schema changes append new @@ -67,6 +112,15 @@ server then served against, silently (ARN-242). version group). - **Keeping the swallow with logging:** a logged-but-served half-migrated schema is still a corrupt deployment; the failure must gate boot. +- **Version-only gating (the first cut of this ADR):** rejected — it made + correctness depend on a human remembering to bump a constant that nothing + checked, re-creating the documented-but-unenforced shape this issue exists + to kill. The fingerprint gate removes the human from the correctness path. +- **A file-based migration system (flyway/sqlx style), where the migration + file IS the ledger unit:** structurally forgetting-proof, but a large + rewrite of an imperative-Rust DDL path. The fingerprint gate gives the same + property at ~40 lines; file-based migrations remain the right long-term + direction. - **Serializing the migration (advisory lock / exclusive transaction):** unnecessary while every statement is idempotent and concurrent-safe (the invariant above), and it would add a cross-backend locking primitive Turso From 5e757b55d746607678813504ce5c2563ec2a2d49 Mon Sep 17 00:00:00 2001 From: Rita Agafonova <36133358+rita-aga@users.noreply.github.com> Date: Mon, 13 Jul 2026 00:07:03 -0700 Subject: [PATCH 4/7] docs(adr): correct the stamp mechanism and record the gate's blind spot (ARN-242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dedicated PR reviewer's two ADR findings, both fair. The concurrency invariant still justified itself with INSERT OR IGNORE while the code uses INSERT OR REPLACE — the conclusion survives (REPLACE is atomic within its statement), but the named mechanism did not exist, and REPLACE's row destruction is exactly what produces the documented rollback caveat, which the ADR never mentioned. And the new gate is schema-shaped: a data migration placed in migrate() would run on fresh databases, leave the fingerprint unchanged, keep CI green, and be skipped forever on every stamped database — a blind spot created by moving the gate onto the schema hash, which bumping SCHEMA_VERSION no longer covers now that the version is deliberately off the correctness path. Also recorded: the mixed-fleet re-run nuance during a fingerprint-only rolling deploy, the corrected statement count (56 CREATEs plus the stamp INSERT), and the reviewer's keying suggestion (PRIMARY KEY on the fingerprint with INSERT OR IGNORE) as the natural follow-up — it removes the rollback caveat entirely, but rebuilding the ledger's primary key on every existing database is precisely the non-idempotent migration this ADR says needs its own gating. Co-Authored-By: Claude Fable 5 --- docs/adrs/0162-turso-migration-ledger.md | 47 +++++++++++++++++++----- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/docs/adrs/0162-turso-migration-ledger.md b/docs/adrs/0162-turso-migration-ledger.md index 4378df23a..30fd36a08 100644 --- a/docs/adrs/0162-turso-migration-ledger.md +++ b/docs/adrs/0162-turso-migration-ledger.md @@ -51,7 +51,16 @@ server then served against, silently (ARN-242). back to an older schema then finds its own retained row and skips, instead of re-running the whole DDL on every boot for the duration of the rollback. -5. **The contract:** EVERY change to the ledgered DDL must update +5. **The gate is SCHEMA-shaped: `migrate()` is DDL-only.** `SCHEMA_FINGERPRINT` + hashes `sqlite_master`, so it is blind to any statement that does not change + the schema. A data migration placed in `migrate()` — a backfill, a seed row, + an `UPDATE … WHERE … IS NULL` — would run on fresh databases, leave the + fingerprint unchanged, keep the test and CI green, and **be skipped forever + on every stamped database**. Bumping `SCHEMA_VERSION` does not save it + either, because the version is deliberately off the correctness path. A data + migration therefore cannot rely on this gate: it needs its own gating row + (e.g. a ledger row keyed by the migration's name) or a separate mechanism. +6. **The contract:** EVERY change to the ledgered DDL must update `SCHEMA_FINGERPRINT` (the `schema_fingerprint_matches_declared_version` test fails otherwise and prints the new value), and should bump `SCHEMA_VERSION`/`SCHEMA_VERSION_NAME` as the human-readable label. @@ -61,11 +70,17 @@ server then served against, silently (ARN-242). must be idempotent AND safe to run concurrently with another booting server (two servers can both see an unstamped ledger and both run the DDL; today every statement is `CREATE … IF NOT EXISTS` or a - benign-tolerated `ADD COLUMN`, and the stamp is `INSERT OR IGNORE` on the - version primary key, so the race is harmless). A future version that - backfills data or issues a bare `CREATE` must serialize the migration - explicitly. -6. **Ordering:** the ledger check runs after the connection PRAGMAs (WAL, + benign-tolerated `ADD COLUMN`, and the stamp is `INSERT OR REPLACE` on the + version primary key — atomic within its statement, so two booters stamping + the same `(version, name, fingerprint)` converge on one identical row and + no third booter can observe a gap and spuriously re-run). Note `REPLACE` + DESTROYS the prior row for that version: when a schema change updates the + fingerprint without bumping the version (the version is only a label), the + older build's row is overwritten — which is precisely the rollback caveat + in Decision 4, and the reason a rolled-back binary re-runs the DDL once + before skipping. A future version that backfills data or issues a bare + `CREATE` must serialize the migration explicitly. +7. **Ordering:** the ledger check runs after the connection PRAGMAs (WAL, busy_timeout) and fully drains its query rows — an undrained statement before WAL was configured held a read lock that deadlocked concurrent writers (caught by the existing projection test during development). @@ -75,9 +90,9 @@ server then served against, silently (ARN-242). - A real migration failure now fails boot loudly instead of leaving a half-migrated schema in service; operators can read the ledger to see what version a database is at. -- Stamped boots skip the whole DDL script — 98 executed schema statements - (57 direct CREATEs, 9 direct ALTERs, and 32 more from three loops), plus - the two ledger statements. The full turso test suite dropped from ~33s to +- Stamped boots skip the whole DDL script — 98 executed statements (56 direct + `CREATE`s, 9 direct `ALTER`s, 32 more from three loops, and the stamp + `INSERT`), on top of the two un-gated ledger statements. The full turso test suite dropped from ~33s to ~4s as a side effect of the reduced lock churn. - **A forgotten schema change cannot silently skip existing databases.** The ledger's own hazard — a stamped database runs no DDL — is closed @@ -91,6 +106,11 @@ server then served against, silently (ARN-242). - A libsql/SQLite upgrade that changed how stored DDL text is rendered would trip the fingerprint test as a false positive; the failure message says so, and following its instruction is harmless (the baseline is idempotent). +- **Rolling deploys where only the fingerprint moved** (no version bump): old + and new replicas each `REPLACE` the other's ledger row, so each boot re-runs + the 98 idempotent statements for the duration of the mixed fleet, rather than + "once and then skips". Harmless (idempotent and concurrent-safe by the + invariant above), and it ends when the fleet converges. - **Multi-tenant fleets:** in the router path a tenant whose migration fails is `warn!`-and-skipped at platform boot and then fails loudly on lazy connect. No half-migrated schema is served either way, but "startup fails" @@ -116,6 +136,15 @@ server then served against, silently (ARN-242). correctness depend on a human remembering to bump a constant that nothing checked, re-creating the documented-but-unenforced shape this issue exists to kill. The fingerprint gate removes the human from the correctness path. +- **Keying the ledger on the fingerprint** (`PRIMARY KEY(fingerprint)` or + `(version, fingerprint)`) with `INSERT OR IGNORE`: rows would accumulate + instead of replacing, which removes the rollback caveat entirely (an older + build's row can never be destroyed) and simplifies the concurrency argument. + It is the smaller design and is likely the right next step — but changing the + ledger's primary key means rebuilding the table on every existing database, + which is exactly the kind of non-idempotent migration this ADR says needs its + own gating and serialization. Recorded as the natural follow-up rather than + folded into this change. - **A file-based migration system (flyway/sqlx style), where the migration file IS the ledger unit:** structurally forgetting-proof, but a large rewrite of an imperative-Rust DDL path. The fingerprint gate gives the same From 2122c057f8bf5b8276d8577f81f0343adb743d29 Mon Sep 17 00:00:00 2001 From: Rita Agafonova <36133358+rita-aga@users.noreply.github.com> Date: Mon, 13 Jul 2026 00:27:24 -0700 Subject: [PATCH 5/7] docs(store-turso): correct SCHEMA_VERSION's doc to match the fingerprint gate (ARN-242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile found the same two staleness bugs in the code doc comments that the PR reviewer found in the ADR: SCHEMA_VERSION's docstring still claimed that failing to bump it makes stamped databases skip a migration (the gate reads the fingerprint and never the version — bumping the version alone changes nothing), and the concurrency invariant still cited INSERT OR IGNORE while the stamp is INSERT OR REPLACE (whose row destruction is what produces the rollback re-run caveat). The constant is now documented as what it is: a human-readable label, off the correctness path, with the fingerprint named as the gate. The invariant states the real stamp and adds the gate's schema-shaped blind spot. Co-Authored-By: Claude Fable 5 --- crates/temper-store-turso/src/store/mod.rs | 29 ++++++++++++++-------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/crates/temper-store-turso/src/store/mod.rs b/crates/temper-store-turso/src/store/mod.rs index 99ad3e1db..280bff3d0 100644 --- a/crates/temper-store-turso/src/store/mod.rs +++ b/crates/temper-store-turso/src/store/mod.rs @@ -43,21 +43,28 @@ pub use field_index::QueryProjectionUpsert; use instrumentation::InstrumentedConnection; use libsql::params; -/// ARN-242: the schema version this build of the store migrates to. +/// ARN-242: a HUMAN-READABLE LABEL for the schema this build migrates to. /// -/// EVERY change to the DDL in [`TursoEventStore::migrate`] (new table, new -/// column, new index) MUST bump this, or databases already stamped at the -/// previous version will skip it. Platform-registry DDL lives in -/// `router.rs::migrate_platform`, OUTSIDE this ledger — bumping this constant -/// does nothing for it; it runs every boot and must stay fail-closed. +/// It is NOT the boot gate and is off the correctness path: the gate is +/// `SCHEMA_FINGERPRINT` (see below), and bumping this constant alone changes +/// nothing — the fingerprint is what decides whether a stamped database +/// re-runs the DDL. Bump it alongside a fingerprint change to give the schema +/// a name; never rely on it to make a migration reach existing databases. +/// +/// Platform-registry DDL lives in `router.rs::migrate_platform`, OUTSIDE this +/// ledger entirely; it runs every boot and must stay fail-closed. /// /// INVARIANT: every statement in the migration must be idempotent AND safe to /// run concurrently with another booting server. Two servers can both observe -/// an unstamped ledger and both run the full DDL; today every statement is -/// `CREATE … IF NOT EXISTS` or a benign-tolerated `ADD COLUMN`, and the stamp -/// is `INSERT OR IGNORE` on the version primary key, so the race is harmless. -/// A future version that backfills data or issues a bare `CREATE` must -/// serialize the migration explicitly. +/// an un-fingerprinted ledger and both run the full DDL; today every statement +/// is `CREATE … IF NOT EXISTS` or a benign-tolerated `ADD COLUMN`, and the +/// stamp is `INSERT OR REPLACE` on the version primary key — atomic within its +/// statement, so concurrent stampers converge on one identical row. (REPLACE +/// destroys the prior row for that version, which is what produces the +/// rollback re-run caveat on `SELECT_SCHEMA_FINGERPRINT_APPLIED`.) A future +/// version that backfills data or issues a bare `CREATE` must serialize the +/// migration explicitly — and note the fingerprint gate is SCHEMA-shaped, so a +/// data migration is invisible to it and needs its own gating row. const SCHEMA_VERSION: i64 = 1; /// Human-readable name recorded in the ledger for [`SCHEMA_VERSION`]. const SCHEMA_VERSION_NAME: &str = "baseline-idempotent-ddl"; From 5cd1b3b812dde3f505faa256528970282c347b8c Mon Sep 17 00:00:00 2001 From: Rita Agafonova <36133358+rita-aga@users.noreply.github.com> Date: Tue, 14 Jul 2026 10:53:19 -0700 Subject: [PATCH 6/7] docs(adr): renumber Turso migration ledger to ADR-0174 (unique) Sibling arena branches claimed 0162 (claude) and 0171 (codex). Use 0174 for the Grok-line ARN-242 record so the ADR number is unique. --- ...-migration-ledger.md => 0174-turso-migration-ledger.md} | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) rename docs/adrs/{0162-turso-migration-ledger.md => 0174-turso-migration-ledger.md} (97%) diff --git a/docs/adrs/0162-turso-migration-ledger.md b/docs/adrs/0174-turso-migration-ledger.md similarity index 97% rename from docs/adrs/0162-turso-migration-ledger.md rename to docs/adrs/0174-turso-migration-ledger.md index 30fd36a08..e024a405b 100644 --- a/docs/adrs/0162-turso-migration-ledger.md +++ b/docs/adrs/0174-turso-migration-ledger.md @@ -1,10 +1,11 @@ -# ADR-0162: Turso Schema Migration Ledger +# ADR-0174: Turso Schema Migration Ledger ## Status -Accepted (2026-07-12) +Accepted (2026-07-14) -(Numbered 0162: 0156–0161 are claimed by concurrently open arena branches.) +(Numbered 0174: unique on this arena branch. Sibling efforts used 0162 +(claude) and 0171 (codex); this record is the Grok-line ADR for ARN-242.) ## Context From e7bfa22cd7ff63195eb65b384a6c6eadb7658e43 Mon Sep 17 00:00:00 2001 From: Rita Agafonova <36133358+rita-aga@users.noreply.github.com> Date: Tue, 14 Jul 2026 12:18:50 -0700 Subject: [PATCH 7/7] fix(store-turso): keep execute_idempotent precondition in release (ARN-242) Promote the ADD COLUMN-only guard from debug_assert! to assert! so a CREATE routed through execute_idempotent cannot reintroduce silent swallow of "already exists" in release builds. Document that sqlite_test_url is UUID-unique so the poison-DB test cannot short- circuit on a leftover stamp. --- crates/temper-store-turso/src/store/mod.rs | 4 +++- crates/temper-store-turso/src/store/tests/mod.rs | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/temper-store-turso/src/store/mod.rs b/crates/temper-store-turso/src/store/mod.rs index 280bff3d0..7a55483f1 100644 --- a/crates/temper-store-turso/src/store/mod.rs +++ b/crates/temper-store-turso/src/store/mod.rs @@ -102,7 +102,9 @@ async fn execute_idempotent( // benign filter safe. Routing a CREATE through here would let a genuine // object-name collision ("already exists") be swallowed — exactly the // defect this function removes. - debug_assert!( + // Release builds keep this guard: tolerating "already exists" is only safe + // for ADD COLUMN. A CREATE routed here would re-introduce the swallow. + assert!( stmt.to_ascii_uppercase().contains("ADD COLUMN"), "PRECONDITION: only ADD COLUMN statements may use execute_idempotent; got: {stmt}" ); diff --git a/crates/temper-store-turso/src/store/tests/mod.rs b/crates/temper-store-turso/src/store/tests/mod.rs index fa28d40d1..a127aa480 100644 --- a/crates/temper-store-turso/src/store/tests/mod.rs +++ b/crates/temper-store-turso/src/store/tests/mod.rs @@ -2105,6 +2105,8 @@ async fn upsert_wasm_module_stores_metadata_only_without_db_blob() { /// instead of serving against a half-migrated schema. #[tokio::test] async fn migrate_surfaces_real_alter_errors() { + // sqlite_test_url embeds a fresh UUID — each run gets a clean file, so a + // prior stamp cannot short-circuit the DDL and turn this into a false pass. let url = sqlite_test_url("migrate-real-error"); // Poison the DB before the store ever runs its schema: a VIEW named