fix: address PR review feedback (set_var safety, parse warnings, db_map efficiency)

1. Replace unsafe set_var keychain caching with OnceLock<String> in
   SecretsConfig::resolve(). Eliminates the env var write from main.rs
   entirely, using a process-wide OnceLock cache instead.

2. Log tracing::warn when database_backend or llm_backend settings
   fail to parse, instead of silently falling back to defaults.

3. Remove O(K*S) get() pre-check in from_db_map(). Instead, let set()
   run and match on "Path not found" errors to skip unknown keys,
   avoiding full Settings serialization per key.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

src/config.rs

@@ -190,7 +190,17 @@ impl DatabaseConfig {
                 message: e,
             })?
         } else if let Some(ref b) = settings.database_backend {
-            b.parse().unwrap_or(DatabaseBackend::default())
+            match b.parse() {
+                Ok(backend) => backend,
+                Err(e) => {
+                    tracing::warn!(
+                        "Invalid database_backend '{}' in settings: {}. Using default.",
+                        b,
+                        e
+                    );
+                    DatabaseBackend::default()
+                }
+            }
         } else {
             DatabaseBackend::default()
         };
@@ -418,7 +428,17 @@ impl LlmConfig {
                 message: e,
             })?
         } else if let Some(ref b) = settings.llm_backend {
-            b.parse().unwrap_or(LlmBackend::NearAi)
+            match b.parse() {
+                Ok(backend) => backend,
+                Err(e) => {
+                    tracing::warn!(
+                        "Invalid llm_backend '{}' in settings: {}. Using default NearAi.",
+                        b,
+                        e
+                    );
+                    LlmBackend::NearAi
+                }
+            }
         } else {
             LlmBackend::NearAi
         };
@@ -866,6 +886,13 @@ impl std::fmt::Debug for SecretsConfig {
     }
 }
 
+/// Process-wide cache for the keychain master key.
+///
+/// Avoids re-prompting the OS keychain on every `SecretsConfig::resolve()` call
+/// (e.g. `Config::from_env()` then `Config::from_db()`). Thread-safe alternative
+/// to caching in a process env var.
+static CACHED_KEYCHAIN_KEY: std::sync::OnceLock<String> = std::sync::OnceLock::new();
+
 impl SecretsConfig {
     async fn resolve(bootstrap: &crate::bootstrap::BootstrapConfig) -> Result<Self, ConfigError> {
         use crate::settings::KeySource;
@@ -875,21 +902,28 @@ impl SecretsConfig {
         } else {
             match bootstrap.secrets_master_key_source {
                 KeySource::Keychain => {
-                    // Try to load from OS keychain (async on Linux)
-                    match crate::secrets::keychain::get_master_key().await {
-                        Ok(key_bytes) => {
-                            let key_hex: String =
-                                key_bytes.iter().map(|b| format!("{:02x}", b)).collect();
-                            (Some(SecretString::from(key_hex)), KeySource::Keychain)
-                        }
-                        Err(_) => {
-                            // Keychain configured but key not found
-                            // This might happen if keychain was cleared
-                            tracing::warn!(
-                                "Secrets configured for keychain but key not found. \
-                                 Run 'ironclaw onboard' to reconfigure."
-                            );
-                            (None, KeySource::None)
+                    // Check process-level cache first (set on previous resolve() call)
+                    if let Some(cached) = CACHED_KEYCHAIN_KEY.get() {
+                        (
+                            Some(SecretString::from(cached.clone())),
+                            KeySource::Keychain,
+                        )
+                    } else {
+                        // Try to load from OS keychain (async on Linux)
+                        match crate::secrets::keychain::get_master_key().await {
+                            Ok(key_bytes) => {
+                                let key_hex: String =
+                                    key_bytes.iter().map(|b| format!("{b:02x}")).collect();
+                                let _ = CACHED_KEYCHAIN_KEY.set(key_hex.clone());
+                                (Some(SecretString::from(key_hex)), KeySource::Keychain)
+                            }
+                            Err(_) => {
+                                tracing::warn!(
+                                    "Secrets configured for keychain but key not found. \
+                                     Run 'ironclaw onboard' to reconfigure."
+                                );
+                                (None, KeySource::None)
+                            }
                         }
                     }
                 }
@@ -1386,7 +1420,9 @@ pub async fn inject_llm_keys_from_secrets(
         }
         match secrets.get_decrypted(user_id, secret_name).await {
             Ok(decrypted) => {
-                // SAFETY: single-threaded at this point in startup
+                // SAFETY: Called from main() before any tokio::spawn(). The tokio
+                // worker threads exist but are idle (no tasks scheduled yet), so
+                // no concurrent std::env::var reads can race with this write.
                 unsafe {
                     std::env::set_var(env_var, decrypted.expose());
                 }

src/main.rs

@@ -45,8 +45,6 @@ use ironclaw::secrets::PostgresSecretsStore;
 use ironclaw::secrets::SecretsCrypto;
 #[cfg(any(feature = "postgres", feature = "libsql"))]
 use ironclaw::setup::{SetupConfig, SetupWizard};
-use secrecy::ExposeSecret as _;
-
 #[tokio::main]
 async fn main() -> anyhow::Result<()> {
     let cli = Cli::parse();
@@ -300,18 +298,8 @@ async fn main() -> anyhow::Result<()> {
         Err(e) => return Err(e.into()),
     };
 
-    // If the master key was loaded from the OS keychain, cache it in the process
-    // env so that later config re-resolution (Config::from_db) doesn't prompt
-    // the user for keychain access again.
-    if config.secrets.source == ironclaw::settings::KeySource::Keychain
-        && std::env::var("SECRETS_MASTER_KEY").is_err()
-        && let Some(key) = config.secrets.master_key()
-    {
-        // SAFETY: Single-threaded at this point in startup.
-        unsafe {
-            std::env::set_var("SECRETS_MASTER_KEY", key.expose_secret());
-        }
-    }
+    // Keychain master key caching is handled by CACHED_KEYCHAIN_KEY OnceLock
+    // in SecretsConfig::resolve(), so repeated resolve() calls skip the keychain.
 
     // Initialize session manager and authenticate before channel setup
     let session_config = SessionConfig {

src/settings.rs

@@ -530,11 +530,6 @@ impl Settings {
         let mut settings = Self::default();
 
         for (key, value) in map {
-            // Check if this key maps to a known Settings field
-            if settings.get(key).is_none() {
-                continue;
-            }
-
             // Convert the JSONB value to a string for the existing set() method
             let value_str = match value {
                 serde_json::Value::String(s) => s.clone(),
@@ -544,13 +539,19 @@ impl Settings {
                 other => other.to_string(),
             };
 
-            if let Err(e) = settings.set(key, &value_str) {
-                tracing::warn!(
-                    "Failed to apply DB setting '{}' = '{}': {}",
-                    key,
-                    value_str,
-                    e
-                );
+            match settings.set(key, &value_str) {
+                Ok(()) => {}
+                // The settings table stores both Settings fields and app-specific
+                // data (e.g. nearai.session_token). Silently skip unknown paths.
+                Err(e) if e.starts_with("Path not found") => {}
+                Err(e) => {
+                    tracing::warn!(
+                        "Failed to apply DB setting '{}' = '{}': {}",
+                        key,
+                        value_str,
+                        e
+                    );
+                }
             }
         }