🏗️ refactor(kebab-store-sqlite): harden open_existing against silent create (fb-27)
Replace `path.exists()` + `Connection::open` (which silently CREATEs on
race) with `Connection::open_with_flags` using READ_WRITE|URI but NOT
CREATE. SQLite surfaces `SQLITE_CANTOPEN` for missing files; we wrap as
NotIndexed { found: None } as before.
Adds open_existing_does_not_create_missing_db regression test pinning
the no-side-effect invariant.
Also documents read-only intent on open_existing, the format contract
on NotIndexed.found, and removes scaffolding comments from kebab-app
error_signal that are no longer load-bearing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -11,5 +11,5 @@
|
||||
pub use crate::doctor_signal::{DoctorUnhealthy, NoHitSignal, RefusalSignal};
|
||||
|
||||
pub use kebab_llm_local::LlmError;
|
||||
pub use kebab_config::ConfigInvalid; // wired in Task 2
|
||||
pub use kebab_store_sqlite::NotIndexed; // wired in Task 3
|
||||
pub use kebab_config::ConfigInvalid;
|
||||
pub use kebab_store_sqlite::NotIndexed;
|
||||
|
||||
@@ -11,7 +11,7 @@ use std::sync::atomic::{AtomicU64, Ordering};
|
||||
use std::sync::{Mutex, MutexGuard};
|
||||
|
||||
use anyhow::{Context, Result};
|
||||
use rusqlite::{Connection, OptionalExtension, params};
|
||||
use rusqlite::{Connection, OpenFlags, OptionalExtension, params};
|
||||
|
||||
use crate::error::StoreError;
|
||||
use crate::schema;
|
||||
@@ -25,6 +25,10 @@ use crate::schema;
|
||||
#[error("not indexed: expected={expected}, found={found:?}")]
|
||||
pub struct NotIndexed {
|
||||
pub expected: String,
|
||||
/// When the DB file exists but the schema is incompatible, this holds
|
||||
/// the highest applied migration version string (e.g. `"V005"`).
|
||||
/// `None` means the file was absent entirely (current Task 3 behavior;
|
||||
/// schema-mismatch wrapping is a deferred follow-up).
|
||||
pub found: Option<String>,
|
||||
}
|
||||
|
||||
@@ -71,21 +75,27 @@ pub struct SqliteStore {
|
||||
}
|
||||
|
||||
impl SqliteStore {
|
||||
/// Open an existing SQLite DB at the path derived from `config`. Unlike
|
||||
/// `open`, this does NOT create the file — if it is missing, returns a
|
||||
/// [`NotIndexed`] signal suitable for `error.v1` translation.
|
||||
/// Open an existing SQLite DB at `path`.
|
||||
///
|
||||
/// Unlike [`Self::open`], this does NOT create the file — if it is
|
||||
/// missing, returns a [`NotIndexed`] signal suitable for `error.v1`
|
||||
/// translation. Stores returned by this method are intended for read-only
|
||||
/// introspection (`schema_with_config`); use [`Self::open`] for any path
|
||||
/// that calls `put_asset_with_bytes`.
|
||||
///
|
||||
/// **Does not run migrations** — call [`Self::run_migrations`] next if
|
||||
/// you need the schema initialised.
|
||||
pub fn open_existing(path: &std::path::Path) -> anyhow::Result<Self> {
|
||||
if !path.exists() {
|
||||
return Err(anyhow::Error::new(NotIndexed {
|
||||
let conn = Connection::open_with_flags(
|
||||
path,
|
||||
OpenFlags::SQLITE_OPEN_READ_WRITE | OpenFlags::SQLITE_OPEN_URI,
|
||||
)
|
||||
.map_err(|_| {
|
||||
anyhow::Error::new(NotIndexed {
|
||||
expected: path.to_string_lossy().to_string(),
|
||||
found: None,
|
||||
}));
|
||||
}
|
||||
let conn = rusqlite::Connection::open(path)
|
||||
.with_context(|| format!("open sqlite at {}", path.display()))?;
|
||||
})
|
||||
})?;
|
||||
apply_pragmas(&conn)?;
|
||||
|
||||
let data_dir = path
|
||||
|
||||
@@ -17,3 +17,11 @@ fn not_indexed_signal_emitted_when_db_missing() {
|
||||
.expect("missing DB error should downcast to NotIndexed");
|
||||
assert_eq!(signal.expected, nonexistent_db.to_string_lossy().as_ref());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn open_existing_does_not_create_missing_db() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let nonexistent_db = dir.path().join("does-not-exist.sqlite");
|
||||
let _ = SqliteStore::open_existing(&nonexistent_db);
|
||||
assert!(!nonexistent_db.exists(), "open_existing must NOT create the file");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user