diff --git a/crates/atomic-core/src/db.rs b/crates/atomic-core/src/db.rs index 6725defb..c34ce2e3 100644 --- a/crates/atomic-core/src/db.rs +++ b/crates/atomic-core/src/db.rs @@ -67,6 +67,61 @@ impl Database { path: &Path, create: bool, pool_size: usize, + ) -> Result { + Self::open_with_pool_size_and_settings_cleanup(path, create, pool_size, false) + } + + /// Acquire a read-only connection from the pool. + /// Tries each pooled connection via try_lock; if all are busy, creates a fresh one. + pub fn read_conn(&self) -> Result, AtomicCoreError> { + for slot in &self.read_pool { + if let Ok(guard) = slot.try_lock() { + return Ok(ReadConn::Pooled(guard)); + } + } + // All pool slots busy — create a temporary connection + let conn = Connection::open(&self.db_path)?; + conn.set_prepared_statement_cache_capacity(STMT_CACHE_CAPACITY); + conn.execute_batch(&format!("{} PRAGMA query_only=ON;", BASE_PRAGMAS))?; + Ok(ReadConn::Temp(conn)) + } + + /// Create a new connection to the same database. + /// Registers sqlite-vec so the connection can query vec_chunks. + pub fn new_connection(&self) -> Result { + // sqlite-vec is registered via sqlite3_auto_extension in open_internal, + // which applies to all connections opened after that call. + let conn = Connection::open(&self.db_path)?; + conn.set_prepared_statement_cache_capacity(STMT_CACHE_CAPACITY); + conn.execute_batch(BASE_PRAGMAS)?; + Ok(conn) + } + + /// Open with a larger read pool sized for server workloads. + /// Creates the DB and parent directories if they don't exist. + pub fn open_for_server(path: impl AsRef) -> Result { + Self::open_with_pool_size(path.as_ref(), true, SERVER_READ_POOL_SIZE) + } + + /// Open a registry-backed data database with a larger read pool. + /// + /// This is the same as `open_for_server`, but runs the one migration step + /// that is only valid when this SQLite file is a per-database store whose + /// defaults live in registry.db. + pub fn open_for_server_with_registry(path: impl AsRef) -> Result { + Self::open_with_pool_size_and_settings_cleanup( + path.as_ref(), + true, + SERVER_READ_POOL_SIZE, + true, + ) + } + + fn open_with_pool_size_and_settings_cleanup( + path: &Path, + create: bool, + pool_size: usize, + cleanup_legacy_seed_settings: bool, ) -> Result { // Register sqlite-vec extension unsafe { @@ -74,7 +129,6 @@ impl Database { sqlite3_auto_extension(Some(std::mem::transmute(sqlite3_vec_init as *const ()))); } - // Create parent directory if needed if create { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; @@ -84,28 +138,18 @@ impl Database { let conn = Connection::open(path)?; conn.set_prepared_statement_cache_capacity(STMT_CACHE_CAPACITY); - // Base PRAGMAs + WAL size limit (64 MB) to prevent unbounded WAL growth conn.execute_batch(&format!( "{} PRAGMA journal_size_limit=67108864;", BASE_PRAGMAS ))?; - - // Checkpoint any WAL from a previous run to start clean conn.execute_batch("PRAGMA wal_checkpoint(TRUNCATE);")?; - // Always run migrations — idempotent, handles both new and existing DBs - Self::run_migrations(&conn)?; + Self::run_migrations_internal(&conn, cleanup_legacy_seed_settings)?; - // Update query planner statistics so the optimizer has fresh data conn.execute_batch("PRAGMA optimize=0x10002;")?; - - // Warm the OS page cache and SQLite page cache by touching the indexes - // and table pages that the most common queries need on startup. Self::warm_cache(&conn); let db_path = path.to_path_buf(); - - // Pre-open read connections for the pool let mut read_pool = Vec::with_capacity(pool_size); for _ in 0..pool_size { let rc = Connection::open(&db_path)?; @@ -121,38 +165,6 @@ impl Database { }) } - /// Acquire a read-only connection from the pool. - /// Tries each pooled connection via try_lock; if all are busy, creates a fresh one. - pub fn read_conn(&self) -> Result, AtomicCoreError> { - for slot in &self.read_pool { - if let Ok(guard) = slot.try_lock() { - return Ok(ReadConn::Pooled(guard)); - } - } - // All pool slots busy — create a temporary connection - let conn = Connection::open(&self.db_path)?; - conn.set_prepared_statement_cache_capacity(STMT_CACHE_CAPACITY); - conn.execute_batch(&format!("{} PRAGMA query_only=ON;", BASE_PRAGMAS))?; - Ok(ReadConn::Temp(conn)) - } - - /// Create a new connection to the same database. - /// Registers sqlite-vec so the connection can query vec_chunks. - pub fn new_connection(&self) -> Result { - // sqlite-vec is registered via sqlite3_auto_extension in open_internal, - // which applies to all connections opened after that call. - let conn = Connection::open(&self.db_path)?; - conn.set_prepared_statement_cache_capacity(STMT_CACHE_CAPACITY); - conn.execute_batch(BASE_PRAGMAS)?; - Ok(conn) - } - - /// Open with a larger read pool sized for server workloads. - /// Creates the DB and parent directories if they don't exist. - pub fn open_for_server(path: impl AsRef) -> Result { - Self::open_with_pool_size(path.as_ref(), true, SERVER_READ_POOL_SIZE) - } - /// Walk the hot indexes and table pages into the OS + SQLite page caches. /// Called once at startup so the first real queries don't pay cold-cache costs. fn warm_cache(conn: &Connection) { @@ -199,9 +211,20 @@ impl Database { /// 1. Add a new `if version < N` block at the end (before the virtual-table section) /// 2. End the block with `PRAGMA user_version = N;` /// 3. Bump LATEST_VERSION - const LATEST_VERSION: i32 = 14; + const LATEST_VERSION: i32 = 15; pub fn run_migrations(conn: &Connection) -> Result<(), AtomicCoreError> { + Self::run_migrations_internal(conn, false) + } + + pub fn run_migrations_for_registry(conn: &Connection) -> Result<(), AtomicCoreError> { + Self::run_migrations_internal(conn, true) + } + + fn run_migrations_internal( + conn: &Connection, + cleanup_legacy_seed_settings: bool, + ) -> Result<(), AtomicCoreError> { let version: i32 = conn.query_row("PRAGMA user_version", [], |row| row.get(0))?; // --- V0 → V1: Baseline schema (tables + indexes) --- @@ -733,6 +756,47 @@ impl Database { )?; } + // --- V14 → V15: Finish settings resolver migration --- + // + // Pre-this-version, every per-DB settings table was seeded with the + // entire `DEFAULT_SETTINGS` map at open time. The override-aware + // resolver in `AtomicCore::get_settings_with_source` treats any + // per-DB row as an override on top of the registry default. As long + // as those legacy rows match the registry value they look harmless + // — but the moment the user edits a setting at N=1 (which writes + // only to the registry), the registry diverges from the stale + // per-DB row and the row starts shadowing the new value, making + // edits appear to do nothing. Wipe the seeds so future resolver + // reads fall through to the registry cleanly. + // + // This cleanup is only valid for registry-backed data DBs. Standalone + // SQLite `AtomicCore::open_or_create` DBs store their canonical + // settings in this table, so plain Database opens preserve rows and + // only advance the schema version. + // + // In registry-backed deployments before this version, settings writes + // routed to the registry, so a per-DB row for one of these keys was + // definitionally a seed (never user-set). Per CLAUDE.md pre-alpha + // policy we accept erasing any in-flight per-DB overrides written by + // builds of this branch before the resolver landed. + if version < 15 { + if cleanup_legacy_seed_settings { + let keys: Vec<&str> = crate::settings::DEFAULT_SETTINGS + .iter() + .map(|(k, _)| *k) + .collect(); + if !keys.is_empty() { + let placeholders = std::iter::repeat("?") + .take(keys.len()) + .collect::>() + .join(","); + let sql = format!("DELETE FROM settings WHERE key IN ({})", placeholders); + conn.execute(&sql, rusqlite::params_from_iter(keys.iter()))?; + } + } + conn.execute_batch("PRAGMA user_version = 15;")?; + } + // --- Triggers (recreated every startup to stay current) --- conn.execute_batch( "DROP TRIGGER IF EXISTS atom_tags_insert_count; @@ -921,7 +985,13 @@ impl Database { )?; } - crate::settings::migrate_settings(conn)?; + // Per-DB settings tables intentionally start empty under the + // registry-defaults + per-DB-overrides model. Defaults live in + // `registry.db` (seeded by `Registry::open` via `migrate_settings_to`); + // per-DB rows only exist when the user explicitly overrides a value. + // Registry-backed opens run the V14→V15 cleanup above to purge any + // legacy seed rows so the resolver's "any per-DB row is an override" + // rule stays correct. Ok(()) } @@ -998,6 +1068,81 @@ mod tests { use super::*; use tempfile::NamedTempFile; + #[test] + fn test_v15_wipes_legacy_per_db_seed_rows() { + // Pre-V15, every per-DB connection got the entire DEFAULT_SETTINGS + // map seeded into its `settings` table. The resolver treats per-DB + // rows as overrides on top of registry defaults — the moment the + // user edits a setting at N=1 (writes to registry only), the + // registry diverges from the stale per-DB row and the row starts + // shadowing the new value, making edits look broken. V15 deletes + // those rows. + // + // Simulate the pre-V15 state by opening a fresh DB (which now runs + // *all* migrations including V15), forcibly downgrading the + // user_version, re-inserting seed rows, then re-running the + // migration. + let temp_file = NamedTempFile::new().unwrap(); + let db = Database::open_or_create(temp_file.path()).unwrap(); + + { + let conn = db.conn.lock().unwrap(); + // Force the schema back to V14 and re-seed the legacy rows. + conn.execute_batch("PRAGMA user_version = 14;").unwrap(); + crate::settings::migrate_settings(&conn).unwrap(); + let pre_count: i64 = conn + .query_row("SELECT COUNT(*) FROM settings", [], |row| row.get(0)) + .unwrap(); + assert!( + pre_count > 0, + "test setup: legacy rows should be present before V15" + ); + + // Run migrations again — V15 should fire and wipe the seed rows. + Database::run_migrations_for_registry(&conn).unwrap(); + + let version: i32 = conn + .query_row("PRAGMA user_version", [], |row| row.get(0)) + .unwrap(); + assert!( + version >= 15, + "user_version should be ≥15 after migration, got {version}" + ); + + // Every key in DEFAULT_SETTINGS must be gone — they were seeds. + for (key, _) in crate::settings::DEFAULT_SETTINGS { + let exists: bool = conn + .query_row("SELECT 1 FROM settings WHERE key = ?1", [key], |_| Ok(true)) + .unwrap_or(false); + assert!( + !exists, + "V15 should have removed legacy seed row for '{key}'" + ); + } + } + } + + #[test] + fn test_v15_preserves_standalone_settings_rows() { + // Registry-less SQLite cores use the data DB's settings table as the + // canonical settings store. The V15 seed cleanup is only valid when a + // registry is attached, so the plain migration path must preserve + // those rows. + let temp_file = NamedTempFile::new().unwrap(); + let db = Database::open_or_create(temp_file.path()).unwrap(); + + { + let conn = db.conn.lock().unwrap(); + conn.execute_batch("PRAGMA user_version = 14;").unwrap(); + crate::settings::set_setting(&conn, "chat_model", "custom/model").unwrap(); + + Database::run_migrations(&conn).unwrap(); + + let value = crate::settings::get_setting(&conn, "chat_model").unwrap(); + assert_eq!(value, "custom/model"); + } + } + #[test] fn test_create_database() { let temp_file = NamedTempFile::new().unwrap(); diff --git a/crates/atomic-core/src/lib.rs b/crates/atomic-core/src/lib.rs index 3ee95d63..b9b7cb18 100644 --- a/crates/atomic-core/src/lib.rs +++ b/crates/atomic-core/src/lib.rs @@ -281,7 +281,11 @@ impl AtomicCore { db_path: impl AsRef, registry: Option>, ) -> Result { - let db = Database::open_for_server(db_path)?; + let db = if registry.is_some() { + Database::open_for_server_with_registry(db_path)? + } else { + Database::open_for_server(db_path)? + }; Self::seed_and_backfill(db, registry) } @@ -473,12 +477,16 @@ impl AtomicCore { Ok(core) } - /// Get settings map for passing to background tasks when registry is present. - /// Returns Some if registry is available, None if settings should be read from data db. - fn settings_for_background(&self) -> Option> { - self.registry - .as_ref() - .and_then(|reg| reg.get_all_settings().ok()) + /// Resolved settings map for background AI operations (embedding, + /// tagging, search, chat, agent). Returns the same merged view as + /// `get_settings` — registry workspace defaults + this DB's per-DB + /// overrides + builtin fallbacks — so background helpers see exactly + /// what the API surfaces. Returning `None` here would make callers fall + /// back to reading just the storage layer, which would skip registry + /// defaults; we never want that, so we always return `Some` (or `None` + /// only on a hard read failure). + async fn settings_for_background(&self) -> Option> { + self.get_settings().await.ok() } /// Get the storage path (for display purposes). @@ -500,15 +508,34 @@ impl AtomicCore { } // ==================== Settings ==================== + // + // Resolution model (see `settings::WORKSPACE_ONLY_KEYS` and + // `settings::SettingSource`): + // + // * Workspace-only keys (theme, font, credentials, machine URLs) live + // ONLY in `registry.db`. Reads and writes always target the registry; + // per-DB rows are ignored. + // + // * Overridable keys read with the precedence + // per-DB row > registry row > DEFAULT_SETTINGS constant. + // Writes go to the per-DB table when the workspace has more than one + // database (so the user's change is scoped to the active DB and other + // DBs keep inheriting). With a single database — the common case — + // writes go to the registry instead, so adding a second database + // later naturally inherits the user's existing preferences without + // any copy-on-promotion gymnastics. + // + // Frontends call `get_settings_with_source` to render override + // affordances; internal Rust callers use `get_settings` for plain values. - /// Get all settings, reading from registry if available. + /// Get all settings as a flat key→value map, with per-DB overrides + /// merged on top of registry defaults. Internal Rust callers (briefing, + /// agent, embedding pipeline) use this — they don't need source info. pub async fn get_settings( &self, ) -> Result, AtomicCoreError> { - if let Some(ref reg) = self.registry { - return reg.get_all_settings(); - } - self.storage.get_all_settings_sync().await + let resolved = self.get_settings_with_source().await?; + Ok(resolved.into_iter().map(|(k, v)| (k, v.value)).collect()) } /// Get all settings as a HashMap. Internal helper used by embedding/agent code. @@ -516,12 +543,119 @@ impl AtomicCore { self.get_settings().await } - /// Set a setting value. - pub async fn set_setting(&self, key: &str, value: &str) -> Result<(), AtomicCoreError> { + /// Get all resolved settings tagged with their source (workspace-only, + /// workspace default, per-DB override, or builtin default). Used by the + /// settings API to power inline override UI. + pub async fn get_settings_with_source( + &self, + ) -> Result, AtomicCoreError> { + let mut merged: HashMap = HashMap::new(); + + // Layer 1: builtin defaults (lowest priority). Defensive — registry + // is normally seeded with these via `migrate_settings`, but we + // include them so a fresh-but-empty registry still resolves. + for (key, value) in settings::DEFAULT_SETTINGS { + merged.insert( + (*key).to_string(), + settings::SettingValue { + value: (*value).to_string(), + source: settings::SettingSource::BuiltinDefault, + }, + ); + } + + // Layer 2: registry — workspace defaults (overridable keys) and the + // single source of truth for workspace-only keys. if let Some(ref reg) = self.registry { - return reg.set_setting(key, value); + for (key, value) in reg.get_all_settings()? { + let source = if settings::is_workspace_only(&key) { + settings::SettingSource::Workspace + } else { + settings::SettingSource::WorkspaceDefault + }; + merged.insert(key, settings::SettingValue { value, source }); + } + } + + // Layer 3: per-DB. Two cases: + // * Registry attached (the SQLite multi-DB shape): per-DB rows are + // true overrides on top of the registry. Workspace-only rows in + // this layer are legacy data — the registry is the single source + // of truth for those keys, so we ignore them here. The V15 + // migration in `db.rs` wipes seeded-default rows so this layer + // only ever contains real overrides. + // * No registry (Postgres deployments today): the storage layer's + // settings table is the only place anything lives. Treat its + // values like a registry layer would — workspace-only → Workspace, + // overridable → WorkspaceDefault. Without a registry there's no + // "per-DB override" concept to surface. + let per_db = self.storage.get_all_settings_sync().await?; + let has_registry = self.registry.is_some(); + for (key, value) in per_db { + let source = if settings::is_workspace_only(&key) { + if has_registry { + continue; + } + settings::SettingSource::Workspace + } else if has_registry { + settings::SettingSource::Override + } else { + settings::SettingSource::WorkspaceDefault + }; + merged.insert(key, settings::SettingValue { value, source }); + } + + Ok(merged) + } + + /// Set a setting value. Routing (see module docs above): + /// workspace-only → registry; overridable + N≤1 → registry as workspace + /// default; overridable + N>1 → per-DB as override for the active DB. + pub async fn set_setting(&self, key: &str, value: &str) -> Result<(), AtomicCoreError> { + let registry = match &self.registry { + Some(r) => r, + None => { + // No registry attached (single-DB embedded use): there's no + // workspace layer, so writes always go to the per-DB table. + return self.storage.set_setting_sync(key, value).await; + } + }; + + if settings::is_workspace_only(key) { + return registry.set_setting(key, value); + } + + // Overridable: route based on database count. + if registry.database_count()? <= 1 { + registry.set_setting(key, value) + } else { + self.storage.set_setting_sync(key, value).await } - self.storage.set_setting_sync(key, value).await + } + + /// Clear the active database's per-DB override for `key`. The next read + /// will resolve to the workspace default (registry) or the builtin + /// default. Errors for workspace-only keys — those have no override to + /// clear. + pub async fn clear_override(&self, key: &str) -> Result<(), AtomicCoreError> { + if settings::is_workspace_only(key) { + return Err(AtomicCoreError::Validation(format!( + "Setting '{}' is workspace-only and has no per-database override", + key + ))); + } + self.storage.delete_setting_sync(key).await + } + + /// Read the per-DB override row for `key`, if any. Skips the registry + /// fallback — used by the "overrides across all databases" endpoint that + /// needs just the override layer for one key, not the merged value. + /// Returns Ok(None) for workspace-only keys (they cannot have overrides). + pub async fn get_setting_override(&self, key: &str) -> Result, AtomicCoreError> { + if settings::is_workspace_only(key) { + return Ok(None); + } + self.storage.get_setting_sync(key).await } // ==================== API Token Operations ==================== @@ -1125,7 +1259,7 @@ impl AtomicCore { return search::search_atoms_with_settings( &sqlite.db, options, - self.settings_for_background(), + self.settings_for_background().await, ) .await .map_err(|e| AtomicCoreError::Search(e)); @@ -1682,7 +1816,7 @@ impl AtomicCore { { let on_event = self.wrap_event_for_cache(on_event); let canvas_cache = Some(self.canvas_cache.clone()); - match self.settings_for_background() { + match self.settings_for_background().await { Some(s) => embedding::process_pending_embeddings_with_settings( self.storage.clone(), on_event, @@ -1706,7 +1840,7 @@ impl AtomicCore { { let on_event = self.wrap_event_for_cache(on_event); let canvas_cache = Some(self.canvas_cache.clone()); - match self.settings_for_background() { + match self.settings_for_background().await { Some(s) => embedding::process_queued_pipeline_jobs_with_settings( self.storage.clone(), on_event, @@ -1739,7 +1873,7 @@ impl AtomicCore { let cutoff_rfc3339 = cutoff.to_rfc3339(); let on_event = self.wrap_event_for_cache(on_event); let canvas_cache = Some(self.canvas_cache.clone()); - let count = match self.settings_for_background() { + let count = match self.settings_for_background().await { Some(s) => { embedding::process_pending_embeddings_due_with_settings( self.storage.clone(), @@ -1908,7 +2042,7 @@ impl AtomicCore { .set_tagging_status_sync(atom_id, "pending", None) .await?; - let mut bg_settings = match self.settings_for_background() { + let mut bg_settings = match self.settings_for_background().await { Some(settings) => settings, None => self.storage.get_all_settings_sync().await?, }; @@ -2103,7 +2237,7 @@ impl AtomicCore { conversation_id, content, on_event, - self.settings_for_background(), + self.settings_for_background().await, ) .await .map_err(|e| AtomicCoreError::DatabaseOperation(e)) @@ -2126,7 +2260,7 @@ impl AtomicCore { conversation_id, content, on_event, - self.settings_for_background(), + self.settings_for_background().await, canvas_context, page_context, Some(self.canvas_cache.clone()), @@ -2557,19 +2691,12 @@ impl AtomicCore { let current_settings = self.get_settings().await?; let value_changed = current_settings.get(key).map(|s| s.as_str()) != Some(value); - let embedding_space_keys = [ - "provider", - "embedding_model", - "ollama_embedding_model", - "openai_compat_embedding_model", - "openai_compat_embedding_dimension", - ]; let mut embedding_space_changed = false; let mut dimension_changed = false; let mut old_dim = 0usize; let mut new_dim = 0usize; - if embedding_space_keys.contains(&key) && value_changed { + if settings::is_embedding_space_key(key) && value_changed { embedding_space_changed = true; let current_config = ProviderConfig::from_settings(¤t_settings); old_dim = current_config.embedding_dimension(); @@ -2590,12 +2717,12 @@ impl AtomicCore { } } - // Write to registry if present, otherwise to storage - if let Some(ref reg) = self.registry { - reg.set_setting(key, value)?; - } else { - self.storage.set_setting_sync(key, value).await?; - } + // Route through the standard resolver: workspace-only keys land in + // registry, overridable keys land in registry while N≤1 and per-DB + // (override for the active DB) when N>1. Re-embedding below targets + // only the active DB, which matches that routing — when an override + // creates divergence, only the changed DB needs re-embedding. + self.set_setting(key, value).await?; let mut queued_reembedding = 0i32; if dimension_changed { @@ -2660,6 +2787,78 @@ impl AtomicCore { }) } + /// Clear a per-DB override, handling embedding-space changes. + /// + /// Clearing an embedding-space override can change the active database's + /// resolved vector space just like setting one can. Dimension changes + /// recreate the active DB's vector index before queueing pending atoms; + /// same-dimension space changes re-embed all atoms so stale vectors are + /// not left behind. + pub async fn clear_override_with_reembed( + &self, + key: &str, + on_event: F, + ) -> Result + where + F: Fn(EmbeddingEvent) + Send + Sync + Clone + 'static, + { + let current_settings = self.get_settings().await?; + let current_config = ProviderConfig::from_settings(¤t_settings); + let current_value = current_settings.get(key).cloned(); + + self.clear_override(key).await?; + + let new_settings = self.get_settings().await?; + let new_config = ProviderConfig::from_settings(&new_settings); + let new_value = new_settings.get(key).cloned(); + let value_changed = current_value != new_value; + + let mut embedding_space_changed = false; + let mut dimension_changed = false; + let mut old_dim = 0usize; + let mut new_dim = 0usize; + let mut queued_reembedding = 0i32; + + if settings::is_embedding_space_key(key) && value_changed { + embedding_space_changed = true; + old_dim = current_config.embedding_dimension(); + new_dim = new_config.embedding_dimension(); + + if old_dim != new_dim { + dimension_changed = true; + tracing::info!( + old_dim, + new_dim, + key, + "Embedding dimension change detected after clearing override" + ); + self.storage.recreate_vector_index_sync(new_dim).await?; + self.canvas_cache.invalidate(); + queued_reembedding = self.spawn_reembed_pending(on_event.clone()).await?; + tracing::info!( + queued_reembedding, + "Queued atoms for re-embedding after clearing dimension override" + ); + } else { + queued_reembedding = self.reembed_all_atoms(on_event.clone()).await?; + tracing::info!( + queued_reembedding, + key, + "Queued atoms for re-embedding after clearing embedding-space override" + ); + } + } + + Ok(SettingChangeResult { + embedding_space_changed, + dimension_changed, + old_dim, + new_dim, + total_atom_count: queued_reembedding, + retried_failed_count: 0, + }) + } + // ==================== Utility Operations ==================== /// Check sqlite-vec version @@ -4161,6 +4360,259 @@ mod tests { .unwrap() } + // ==================== Settings Resolver Tests ==================== + // + // These exercise the registry-defaults + per-DB-overrides model end-to-end: + // workspace-only keys go to the registry, overridable keys route by N, + // and `get_settings_with_source` reports the right source for each layer. + + use crate::settings::SettingSource; + use registry::Registry; + use std::sync::Arc; + use tempfile::TempDir; + + /// Build a registry plus N AtomicCore instances bound to that registry, + /// one per "database". The registry already starts with a "default" DB, + /// so requesting `extra_dbs=0` gives N=1. + fn make_workspace(extra_dbs: usize) -> (Arc, Vec, TempDir) { + let dir = TempDir::new().unwrap(); + let registry = Arc::new(Registry::open_or_create(dir.path()).unwrap()); + for i in 0..extra_dbs { + registry.create_database(&format!("db-{i}")).unwrap(); + } + let dbs = registry.list_databases().unwrap(); + let cores: Vec = dbs + .iter() + .map(|info| { + let path = dir.path().join(format!("{}.db", info.id)); + AtomicCore::open_for_server_with_registry(&path, Some(Arc::clone(®istry))) + .unwrap() + }) + .collect(); + (registry, cores, dir) + } + + fn assert_vec_chunks_dimension(core: &AtomicCore, dimension: usize) { + let sqlite = core.storage.as_sqlite().unwrap(); + let conn = sqlite.db.conn.lock().unwrap(); + let sql: String = conn + .query_row( + "SELECT sql FROM sqlite_master WHERE type='table' AND name='vec_chunks'", + [], + |row| row.get(0), + ) + .unwrap(); + assert!( + sql.contains(&format!("float[{dimension}]")), + "vec_chunks schema should use float[{dimension}], got {sql}" + ); + } + + #[tokio::test] + async fn test_workspace_only_writes_always_hit_registry() { + // theme is workspace-only — even with multiple DBs, set_setting on + // any core lands in registry.db, and every core sees the same value. + let (registry, cores, _dir) = make_workspace(2); + cores[0].set_setting("theme", "dracula").await.unwrap(); + + for core in &cores { + let settings = core.get_settings().await.unwrap(); + assert_eq!(settings.get("theme").map(String::as_str), Some("dracula")); + } + assert_eq!(registry.get_setting("theme").unwrap(), "dracula"); + } + + #[tokio::test] + async fn test_overridable_with_n1_writes_to_registry() { + // With one DB, set_setting(provider, ...) goes to the registry as a + // workspace default — so a future second DB inherits it instead of + // starting on the builtin default. + let (registry, cores, dir) = make_workspace(0); + cores[0] + .set_setting("chat_model", "openai/gpt-4o") + .await + .unwrap(); + + // The single DB's per-DB settings table stays empty for this key. + let per_db = cores[0].storage.get_all_settings_sync().await.unwrap(); + assert!(!per_db.contains_key("chat_model")); + assert_eq!(registry.get_setting("chat_model").unwrap(), "openai/gpt-4o"); + + // Spin up a second DB; it inherits the workspace default. + registry.create_database("second").unwrap(); + let second_path = dir.path().join("second.db"); + let second = + AtomicCore::open_for_server_with_registry(&second_path, Some(Arc::clone(®istry))) + .unwrap(); + let settings = second.get_settings().await.unwrap(); + assert_eq!( + settings.get("chat_model").map(String::as_str), + Some("openai/gpt-4o"), + ); + } + + #[tokio::test] + async fn test_overridable_with_n2_writes_per_db() { + // With two DBs, set_setting(provider, ...) on one core writes only + // to that DB's per-DB table. The other DB keeps inheriting from + // the workspace default. + let (registry, cores, _dir) = make_workspace(1); + registry + .set_setting("chat_model", "workspace/default") + .unwrap(); + + cores[0] + .set_setting("chat_model", "override/for-first") + .await + .unwrap(); + + // First DB sees its override. + let s0 = cores[0].get_settings().await.unwrap(); + assert_eq!( + s0.get("chat_model").map(String::as_str), + Some("override/for-first"), + ); + + // Second DB still sees the workspace default. + let s1 = cores[1].get_settings().await.unwrap(); + assert_eq!( + s1.get("chat_model").map(String::as_str), + Some("workspace/default"), + ); + } + + #[tokio::test] + async fn test_clear_override_falls_back_to_default() { + let (registry, cores, _dir) = make_workspace(1); + registry + .set_setting("chat_model", "workspace/default") + .unwrap(); + cores[0] + .set_setting("chat_model", "override/for-first") + .await + .unwrap(); + + cores[0].clear_override("chat_model").await.unwrap(); + + let s = cores[0].get_settings().await.unwrap(); + assert_eq!( + s.get("chat_model").map(String::as_str), + Some("workspace/default"), + "after clearing override, resolves back to workspace default" + ); + } + + #[tokio::test] + async fn test_clear_override_rejects_workspace_only() { + let (_registry, cores, _dir) = make_workspace(0); + let result = cores[0].clear_override("theme").await; + assert!( + result.is_err(), + "clear_override on a workspace-only key must error" + ); + } + + #[tokio::test] + async fn test_clear_embedding_dimension_override_recreates_vector_index() { + let (registry, cores, _dir) = make_workspace(1); + registry.set_setting("provider", "openai_compat").unwrap(); + registry + .set_setting("openai_compat_embedding_dimension", "1536") + .unwrap(); + + cores[0] + .set_setting_with_reembed("openai_compat_embedding_dimension", "768", |_| {}) + .await + .unwrap(); + assert_vec_chunks_dimension(&cores[0], 768); + + let result = cores[0] + .clear_override_with_reembed("openai_compat_embedding_dimension", |_| {}) + .await + .unwrap(); + + assert!(result.embedding_space_changed); + assert!(result.dimension_changed); + assert_eq!(result.old_dim, 768); + assert_eq!(result.new_dim, 1536); + assert_vec_chunks_dimension(&cores[0], 1536); + } + + #[tokio::test] + async fn test_clear_embedding_model_override_marks_space_changed() { + let (registry, cores, _dir) = make_workspace(1); + registry + .set_setting("embedding_model", "openai/text-embedding-3-small") + .unwrap(); + + cores[0] + .set_setting("embedding_model", "custom/same-dimension") + .await + .unwrap(); + + let result = cores[0] + .clear_override_with_reembed("embedding_model", |_| {}) + .await + .unwrap(); + + assert!(result.embedding_space_changed); + assert!(!result.dimension_changed); + assert_eq!(result.old_dim, 1536); + assert_eq!(result.new_dim, 1536); + } + + #[tokio::test] + async fn test_get_settings_with_source_labels_each_layer() { + let (registry, cores, _dir) = make_workspace(1); + // Workspace default for an overridable key. + registry + .set_setting("chat_model", "workspace/default") + .unwrap(); + // Per-DB override for another overridable key on the first DB. + cores[0] + .set_setting("tagging_model", "override/tag") + .await + .unwrap(); + // Workspace-only key (theme). + registry.set_setting("theme", "dracula").unwrap(); + + let s = cores[0].get_settings_with_source().await.unwrap(); + + assert_eq!(s["theme"].source, SettingSource::Workspace); + assert_eq!(s["theme"].value, "dracula"); + + assert_eq!(s["chat_model"].source, SettingSource::WorkspaceDefault); + assert_eq!(s["chat_model"].value, "workspace/default"); + + assert_eq!(s["tagging_model"].source, SettingSource::Override); + assert_eq!(s["tagging_model"].value, "override/tag"); + + // The second DB sees the same workspace default but no override. + let s2 = cores[1].get_settings_with_source().await.unwrap(); + assert_eq!(s2["tagging_model"].source, SettingSource::WorkspaceDefault); + } + + #[tokio::test] + async fn test_per_db_row_for_workspace_only_key_is_ignored() { + // A legacy per-DB row for a workspace-only key (left over from before + // the resolver landed) must not poison the resolved value. + let (registry, cores, _dir) = make_workspace(0); + registry.set_setting("theme", "registry-value").unwrap(); + // Sneak a legacy row directly into the per-DB table. + cores[0] + .storage + .set_setting_sync("theme", "stale-per-db-value") + .await + .unwrap(); + + let s = cores[0].get_settings().await.unwrap(); + assert_eq!( + s.get("theme").map(String::as_str), + Some("registry-value"), + "workspace-only keys ignore per-DB rows even when present" + ); + } + // ==================== Atom CRUD Tests ==================== #[tokio::test] diff --git a/crates/atomic-core/src/manager.rs b/crates/atomic-core/src/manager.rs index dfb0a10d..b6155a9f 100644 --- a/crates/atomic-core/src/manager.rs +++ b/crates/atomic-core/src/manager.rs @@ -477,35 +477,6 @@ impl DatabaseManager { self.sqlite_registry().set_default_database(id) } - /// Recreate vector indexes on all known databases *except* `skip_id` with the - /// given dimension. Existing chunk rows are preserved so embed-only jobs can - /// reuse current chunk boundaries. `skip_id` is typically the active database - /// whose index was already recreated. - pub async fn recreate_other_vector_indexes( - &self, - new_dim: usize, - skip_id: &str, - ) -> Result<(), AtomicCoreError> { - #[cfg(feature = "postgres")] - if self.is_postgres() { - // In Postgres mode all databases share the same atom_chunks table — - // the caller already recreated it, nothing more to do. - return Ok(()); - } - - // SQLite: each database has its own vec_chunks table. - let databases = self.sqlite_registry().list_databases()?; - for db_info in &databases { - if db_info.id == skip_id { - continue; - } - let core = self.get_core(&db_info.id).await?; - core.storage.recreate_vector_index_sync(new_dim).await?; - tracing::info!(db_id = %db_info.id, db_name = %db_info.name, new_dim, "Recreated vector index"); - } - Ok(()) - } - /// Optimize all loaded cores (call on shutdown). pub fn optimize_all(&self) { if let Ok(cores) = self.cores.read() { diff --git a/crates/atomic-core/src/registry.rs b/crates/atomic-core/src/registry.rs index 9a5a27b2..832d9241 100644 --- a/crates/atomic-core/src/registry.rs +++ b/crates/atomic-core/src/registry.rs @@ -535,6 +535,29 @@ impl Registry { crate::settings::set_setting(&conn, key, value) } + /// Delete a setting from the registry. Used by the resolver when clearing + /// a workspace default (rare — most clears target per-DB overrides). + pub fn delete_setting(&self, key: &str) -> Result<(), AtomicCoreError> { + let conn = self + .conn + .lock() + .map_err(|e| AtomicCoreError::Lock(e.to_string()))?; + crate::settings::delete_setting(&conn, key) + } + + /// Number of databases currently registered. Cheap COUNT — used by the + /// settings resolver to decide whether per-DB writes are meaningful + /// (with N=1, writes go to the registry as workspace defaults so a + /// future second DB can inherit them). + pub fn database_count(&self) -> Result { + let conn = self + .conn + .lock() + .map_err(|e| AtomicCoreError::Lock(e.to_string()))?; + let count: i64 = conn.query_row("SELECT COUNT(*) FROM databases", [], |row| row.get(0))?; + Ok(count as usize) + } + // ==================== API Tokens (shared across databases) ==================== /// Create a new named API token. diff --git a/crates/atomic-core/src/settings.rs b/crates/atomic-core/src/settings.rs index ce65882b..6d00b00d 100644 --- a/crates/atomic-core/src/settings.rs +++ b/crates/atomic-core/src/settings.rs @@ -9,6 +9,43 @@ use std::collections::HashMap; /// Default Ollama host URL pub const DEFAULT_OLLAMA_HOST: &str = "http://127.0.0.1:11434"; +/// Settings whose values are properties of the user/machine, not the knowledge +/// base, so they always live in `registry.db` and can never be overridden +/// per-database. Everything *not* in this list is overridable: the registry +/// row holds the workspace default, and a per-DB row (when present) overrides +/// it for that database only. +pub const WORKSPACE_ONLY_KEYS: &[&str] = &[ + // UI preferences — one per user + "theme", + "font", + // Credentials — one set per user/account + "openrouter_api_key", + "openai_compat_api_key", + // Machine-level URLs — one host per machine + "ollama_host", + "openai_compat_base_url", +]; + +/// True if `key` must live in `registry.db` and cannot be overridden per-DB. +pub fn is_workspace_only(key: &str) -> bool { + WORKSPACE_ONLY_KEYS.contains(&key) +} + +/// Settings whose resolved value defines the embedding vector space. Changing +/// or clearing any of these requires re-embedding the affected database. +pub const EMBEDDING_SPACE_KEYS: &[&str] = &[ + "provider", + "embedding_model", + "ollama_embedding_model", + "openai_compat_embedding_model", + "openai_compat_embedding_dimension", +]; + +/// True if `key` affects the embedding vector space. +pub fn is_embedding_space_key(key: &str) -> bool { + EMBEDDING_SPACE_KEYS.contains(&key) +} + /// Default settings with their values pub const DEFAULT_SETTINGS: &[(&str, &str)] = &[ ("provider", "openrouter"), @@ -108,6 +145,39 @@ pub fn set_setting(conn: &Connection, key: &str, value: &str) -> Result<(), Atom Ok(()) } +/// Delete a setting row. Returns Ok(()) whether or not the row existed. +/// Used to clear a per-DB override so the resolver falls back to the +/// workspace default in `registry.db`. +pub fn delete_setting(conn: &Connection, key: &str) -> Result<(), AtomicCoreError> { + conn.execute("DELETE FROM settings WHERE key = ?1", [key])?; + Ok(()) +} + +/// Source of a resolved setting value. Powers the override UI: the frontend +/// uses this to decide whether to render "Default", "Overridden", or to +/// suppress the override affordance entirely. +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum SettingSource { + /// Key is in `WORKSPACE_ONLY_KEYS` — value lives in `registry.db` and + /// can never be overridden per-DB. + Workspace, + /// Overridable key, currently using the workspace default from `registry.db`. + WorkspaceDefault, + /// Overridable key, currently overridden by a row in this DB's settings table. + Override, + /// No row in registry or per-DB; value comes from the `DEFAULT_SETTINGS` + /// constant baked into the binary. + BuiltinDefault, +} + +/// Resolved setting: the value the caller will see, plus where it came from. +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct SettingValue { + pub value: String, + pub source: SettingSource, +} + #[cfg(test)] mod tests { use super::*; @@ -121,22 +191,41 @@ mod tests { } #[test] - fn test_get_all_settings_has_defaults() { + fn test_per_db_settings_table_starts_empty() { + // Per-DB tables are no longer seeded with DEFAULT_SETTINGS — defaults + // live in registry.db. A freshly-opened per-DB connection should have + // an empty settings table; rows only appear when the user explicitly + // overrides a value. let (db, _temp) = create_test_db(); let conn = db.conn.lock().unwrap(); + let settings = get_all_settings(&conn).unwrap(); + assert!( + settings.is_empty(), + "Per-DB settings table should start empty (defaults live in registry)" + ); + } + + #[test] + fn test_migrate_settings_seeds_defaults() { + // The migration function itself still seeds DEFAULT_SETTINGS — it's + // just no longer called from the per-DB open path. Callers like + // Registry::open invoke it explicitly to seed registry.db. + let (db, _temp) = create_test_db(); + let conn = db.conn.lock().unwrap(); + + migrate_settings(&conn).unwrap(); let settings = get_all_settings(&conn).unwrap(); - // After migration, should have default settings assert!( !settings.is_empty(), - "Should have default settings after migration" + "migrate_settings should seed defaults" ); - assert!( - settings.contains_key("provider"), - "Should have provider setting" + assert_eq!( + settings.get("provider").map(String::as_str), + Some("openrouter"), + "provider default should be openrouter" ); - assert_eq!(settings.get("provider").unwrap(), "openrouter"); } #[test] @@ -187,14 +276,42 @@ mod tests { let (db, _temp) = create_test_db(); let conn = db.conn.lock().unwrap(); - // Get settings after first migration (done in open_or_create) + // Per-DB connections aren't seeded automatically — drive migration + // ourselves and confirm a second run leaves the row count unchanged. + migrate_settings(&conn).unwrap(); let settings1 = get_all_settings(&conn).unwrap(); - // Run migration again migrate_settings(&conn).unwrap(); - - // Settings should be the same let settings2 = get_all_settings(&conn).unwrap(); assert_eq!(settings1.len(), settings2.len()); } + + #[test] + fn test_workspace_only_classification() { + // Sanity-check the small static list that gates the resolver. + assert!(is_workspace_only("theme")); + assert!(is_workspace_only("openrouter_api_key")); + assert!(is_workspace_only("ollama_host")); + assert!(!is_workspace_only("provider")); + assert!(!is_workspace_only("embedding_model")); + assert!(!is_workspace_only("auto_tagging_enabled")); + } + + #[test] + fn test_delete_setting_removes_row() { + let (db, _temp) = create_test_db(); + let conn = db.conn.lock().unwrap(); + + set_setting(&conn, "provider", "ollama").unwrap(); + assert_eq!(get_setting(&conn, "provider").unwrap(), "ollama"); + + delete_setting(&conn, "provider").unwrap(); + assert!( + get_setting(&conn, "provider").is_err(), + "delete_setting should remove the row so subsequent reads fail" + ); + + // Deleting a missing key is a no-op (does not error). + delete_setting(&conn, "never_existed").unwrap(); + } } diff --git a/crates/atomic-core/src/storage/mod.rs b/crates/atomic-core/src/storage/mod.rs index 8489845b..13fcb2db 100644 --- a/crates/atomic-core/src/storage/mod.rs +++ b/crates/atomic-core/src/storage/mod.rs @@ -623,6 +623,8 @@ dispatch! { => sqlite: get_setting_sync, pg_trait: SettingsStore, pg_method: get_setting; fn set_setting_sync(&self, key: &str, value: &str) -> Result<(), AtomicCoreError> => sqlite: set_setting_sync, pg_trait: SettingsStore, pg_method: set_setting; + fn delete_setting_sync(&self, key: &str) -> Result<(), AtomicCoreError> + => sqlite: delete_setting_sync, pg_trait: SettingsStore, pg_method: delete_setting; // ---- TokenStore ---- fn create_api_token_sync(&self, name: &str) -> Result<(crate::tokens::ApiTokenInfo, String), AtomicCoreError> diff --git a/crates/atomic-core/src/storage/postgres/settings.rs b/crates/atomic-core/src/storage/postgres/settings.rs index 0a8443b1..317b9f60 100644 --- a/crates/atomic-core/src/storage/postgres/settings.rs +++ b/crates/atomic-core/src/storage/postgres/settings.rs @@ -48,6 +48,16 @@ impl SettingsStore for PostgresStorage { Ok(()) } + + async fn delete_setting(&self, key: &str) -> StorageResult<()> { + sqlx::query("DELETE FROM settings WHERE key = $1") + .bind(key) + .execute(&self.pool) + .await + .map_err(|e| AtomicCoreError::DatabaseOperation(e.to_string()))?; + + Ok(()) + } } // ==================== Tokens ==================== diff --git a/crates/atomic-core/src/storage/sqlite/settings.rs b/crates/atomic-core/src/storage/sqlite/settings.rs index e4b29dff..f8a98e4c 100644 --- a/crates/atomic-core/src/storage/sqlite/settings.rs +++ b/crates/atomic-core/src/storage/sqlite/settings.rs @@ -30,6 +30,15 @@ impl SqliteStorage { .map_err(|e| AtomicCoreError::Lock(e.to_string()))?; crate::settings::set_setting(&conn, key, value) } + + pub(crate) fn delete_setting_sync(&self, key: &str) -> StorageResult<()> { + let conn = self + .db + .conn + .lock() + .map_err(|e| AtomicCoreError::Lock(e.to_string()))?; + crate::settings::delete_setting(&conn, key) + } } #[async_trait] @@ -45,6 +54,10 @@ impl SettingsStore for SqliteStorage { async fn set_setting(&self, key: &str, value: &str) -> StorageResult<()> { self.set_setting_sync(key, value) } + + async fn delete_setting(&self, key: &str) -> StorageResult<()> { + self.delete_setting_sync(key) + } } // ==================== Tokens ==================== diff --git a/crates/atomic-core/src/storage/traits.rs b/crates/atomic-core/src/storage/traits.rs index 2fc4bb1e..6356fb5a 100644 --- a/crates/atomic-core/src/storage/traits.rs +++ b/crates/atomic-core/src/storage/traits.rs @@ -858,6 +858,10 @@ pub trait SettingsStore: Send + Sync { /// Set a setting value (upsert). async fn set_setting(&self, key: &str, value: &str) -> StorageResult<()>; + + /// Delete a setting row. No-op if the key isn't present. Used to clear a + /// per-DB override so the resolver falls back to the workspace default. + async fn delete_setting(&self, key: &str) -> StorageResult<()>; } // ==================== Token Storage ==================== diff --git a/crates/atomic-core/tests/multi_db_tests.rs b/crates/atomic-core/tests/multi_db_tests.rs index 775d85ae..5e3ddb3f 100644 --- a/crates/atomic-core/tests/multi_db_tests.rs +++ b/crates/atomic-core/tests/multi_db_tests.rs @@ -1,22 +1,25 @@ //! Multi-database isolation tests. //! //! CLAUDE.md flags this as the area most prone to silent cross-contamination -//! bugs: per-DB data must not leak, and `AtomicCore::{get_settings, set_setting}` -//! intentionally routes to the shared registry when one is attached (so it is -//! *global*, not per-DB). Anything that needs per-DB state — scheduler -//! last-run timestamps, per-DB feature flags — must bypass that routing. +//! bugs. The settings routing has two flavors: workspace-only keys (theme, +//! font, credentials, machine URLs — see `settings::WORKSPACE_ONLY_KEYS`) +//! always live in `registry.db` and are shared across every database; +//! everything else is *overridable*, meaning a per-DB row in that database's +//! settings table wins over the registry-side workspace default. //! //! These tests open a real `DatabaseManager`, create two data databases //! inside it, and assert: //! //! 1. Atoms created in DB1 are invisible from DB2 (and vice versa). //! 2. Tags are isolated per-database. -//! 3. `set_setting` is *intentionally global* when a registry is attached — -//! this test pins that contract so anyone refactoring settings routing -//! sees the consequence and remembers to route per-DB state elsewhere. -//! -//! Runs against both SQLite (via `Registry` + per-file data DBs) and Postgres -//! (single shared pool, rows keyed by `db_id`). +//! 3. Workspace-only keys (e.g. `theme`) are shared — setting one on DB1 +//! is visible from DB2. +//! 4. Overridable keys (e.g. `provider`) routed via `set_setting` with a +//! registry attached and N≥2 land in the active DB's per-DB table — +//! DB2 keeps seeing the workspace default until it sets its own override. +//! Postgres mode currently has no registry, so settings fall through to +//! a single shared `settings` table; the SQLite-only assertion below +//! pins the per-DB behavior we actually want. mod support; @@ -30,6 +33,58 @@ async fn isolation_sqlite() { let dir = TempDir::new().expect("tempdir"); let manager = DatabaseManager::new(dir.path()).expect("open manager"); run_isolation(&manager).await; + + // ---------- Overridable settings are per-DB (SQLite + registry only) ---------- + // + // Postgres mode has no registry today, so the Postgres `set_setting` + // path falls through to the shared storage table — there's no override + // layer to test. SQLite always has a registry, so this is the deployment + // where the override semantics actually take effect. + let dbs = manager.list_databases().await.expect("list").0; + let alpha = dbs + .iter() + .find(|d| d.name == "isolation_alpha") + .expect("alpha exists"); + let beta = dbs + .iter() + .find(|d| d.name == "isolation_beta") + .expect("beta exists"); + let core1 = manager.get_core(&alpha.id).await.expect("get_core alpha"); + let core2 = manager.get_core(&beta.id).await.expect("get_core beta"); + + // Set an overridable key on alpha. With N≥2 this writes to alpha's per-DB + // settings table — beta should keep seeing the workspace default. + core1 + .set_setting("provider", "ollama") + .await + .expect("set provider override on alpha"); + + let s1 = core1.get_settings().await.expect("get_settings alpha"); + let s2 = core2.get_settings().await.expect("get_settings beta"); + assert_eq!( + s1.get("provider").map(String::as_str), + Some("ollama"), + "alpha sees its own override" + ); + assert_ne!( + s2.get("provider").map(String::as_str), + Some("ollama"), + "beta MUST NOT see alpha's per-DB override — that would leak the active \ + DB's choice into every other DB and break the inheritance model." + ); + + // Clearing the override on alpha makes it fall back to whatever beta sees + // (the workspace default in registry.db). + core1 + .clear_override("provider") + .await + .expect("clear override on alpha"); + let s1_after = core1.get_settings().await.expect("get_settings alpha"); + assert_eq!( + s1_after.get("provider").map(String::as_str), + s2.get("provider").map(String::as_str), + "after clear_override, alpha resolves to the same value beta sees" + ); } #[cfg(feature = "postgres")] @@ -155,26 +210,23 @@ async fn run_isolation(manager: &DatabaseManager) { names2 ); - // ---------- The registry footgun ---------- + // ---------- Workspace-only keys are shared across DBs ---------- // - // `AtomicCore::set_setting` routes through the registry when attached - // (SQLite DatabaseManager always has one; Postgres DatabaseManager has - // none but `AtomicCore` still stores settings at the db-id scope). The - // important invariant is the *documented* behavior: settings written - // via `set_setting` are visible from both cores. If a future refactor - // quietly swaps this to per-DB routing, this assertion fires — giving - // the author a chance to go audit the scheduler and anywhere else that - // relies on the current shape. + // Workspace-only keys (theme, font, credentials, machine URLs) live in + // the registry and are intentionally global — setting `theme` on alpha + // must show up on beta. This is the contract we *want* for these keys; + // overridable keys behave differently and are pinned per-deployment in + // each test entry point. core1 - .set_setting("provider", "ollama") + .set_setting("theme", "dracula") .await - .expect("set_setting on alpha"); + .expect("set theme on alpha"); let s2 = core2.get_settings().await.expect("get_settings on beta"); assert_eq!( - s2.get("provider").map(String::as_str), - Some("ollama"), - "set_setting/get_settings share state across DBs (registry when SQLite, \ - per-db_id rows in Postgres). Per-DB state MUST use a different mechanism — \ - see crates/atomic-core/src/scheduler/state.rs for the canonical pattern." + s2.get("theme").map(String::as_str), + Some("dracula"), + "workspace-only keys (here: theme) MUST be visible across all \ + databases — they live in registry.db and the resolver short-circuits \ + the per-DB layer for them." ); } diff --git a/crates/atomic-server/src/lib.rs b/crates/atomic-server/src/lib.rs index 7704ced1..d7877266 100644 --- a/crates/atomic-server/src/lib.rs +++ b/crates/atomic-server/src/lib.rs @@ -77,6 +77,8 @@ pub use utoipa_scalar::{Scalar, Servable}; // Settings routes::settings::get_settings, routes::settings::set_setting, + routes::settings::clear_setting_override, + routes::settings::list_setting_overrides, routes::settings::test_openrouter_connection, routes::settings::test_openai_compat_connection, routes::settings::get_available_llm_models, @@ -248,6 +250,7 @@ pub use utoipa_scalar::{Scalar, Servable}; routes::wiki::GenerateWikiBody, routes::settings::SetSettingBody, routes::settings::TestOpenRouterBody, + routes::settings::OverrideEntry, routes::canvas::CanvasLevelBody, routes::clustering::ComputeClustersBody, routes::chat::CreateConversationBody, diff --git a/crates/atomic-server/src/routes/mod.rs b/crates/atomic-server/src/routes/mod.rs index efc0c5e3..0f6f6f78 100644 --- a/crates/atomic-server/src/routes/mod.rs +++ b/crates/atomic-server/src/routes/mod.rs @@ -141,6 +141,14 @@ pub fn configure_routes(cfg: &mut web::ServiceConfig) { // Settings cfg.route("/settings", web::get().to(settings::get_settings)); cfg.route("/settings/{key}", web::put().to(settings::set_setting)); + cfg.route( + "/settings/{key}", + web::delete().to(settings::clear_setting_override), + ); + cfg.route( + "/settings/{key}/overrides", + web::get().to(settings::list_setting_overrides), + ); cfg.route( "/settings/test-openrouter", web::post().to(settings::test_openrouter_connection), diff --git a/crates/atomic-server/src/routes/settings.rs b/crates/atomic-server/src/routes/settings.rs index 09580e0f..0d44b9bc 100644 --- a/crates/atomic-server/src/routes/settings.rs +++ b/crates/atomic-server/src/routes/settings.rs @@ -1,4 +1,20 @@ //! Settings routes +//! +//! Settings come in two flavors (see `atomic_core::settings`): workspace-only +//! keys live exclusively in `registry.db`; overridable keys default in the +//! registry but each database can override them in its own settings table. +//! `GET /api/settings` returns the resolved values *with their source* so the +//! frontend can render override affordances. `DELETE /api/settings/{key}` +//! clears an override on the active DB. `GET /api/settings/{key}/overrides` +//! lists which databases currently override the key. +//! +//! There is intentionally no endpoint to write a workspace default +//! out-of-band: changing the registry value for an embedding-space key would +//! silently shift every inheriting DB's resolved setting without recreating +//! their vector indexes or re-embedding their atoms, leaving them in a +//! broken state. If a "change for all DBs" feature ever ships it will need +//! its own dedicated route that walks every inheriting DB and queues +//! reembeds. use crate::db_extractor::Db; use crate::error::{ok_or_error, ApiErrorResponse}; @@ -7,9 +23,9 @@ use actix_web::{web, HttpResponse}; use serde::{Deserialize, Serialize}; use utoipa::ToSchema; -#[utoipa::path(get, path = "/api/settings", responses((status = 200, description = "All settings as key-value map")), tag = "settings")] +#[utoipa::path(get, path = "/api/settings", responses((status = 200, description = "All resolved settings tagged with source")), tag = "settings")] pub async fn get_settings(db: Db) -> HttpResponse { - ok_or_error(db.0.get_settings().await) + ok_or_error(db.0.get_settings_with_source().await) } #[derive(Deserialize, Serialize, ToSchema)] @@ -29,84 +45,98 @@ pub async fn set_setting( let value = body.into_inner().value; // Handle embedding-space settings via set_setting_with_reembed (avoids deadlock) - let embedding_space_keys = [ - "provider", - "embedding_model", - "ollama_embedding_model", - "openai_compat_embedding_model", - "openai_compat_embedding_dimension", - ]; - if embedding_space_keys.contains(&key.as_str()) { - let manager = state.manager.clone(); - let active_id = state.manager.active_id().unwrap_or_default(); + if atomic_core::settings::is_embedding_space_key(&key) { let on_event = crate::event_bridge::embedding_event_callback(state.event_tx.clone()); - let result = - db.0.set_setting_with_reembed(&key, &value, on_event.clone()) - .await; - // Embedding model/provider changes affect every database because the - // settings live in the registry. Dimension changes also need each - // database's vector index reset before embed-only jobs are queued. - if let Ok(ref r) = &result { - if r.embedding_space_changed { - if r.dimension_changed { - if let Err(e) = manager - .recreate_other_vector_indexes(r.new_dim, &active_id) - .await - { - tracing::error!( - "Failed to recreate vector indexes on other databases: {}", - e - ); - return ok_or_error(result); - } - } - - match manager.list_databases().await { - Ok((dbs, _)) => { - for db_info in dbs { - if db_info.id == active_id { - continue; - } - match manager.get_core(&db_info.id).await { - Ok(other_core) => { - let queue_result = if r.dimension_changed { - other_core.spawn_reembed_pending(on_event.clone()).await - } else { - other_core.reembed_all_atoms(on_event.clone()).await - }; - match queue_result { - Ok(n) => tracing::info!( - db_id = %db_info.id, - db_name = %db_info.name, - queued = n, - dimension_changed = r.dimension_changed, - "Queued re-embedding for non-active database" - ), - Err(e) => tracing::error!( - db_id = %db_info.id, - "Failed to queue re-embedding: {}", - e - ), - } - } - Err(e) => tracing::error!( - db_id = %db_info.id, - "Failed to load core for re-embed: {}", - e - ), - } - } - } - Err(e) => tracing::error!("Failed to list databases for re-embed: {}", e), - } - } - } + // `set_setting_with_reembed` writes through the resolver's routing: + // workspace-only → registry, overridable + N≤1 → registry, overridable + // + N>1 → per-DB override for the active database. It then re-embeds + // **the active database only**, which is correct in every routing + // case here: + // * N=1: there are no other DBs to fan out to. + // * N>1: the write created/updated a per-DB override. Other DBs + // keep inheriting the workspace default — their resolved value + // didn't change, so re-embedding them would corrupt their vec + // indexes (especially for dimension changes). The previous + // fan-out across all databases assumed the registry-global write + // model and is gone deliberately. + // A future "change for all DBs" operation that updates the workspace + // default cascade-style would need its own dedicated route that + // walks every DB without an override and re-embeds them. + let result = db.0.set_setting_with_reembed(&key, &value, on_event).await; ok_or_error(result) } else { ok_or_error(db.0.set_setting(&key, &value).await) } } +#[utoipa::path(delete, path = "/api/settings/{key}", params(("key" = String, Path, description = "Setting key")), responses((status = 200, description = "Override cleared"), (status = 400, description = "Key is workspace-only", body = ApiErrorResponse)), tag = "settings")] +pub async fn clear_setting_override( + state: web::Data, + db: Db, + path: web::Path, +) -> HttpResponse { + let key = path.into_inner(); + if atomic_core::settings::is_embedding_space_key(&key) { + let on_event = crate::event_bridge::embedding_event_callback(state.event_tx.clone()); + ok_or_error(db.0.clear_override_with_reembed(&key, on_event).await) + } else { + ok_or_error(db.0.clear_override(&key).await) + } +} + +#[derive(Serialize, ToSchema)] +pub struct OverrideEntry { + pub db_id: String, + pub db_name: String, + pub value: String, +} + +#[utoipa::path(get, path = "/api/settings/{key}/overrides", params(("key" = String, Path, description = "Setting key")), responses((status = 200, description = "List of databases overriding the key", body = Vec)), tag = "settings")] +pub async fn list_setting_overrides( + state: web::Data, + path: web::Path, +) -> HttpResponse { + let key = path.into_inner(); + + // Workspace-only keys can't be overridden — short-circuit so the frontend + // can render "no overrides" without spinning up cores for every DB. + if atomic_core::settings::is_workspace_only(&key) { + return HttpResponse::Ok().json(Vec::::new()); + } + + let (databases, _active) = match state.manager.list_databases().await { + Ok(v) => v, + Err(e) => return crate::error::error_response(e), + }; + + let mut overrides: Vec = Vec::new(); + for info in databases { + let core = match state.manager.get_core(&info.id).await { + Ok(c) => c, + Err(e) => { + tracing::error!(db_id = %info.id, "Failed to load core for override lookup: {}", e); + continue; + } + }; + match core.get_setting_override(&key).await { + Ok(Some(value)) => overrides.push(OverrideEntry { + db_id: info.id, + db_name: info.name, + value, + }), + Ok(None) => {} + Err(e) => tracing::error!( + db_id = %info.id, + key = %key, + "Failed to read override: {}", + e + ), + } + } + + HttpResponse::Ok().json(overrides) +} + #[derive(Deserialize, Serialize, ToSchema)] pub struct TestOpenRouterBody { /// OpenRouter API key to test diff --git a/src/components/settings/OverrideControls.tsx b/src/components/settings/OverrideControls.tsx new file mode 100644 index 00000000..10cd367d --- /dev/null +++ b/src/components/settings/OverrideControls.tsx @@ -0,0 +1,74 @@ +import { useDatabasesStore } from '../../stores/databases'; +import { useSettingsStore } from '../../stores/settings'; +import { isWorkspaceOnly } from '../../lib/settings'; + +interface Props { + /** The settings key this control governs (e.g. `chat_model`). */ + settingKey: string; +} + +/** + * Renders the per-field override affordance — a small chip showing whether + * the field is using the workspace default or a per-DB override, plus a + * `Reset` action when overridden. + * + * Renders nothing when: + * - the workspace has only one database (overrides are meaningless), or + * - the key is workspace-only (e.g. theme, API keys — registry-locked). + * + * Editing the field while N>1 implicitly creates an override (the resolver + * routes writes to the per-DB table); the chip flips to "Overridden" via the + * store's optimistic source update. + */ +export function OverrideControls({ settingKey }: Props) { + const databases = useDatabasesStore((s) => s.databases); + const activeId = useDatabasesStore((s) => s.activeId); + const source = useSettingsStore((s) => s.sources[settingKey]); + const clearOverride = useSettingsStore((s) => s.clearOverride); + + if (isWorkspaceOnly(settingKey) || databases.length <= 1) { + return null; + } + + const activeDbName = + databases.find((d) => d.id === activeId)?.name ?? 'this database'; + const isOverridden = source === 'override'; + + const handleReset = async () => { + try { + await clearOverride(settingKey); + } catch (e) { + console.error(`Failed to clear override for ${settingKey}:`, e); + } + }; + + return ( +
+ {isOverridden ? ( + <> + + Overridden for{' '} + {activeDbName} + + + + ) : ( + + Workspace default + + )} +
+ ); +} diff --git a/src/components/settings/SettingsModal.tsx b/src/components/settings/SettingsModal.tsx index 5ba7a209..0f3038e0 100644 --- a/src/components/settings/SettingsModal.tsx +++ b/src/components/settings/SettingsModal.tsx @@ -74,6 +74,7 @@ const MACOS_FULL_DISK_ACCESS_URL = 'x-apple.systempreferences:com.apple.settings.PrivacySecurity.extension?Privacy_AllFiles'; import { formatRelativeDate } from '../../lib/date'; import { useDatabasesStore, type DatabaseInfo, type DatabaseStats } from '../../stores/databases'; +import { OverrideControls } from './OverrideControls'; export type SettingsTab = 'general' | 'ai' | 'tag-categories' | 'connection' | 'integrations' | 'databases'; @@ -1565,33 +1566,6 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp /> - {/* Auto-tagging Toggle Section */} -
-
- -

