diff --git a/README.md b/README.md index 4f187be2..47896064 100644 --- a/README.md +++ b/README.md @@ -314,7 +314,7 @@ milestone. | **Every Ubuntu LTS validated** โ€” 22.04, 24.04 and 26.04 all at 79/79, each with a replay twin that reproduces it | โœ… | | Telegram approval interface | ๐Ÿ“‹ roadmap | -**1,853 Rust tests and 72 frontend tests** form the current deterministic +**1,854 Rust tests and 72 frontend tests** form the current deterministic release baseline. ## Configure your LLM diff --git a/crates/sysknife-daemon/src/store.rs b/crates/sysknife-daemon/src/store.rs index d2996884..b946364d 100644 --- a/crates/sysknife-daemon/src/store.rs +++ b/crates/sysknife-daemon/src/store.rs @@ -49,6 +49,28 @@ use crate::transactions::{ pub mod postgres; +/// Column list every `ChainRow` read shares, on both backends. The SQLite +/// path (`transactions.rs`) maps rows positionally and the Postgres path +/// (`store/postgres.rs`) maps them by name, but both SELECT this exact list +/// in this exact order, so the list, the column count, and the mapper order +/// are one invariant. +/// +/// This used to be declared twice โ€” once per backend โ€” kept in sync only by +/// a "Mirrors" doc comment. Adding the caller-identity columns updated the +/// mapper and one of the two queries, and the miss showed up only as a +/// runtime "no column found for name: chain_version" from the live-Postgres +/// test โ€” the unit tests, which never touch this SQL, stayed green. One +/// declaration means that class of drift is a compile-time impossibility +/// rather than a live-database surprise (#397). +/// +/// Parameter placeholders stay per-backend on purpose: SQLite binds `?1`, +/// Postgres binds `$1`. Only the column list is shared. +pub(crate) const CHAIN_ROW_COLUMNS: &str = + "seq, key_id, transaction_id, request_id, request_hash, \ + action_name, risk_level, summary, approval_id, warnings_json, \ + created_at, prev_chain_hash, chain_hash, chain_version, caller_role, event_tip, \ + caller_principal"; + /// Async, polymorphic interface to the audit log. Implemented by /// [`SqliteStore`] (rusqlite, blocking under the hood) and /// [`postgres::PostgresStore`] (sqlx, native async). @@ -417,6 +439,47 @@ mod tests { SqliteStore::new(crate::transactions::TransactionStore::open_with_key(path, key).unwrap()) } + /// `CHAIN_ROW_COLUMNS` is now declared once (store.rs), so there is no + /// second copy left to compare โ€” the drift test that justified the hoist + /// ran green first, was mutated red on purpose (see #397's PR), and is + /// replaced by this pin. The invariant that matters is the one the + /// positional mapper (`chain_row_from_sqlite`) and the by-name mapper + /// (`row_to_chain_row`) both depend on: 17 columns, in this exact order. + /// Adding a column to the chain means updating this list deliberately, + /// together with both mappers. + #[test] + fn chain_row_columns_pinned() { + let columns: Vec<&str> = crate::store::CHAIN_ROW_COLUMNS + .split(',') + .map(str::trim) + .collect(); + assert_eq!( + columns, + [ + "seq", + "key_id", + "transaction_id", + "request_id", + "request_hash", + "action_name", + "risk_level", + "summary", + "approval_id", + "warnings_json", + "created_at", + "prev_chain_hash", + "chain_hash", + "chain_version", + "caller_role", + "event_tip", + "caller_principal", + ], + "CHAIN_ROW_COLUMNS changed โ€” both ChainRow mappers read these \ + columns in this order, so update them in the same change" + ); + assert_eq!(columns.len(), 17, "column count drifted from 17"); + } + fn new_tx(request_id: &str) -> NewTransaction { NewTransaction { request_id: request_id.to_string(), diff --git a/crates/sysknife-daemon/src/store/postgres.rs b/crates/sysknife-daemon/src/store/postgres.rs index 8e252fbe..2f62f51f 100644 --- a/crates/sysknife-daemon/src/store/postgres.rs +++ b/crates/sysknife-daemon/src/store/postgres.rs @@ -48,17 +48,15 @@ use crate::transactions::{ const MIGRATION_LOCK_ID: i64 = 0x5359_534b_4e49_4645; -/// Column list every `ChainRow` read shares, kept next to `row_to_chain_row`. +/// Column list every `ChainRow` read shares, declared once in `store.rs` +/// (#397) and kept next to `row_to_chain_row` here by import. /// /// The two read paths each spelled the list out. Adding the caller-identity /// columns updated the mapper and one of the two queries, and the miss showed /// up only as a runtime "no column found for name: chain_version" from the /// live-Postgres test โ€” the unit tests, which never touch this SQL, stayed -/// green. Mirrors `CHAIN_ROW_COLUMNS` in `transactions.rs`. -const CHAIN_ROW_COLUMNS: &str = "seq, key_id, transaction_id, request_id, request_hash, \ - action_name, risk_level, summary, approval_id, warnings_json, \ - created_at, prev_chain_hash, chain_hash, chain_version, caller_role, event_tip, \ - caller_principal"; +/// green. That history now lives on the single declaration. +use crate::store::CHAIN_ROW_COLUMNS; struct Migration { version: i64, diff --git a/crates/sysknife-daemon/src/transactions.rs b/crates/sysknife-daemon/src/transactions.rs index ddf606db..c50a3f78 100644 --- a/crates/sysknife-daemon/src/transactions.rs +++ b/crates/sysknife-daemon/src/transactions.rs @@ -204,16 +204,10 @@ const SQLITE_MIGRATIONS: &[SqliteMigration] = &[ }, ]; -/// Column list for every `ChainRow` read, kept next to the mapper below. -/// -/// The two read paths (`fetch_chain_rows`, `fetch_chain_row`) used to repeat -/// both the SELECT and a positional `row.get(n)` block. Adding a column meant -/// editing four places in step, and a mismatch between the two would surface -/// as a verification failure rather than a compile error. -const CHAIN_ROW_COLUMNS: &str = "seq, key_id, transaction_id, request_id, request_hash, \ - action_name, risk_level, summary, approval_id, warnings_json, \ - created_at, prev_chain_hash, chain_hash, chain_version, caller_role, event_tip, \ - caller_principal"; +/// Column list for every `ChainRow` read, shared with the Postgres backend +/// and declared once in `store.rs` (#397) โ€” the mapper below reads columns +/// positionally in exactly that order. +use crate::store::CHAIN_ROW_COLUMNS; fn chain_row_from_sqlite(row: &rusqlite::Row<'_>) -> rusqlite::Result { Ok(ChainRow { diff --git a/docs/distro-support.md b/docs/distro-support.md index 0578df7f..5b93108d 100644 --- a/docs/distro-support.md +++ b/docs/distro-support.md @@ -91,7 +91,7 @@ family and the atomic story family are implemented and covered by the workspace suite. What is missing is a way to put the helpers somewhere the daemon's own grants already point. -The deterministic workspace baseline is 1,853 Rust tests plus 72 frontend +The deterministic workspace baseline is 1,854 Rust tests plus 72 frontend tests. Those tests verify action construction, policy, approval, storage, and UI behavior, but they do not replace a real distribution VM run. diff --git a/docs/introduction.md b/docs/introduction.md index 8a85996c..5c4123f7 100644 --- a/docs/introduction.md +++ b/docs/introduction.md @@ -144,7 +144,7 @@ flow. ## Status -190 typed actions ยท 1,853 Rust tests + 72 frontend tests ยท MIT +190 typed actions ยท 1,854 Rust tests + 72 frontend tests ยท MIT SysKnife is the reference implementation of the [LACS specification](https://github.com/lacs-project/specification) โ€” a diff --git a/tests/evidence/workspace-tests.json b/tests/evidence/workspace-tests.json index e175f7b4..c2a9e65d 100644 --- a/tests/evidence/workspace-tests.json +++ b/tests/evidence/workspace-tests.json @@ -4,6 +4,6 @@ "tests": "cargo nextest run --workspace --locked" }, "frontend_tests": 72, - "tests": 1853, + "tests": 1854, "version": 2 }