[ac-pplx]

Summary

Milestone 2 of 3, stacked on #561. It concerns sbx-mgr's node-local SQLite database (sandboxes and every other sbx-mgr table), not the control-plane Postgres. The store, crates/libs/sbx/mgr/server/src/db, now defines every type its callers hold. No code outside db/ names rusqlite, and the sbxmgr_sqlite ratchet baseline is empty.

The store's handle types (db/handle.rs)

Type	Replaces	Behavior
db::SharedConnection	Arc<Mutex<rusqlite::Connection>>	Wraps the same Arc<Mutex<rusqlite::Connection>>, because common-jobs' BackgroundWorker must share that mutex (SharedConnection::background_worker). lock() returns LockResult<ConnectionGuard> with Mutex::lock's poisoning: a poisoned lock still yields its guard through the PoisonError, whose text is unchanged. is_poisoned() delegates.
db::ConnectionGuard	MutexGuard<'_, rusqlite::Connection>	Derefs to db::Connection.
db::Connection	rusqlite::Connection	The connection view every db function takes as &Connection. It is #[repr(transparent)] over rusqlite's connection. #[derive(RefCastCustom)] generates only a private view(&rusqlite::Connection) -> &Connection; there is no public RefCast impl. The crate keeps #![forbid(unsafe_code)]. raw(), execute, prepare, prepare_cached and query_row are private to db in production builds (see below).
db::Transaction::{immediate, deferred}	rusqlite::Transaction::new_unchecked(.., Immediate), unchecked_transaction(), transaction()	Wraps rusqlite's transaction, derefs to Connection, and rolls back on drop. commit/rollback return db::Result.
db::Error / db::Result	rusqlite::Error / rusqlite::Result	thiserror, #[error(transparent)] over rusqlite::Error, so Display and source() are byte-for-byte the same. Callers use is_no_rows()/is_invalid_query() instead of matching rusqlite variants. Error::raw() is private to db in production builds. The constructors invalid_query, no_rows, poisoned and to_sql_conversion build the same rusqlite variants as before, so their texts are identical.
db::OpenError	pplx_space_common_svc::sql::sqlite::SqliteError in run.rs	Transparent over the shared helper's error.

Every db function and db error enum now takes these types and returns or wraps db::Error. Row decoders inside db keep rusqlite::Result, since rusqlite's row callbacks require it.

Opening and migrating move into db

• run.rs's migration moves to db::migrate() / db::migrate_at(path), with the same migrators in the same order.
• db::open() keeps the same open helper, pragmas and statement-cache capacity.
• RunSbxMgrError::{Migrate, OpenDatabase} keep their messages ("failed to migrate the sandbox-manager database: {0}", "failed to open the sandbox-manager database: {0}"). Their {0} and source chain are unchanged because OpenError is transparent.
• The test connections (test_conn, create_test_schema*) and the serialize-based schema image were already in db.

Callers

• Every production caller outside db/ is converted:
  • 116 Arc<Mutex<rusqlite::Connection>>/&rusqlite::Connection spellings;
  • the 16 transaction openings;
  • the error matches in service/mod.rs, activation_support.rs and grpc_lifecycle/teardown_suspend.rs;
  • the poison-mapping helpers.
• connection_or_signal_failure takes &SharedConnection and keeps its semantics exactly: on a poisoned lock it logs, notifies the supervised outbox worker and returns DatabaseMutexPoisoned.
• main's code since the original base is converted the same way. The retired-teardown release claim in db/eviction.rs ([backup] refuse teardown-suspend backups from retired executions and release their runtimes #331) passes Connection::raw() to the job store from inside db, and collects its artifact rows into db::Result as the other readers do. main's new tests take db::Connection, bind with db::fixture::params! and pass conn.raw() to the job store: grpc_lifecycle/retired_teardown_tests.rs, service/tests/node_manager_transport_tests.rs and db/tests/stopped_artifact_pin.rs.
• sbx-mgr: route every SQLite statement through db/ and fence the rest with cargo arch #561 round-1 item A1: service/cleanup_executor.rs no longer imports pplx_space_common_jobs::db. Its reset, get, claim, yield and retry calls go through new db::cleanup_jobs functions (reset_running, reset_inactive_running, get_job, mark_running, yield_running, retry_running) that wrap the same job-store calls.

What production code outside db can reach

The boundary, exactly. In production builds (not(test)), code outside db gets no rusqlite value from the store's types or from the jobs worker the store builds:

• Methods private to db: Connection::raw(), Error::raw(), the statement methods execute, prepare, prepare_cached and query_row, Connection::from_raw and Error::decode are pub(in crate::db). So are the row-level helpers that take or return rusqlite values: timestamps::EpochSeconds::decode, timestamps::diagnostic_text and inventory_read::check_row. The ref-cast view is private to db/handle.rs.
• The jobs worker: SharedConnection::background_worker builds common-jobs' BackgroundWorker inside db from the private Arc<Mutex<rusqlite::Connection>> and returns only the worker. The worker's one public method that returned a rusqlite value, try_prune_terminal_jobs_once, now returns common-jobs' own PruneTerminalJobsError. That type is #[error(transparent)] over a private rusqlite::Error field and has no From impl, so its Display (logged by snapshot_upload_jobs.rs) is byte-identical and no rusqlite value comes out. snapshot_upload_jobs.rs is that method's only caller outside common-jobs.
• What remains public: db::Error's #[from] rusqlite::Error conversion, an inbound From impl that hands nothing out. This PR keeps everything in one crate, so cfg(test) widens these methods to the whole crate; milestone 3 replaces that with the store's test-support feature.

