Skip to main content

max / makenotwork

sando: seal tier-state forward-advance; WAL+busy_timeout; lock /confirm (Run 2 S1/F2/F3) S1: the host build path advanced tier_state with a non-atomic SELECT-then-UPDATE outside deploy_lock — the one CF3 site the Run 1 fix never reached. Add a single sealed runs::advance_tier op (atomic self-referential UPDATE) and route both /promote and the host path through it; the host advance now runs under deploy_lock so it can't interleave with /rollback host. The promote atomicity test now drives the primitive instead of a copy of its SQL. F2: WAL journal mode + 5s busy_timeout on the SQLite pool so a second writer waits for the lock instead of surfacing SQLITE_BUSY as a 500 mid-deploy. F3: /confirm now holds deploy_lock so its read of current_version and the row it inserts reference the same tier state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-23 00:52 UTC
Commit: ff98486d0c667b205acff8f9c111b07b0d4e6880
Parent: 2d2eb4e
4 files changed, +70 insertions, -46 deletions
M sando/daemon/src/build.rs +9 -13
@@ -193,6 +193,7 @@ pub async fn build_and_run_host(
193 193 sha: GitSha,
194 194 events: crate::events::EventTx,
195 195 run_id: RunId,
196 + deploy_lock: Arc<tokio::sync::Mutex<()>>,
196 197 ) -> Result<()> {
197 198 let art = run(pool.clone(), cfg.clone(), topo.clone(), sha, events.clone(), run_id).await?;
198 199
@@ -233,19 +234,14 @@ pub async fn build_and_run_host(
233 234 let ok = gates::run_all(&ctx, &host.gates).await?;
234 235
235 236 if ok {
236 - let prev: Option<String> = sqlx::query_scalar(
237 - "SELECT current_version FROM tier_state WHERE tier = 'host'",
238 - )
239 - .fetch_optional(&pool).await?.flatten();
240 - sqlx::query(
241 - "UPDATE tier_state SET previous_version = ?, current_version = ?, burn_in_started_at = ?
242 - WHERE tier = 'host'",
243 - )
244 - .bind(prev)
245 - .bind(&art.version)
246 - .bind(Utc::now().to_rfc3339())
247 - .execute(&pool)
248 - .await?;
237 + // Advance the host tier through the single sealed forward-advance op, under
238 + // deploy_lock so this can't interleave with a concurrent `/rollback host`
239 + // (the old fetch-then-write here was the one CF3 site outside the lock —
240 + // ultra-fuzz Run 2, S1). Held only for the atomic UPDATE, never the gates.
241 + {
242 + let _deploy_guard = deploy_lock.lock().await;
243 + crate::runs::advance_tier(&pool, "host", &art.version).await?;
244 + }
249 245 crate::runs::mark_passed(&pool, run_id).await.ok();
250 246 tracing::info!(version = %art.version, "host pipeline green; ready to promote to next tier");
251 247 } else {
M sando/daemon/src/db.rs +8 -2
@@ -1,14 +1,20 @@
1 1 use anyhow::Result;
2 - use sqlx::sqlite::{SqliteConnectOptions, SqlitePoolOptions};
2 + use sqlx::sqlite::{SqliteConnectOptions, SqliteJournalMode, SqlitePoolOptions};
3 3 use sqlx::SqlitePool;
4 4 use std::path::Path;
5 5 use std::str::FromStr;
6 + use std::time::Duration;
6 7
7 8 pub async fn connect(path: &Path) -> Result<SqlitePool> {
8 9 let url = format!("sqlite://{}?mode=rwc", path.display());
9 10 let opts = SqliteConnectOptions::from_str(&url)?
10 11 .create_if_missing(true)
11 - .foreign_keys(true);
12 + .foreign_keys(true)
13 + // WAL lets readers run concurrently with a writer; busy_timeout makes a
14 + // second writer wait for the lock instead of erroring out as SQLITE_BUSY
15 + // (which surfaced as a 500 to the operator mid-deploy — ultra-fuzz Run 2).
16 + .journal_mode(SqliteJournalMode::Wal)
17 + .busy_timeout(Duration::from_secs(5));
12 18 let pool = SqlitePoolOptions::new().max_connections(4).connect_with(opts).await?;
13 19 Ok(pool)
14 20 }
M sando/daemon/src/routes.rs +20 -31
@@ -436,24 +436,15 @@ async fn promote(
436 436 }
437 437 }
438 438
439 - // 4. Advance tier_state in a single atomic UPDATE: `previous_version` is set
440 - // from the row's *old* `current_version` (SQLite evaluates all RHS
441 - // expressions against the original row), so there is no read-modify-write
442 - // to lose under concurrency (CF3) — the deploy_lock serializes promotes,
443 - // and this statement needs no separate SELECT. burn_in_started_at = now
444 - // starts the target tier's burn_in clock ticking. reset_burn_in on the
445 - // *source* tier nulls its clock only when the operator explicitly asked.
446 - sqlx::query(
447 - "UPDATE tier_state
448 - SET previous_version = current_version,
449 - current_version = ?,
450 - burn_in_started_at = ?
451 - WHERE tier = ?",
452 - )
453 - .bind(&version)
454 - .bind(chrono::Utc::now().to_rfc3339())
455 - .bind(&target.name)
456 - .execute(&s.pool).await.map_err(crate::error::Error::Db)?;
439 + // 4. Advance tier_state through the single sealed forward-advance op (atomic
440 + // self-referential UPDATE; no read-modify-write to lose under concurrency,
441 + // CF3). We hold deploy_lock for this whole handler, so the advance is
442 + // serialized against rollback and the host build path's advance.
443 + // reset_burn_in on the *source* tier nulls its clock only when the operator
444 + // explicitly asked.
445 + crate::runs::advance_tier(&s.pool, target.name.as_str(), &version)
446 + .await
447 + .map_err(crate::error::Error::Db)?;
457 448
458 449 if body.reset_burn_in {
459 450 sqlx::query("UPDATE tier_state SET burn_in_started_at = NULL WHERE tier = ?")
@@ -824,8 +815,9 @@ async fn rebuild(
824 815 let sha_for_task = sha.clone();
825 816 let sha_response = sha.to_string();
826 817 let pool_for_task = s.pool.clone();
818 + let deploy_lock = s.deploy_lock.clone();
827 819 let handle = tokio::spawn(async move {
828 - if let Err(e) = crate::build::build_and_run_host(pool, cfg, topo, sha_for_task.clone(), events_for_task, run_id).await {
820 + if let Err(e) = crate::build::build_and_run_host(pool, cfg, topo, sha_for_task.clone(), events_for_task, run_id, deploy_lock).await {
829 821 tracing::error!(sha = %sha_for_task, error = %e, "rebuild pipeline failed");
830 822 // Pre-gate bails (fetch/checkout/version/scratch) don't settle the
831 823 // run themselves; the build-step compile error already did. First
@@ -954,6 +946,11 @@ async fn confirm(
954 946 // Operator-driven satisfaction of a `manual_confirm` gate. Looks up the
955 947 // pending version (current MM version, or the tier's own if non-mm) and
956 948 // inserts a passing gate_runs row so /promote can advance.
949 + //
950 + // Hold deploy_lock so the version we read and the row we insert reference the
951 + // same tier state — a concurrent rollback can't change current_version between
952 + // the read and the insert (ultra-fuzz Run 2, F3).
953 + let _deploy_guard = s.deploy_lock.lock().await;
957 954 let tier = crate::domain::TierId::new(tier);
958 955 let target = s.topo.tiers.iter().find(|t| t.name == tier)
959 956 .ok_or(crate::error::Error::NotFound)?;
@@ -1708,18 +1705,10 @@ mod tests {
1708 1705 sqlx::query("UPDATE tier_state SET current_version = '1.0.0' WHERE tier = 'a'")
1709 1706 .execute(&pool).await.unwrap();
1710 1707
1711 - // The exact advance statement /promote runs.
1712 - sqlx::query(
1713 - "UPDATE tier_state
1714 - SET previous_version = current_version,
1715 - current_version = ?,
1716 - burn_in_started_at = ?
1717 - WHERE tier = ?",
1718 - )
1719 - .bind("2.0.0")
1720 - .bind("2026-01-01T00:00:00Z")
1721 - .bind("a")
1722 - .execute(&pool).await.unwrap();
1708 + // Exercise the sealed forward-advance primitive itself — the same op
1709 + // /promote and the host build path both call (S1), not a copy of its SQL.
1710 + let v = crate::domain::Version::parse("2.0.0").unwrap();
1711 + crate::runs::advance_tier(&pool, "a", &v).await.unwrap();
1723 1712
1724 1713 let (cur, prev): (Option<String>, Option<String>) = sqlx::query_as(
1725 1714 "SELECT current_version, previous_version FROM tier_state WHERE tier = 'a'",
M sando/daemon/src/runs.rs +33
@@ -66,6 +66,39 @@ pub async fn set_phase(pool: &SqlitePool, run_id: RunId, phase: Phase) -> Result
66 66 Ok(())
67 67 }
68 68
69 + /// Forward-advance a tier to `version` in a single atomic UPDATE. `previous_version`
70 + /// is set from the row's *old* `current_version` (SQLite evaluates every RHS against
71 + /// the original row), so there is no read-modify-write to lose under concurrency
72 + /// (CF3) — no separate SELECT exists to race. `burn_in_started_at = now` starts the
73 + /// tier's burn-in clock.
74 + ///
75 + /// This is the *only* forward-advance writer of `tier_state`. The host build path
76 + /// and `/promote` both go through it; keeping the fetch-then-write shape out of the
77 + /// codebase is the point (ultra-fuzz Run 2, S1). Callers MUST hold `deploy_lock` so
78 + /// the logical advance is serialized against `/rollback`.
79 + ///
80 + /// Returns the raw `sqlx::Error` so the route layer can map it to its typed
81 + /// `Error::Db`; anyhow callers (the build pipeline) get `?`-conversion for free.
82 + pub async fn advance_tier(
83 + pool: &SqlitePool,
84 + tier: &str,
85 + version: &Version,
86 + ) -> Result<(), sqlx::Error> {
87 + sqlx::query(
88 + "UPDATE tier_state
89 + SET previous_version = current_version,
90 + current_version = ?,
91 + burn_in_started_at = ?
92 + WHERE tier = ?",
93 + )
94 + .bind(version)
95 + .bind(Utc::now().to_rfc3339())
96 + .bind(tier)
97 + .execute(pool)
98 + .await?;
99 + Ok(())
100 + }
101 +
69 102 /// Record the version once it's been read from the worktree's Cargo.toml.
70 103 pub async fn set_version(pool: &SqlitePool, run_id: RunId, version: &Version) -> Result<()> {
71 104 sqlx::query("UPDATE build_runs SET version = ? WHERE id = ? AND result = 'building'")