- Automatically suggest tags when creating atoms -

-
- -
- {/* Troubleshooting */}
)} + {/* OpenRouter Settings */} @@ -1723,6 +1698,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp }))} placeholder="Select embedding model..." /> + {/* Tagging Model */} @@ -1740,6 +1716,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp isLoading={isLoadingModels} placeholder="Select tagging model..." /> + {/* Wiki Model */} @@ -1757,6 +1734,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp isLoading={isLoadingModels} placeholder="Select wiki model..." /> + {/* Wiki Strategy */} @@ -1775,6 +1753,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp { value: 'agentic', label: 'Agentic — AI agent searches and curates sources' }, ]} /> + {/* Wiki Generation Prompt */} @@ -1801,6 +1780,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp Reset to default )} + {/* Wiki Update Prompt */} @@ -1827,6 +1807,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp Reset to default )} + {/* Chat Model */} @@ -1844,6 +1825,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp isLoading={isLoadingModels} placeholder="Select chat model..." /> + {/* Context Length */} @@ -1868,6 +1850,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp { value: '1000000', label: '1M' }, ]} /> + @@ -1917,6 +1900,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp No embedding models found. Run: ollama pull nomic-embed-text )} + {/* Ollama LLM Model */} @@ -1940,6 +1924,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp No LLM models found. Run: ollama pull llama3.2 )} + {/* Context Length */} @@ -1965,6 +1950,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp { value: '1000000', label: '1M' }, ]} /> + {/* Timeout */} @@ -1987,6 +1973,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp { value: '600', label: '10 minutes' }, ]} /> + )} @@ -2107,6 +2094,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp placeholder="text-embedding-3-small" className="w-full px-3 py-2 bg-[var(--color-bg-card)] border border-[var(--color-border)] rounded-md text-[var(--color-text-primary)] placeholder-[var(--color-text-secondary)] focus:outline-none focus:ring-2 focus:ring-[var(--color-accent)] focus:border-transparent transition-colors duration-150" /> + {/* Embedding Dimension */} @@ -2129,6 +2117,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp placeholder="1536" className="w-full px-3 py-2 bg-[var(--color-bg-card)] border border-[var(--color-border)] rounded-md text-[var(--color-text-primary)] placeholder-[var(--color-text-secondary)] focus:outline-none focus:ring-2 focus:ring-[var(--color-accent)] focus:border-transparent transition-colors duration-150" /> + {/* LLM Model */} @@ -2147,6 +2136,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp placeholder="meta-llama/Llama-3.1-8B-Instruct" className="w-full px-3 py-2 bg-[var(--color-bg-card)] border border-[var(--color-border)] rounded-md text-[var(--color-text-primary)] placeholder-[var(--color-text-secondary)] focus:outline-none focus:ring-2 focus:ring-[var(--color-accent)] focus:border-transparent transition-colors duration-150" /> + {/* Context Length */} @@ -2172,6 +2162,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp { value: '1000000', label: '1M' }, ]} /> + {/* Timeout */} @@ -2194,6 +2185,7 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp { value: '600', label: '10 minutes' }, ]} /> + @@ -2203,7 +2195,40 @@ export function SettingsModal({ isOpen, onClose, initialTab }: SettingsModalProp {/* ===== TAG CATEGORIES TAB ===== */} {activeTab === 'tag-categories' && ( - + <> + {/* Auto-tagging master toggle — gates everything below; + tag categories only matter when this is on. */} +
+
+
+ +

+ Automatically suggest tags when creating atoms +

+
+ +
+ +
+ + + )} {/* ===== CONNECTION TAB ===== */} diff --git a/src/lib/settings.ts b/src/lib/settings.ts new file mode 100644 index 00000000..65d811da --- /dev/null +++ b/src/lib/settings.ts @@ -0,0 +1,41 @@ +/** + * Setting source classification — mirrors `atomic_core::settings::SettingSource`. + * + * - `workspace` — key is workspace-only (theme, font, credentials, + * machine URLs); lives in registry.db, never overridable. + * - `workspace_default` — overridable key, currently using the value stored + * in registry.db. The active DB has no override. + * - `override` — overridable key, the active DB has its own row in + * its per-DB settings table. + * - `builtin_default` — no row in registry or per-DB; value is the constant + * baked into the binary. + */ +export type SettingSource = + | 'workspace' + | 'workspace_default' + | 'override' + | 'builtin_default'; + +export interface SettingValue { + value: string; + source: SettingSource; +} + +/** + * Keys that are workspace-only — lives in registry.db, cannot be overridden + * per-DB. Mirrors `atomic_core::settings::WORKSPACE_ONLY_KEYS`. Kept in sync + * by hand; the override UI uses this to suppress override affordances on + * fields that aren't overridable in the first place. + */ +export const WORKSPACE_ONLY_KEYS: readonly string[] = [ + 'theme', + 'font', + 'openrouter_api_key', + 'openai_compat_api_key', + 'ollama_host', + 'openai_compat_base_url', +]; + +export function isWorkspaceOnly(key: string): boolean { + return WORKSPACE_ONLY_KEYS.includes(key); +} diff --git a/src/lib/transport/command-map.ts b/src/lib/transport/command-map.ts index e5db487d..4bac9c39 100644 --- a/src/lib/transport/command-map.ts +++ b/src/lib/transport/command-map.ts @@ -343,6 +343,10 @@ export const COMMAND_MAP: Record = { argsMode: 'body', transformArgs: (a) => ({ value: a.value }), }, + clear_setting_override: { + method: 'DELETE', + path: (a) => `/api/settings/${encodeURIComponent(a.key as string)}`, + }, test_openrouter_connection: { method: 'POST', path: '/api/settings/test-openrouter', diff --git a/src/stores/settings.ts b/src/stores/settings.ts index 4daa392d..47308740 100644 --- a/src/stores/settings.ts +++ b/src/stores/settings.ts @@ -1,48 +1,117 @@ import { create } from 'zustand'; import { getTransport } from '../lib/transport'; +import { + isWorkspaceOnly, + type SettingSource, + type SettingValue, +} from '../lib/settings'; +import { useDatabasesStore } from './databases'; +/** + * The store keeps two parallel maps so existing call sites can keep reading + * `settings.foo` for the resolved value, while override-aware UI reads + * `sources.foo` to decide whether to render override affordances. + * + * Mutations: + * - `setSetting` — routes per the resolver (workspace-only → registry; + * overridable + N≤1 → registry default; overridable + + * N>1 → per-DB override). + * - `clearOverride` — DELETE the per-DB override; resolver falls back to + * the workspace default. Refetches to pick up the new + * resolved value/source. + * + * Note: `set_workspace_default` exists on the backend (`PUT + * /api/settings/defaults/{key}`) as a primitive for a possible future + * "change for all DBs" feature, but isn't wired into the store — multi-DB + * users edit per-DB only. Workspace defaults are the inheritance source for + * new DBs, frozen at whatever values were live during the N=1 phase. + */ interface SettingsStore { settings: Record; + sources: Record; isLoading: boolean; error: string | null; - + fetchSettings: () => Promise; setSetting: (key: string, value: string) => Promise; + clearOverride: (key: string) => Promise; testOpenRouterConnection: (apiKey: string) => Promise; } +function splitResolvedSettings( + resolved: Record, +): { settings: Record; sources: Record } { + const settings: Record = {}; + const sources: Record = {}; + for (const [key, entry] of Object.entries(resolved)) { + settings[key] = entry.value; + sources[key] = entry.source; + } + return { settings, sources }; +} + export const useSettingsStore = create((set) => ({ settings: {}, + sources: {}, isLoading: false, error: null, - + fetchSettings: async () => { set({ isLoading: true, error: null }); try { - const settings = await getTransport().invoke>('get_settings'); - set({ settings, isLoading: false }); + const resolved = await getTransport().invoke>( + 'get_settings', + ); + const { settings, sources } = splitResolvedSettings(resolved); + set({ settings, sources, isLoading: false }); } catch (e) { set({ error: String(e), isLoading: false }); } }, - + setSetting: async (key: string, value: string) => { const current = useSettingsStore.getState().settings[key]; if (current === value) return; try { await getTransport().invoke('set_setting', { key, value }); + // Mirror the backend resolver's routing so the override chip flips + // immediately on edit instead of waiting for a refetch. The rules + // here must stay in sync with `AtomicCore::set_setting` — workspace- + // only keys land in registry, overridable keys land in registry while + // N≤1 (workspace default) and per-DB once N≥2 (override). + const dbCount = useDatabasesStore.getState().databases.length; + const newSource: SettingSource = isWorkspaceOnly(key) + ? 'workspace' + : dbCount <= 1 + ? 'workspace_default' + : 'override'; set((state) => ({ - settings: { ...state.settings, [key]: value } + settings: { ...state.settings, [key]: value }, + sources: { ...state.sources, [key]: newSource }, })); } catch (e) { set({ error: String(e) }); throw e; } }, - + + clearOverride: async (key: string) => { + try { + await getTransport().invoke('clear_setting_override', { key }); + // Resolved value flips to the workspace default, which we don't know + // without re-asking the server. + await useSettingsStore.getState().fetchSettings(); + } catch (e) { + set({ error: String(e) }); + throw e; + } + }, + testOpenRouterConnection: async (apiKey: string) => { - const result = await getTransport().invoke('test_openrouter_connection', { apiKey }); + const result = await getTransport().invoke( + 'test_openrouter_connection', + { apiKey }, + ); return result; }, })); -