Under cfg(test), the six raw/statement methods above are pub(crate) for fixtures and tripwires. One macro in handle.rs declares each method twice, once per cfg, so both visibilities share one body. I checked this by adding conn.raw(), error.raw() and conn.prepare(..) to a production module: cargo check --lib rejects all three with E0624.

Tests

Exactly these items are test-only (#[cfg(test)], pub(crate)):

• Connection::{open_in_memory, open_at, pragma_update, pragma_query_value, busy_timeout, trace_v2, execute_batch};
• SharedConnection::try_lock;
• Error::{is_database_busy, conversion_failure, into_raw};
• db::fixture, which re-exports params!, the trace types, Value, Type and StatementStatus;
• the test connections and schema image (test_conn, create_test_schema*, seed_historical_lifecycle_rows);
• store functions that were already #[cfg(test)] before this PR:
  • complete_row_intent_cleanup{,_with_receipt}, clear_cleanup_intent, list_requested_cleanup_recovery_candidate_ids, mark_quarantined and scan_quarantine_recovery_candidates;
  • route_activation_claims::claim_exists;
  • mark_{stopping,deleting}_superseding_activation (and their private helper), mark_{starting,restoring}_quarantined, rollback_{starting,restoring} and update_state_from_states.

The six methods above, raw, execute, prepare, prepare_cached, query_row and Error::raw, are test-visible but db-private in production. is_autocommit() is pub(crate) in every build, because it returns only a bool.

Tests that call the shared job store directly pass Connection::raw(). run/tests.rs' worker-drain test now builds its worker over db::test_conn() (the full schema, which includes jobs) instead of a hand-migrated common-schema connection. No test changed what it asserts.

Counts (outside db/)

	#561 head	this PR
SQL statement lines (production)	0	0
rusqlite:: / use rusqlite lines, production	185 lines in 33 files	0
rusqlite:: / use rusqlite lines, tests	305	0
[pattern_ratchet.sbxmgr_sqlite] baseline	34 entries, 203 lines	0 entries (empty table)
[pattern_ratchet.sbxmgr_sqlite_schema] baseline	2 entries, 8 lines	1 entry, 5 lines (the database migrator, moved in milestone 3)

Counts are code lines, comments excluded, as archcheck counts them.

file_size: no cap raised. Caps are lowered where files shrank, and one is tightened:

• db/auto_lifecycle_delete_receipts.rs 1158→1149;
• db/sandboxes.rs 1547→1541;
• db/state_transitions.rs 1016→1012;
• run.rs 1101→1091;
• service/cache_cleanup.rs 1455→1452. This one tightens the cap to the file's current size; the file did not shrink in this PR.
• service/auth_lookup.rs keeps 1221 and db/cleanup_episodes.rs keeps 1833: main grew both past their caps (to 1222 and 1845), and this PR brings them back to exactly those caps.

Hidden observable changes

• Log targets: none renamed; no tracing event moved between modules.
• sql! locators: sql! statements in db files this PR edits keep their module paths. Their line!() values shift with the edits above them. No statement changed text otherwise.
• Error source chains:
  • #[from]/#[source] fields that held rusqlite::Error now hold db::Error, whose Display and source() are rusqlite's.
  • So rendered chains are identical, but source().downcast_ref::<rusqlite::Error>() would now see db::Error. Nothing in the workspace downcasts that way.
• No file!()-derived values are involved.

Review round 2

Finding	Action
[MAJOR] the worker leaks a raw rusqlite::Error through try_prune_terminal_jobs_once	The method returns common-jobs' new PruneTerminalJobsError (transparent Display, private rusqlite::Error field, no From). git grep finds one caller outside common-jobs, snapshot_upload_jobs.rs. It logs it with %error, so the log text is unchanged. prune_terminal_jobs_once logs the same Display as before.
[MAJOR] crate-visible signatures with rusqlite types	EpochSeconds::decode, diagnostic_text and check_row are pub(in crate::db); all their callers are in db/. A sweep of every pub/pub(crate) signature and field in db/, including names imported from rusqlite under short aliases, finds only #[cfg(test)] items and private tuple fields.
body and commit message	The boundary is stated exactly above, and the test-only list includes the store functions that were already #[cfg(test)].

Review round 1

Finding	Action
3 [MAJOR] #[derive(RefCast)] publishes the raw type	#[derive(RefCastCustom)] with a private #[ref_cast_custom] fn view. There is no RefCast impl, and forbid(unsafe_code) holds.
4 [MAJOR/MINOR] Connection::raw() and Error::raw() reachable from production	Both are pub(in crate::db) outside cfg(test) and pub(crate) under it. So are execute, prepare, prepare_cached and query_row, which return rusqlite statements. A production probe fails with E0624.
5 [NIT] due_job_ids' doc comment displaced	Moved back onto due_job_ids.
6 [NIT] no_new_entries advisory with an empty baseline	Now required, and the comment is fixed.
7 [NIT] body	cache_cleanup.rs is described as tightened; the test-only surface is listed exactly.

Rebased onto #561's new head (round 2). The only conflict was the sbxmgr_sqlite_schema baseline, which now lists the migrator at 5 lines.

Rebased with the stack onto main 825b786. Conflicts: Cargo.lock (regenerated); the baseline (file_size caps recomputed, node_manager_commands kept); and db/local_artifacts.rs, where main made row_to_local_artifact pub(super) for db/eviction.rs and this PR made it return rusqlite::Result. It now does both.

Rebased with the stack onto main c4be884, without conflicts. The caps are unchanged.

Decisions

• ref-cast's RefCastCustom rather than unsafe, and the raw mutex kept. The jobs worker must share sbx-mgr's mutex, so the guard derefs a rusqlite::Connection that has to be viewed as db::Connection. ref-cast 1.0.27, already in Cargo.lock, does that safely under forbid(unsafe_code). RefCastCustom keeps the view private, where #[derive(RefCast)] would have published <Connection as RefCast>::From = rusqlite::Connection. It is added as a workspace dependency; ci/rust-artifact-inputs.json is unchanged.
• The raw accessors and statement methods are db-private in production, test-visible in tests. The compiler, not cargo arch, keeps production code outside db from reaching a raw connection, statement or error. Milestone 3 moves the same split onto the store crate's test-support feature.
• sbxmgr_sqlite.no_new_entries is required. The baseline is empty, so no new entry is allowed. The rule's comment now calls its patterns a line-based tripwire.
• No new behavior tests. The conversion is type-level and existing coverage pins the behavior; the full suite passes unchanged.

Validation

Check	Result
bash ci/scripts/run-rust-checks.sh fmt (includes the Rust error policy)	pass
bash ci/scripts/run-rust-checks.sh clippy (CI's clippy mode, including the workspace --all-targets --all-features pass)	pass
cargo arch --base-baseline <main> --base-architecture <main>	0 required findings (advisory: pre-existing file_size drift and the schema table's no_new_entries)
SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --locked -p pplx-space-sbx-mgr -p pplx-space-db-migrate -p pplx-space-common-jobs --retries 0	1843 passed (sbx-mgr 1731, db-migrate 53, common-jobs 59), 0 failed; 1 skip is the root-only #[ignore] transfer_directory UID test, not a dependency skip
cargo test --locked -p pplx-space-archcheck	pass (47 CLI, 5 repository)
no-feature cargo clippy --locked -p pplx-space-sbx-mgr -p pplx-space-sbxmgr-svc -p pplx-space-db-migrate --lib --bins -- -D warnings	pass

After the c4be884 rebase, fmt, clippy, cargo arch against main and the archcheck tests re-ran on every head of the stack and passed. Nextest ran once, on #586's head d9f6b99: SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --locked --all-features -p pplx-space-sbx-mgr -p pplx-space-sbx-mgr-store -p pplx-space-db-migrate -p pplx-space-sqlite -p pplx-space-db-test-fixtures --retries 0 gave 1808 passed, 0 failed (server 1367, store 366, migrator 53, sqlite 12, test fixtures 10). The 1 skip is the root-only #[ignore] transfer_directory UID test. Kind was not re-run, because the rebase changed no code.

🤖 Generated with Claude Code

[the-gigi-pplx]

Review — 2 findings

No correctness findings. I checked that db::migrate() still targets SANDBOX_MANAGER_DB with the same migrators in the same order. I also checked that SharedConnection::lock keeps Mutex::lock's poisoning, connection_or_signal_failure keeps its log, notify and DatabaseMutexPoisoned path, and the error matches in service/mod.rs, activation_support.rs and grpc_lifecycle/teardown_suspend.rs are equivalent to the rusqlite-variant matches they replace. try_prune_terminal_jobs_once has one caller outside common-jobs, and that caller only logs its Display.

Low

1. snapshot_upload_jobs.rs:64: the rewritten doc comment names the wrong type.
   It says "Caller owns the crate::db::Connection", but build_worker takes a crate::db::SharedConnection and builds the worker through background_worker. db::Connection is the per-statement view, not the mutex-backed handle the worker holds. It should say SharedConnection. The line is unchanged in the next PR in the stack.
2. db.rs:277: immediate_transaction survives as a one-line alias for Transaction::immediate.
   Production code outside db now spells it Transaction::immediate, while the code inside db still calls immediate_transaction( at roughly 70 lines. Anyone grepping for immediate-vs-deferred transactions has to track both names. Inlining the alias would leave one spelling. Take it or leave it.

Verified against 20e4085051. Reproduced locally: I read the base-vs-head diff of the converted call sites and grepped the head for both transaction spellings and for build_worker's parameter type. I didn't rerun the test suite; CI is green on this head.

[the-gigi-pplx]

Approved. 2 non-blocking findings in the review comment above.