sbx-mgr: callers use the store's own handle and transaction types

ARCHITECTURE.toml

@@ -250,10 +250,12 @@ mode = "required"
 # connection or transaction, using the shared SQLite open and migration
 # helpers or the shared job store's SQL functions, binding parameters,
 # executing a statement, or writing an SQL statement literal outside the
-# store is a crossing. Other services' crates use rusqlite for their own
-# databases, so `paths` limits the rule to sbx-mgr's. Existing callers that
-# still name the connection, error and transaction types are baselined
-# until they use the store's own.
+# store is a crossing; callers use the store's own handle, transaction and
+# error types. Other services' crates use rusqlite for their own databases,
+# so `paths` limits the rule to sbx-mgr's. The patterns are a line-based
+# tripwire: aliases, wrapped imports and calls through names imported
+# earlier escape them, so they catch common crossings rather than prove
+# their absence.
 [rules.pattern_ratchet.sbxmgr_sqlite]
 patterns = [
   '\brusqlite\s*::',
@@ -278,9 +280,9 @@ allow = [
 [rules.pattern_ratchet.sbxmgr_sqlite.no_crossing]
 mode = "required"
 
-# Seeding the ratchet reports its initial entries against the base.
+# The baseline is empty: no file outside the store may be added to it.
 [rules.pattern_ratchet.sbxmgr_sqlite.no_new_entries]
-mode = "advisory"
+mode = "required"
 
 [rules.pattern_ratchet.sbxmgr_sqlite.no_raised_caps]
 mode = "required"

Cargo.toml

@@ -226,6 +226,7 @@ prost-types = "0.14"
 rand = "0.10"
 rcgen = { version = "0.14", default-features = false }
 redis = { version = "1", default-features = false, features = ["cluster-async", "script", "tokio-rustls-comp", "tls-rustls-webpki-roots"] }
+ref-cast = "1.0.27"
 regex = "1"
 reqwest = { version = "0.13", default-features = false, features = ["json", "rustls"] }
 rstest = { version = "0.27", default-features = false }

architecture-baseline.toml

@@ -75,16 +75,16 @@
 "crates/libs/object-store/src/lib.rs" = 1810
 "crates/libs/observability/src/lib.rs" = 1063
 "crates/libs/public/api/src/lib.rs" = 1793
-"crates/libs/sbx/mgr/server/src/db/auto_lifecycle_delete_receipts.rs" = 1158
+"crates/libs/sbx/mgr/server/src/db/auto_lifecycle_delete_receipts.rs" = 1149
 "crates/libs/sbx/mgr/server/src/db/cleanup_episodes.rs" = 1833
 "crates/libs/sbx/mgr/server/src/db/quarantine_recovery.rs" = 1042
-"crates/libs/sbx/mgr/server/src/db/sandboxes.rs" = 1547
-"crates/libs/sbx/mgr/server/src/db/state_transitions.rs" = 1016
-"crates/libs/sbx/mgr/server/src/run.rs" = 1101
+"crates/libs/sbx/mgr/server/src/db/sandboxes.rs" = 1541
+"crates/libs/sbx/mgr/server/src/db/state_transitions.rs" = 1012
+"crates/libs/sbx/mgr/server/src/run.rs" = 1091
 "crates/libs/sbx/mgr/server/src/sandbox_eviction.rs" = 1253
 "crates/libs/sbx/mgr/server/src/service/activation_support.rs" = 1574
 "crates/libs/sbx/mgr/server/src/service/auth_lookup.rs" = 1221
-"crates/libs/sbx/mgr/server/src/service/cache_cleanup.rs" = 1455
+"crates/libs/sbx/mgr/server/src/service/cache_cleanup.rs" = 1452
 "crates/libs/sbx/mgr/server/src/service/cleanup.rs" = 2704
 "crates/libs/sbx/mgr/server/src/service/create.rs" = 1195
 "crates/libs/sbx/mgr/server/src/service/grpc_core.rs" = 1087
@@ -145,50 +145,14 @@
 "crates/libs/node/mgr/server/src/service/artifact_fs/receive.rs" = 4
 "crates/libs/node/mgr/server/src/net/fallback.rs" = 1
 
-# sbx-mgr code outside its SQLite store that still names rusqlite types, the
-# shared SQLite helpers or the shared job store, or opens a transaction itself.
+# sbx-mgr code outside its SQLite store uses only the store's own handle,
+# transaction and error types, so no file is listed.
 [pattern_ratchet.sbxmgr_sqlite]
-"crates/libs/sbx/mgr/server/src/lifecycle_metrics.rs" = 1
-"crates/libs/sbx/mgr/server/src/local_artifacts.rs" = 28
-"crates/libs/sbx/mgr/server/src/node_state_sync.rs" = 1
-"crates/libs/sbx/mgr/server/src/route_activation_claims.rs" = 2
-"crates/libs/sbx/mgr/server/src/run.rs" = 6
-"crates/libs/sbx/mgr/server/src/runtime_reconciliation.rs" = 1
-"crates/libs/sbx/mgr/server/src/sandbox_eviction.rs" = 5
-"crates/libs/sbx/mgr/server/src/service/activation_support.rs" = 9
-"crates/libs/sbx/mgr/server/src/service/auth_lookup.rs" = 5
-"crates/libs/sbx/mgr/server/src/service/cache_cleanup.rs" = 12
-"crates/libs/sbx/mgr/server/src/service/cleanup.rs" = 15
-"crates/libs/sbx/mgr/server/src/service/cleanup_executor.rs" = 7
-"crates/libs/sbx/mgr/server/src/service/cleanup_support.rs" = 7
-"crates/libs/sbx/mgr/server/src/service/cleanup_targets.rs" = 1
-"crates/libs/sbx/mgr/server/src/service/create_support.rs" = 5
-"crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/pause_resume.rs" = 2
-"crates/libs/sbx/mgr/server/src/service/grpc_lifecycle/teardown_suspend.rs" = 2
-"crates/libs/sbx/mgr/server/src/service/grpc_pool_wait_clone.rs" = 1
-"crates/libs/sbx/mgr/server/src/service/grpc_spaced_process_snapshots/rollback_state.rs" = 1
-"crates/libs/sbx/mgr/server/src/service/grpc_state_snapshot.rs" = 1
-"crates/libs/sbx/mgr/server/src/service/mod.rs" = 13
-"crates/libs/sbx/mgr/server/src/service/network_runtime.rs" = 5
-"crates/libs/sbx/mgr/server/src/service/recovery.rs" = 5
-"crates/libs/sbx/mgr/server/src/service/recovery_cross_node.rs" = 12
-"crates/libs/sbx/mgr/server/src/service/start_support.rs" = 1
-"crates/libs/sbx/mgr/server/src/service/startup_resident.rs" = 1
-"crates/libs/sbx/mgr/server/src/service/storage.rs" = 1
-"crates/libs/sbx/mgr/server/src/service/template_snapshot_support.rs" = 3
-"crates/libs/sbx/mgr/server/src/service/test_options.rs" = 1
-"crates/libs/sbx/mgr/server/src/snapshot_gc.rs" = 1
-"crates/libs/sbx/mgr/server/src/snapshot_restore.rs" = 5
-"crates/libs/sbx/mgr/server/src/snapshot_scheduler.rs" = 3
-"crates/libs/sbx/mgr/server/src/snapshot_upload_jobs.rs" = 19
-"crates/libs/sbx/mgr/server/src/watchdog.rs" = 21
 
 # Code outside the store that still names sbx-mgr's migrations or database
-# path: its open and migration in `run.rs`, and the migrator, until it calls
-# the store.
+# path: the database migrator, until it calls the store.
 [pattern_ratchet.sbxmgr_sqlite_schema]
 "crates/libs/db/migrator/src/lib.rs" = 5
-"crates/libs/sbx/mgr/server/src/run.rs" = 3
 
 # [crate_names] maps each workspace package with a catch-all name (a part such
 # as `common`, `utils` or `core`) to its member path. Only remove entries: a pull

crates/libs/common/jobs/src/lib.rs

@@ -31,7 +31,8 @@ mod worker;
 
 pub use worker::{
     BackgroundWorker, COMPLETED_JOB_RETENTION, DuplicatePolicy, FAILED_JOB_RETENTION, LeaseConfig,
-    PriorityBand, SubjectConcurrency, TERMINAL_JOB_PRUNE_INTERVAL, WorkerConfig,
+    PriorityBand, PruneTerminalJobsError, SubjectConcurrency, TERMINAL_JOB_PRUNE_INTERVAL,
+    WorkerConfig,
 };
 
 /// Caller-selected scheduling priority. Higher values run first within a job kind.

crates/libs/common/jobs/src/worker.rs

@@ -1,4 +1,4 @@
 use pplx_space_common_core::types::{JobId, SubjectId};
 use std::collections::HashMap;
 use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::{Arc, Mutex, MutexGuard};
@@ -370,6 +370,13 @@
     Serialized,
 }
 
+/// A terminal-job prune that failed in the database. `Display` and `source`
+/// are the SQLite error's; the error itself stays private to this crate, so a
+/// worker's owner gets no database value back.
+#[derive(Debug, thiserror::Error)]
+#[error(transparent)]
+pub struct PruneTerminalJobsError(rusqlite::Error);
+
 /// A background job worker that persists jobs to SQLite and drives execution
 /// with bounded concurrency, exponential-backoff retry, and graceful drain.
 ///
@@ -518,10 +525,14 @@
 
     /// Preserve database failures so maintenance callers can retry without
     /// treating an unsuccessful pass as a successful zero-deletion pass.
-    pub fn try_prune_terminal_jobs_once(&self, worker_label: &str) -> rusqlite::Result<usize> {
+    pub fn try_prune_terminal_jobs_once(
+        &self,
+        worker_label: &str,
+    ) -> Result<usize, PruneTerminalJobsError> {
         let pruned = {
             let conn = self.lock_db_for_background();
-            db::prune_terminal_jobs(&conn, COMPLETED_JOB_RETENTION, FAILED_JOB_RETENTION)?
+            db::prune_terminal_jobs(&conn, COMPLETED_JOB_RETENTION, FAILED_JOB_RETENTION)
+                .map_err(PruneTerminalJobsError)?
         };
         if pruned > 0 {
             tracing::info!(

crates/libs/sbx/mgr/server/Cargo.toml

@@ -41,6 +41,7 @@ jiff = { workspace = true }
 libc = { workspace = true }
 prost = { workspace = true }
 prost-types = { workspace = true }
+ref-cast = { workspace = true }
 rusqlite = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
 serde_json = { workspace = true }

crates/libs/sbx/mgr/server/src/db.rs

@@ -30,7 +30,11 @@ use pplx_space_net_common::{NetworkPolicy, proto::NetworkPolicySource};
 use rusqlite::types::Type;
 
 use pplx_space_common_core::sql;
-use rusqlite::{Connection, OptionalExtension, Result, params};
+use rusqlite::{OptionalExtension as _, params};
+
+mod handle;
+pub(crate) use handle::OptionalExtension;
+pub use handle::{Connection, ConnectionGuard, Error, Result, SharedConnection, Transaction};
 
 pub(crate) mod auto_lifecycle_delete_receipts;
 pub(crate) mod cleanup_jobs;
@@ -109,7 +113,7 @@ pub use pplx_space_common_core::types::QuarantineRecoveryNoProgressReason;
 
 const EXECUTION_DELETE_RECEIPT_RETENTION_SECS: i64 = 7 * 24 * 60 * 60;
 
-fn parse_policy_authority(value: &str, column: usize) -> Result<PolicyAuthority> {
+fn parse_policy_authority(value: &str, column: usize) -> rusqlite::Result<PolicyAuthority> {
     value.parse().map_err(|error| {
         rusqlite::Error::FromSqlConversionFailure(column, Type::Text, Box::new(error))
     })
@@ -218,16 +222,47 @@ impl CleanupCompletion {
 /// background loops evict them.
 const STATEMENT_CACHE_CAPACITY: usize = 128;
 
-pub fn open() -> Result<Connection, pplx_space_common_svc::sql::sqlite::SqliteError> {
+/// Opening or migrating the database failed. `Display` and `source` are the
+/// shared SQLite helpers' error's, unchanged.
+#[derive(Debug, thiserror::Error)]
+#[error(transparent)]
+pub struct OpenError(#[from] pplx_space_common_svc::sql::sqlite::SqliteError);
+
+/// Apply the service and common schema migrations to the database.
+pub async fn migrate() -> Result<(), OpenError> {
+    migrate_at(Path::new(DB_PATH)).await
+}
+
+/// [`migrate`] for the database at `db_path`.
+pub async fn migrate_at(db_path: &Path) -> Result<(), OpenError> {
+    use pplx_space_sbxmgr_migrations::{COMMON_SCHEMA_MIGRATOR, SANDBOX_MANAGER_MIGRATOR};
+    Ok(pplx_space_common_svc::sql::sqlite::migrate_embedded(
+        "sandbox-manager",
+        db_path,
+        pplx_space_common_svc::sql::sqlite::EmbeddedMigrators {
+            service: &SANDBOX_MANAGER_MIGRATOR,
+            common: Some(&COMMON_SCHEMA_MIGRATOR),
+        },
+    )
+    .await?)
+}
+
+/// Open the migrated database, with the connection settings every store
+/// operation assumes.
+pub fn open() -> Result<Connection, OpenError> {
     let conn = pplx_space_common_svc::sql::sqlite::open_rusqlite(
         Path::new(DB_PATH),
         pplx_space_common_svc::sql::sqlite::RusqliteOpenOptions::default(),
     )?;
     conn.set_prepared_statement_cache_capacity(STATEMENT_CACHE_CAPACITY);
-    Ok(conn)
+    Ok(Connection::from_raw(conn))
+}
+
+pub(crate) fn describe_error(error: &Error) -> String {
+    describe_raw_error(error.raw())
 }
 
-pub(crate) fn describe_error(error: &rusqlite::Error) -> String {
+fn describe_raw_error(error: &rusqlite::Error) -> String {
     match error.sqlite_error() {
         Some(sqlite) => format!(
             "{error} (sqlite_code={:?}, sqlite_extended_code={})",
@@ -237,8 +272,8 @@ pub(crate) fn describe_error(error: &rusqlite::Error) -> String {
     }
 }
 
-pub(crate) fn immediate_transaction(conn: &Connection) -> Result<rusqlite::Transaction<'_>> {
-    rusqlite::Transaction::new_unchecked(conn, rusqlite::TransactionBehavior::Immediate)
+pub(crate) fn immediate_transaction(conn: &Connection) -> Result<Transaction<'_>> {
+    Transaction::immediate(conn)
 }
 
 pub(crate) fn now() -> Result<String> {
@@ -257,9 +292,7 @@ struct PersistedClockBeforeEpoch;
 
 fn persisted_epoch_seconds(timestamp: jiff::Timestamp) -> Result<i64> {
     if timestamp < jiff::Timestamp::UNIX_EPOCH {
-        return Err(rusqlite::Error::ToSqlConversionFailure(Box::new(
-            PersistedClockBeforeEpoch,
-        )));
+        return Err(Error::to_sql_conversion(PersistedClockBeforeEpoch));
     }
     Ok(timestamp.as_second())
 }
@@ -480,9 +513,9 @@ fn test_schema_image() -> &'static [u8] {
         if let Some(image) = cache.as_ref().and_then(|cache| cache.read()) {
             return image;
         }
-        let mut conn = Connection::open_in_memory().unwrap();
+        let mut conn = Connection::from_raw(rusqlite::Connection::open_in_memory().unwrap());
         create_test_schema(&mut conn);
-        let image = conn.serialize(rusqlite::MAIN_DB).unwrap().to_vec();
+        let image = conn.raw().serialize(rusqlite::MAIN_DB).unwrap().to_vec();
         if let Some(cache) = &cache {
             cache.write(&image);
         }
@@ -511,10 +544,10 @@ pub(crate) fn create_test_schema_on_disk(db_path: &std::path::Path) {
 #[cfg(test)]
 pub(crate) fn test_conn() -> Connection {
     let image = test_schema_image();
-    let mut conn = Connection::open_in_memory().unwrap();
+    let mut conn = rusqlite::Connection::open_in_memory().unwrap();
     conn.deserialize_read_exact(rusqlite::MAIN_DB, image, image.len(), false)
         .unwrap();
-    conn
+    Connection::from_raw(conn)
 }
 
 /// Seed pre-validation lifecycle evidence without weakening subsequent writes.
@@ -529,7 +562,7 @@ pub(crate) fn seed_historical_lifecycle_rows<T>(
     conn: &Connection,
     seed: impl FnOnce(&Connection) -> T,
 ) -> T {
-    let tx = conn.unchecked_transaction().unwrap();
+    let tx = Transaction::deferred(conn).unwrap();
     let validators: Vec<(String, String)> = tx
         .prepare(
             "SELECT name, sql FROM sqlite_master
@@ -541,7 +574,7 @@ pub(crate) fn seed_historical_lifecycle_rows<T>(
         .unwrap()
         .query_map([], |row| Ok((row.get(0)?, row.get(1)?)))
         .unwrap()
-        .collect::<Result<_>>()
+        .collect::<rusqlite::Result<_>>()
         .unwrap();
     assert!(
         !validators.is_empty(),
@@ -558,6 +591,15 @@ pub(crate) fn seed_historical_lifecycle_rows<T>(
     result
 }
 
+/// What tests outside the store need for raw SQL fixtures and tripwires,
+/// besides the statement methods of [`Connection`].
+#[cfg(test)]
+pub(crate) mod fixture {
+    pub(crate) use rusqlite::trace::{TraceEvent, TraceEventCodes};
+    pub(crate) use rusqlite::types::{Type, Value};
+    pub(crate) use rusqlite::{StatementStatus, params};
+}
+
 #[cfg(test)]
 pub(crate) mod tests;
 

crates/libs/sbx/mgr/server/src/db/auto_lifecycle.rs

@@ -121,10 +121,10 @@ pub(crate) fn due_auto_lifecycle_batch(
         let (id, deadline) = match (row.get::<_, String>(1), row.get::<_, Option<i64>>(2)) {
             (Ok(id), Ok(deadline)) => (id, deadline),
             (Err(error), _) | (_, Err(error)) if is_row_decode_error(&error) => {
-                tracing::warn!(rowid = row.get::<_, i64>(0)?, error = %describe_error(&error), "skipping malformed auto lifecycle discovery row");
+                tracing::warn!(rowid = row.get::<_, i64>(0)?, error = %describe_raw_error(&error), "skipping malformed auto lifecycle discovery row");
                 return Ok(None);
             }
-            (Err(error), _) | (_, Err(error)) => return Err(error),
+            (Err(error), _) | (_, Err(error)) => return Err(Error::from(error)),
         };
         let Some(deadline) = deadline else {
             return Ok(None);
@@ -140,10 +140,10 @@ pub(crate) fn due_auto_lifecycle_batch(
         let state = match row.get::<_, String>(3) {
             Ok(state) => state,
             Err(error) if is_row_decode_error(&error) => {
-                tracing::warn!(rowid = row.get::<_, i64>(0)?, error = %describe_error(&error), "skipping malformed auto lifecycle discovery row");
+                tracing::warn!(rowid = row.get::<_, i64>(0)?, error = %describe_raw_error(&error), "skipping malformed auto lifecycle discovery row");
                 return Ok(None);
             }
-            Err(error) => return Err(error),
+            Err(error) => return Err(Error::from(error)),
         };
         if state.parse::<SandboxState>().ok() == Some(SandboxState::Deleting) {
             return Ok(None);