From f1fa303abc75e08f0b81b916b2222eeca502eb42 Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Fri, 27 Mar 2026 23:04:14 -0700 Subject: [PATCH] feat(config): unify all settings to DB > env > default priority MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously only LLM settings used DB-first priority while all other subsystems (agent, channels, tunnel, heartbeat, embeddings, sandbox, wasm, safety, builder, transcription, routines, skills, hygiene, search) used env-first. This made web UI settings changes unreliable for non-LLM config — env vars would silently override DB values. Now all subsystems follow the same priority: DB > env > TOML > default. - Add db_first_or_default, db_first_bool, db_first_optional_string, db_first_option helpers to config/helpers.rs with shadow warnings - Flip 10 Group 1 resolvers (agent, channels, tunnel, heartbeat, embeddings, sandbox, wasm, safety, builder, transcription) from parse_optional_env/parse_bool_env to db_first_* equivalents - Add Settings structs for 4 Group 2 resolvers (routines, skills, hygiene, search) that previously had no DB persistence - Update Config::build() call sites and cli/doctor.rs caller - Security-sensitive fields stay env-only: allow_local_tools, allow_full_access, cost/rate limits, auth tokens, API keys - Bootstrap configs (database, secrets) stay env-only Closes #1119 (partial — config unification phases 1-2) Co-Authored-By: Claude Opus 4.6 (1M context) --- src/cli/doctor.rs | 12 +- src/config/agent.rs | 64 ++++++---- src/config/builder.rs | 51 ++++++-- src/config/channels.rs | 138 +++++++++++++--------- src/config/embeddings.rs | 101 +++++++++++++--- src/config/heartbeat.rs | 213 +++++++++++++++++++++++++++------ src/config/helpers.rs | 227 ++++++++++++++++++++++++++++++++++++ src/config/hygiene.rs | 27 +++-- src/config/mod.rs | 29 ++--- src/config/routines.rs | 46 ++++++-- src/config/safety.rs | 36 +++++- src/config/sandbox.rs | 96 +++++++++++---- src/config/search.rs | 101 ++++++++++++---- src/config/skills.rs | 23 +++- src/config/transcription.rs | 8 +- src/config/tunnel.rs | 71 ++++++----- src/config/wasm.rs | 60 +++++++--- src/settings.rs | 211 ++++++++++++++++++++++++++++++++- 18 files changed, 1229 insertions(+), 285 deletions(-) diff --git a/src/cli/doctor.rs b/src/cli/doctor.rs index 023ac4e1..dca408d7 100644 --- a/src/cli/doctor.rs +++ b/src/cli/doctor.rs @@ -80,7 +80,7 @@ pub async fn run_doctor_command() -> anyhow::Result<()> { check( "Routines config", - check_routines_config(), + check_routines_config(&settings), &mut passed, &mut failed, &mut skipped, @@ -434,8 +434,8 @@ fn check_embeddings(settings: &Settings) -> CheckResult { // ── Routines config ───────────────────────────────────────── -fn check_routines_config() -> CheckResult { - match crate::config::RoutineConfig::resolve() { +fn check_routines_config(settings: &Settings) -> CheckResult { + match crate::config::RoutineConfig::resolve(settings) { Ok(config) => { if config.enabled { CheckResult::Pass(format!( @@ -737,7 +737,8 @@ mod tests { #[test] fn check_routines_config_does_not_panic() { - let result = check_routines_config(); + let settings = Settings::default(); + let result = check_routines_config(&settings); match result { CheckResult::Pass(_) | CheckResult::Fail(_) | CheckResult::Skip(_) => {} } @@ -866,7 +867,8 @@ mod tests { unsafe { std::env::remove_var("ROUTINES_ENABLED"); } - match check_routines_config() { + let settings = Settings::default(); + match check_routines_config(&settings) { CheckResult::Pass(msg) => { assert!( msg.contains("enabled"), diff --git a/src/config/agent.rs b/src/config/agent.rs index cfa0879a..5e1d83be 100644 --- a/src/config/agent.rs +++ b/src/config/agent.rs @@ -1,6 +1,8 @@ use std::time::Duration; -use crate::config::helpers::{optional_env, parse_bool_env, parse_option_env, parse_optional_env}; +use crate::config::helpers::{ + db_first_bool, db_first_or_default, optional_env, parse_bool_env, parse_option_env, +}; use crate::error::ConfigError; use crate::settings::Settings; @@ -70,49 +72,64 @@ impl AgentConfig { } pub(crate) fn resolve(settings: &Settings) -> Result { + let defaults = crate::settings::AgentSettings::default(); + Ok(Self { - name: parse_optional_env("AGENT_NAME", settings.agent.name.clone())?, - max_parallel_jobs: parse_optional_env( + name: db_first_or_default(&settings.agent.name, &defaults.name, "AGENT_NAME")?, + max_parallel_jobs: db_first_or_default( + &(settings.agent.max_parallel_jobs as usize), + &(defaults.max_parallel_jobs as usize), "AGENT_MAX_PARALLEL_JOBS", - settings.agent.max_parallel_jobs as usize, )?, - job_timeout: Duration::from_secs(parse_optional_env( + job_timeout: Duration::from_secs(db_first_or_default( + &settings.agent.job_timeout_secs, + &defaults.job_timeout_secs, "AGENT_JOB_TIMEOUT_SECS", - settings.agent.job_timeout_secs, )?), - stuck_threshold: Duration::from_secs(parse_optional_env( + stuck_threshold: Duration::from_secs(db_first_or_default( + &settings.agent.stuck_threshold_secs, + &defaults.stuck_threshold_secs, "AGENT_STUCK_THRESHOLD_SECS", - settings.agent.stuck_threshold_secs, )?), - repair_check_interval: Duration::from_secs(parse_optional_env( + repair_check_interval: Duration::from_secs(db_first_or_default( + &settings.agent.repair_check_interval_secs, + &defaults.repair_check_interval_secs, "SELF_REPAIR_CHECK_INTERVAL_SECS", - settings.agent.repair_check_interval_secs, )?), - max_repair_attempts: parse_optional_env( + max_repair_attempts: db_first_or_default( + &settings.agent.max_repair_attempts, + &defaults.max_repair_attempts, "SELF_REPAIR_MAX_ATTEMPTS", - settings.agent.max_repair_attempts, )?, - use_planning: parse_bool_env("AGENT_USE_PLANNING", settings.agent.use_planning)?, - session_idle_timeout: Duration::from_secs(parse_optional_env( + use_planning: db_first_bool( + settings.agent.use_planning, + defaults.use_planning, + "AGENT_USE_PLANNING", + )?, + session_idle_timeout: Duration::from_secs(db_first_or_default( + &settings.agent.session_idle_timeout_secs, + &defaults.session_idle_timeout_secs, "SESSION_IDLE_TIMEOUT_SECS", - settings.agent.session_idle_timeout_secs, )?), allow_local_tools: parse_bool_env("ALLOW_LOCAL_TOOLS", false)?, max_cost_per_day_cents: parse_option_env("MAX_COST_PER_DAY_CENTS")?, max_actions_per_hour: parse_option_env("MAX_ACTIONS_PER_HOUR")?, max_cost_per_user_per_day_cents: parse_option_env("MAX_COST_PER_USER_PER_DAY_CENTS")?, - max_tool_iterations: parse_optional_env( + max_tool_iterations: db_first_or_default( + &settings.agent.max_tool_iterations, + &defaults.max_tool_iterations, "AGENT_MAX_TOOL_ITERATIONS", - settings.agent.max_tool_iterations, )?, - auto_approve_tools: parse_bool_env( - "AGENT_AUTO_APPROVE_TOOLS", + auto_approve_tools: db_first_bool( settings.agent.auto_approve_tools, + defaults.auto_approve_tools, + "AGENT_AUTO_APPROVE_TOOLS", )?, default_timezone: { - let tz: String = parse_optional_env( + let tz: String = db_first_or_default( + &settings.agent.default_timezone, + &defaults.default_timezone, "DEFAULT_TIMEZONE", - settings.agent.default_timezone.clone(), )?; if crate::timezone::parse_timezone(&tz).is_none() { return Err(ConfigError::InvalidValue { @@ -122,9 +139,10 @@ impl AgentConfig { } tz }, - max_tokens_per_job: parse_optional_env( + max_tokens_per_job: db_first_or_default( + &settings.agent.max_tokens_per_job, + &defaults.max_tokens_per_job, "AGENT_MAX_TOKENS_PER_JOB", - settings.agent.max_tokens_per_job, )?, // Auto-detected from GATEWAY_USER_TOKENS presence. Not a separate // knob — multi-tenant mode is always implied by configuring user tokens. diff --git a/src/config/builder.rs b/src/config/builder.rs index f7bad12c..73c8930a 100644 --- a/src/config/builder.rs +++ b/src/config/builder.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; use std::time::Duration; -use crate::config::helpers::{optional_env, parse_bool_env, parse_optional_env}; +use crate::config::helpers::{db_first_bool, db_first_or_default, optional_env}; use crate::error::ConfigError; /// Builder mode configuration. @@ -34,14 +34,29 @@ impl Default for BuilderModeConfig { impl BuilderModeConfig { pub(crate) fn resolve(settings: &crate::settings::Settings) -> Result { let bs = &settings.builder; + let defaults = crate::settings::BuilderSettings::default(); Ok(Self { - enabled: parse_bool_env("BUILDER_ENABLED", bs.enabled)?, - build_dir: optional_env("BUILDER_DIR")? - .map(PathBuf::from) - .or_else(|| bs.build_dir.clone()), - max_iterations: parse_optional_env("BUILDER_MAX_ITERATIONS", bs.max_iterations)?, - timeout_secs: parse_optional_env("BUILDER_TIMEOUT_SECS", bs.timeout_secs)?, - auto_register: parse_bool_env("BUILDER_AUTO_REGISTER", bs.auto_register)?, + enabled: db_first_bool(bs.enabled, defaults.enabled, "BUILDER_ENABLED")?, + build_dir: if let Some(ref dir) = bs.build_dir { + Some(dir.clone()) + } else { + optional_env("BUILDER_DIR")?.map(PathBuf::from) + }, + max_iterations: db_first_or_default( + &bs.max_iterations, + &defaults.max_iterations, + "BUILDER_MAX_ITERATIONS", + )?, + timeout_secs: db_first_or_default( + &bs.timeout_secs, + &defaults.timeout_secs, + "BUILDER_TIMEOUT_SECS", + )?, + auto_register: db_first_bool( + bs.auto_register, + defaults.auto_register, + "BUILDER_AUTO_REGISTER", + )?, }) } @@ -79,7 +94,7 @@ mod tests { } #[test] - fn env_overrides_settings() { + fn db_settings_override_env() { let _guard = lock_env(); let mut settings = Settings::default(); settings.builder.timeout_secs = 123; @@ -89,6 +104,22 @@ mod tests { let cfg = BuilderModeConfig::resolve(&settings).expect("resolve"); unsafe { std::env::remove_var("BUILDER_TIMEOUT_SECS") }; - assert_eq!(cfg.timeout_secs, 3); + assert_eq!(cfg.timeout_secs, 123, "DB setting should win over env"); + } + + #[test] + fn env_used_when_no_db_setting() { + let _guard = lock_env(); + let settings = Settings::default(); + + // SAFETY: Under ENV_MUTEX, no concurrent env access. + unsafe { std::env::set_var("BUILDER_TIMEOUT_SECS", "42") }; + let cfg = BuilderModeConfig::resolve(&settings).expect("resolve"); + unsafe { std::env::remove_var("BUILDER_TIMEOUT_SECS") }; + + assert_eq!( + cfg.timeout_secs, 42, + "env should be used when DB has the default value" + ); } } diff --git a/src/config/channels.rs b/src/config/channels.rs index d9c2c0a9..759e005b 100644 --- a/src/config/channels.rs +++ b/src/config/channels.rs @@ -5,9 +5,11 @@ use secrecy::SecretString; use serde::Deserialize; use crate::bootstrap::ironclaw_base_dir; -use crate::config::helpers::{optional_env, parse_bool_env, parse_optional_env}; +use crate::config::helpers::{ + db_first_bool, db_first_optional_string, db_first_or_default, optional_env, parse_optional_env, +}; use crate::error::ConfigError; -use crate::settings::Settings; +use crate::settings::{ChannelSettings, Settings}; /// Channel configurations. #[derive(Debug, Clone)] @@ -114,15 +116,24 @@ pub struct SignalConfig { impl ChannelsConfig { pub(crate) fn resolve(settings: &Settings, owner_id: &str) -> Result { let cs = &settings.channels; + let defaults = ChannelSettings::default(); let http_enabled_by_env = optional_env("HTTP_PORT")?.is_some() || optional_env("HTTP_HOST")?.is_some(); - let http = if http_enabled_by_env || cs.http_enabled { + let http_enabled_by_db = + db_first_bool(cs.http_enabled, defaults.http_enabled, "HTTP_ENABLED")?; + let http = if http_enabled_by_env || http_enabled_by_db { Some(HttpConfig { - host: optional_env("HTTP_HOST")? - .or_else(|| cs.http_host.clone()) + host: db_first_optional_string(&cs.http_host, "HTTP_HOST")? .unwrap_or_else(|| "0.0.0.0".to_string()), - port: parse_optional_env("HTTP_PORT", cs.http_port.unwrap_or(8080))?, + port: { + // defaults.http_port is None, so any Some(..) is an explicit DB override. + if let Some(ref db_port) = cs.http_port { + db_first_or_default(db_port, &8080, "HTTP_PORT")? + } else { + parse_optional_env("HTTP_PORT", 8080)? + } + }, webhook_secret: optional_env("HTTP_WEBHOOK_SECRET")?.map(SecretString::from), user_id: owner_id.to_string(), }) @@ -130,10 +141,13 @@ impl ChannelsConfig { None }; - let gateway_enabled = parse_bool_env("GATEWAY_ENABLED", cs.gateway_enabled)?; + let gateway_enabled = db_first_bool( + cs.gateway_enabled, + defaults.gateway_enabled, + "GATEWAY_ENABLED", + )?; let gateway = if gateway_enabled { - let user_id = optional_env("GATEWAY_USER_ID")? - .or_else(|| cs.gateway_user_id.clone()) + let user_id = db_first_optional_string(&cs.gateway_user_id, "GATEWAY_USER_ID")? .unwrap_or_else(|| owner_id.to_string()); let memory_layers: Vec = @@ -249,13 +263,16 @@ impl ChannelsConfig { } } Some(GatewayConfig { - host: optional_env("GATEWAY_HOST")? - .or_else(|| cs.gateway_host.clone()) + host: db_first_optional_string(&cs.gateway_host, "GATEWAY_HOST")? .unwrap_or_else(|| "127.0.0.1".to_string()), - port: parse_optional_env( - "GATEWAY_PORT", - cs.gateway_port.unwrap_or(DEFAULT_GATEWAY_PORT), - )?, + port: { + // defaults.gateway_port is None, so any Some(..) is an explicit DB override. + if let Some(ref db_port) = cs.gateway_port { + db_first_or_default(db_port, &DEFAULT_GATEWAY_PORT, "GATEWAY_PORT")? + } else { + parse_optional_env("GATEWAY_PORT", DEFAULT_GATEWAY_PORT)? + } + }, auth_token: optional_env("GATEWAY_AUTH_TOKEN")? .or_else(|| cs.gateway_auth_token.clone()), user_id, @@ -267,16 +284,22 @@ impl ChannelsConfig { None }; - let signal_url = optional_env("SIGNAL_HTTP_URL")?.or_else(|| cs.signal_http_url.clone()); - let signal = if let Some(http_url) = signal_url { - let account = optional_env("SIGNAL_ACCOUNT")? - .or_else(|| cs.signal_account.clone()) - .ok_or(ConfigError::InvalidValue { + let signal_enabled = + db_first_bool(cs.signal_enabled, defaults.signal_enabled, "SIGNAL_ENABLED")?; + let signal_url = db_first_optional_string(&cs.signal_http_url, "SIGNAL_HTTP_URL")?; + let signal = if signal_enabled || signal_url.is_some() { + let http_url = signal_url.ok_or(ConfigError::InvalidValue { + key: "SIGNAL_HTTP_URL".to_string(), + message: "SIGNAL_HTTP_URL is required when Signal is enabled".to_string(), + })?; + let account = db_first_optional_string(&cs.signal_account, "SIGNAL_ACCOUNT")?.ok_or( + ConfigError::InvalidValue { key: "SIGNAL_ACCOUNT".to_string(), message: "SIGNAL_ACCOUNT is required when SIGNAL_HTTP_URL is set".to_string(), - })?; + }, + )?; let allow_from = - match optional_env("SIGNAL_ALLOW_FROM")?.or_else(|| cs.signal_allow_from.clone()) { + match db_first_optional_string(&cs.signal_allow_from, "SIGNAL_ALLOW_FROM")? { None => vec![account.clone()], Some(s) => s .split(',') @@ -284,36 +307,39 @@ impl ChannelsConfig { .filter(|s| !s.is_empty()) .collect(), }; - let dm_policy = optional_env("SIGNAL_DM_POLICY")? - .or_else(|| cs.signal_dm_policy.clone()) + let dm_policy = db_first_optional_string(&cs.signal_dm_policy, "SIGNAL_DM_POLICY")? .unwrap_or_else(|| "pairing".to_string()); - let group_policy = optional_env("SIGNAL_GROUP_POLICY")? - .or_else(|| cs.signal_group_policy.clone()) - .unwrap_or_else(|| "allowlist".to_string()); + let group_policy = + db_first_optional_string(&cs.signal_group_policy, "SIGNAL_GROUP_POLICY")? + .unwrap_or_else(|| "allowlist".to_string()); Some(SignalConfig { http_url, account, allow_from, - allow_from_groups: optional_env("SIGNAL_ALLOW_FROM_GROUPS")? - .or_else(|| cs.signal_allow_from_groups.clone()) - .map(|s| { - s.split(',') - .map(|e| e.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect() - }) - .unwrap_or_default(), + allow_from_groups: db_first_optional_string( + &cs.signal_allow_from_groups, + "SIGNAL_ALLOW_FROM_GROUPS", + )? + .map(|s| { + s.split(',') + .map(|e| e.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect() + }) + .unwrap_or_default(), dm_policy, group_policy, - group_allow_from: optional_env("SIGNAL_GROUP_ALLOW_FROM")? - .or_else(|| cs.signal_group_allow_from.clone()) - .map(|s| { - s.split(',') - .map(|e| e.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect() - }) - .unwrap_or_default(), + group_allow_from: db_first_optional_string( + &cs.signal_group_allow_from, + "SIGNAL_GROUP_ALLOW_FROM", + )? + .map(|s| { + s.split(',') + .map(|e| e.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect() + }) + .unwrap_or_default(), ignore_attachments: optional_env("SIGNAL_IGNORE_ATTACHMENTS")? .map(|s| s.to_lowercase() == "true" || s == "1") .unwrap_or(false), @@ -325,7 +351,7 @@ impl ChannelsConfig { None }; - let cli_enabled = parse_bool_env("CLI_ENABLED", cs.cli_enabled)?; + let cli_enabled = db_first_bool(cs.cli_enabled, defaults.cli_enabled, "CLI_ENABLED")?; Ok(Self { cli: CliConfig { @@ -334,13 +360,21 @@ impl ChannelsConfig { http, gateway, signal, - wasm_channels_dir: optional_env("WASM_CHANNELS_DIR")? - .map(PathBuf::from) - .or_else(|| cs.wasm_channels_dir.clone()) - .unwrap_or_else(default_channels_dir), - wasm_channels_enabled: parse_bool_env( - "WASM_CHANNELS_ENABLED", + wasm_channels_dir: { + // DB-first: use settings if explicitly set, else env, else default. + // defaults.wasm_channels_dir is None, so any Some(..) is an explicit DB override. + if let Some(ref db_dir) = cs.wasm_channels_dir { + db_dir.clone() + } else { + optional_env("WASM_CHANNELS_DIR")? + .map(PathBuf::from) + .unwrap_or_else(default_channels_dir) + } + }, + wasm_channels_enabled: db_first_bool( cs.wasm_channels_enabled, + defaults.wasm_channels_enabled, + "WASM_CHANNELS_ENABLED", )?, wasm_channel_owner_ids: { let mut ids = cs.wasm_channel_owner_ids.clone(); diff --git a/src/config/embeddings.rs b/src/config/embeddings.rs index 98183976..193268be 100644 --- a/src/config/embeddings.rs +++ b/src/config/embeddings.rs @@ -2,7 +2,9 @@ use std::sync::Arc; use secrecy::{ExposeSecret, SecretString}; -use crate::config::helpers::{optional_env, parse_bool_env, parse_optional_env, validate_base_url}; +use crate::config::helpers::{ + db_first_bool, db_first_or_default, optional_env, parse_optional_env, validate_base_url, +}; use crate::error::ConfigError; use crate::llm::SessionManager; use crate::settings::Settings; @@ -71,22 +73,41 @@ pub(crate) fn default_dimension_for_model(model: &str) -> usize { impl EmbeddingsConfig { pub(crate) fn resolve(settings: &Settings) -> Result { + let defaults = crate::settings::EmbeddingsSettings::default(); + let openai_api_key = optional_env("OPENAI_API_KEY")?.map(SecretString::from); - let provider = optional_env("EMBEDDING_PROVIDER")? - .unwrap_or_else(|| settings.embeddings.provider.clone()); + let provider = db_first_or_default( + &settings.embeddings.provider, + &defaults.provider, + "EMBEDDING_PROVIDER", + )?; - let model = - optional_env("EMBEDDING_MODEL")?.unwrap_or_else(|| settings.embeddings.model.clone()); + let model = db_first_or_default( + &settings.embeddings.model, + &defaults.model, + "EMBEDDING_MODEL", + )?; - let ollama_base_url = optional_env("OLLAMA_BASE_URL")? - .or_else(|| settings.ollama_base_url.clone()) - .unwrap_or_else(|| "http://localhost:11434".to_string()); + // ollama_base_url lives on the top-level Settings, not the embeddings + // sub-struct. Use a manual DB > env > default chain. + let default_ollama_url = "http://localhost:11434".to_string(); + let ollama_base_url = settings + .ollama_base_url + .as_ref() + .filter(|s| !s.is_empty()) + .cloned() + .or_else(|| optional_env("OLLAMA_BASE_URL").ok().flatten()) + .unwrap_or(default_ollama_url); - let dimension = - parse_optional_env("EMBEDDING_DIMENSION", default_dimension_for_model(&model))?; + let dim_default = default_dimension_for_model(&model); + let dimension = db_first_or_default(&dim_default, &dim_default, "EMBEDDING_DIMENSION")?; - let enabled = parse_bool_env("EMBEDDING_ENABLED", settings.embeddings.enabled)?; + let enabled = db_first_bool( + settings.embeddings.enabled, + defaults.enabled, + "EMBEDDING_ENABLED", + )?; let openai_base_url = optional_env("EMBEDDING_BASE_URL")?; @@ -207,9 +228,11 @@ mod tests { std::env::remove_var("EMBEDDING_ENABLED"); std::env::remove_var("EMBEDDING_PROVIDER"); std::env::remove_var("EMBEDDING_MODEL"); + std::env::remove_var("EMBEDDING_DIMENSION"); std::env::remove_var("OPENAI_API_KEY"); std::env::remove_var("EMBEDDING_BASE_URL"); std::env::remove_var("EMBEDDING_CACHE_SIZE"); + std::env::remove_var("OLLAMA_BASE_URL"); } } @@ -264,18 +287,21 @@ mod tests { } #[test] - fn embeddings_env_override_takes_precedence() { + fn db_settings_override_env() { let _guard = lock_env(); clear_embedding_env(); // SAFETY: Under ENV_MUTEX. unsafe { - std::env::set_var("EMBEDDING_ENABLED", "true"); + std::env::set_var("EMBEDDING_ENABLED", "false"); + std::env::set_var("EMBEDDING_PROVIDER", "ollama"); + std::env::set_var("EMBEDDING_MODEL", "all-minilm"); } let settings = Settings { embeddings: EmbeddingsSettings { - enabled: false, - ..Default::default() + enabled: true, + provider: "openai".to_string(), + model: "text-embedding-3-large".to_string(), }, ..Default::default() }; @@ -283,12 +309,55 @@ mod tests { let config = EmbeddingsConfig::resolve(&settings).expect("resolve should succeed"); assert!( config.enabled, - "EMBEDDING_ENABLED=true env var should override settings" + "DB enabled=true should win over env EMBEDDING_ENABLED=false" + ); + assert_eq!(config.provider, "openai", "DB provider should win over env"); + assert_eq!( + config.model, "text-embedding-3-large", + "DB model should win over env" ); // SAFETY: Under ENV_MUTEX. unsafe { std::env::remove_var("EMBEDDING_ENABLED"); + std::env::remove_var("EMBEDDING_PROVIDER"); + std::env::remove_var("EMBEDDING_MODEL"); + } + } + + #[test] + fn env_used_when_no_db_setting() { + let _guard = lock_env(); + clear_embedding_env(); + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::set_var("EMBEDDING_ENABLED", "true"); + std::env::set_var("EMBEDDING_PROVIDER", "ollama"); + std::env::set_var("EMBEDDING_MODEL", "nomic-embed-text"); + } + + // Settings left at defaults — no explicit DB/TOML override + let settings = Settings::default(); + + let config = EmbeddingsConfig::resolve(&settings).expect("resolve should succeed"); + assert!( + config.enabled, + "env EMBEDDING_ENABLED should be used when settings at default" + ); + assert_eq!( + config.provider, "ollama", + "env EMBEDDING_PROVIDER should be used when settings at default" + ); + assert_eq!( + config.model, "nomic-embed-text", + "env EMBEDDING_MODEL should be used when settings at default" + ); + + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("EMBEDDING_ENABLED"); + std::env::remove_var("EMBEDDING_PROVIDER"); + std::env::remove_var("EMBEDDING_MODEL"); } } diff --git a/src/config/heartbeat.rs b/src/config/heartbeat.rs index 09b8f0cd..0c22b513 100644 --- a/src/config/heartbeat.rs +++ b/src/config/heartbeat.rs @@ -1,4 +1,7 @@ -use crate::config::helpers::{optional_env, parse_bool_env, parse_option_env, parse_optional_env}; +use crate::config::helpers::{ + db_first_bool, db_first_optional_string, db_first_or_default, optional_env, parse_bool_env, + parse_option_env, +}; use crate::error::ConfigError; use crate::settings::Settings; @@ -44,8 +47,11 @@ impl Default for HeartbeatConfig { impl HeartbeatConfig { pub(crate) fn resolve(settings: &Settings) -> Result { + let defaults = crate::settings::HeartbeatSettings::default(); + + // fire_at: DB > env, then parse into NaiveTime let fire_at_str = - optional_env("HEARTBEAT_FIRE_AT")?.or_else(|| settings.heartbeat.fire_at.clone()); + db_first_optional_string(&settings.heartbeat.fire_at, "HEARTBEAT_FIRE_AT")?; let fire_at = fire_at_str .map(|s| { chrono::NaiveTime::parse_from_str(&s, "%H:%M").map_err(|e| { @@ -57,44 +63,62 @@ impl HeartbeatConfig { }) .transpose()?; + // quiet_hours: settings first, then env fallback + let quiet_hours_start = settings + .heartbeat + .quiet_hours_start + .or(parse_option_env::("HEARTBEAT_QUIET_START")?) + .map(|h| { + if h > 23 { + return Err(ConfigError::InvalidValue { + key: "HEARTBEAT_QUIET_START".into(), + message: "must be 0-23".into(), + }); + } + Ok(h) + }) + .transpose()?; + + let quiet_hours_end = settings + .heartbeat + .quiet_hours_end + .or(parse_option_env::("HEARTBEAT_QUIET_END")?) + .map(|h| { + if h > 23 { + return Err(ConfigError::InvalidValue { + key: "HEARTBEAT_QUIET_END".into(), + message: "must be 0-23".into(), + }); + } + Ok(h) + }) + .transpose()?; + Ok(Self { - enabled: parse_bool_env("HEARTBEAT_ENABLED", settings.heartbeat.enabled)?, - interval_secs: parse_optional_env( + enabled: db_first_bool( + settings.heartbeat.enabled, + defaults.enabled, + "HEARTBEAT_ENABLED", + )?, + interval_secs: db_first_or_default( + &settings.heartbeat.interval_secs, + &defaults.interval_secs, "HEARTBEAT_INTERVAL_SECS", - settings.heartbeat.interval_secs, + )?, + notify_channel: db_first_optional_string( + &settings.heartbeat.notify_channel, + "HEARTBEAT_NOTIFY_CHANNEL", + )?, + notify_user: db_first_optional_string( + &settings.heartbeat.notify_user, + "HEARTBEAT_NOTIFY_USER", )?, - notify_channel: optional_env("HEARTBEAT_NOTIFY_CHANNEL")? - .or_else(|| settings.heartbeat.notify_channel.clone()), - notify_user: optional_env("HEARTBEAT_NOTIFY_USER")? - .or_else(|| settings.heartbeat.notify_user.clone()), fire_at, - quiet_hours_start: parse_option_env::("HEARTBEAT_QUIET_START")? - .or(settings.heartbeat.quiet_hours_start) - .map(|h| { - if h > 23 { - return Err(ConfigError::InvalidValue { - key: "HEARTBEAT_QUIET_START".into(), - message: "must be 0-23".into(), - }); - } - Ok(h) - }) - .transpose()?, - quiet_hours_end: parse_option_env::("HEARTBEAT_QUIET_END")? - .or(settings.heartbeat.quiet_hours_end) - .map(|h| { - if h > 23 { - return Err(ConfigError::InvalidValue { - key: "HEARTBEAT_QUIET_END".into(), - message: "must be 0-23".into(), - }); - } - Ok(h) - }) - .transpose()?, + quiet_hours_start, + quiet_hours_end, timezone: { - let tz = optional_env("HEARTBEAT_TIMEZONE")? - .or_else(|| settings.heartbeat.timezone.clone()); + let tz = + db_first_optional_string(&settings.heartbeat.timezone, "HEARTBEAT_TIMEZONE")?; if let Some(ref tz_str) = tz && crate::timezone::parse_timezone(tz_str).is_none() { @@ -106,7 +130,7 @@ impl HeartbeatConfig { tz }, // Auto-detect multi-tenant mode from GATEWAY_USER_TOKENS presence, - // or allow explicit override via HEARTBEAT_MULTI_TENANT. + // or allow explicit override via HEARTBEAT_MULTI_TENANT. Stays env-only. multi_tenant: parse_bool_env( "HEARTBEAT_MULTI_TENANT", optional_env("GATEWAY_USER_TOKENS")?.is_some(), @@ -118,10 +142,11 @@ impl HeartbeatConfig { #[cfg(test)] mod tests { use super::*; + use crate::config::helpers::lock_env; #[test] - fn test_quiet_hours_settings_fallback() { - // When env vars are not set, settings values should be used + fn test_quiet_hours_settings_have_priority() { + // DB/settings values should take priority over env let mut settings = Settings::default(); settings.heartbeat.quiet_hours_start = Some(22); settings.heartbeat.quiet_hours_end = Some(6); @@ -168,4 +193,116 @@ mod tests { let config = HeartbeatConfig::resolve(&settings).expect("resolve"); assert_eq!(config.timezone.as_deref(), Some("America/New_York")); } + + #[test] + fn test_db_first_enabled_beats_env() { + let _guard = lock_env(); + // SAFETY: under ENV_MUTEX + unsafe { std::env::set_var("HEARTBEAT_ENABLED", "false") }; + + let mut settings = Settings::default(); + settings.heartbeat.enabled = true; // DB says enabled + + let config = HeartbeatConfig::resolve(&settings).expect("resolve"); + assert!(config.enabled, "DB value (true) should beat env (false)"); + + unsafe { std::env::remove_var("HEARTBEAT_ENABLED") }; + } + + #[test] + fn test_db_first_interval_beats_env() { + let _guard = lock_env(); + unsafe { std::env::set_var("HEARTBEAT_INTERVAL_SECS", "999") }; + + let mut settings = Settings::default(); + settings.heartbeat.interval_secs = 600; // DB says 600 + + let config = HeartbeatConfig::resolve(&settings).expect("resolve"); + assert_eq!(config.interval_secs, 600, "DB value should beat env"); + + unsafe { std::env::remove_var("HEARTBEAT_INTERVAL_SECS") }; + } + + #[test] + fn test_db_first_notify_channel_beats_env() { + let _guard = lock_env(); + unsafe { std::env::set_var("HEARTBEAT_NOTIFY_CHANNEL", "env-channel") }; + + let mut settings = Settings::default(); + settings.heartbeat.notify_channel = Some("db-channel".to_string()); + + let config = HeartbeatConfig::resolve(&settings).expect("resolve"); + assert_eq!( + config.notify_channel.as_deref(), + Some("db-channel"), + "DB value should beat env" + ); + + unsafe { std::env::remove_var("HEARTBEAT_NOTIFY_CHANNEL") }; + } + + #[test] + fn test_env_fallback_when_db_at_default() { + let _guard = lock_env(); + unsafe { std::env::set_var("HEARTBEAT_INTERVAL_SECS", "999") }; + + // Settings at default => env should win + let settings = Settings::default(); + + let config = HeartbeatConfig::resolve(&settings).expect("resolve"); + assert_eq!( + config.interval_secs, 999, + "env should win when DB at default" + ); + + unsafe { std::env::remove_var("HEARTBEAT_INTERVAL_SECS") }; + } + + #[test] + fn test_fire_at_db_first() { + let _guard = lock_env(); + unsafe { std::env::set_var("HEARTBEAT_FIRE_AT", "08:00") }; + + let mut settings = Settings::default(); + settings.heartbeat.fire_at = Some("14:30".to_string()); + + let config = HeartbeatConfig::resolve(&settings).expect("resolve"); + assert_eq!( + config.fire_at, + Some(chrono::NaiveTime::from_hms_opt(14, 30, 0).unwrap()), + "DB fire_at should beat env" + ); + + unsafe { std::env::remove_var("HEARTBEAT_FIRE_AT") }; + } + + #[test] + fn test_timezone_db_first() { + let _guard = lock_env(); + unsafe { std::env::set_var("HEARTBEAT_TIMEZONE", "UTC") }; + + let mut settings = Settings::default(); + settings.heartbeat.timezone = Some("America/New_York".to_string()); + + let config = HeartbeatConfig::resolve(&settings).expect("resolve"); + assert_eq!( + config.timezone.as_deref(), + Some("America/New_York"), + "DB timezone should beat env" + ); + + unsafe { std::env::remove_var("HEARTBEAT_TIMEZONE") }; + } + + #[test] + fn test_multi_tenant_stays_env_only() { + let _guard = lock_env(); + unsafe { std::env::set_var("HEARTBEAT_MULTI_TENANT", "true") }; + + let settings = Settings::default(); + let config = HeartbeatConfig::resolve(&settings).expect("resolve"); + assert!(config.multi_tenant, "multi_tenant should read from env"); + + unsafe { std::env::remove_var("HEARTBEAT_MULTI_TENANT") }; + } } diff --git a/src/config/helpers.rs b/src/config/helpers.rs index ff5ee706..f5a06595 100644 --- a/src/config/helpers.rs +++ b/src/config/helpers.rs @@ -331,6 +331,93 @@ pub(crate) fn validate_base_url(url: &str, field_name: &str) -> Result<(), Confi Ok(()) } +// --------------------------------------------------------------------------- +// DB-first resolution helpers (DB > env > default) +// --------------------------------------------------------------------------- + +/// Log a warning when a DB/TOML setting shadows a set env var. +fn warn_if_db_shadows_env(env_key: &str, db_value: &dyn std::fmt::Display) { + if let Ok(env_val) = std::env::var(env_key) + && !env_val.is_empty() + { + tracing::warn!( + db_value = %db_value, + env_value = %env_val, + "{env_key} env var is set but DB/TOML setting takes priority. \ + Remove the setting from the DB to use the env var." + ); + } +} + +/// Resolve with DB > env > default priority for concrete settings fields. +/// +/// If `settings_val != default_val`, the settings value wins (it was explicitly +/// set in DB or TOML). Otherwise falls back to `optional_env(env_key)`, then +/// `default_val`. +pub(crate) fn db_first_or_default( + settings_val: &T, + default_val: &T, + env_key: &str, +) -> Result +where + T: std::str::FromStr + Clone + PartialEq + std::fmt::Display, + T::Err: std::fmt::Display, +{ + if settings_val != default_val { + warn_if_db_shadows_env(env_key, settings_val); + return Ok(settings_val.clone()); + } + parse_optional_env(env_key, default_val.clone()) +} + +/// Resolve a bool with DB > env > default priority. +pub(crate) fn db_first_bool( + settings_val: bool, + default_val: bool, + env_key: &str, +) -> Result { + if settings_val != default_val { + warn_if_db_shadows_env(env_key, &settings_val); + return Ok(settings_val); + } + parse_bool_env(env_key, default_val) +} + +/// Resolve an `Option` with DB > env priority (no hardcoded default). +/// +/// Non-empty `Some` means DB set it; `None` or empty falls back to env. +pub(crate) fn db_first_optional_string( + settings_val: &Option, + env_key: &str, +) -> Result, ConfigError> { + if let Some(val) = settings_val + && !val.is_empty() + { + warn_if_db_shadows_env(env_key, val); + return Ok(Some(val.clone())); + } + optional_env(env_key) +} + +/// Resolve an `Option` with DB > env priority (no hardcoded default). +/// +/// `Some(v)` means DB set it; `None` falls back to env. +#[allow(dead_code)] // Used by Group 2 resolvers (routines, skills, etc.) +pub(crate) fn db_first_option( + settings_val: &Option, + env_key: &str, +) -> Result, ConfigError> +where + T: std::str::FromStr + Clone + std::fmt::Display, + T::Err: std::fmt::Display, +{ + if let Some(val) = settings_val { + warn_if_db_shadows_env(env_key, val); + return Ok(Some(val.clone())); + } + parse_option_env(env_key) +} + #[cfg(test)] mod tests { use super::*; @@ -519,4 +606,144 @@ mod tests { "Expected DNS resolution failure, got: {err}" ); } + + // --- db_first_* helper tests --- + + #[test] + fn db_first_or_default_prefers_settings_over_env() { + let _guard = lock_env(); + let key = "IRONCLAW_TEST_DB_FIRST_1"; + // SAFETY: under ENV_MUTEX + unsafe { std::env::set_var(key, "from-env") }; + + let result: String = + db_first_or_default(&"from-db".to_string(), &"default".to_string(), key) + .expect("should resolve"); + assert_eq!(result, "from-db", "DB value should win over env"); + + unsafe { std::env::remove_var(key) }; + } + + #[test] + fn db_first_or_default_falls_back_to_env() { + let _guard = lock_env(); + let key = "IRONCLAW_TEST_DB_FIRST_2"; + unsafe { std::env::set_var(key, "from-env") }; + + // settings_val == default_val → treated as "unset" + let result: String = + db_first_or_default(&"default".to_string(), &"default".to_string(), key) + .expect("should resolve"); + assert_eq!( + result, "from-env", + "env should win when settings at default" + ); + + unsafe { std::env::remove_var(key) }; + } + + #[test] + fn db_first_or_default_uses_default_when_neither_set() { + let _guard = lock_env(); + let key = "IRONCLAW_TEST_DB_FIRST_3"; + unsafe { std::env::remove_var(key) }; + + let result: String = + db_first_or_default(&"default".to_string(), &"default".to_string(), key) + .expect("should resolve"); + assert_eq!(result, "default"); + } + + #[test] + fn db_first_bool_prefers_settings() { + let _guard = lock_env(); + let key = "IRONCLAW_TEST_DB_FIRST_BOOL_1"; + unsafe { std::env::set_var(key, "false") }; + + let result = db_first_bool(true, false, key).expect("should resolve"); + assert!(result, "DB true should win over env false"); + + unsafe { std::env::remove_var(key) }; + } + + #[test] + fn db_first_bool_falls_back_to_env() { + let _guard = lock_env(); + let key = "IRONCLAW_TEST_DB_FIRST_BOOL_2"; + unsafe { std::env::set_var(key, "true") }; + + // settings == default → falls back to env + let result = db_first_bool(false, false, key).expect("should resolve"); + assert!(result, "env should win when settings at default"); + + unsafe { std::env::remove_var(key) }; + } + + #[test] + fn db_first_optional_string_prefers_settings() { + let _guard = lock_env(); + let key = "IRONCLAW_TEST_DB_FIRST_OPT_1"; + unsafe { std::env::set_var(key, "from-env") }; + + let val = Some("from-db".to_string()); + let result = db_first_optional_string(&val, key).expect("should resolve"); + assert_eq!(result, Some("from-db".to_string())); + + unsafe { std::env::remove_var(key) }; + } + + #[test] + fn db_first_optional_string_falls_back_to_env() { + let _guard = lock_env(); + let key = "IRONCLAW_TEST_DB_FIRST_OPT_2"; + unsafe { std::env::set_var(key, "from-env") }; + + let result = db_first_optional_string(&None, key).expect("should resolve"); + assert_eq!(result, Some("from-env".to_string())); + + unsafe { std::env::remove_var(key) }; + } + + #[test] + fn db_first_optional_string_empty_treated_as_unset() { + let _guard = lock_env(); + let key = "IRONCLAW_TEST_DB_FIRST_OPT_3"; + unsafe { std::env::set_var(key, "from-env") }; + + let val = Some(String::new()); + let result = db_first_optional_string(&val, key).expect("should resolve"); + assert_eq!( + result, + Some("from-env".to_string()), + "empty string should be treated as unset" + ); + + unsafe { std::env::remove_var(key) }; + } + + #[test] + fn db_first_option_prefers_settings() { + let _guard = lock_env(); + let key = "IRONCLAW_TEST_DB_FIRST_OPT_T_1"; + unsafe { std::env::set_var(key, "99") }; + + let val: Option = Some(42); + let result = db_first_option(&val, key).expect("should resolve"); + assert_eq!(result, Some(42)); + + unsafe { std::env::remove_var(key) }; + } + + #[test] + fn db_first_option_falls_back_to_env() { + let _guard = lock_env(); + let key = "IRONCLAW_TEST_DB_FIRST_OPT_T_2"; + unsafe { std::env::set_var(key, "99") }; + + let val: Option = None; + let result = db_first_option(&val, key).expect("should resolve"); + assert_eq!(result, Some(99)); + + unsafe { std::env::remove_var(key) }; + } } diff --git a/src/config/hygiene.rs b/src/config/hygiene.rs index b510933a..f408de45 100644 --- a/src/config/hygiene.rs +++ b/src/config/hygiene.rs @@ -1,6 +1,7 @@ use crate::bootstrap::ironclaw_base_dir; -use crate::config::helpers::{parse_bool_env, parse_optional_env}; +use crate::config::helpers::{db_first_bool, db_first_or_default}; use crate::error::ConfigError; +use crate::settings::Settings; /// Memory hygiene configuration. /// @@ -30,15 +31,27 @@ impl Default for HygieneConfig { } impl HygieneConfig { - pub(crate) fn resolve() -> Result { + pub(crate) fn resolve(settings: &Settings) -> Result { + let defaults = crate::settings::HygieneSettings::default(); + let hs = &settings.hygiene; + Ok(Self { - enabled: parse_bool_env("MEMORY_HYGIENE_ENABLED", true)?, - daily_retention_days: parse_optional_env("MEMORY_HYGIENE_DAILY_RETENTION_DAYS", 30)?, - conversation_retention_days: parse_optional_env( + enabled: db_first_bool(hs.enabled, defaults.enabled, "MEMORY_HYGIENE_ENABLED")?, + daily_retention_days: db_first_or_default( + &hs.daily_retention_days, + &defaults.daily_retention_days, + "MEMORY_HYGIENE_DAILY_RETENTION_DAYS", + )?, + conversation_retention_days: db_first_or_default( + &hs.conversation_retention_days, + &defaults.conversation_retention_days, "MEMORY_HYGIENE_CONVERSATION_RETENTION_DAYS", - 7, + )?, + cadence_hours: db_first_or_default( + &hs.cadence_hours, + &defaults.cadence_hours, + "MEMORY_HYGIENE_CADENCE_HOURS", )?, - cadence_hours: parse_optional_env("MEMORY_HYGIENE_CADENCE_HOURS", 12)?, }) } diff --git a/src/config/mod.rs b/src/config/mod.rs index 241eacb8..5310758e 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -1,10 +1,12 @@ //! Configuration for IronClaw. //! -//! Settings are loaded from env vars, the DB settings table, TOML config, -//! and built-in defaults. Priority varies by subsystem: +//! Settings are loaded with priority: **DB > env > TOML > default**. //! -//! - **LLM settings** (backend, model, api_key, base_url): DB > env > default -//! - **Most other settings** (agent, channels, tunnel, …): env > DB > default +//! Exceptions: +//! - Bootstrap configs (database, secrets): env-only (DB not yet available) +//! - Security-sensitive fields (allow_local_tools, allow_full_access, +//! cost limits, auth tokens): env-only +//! - API keys: env/secrets store only //! //! `DATABASE_URL` lives in `~/.ironclaw/.env` (loaded via dotenvy early //! in startup). @@ -190,9 +192,9 @@ impl Config { /// Load configuration from environment variables and the database. /// - /// TOML is loaded first as a base, then DB values are merged on top - /// (DB wins over TOML). Individual subsystem resolvers then apply - /// their own env-vs-DB priority — see module docs for details. + /// Priority: DB > env > TOML > default. TOML is loaded first as a + /// base, then DB values are merged on top. Subsystem resolvers check + /// DB-backed settings before env vars (except bootstrap/security fields). pub async fn from_db( store: &(dyn crate::db::SettingsStore + Sync), user_id: &str, @@ -202,9 +204,8 @@ impl Config { /// Load from DB with an optional TOML config file overlay. /// - /// TOML is loaded first as a base, then DB values are merged on top - /// (DB wins over TOML). Per-subsystem resolvers then decide whether - /// env vars or DB values take final precedence — see module docs. + /// Priority: DB > env > TOML > default. TOML is loaded as the base, + /// then DB values are merged on top. See module docs for exceptions. pub async fn from_db_with_toml( store: &(dyn crate::db::SettingsStore + Sync), user_id: &str, @@ -365,13 +366,13 @@ impl Config { secrets: SecretsConfig::resolve().await?, builder: BuilderModeConfig::resolve(settings)?, heartbeat: HeartbeatConfig::resolve(settings)?, - hygiene: HygieneConfig::resolve()?, - routines: RoutineConfig::resolve()?, + hygiene: HygieneConfig::resolve(settings)?, + routines: RoutineConfig::resolve(settings)?, sandbox: SandboxModeConfig::resolve(settings)?, claude_code: ClaudeCodeConfig::resolve(settings)?, - skills: SkillsConfig::resolve()?, + skills: SkillsConfig::resolve(settings)?, transcription: TranscriptionConfig::resolve(settings)?, - search: WorkspaceSearchConfig::resolve()?, + search: WorkspaceSearchConfig::resolve(settings)?, workspace, observability: crate::observability::ObservabilityConfig { backend: std::env::var("OBSERVABILITY_BACKEND").unwrap_or_else(|_| "none".into()), diff --git a/src/config/routines.rs b/src/config/routines.rs index c82aa8b5..d28c2f50 100644 --- a/src/config/routines.rs +++ b/src/config/routines.rs @@ -1,5 +1,6 @@ -use crate::config::helpers::{parse_bool_env, parse_optional_env}; +use crate::config::helpers::{db_first_bool, db_first_or_default}; use crate::error::ConfigError; +use crate::settings::Settings; /// Routines configuration. #[derive(Debug, Clone)] @@ -35,15 +36,42 @@ impl Default for RoutineConfig { } impl RoutineConfig { - pub(crate) fn resolve() -> Result { - let max_iterations: u32 = parse_optional_env("ROUTINES_LIGHTWEIGHT_MAX_ITERATIONS", 3)?; + pub(crate) fn resolve(settings: &Settings) -> Result { + let defaults = crate::settings::RoutineSettings::default(); + let rs = &settings.routines; + + let max_iterations: u32 = db_first_or_default( + &rs.lightweight_max_iterations, + &defaults.lightweight_max_iterations, + "ROUTINES_LIGHTWEIGHT_MAX_ITERATIONS", + )?; Ok(Self { - enabled: parse_bool_env("ROUTINES_ENABLED", true)?, - cron_check_interval_secs: parse_optional_env("ROUTINES_CRON_INTERVAL", 15)?, - max_concurrent_routines: parse_optional_env("ROUTINES_MAX_CONCURRENT", 10)?, - default_cooldown_secs: parse_optional_env("ROUTINES_DEFAULT_COOLDOWN", 300)?, - max_lightweight_tokens: parse_optional_env("ROUTINES_MAX_TOKENS", 4096)?, - lightweight_tools_enabled: parse_bool_env("ROUTINES_LIGHTWEIGHT_TOOLS", true)?, + enabled: db_first_bool(rs.enabled, defaults.enabled, "ROUTINES_ENABLED")?, + cron_check_interval_secs: db_first_or_default( + &rs.cron_check_interval_secs, + &defaults.cron_check_interval_secs, + "ROUTINES_CRON_INTERVAL", + )?, + max_concurrent_routines: db_first_or_default( + &rs.max_concurrent_routines, + &defaults.max_concurrent_routines, + "ROUTINES_MAX_CONCURRENT", + )?, + default_cooldown_secs: db_first_or_default( + &rs.default_cooldown_secs, + &defaults.default_cooldown_secs, + "ROUTINES_DEFAULT_COOLDOWN", + )?, + max_lightweight_tokens: db_first_or_default( + &rs.max_lightweight_tokens, + &defaults.max_lightweight_tokens, + "ROUTINES_MAX_TOKENS", + )?, + lightweight_tools_enabled: db_first_bool( + rs.lightweight_tools_enabled, + defaults.lightweight_tools_enabled, + "ROUTINES_LIGHTWEIGHT_TOOLS", + )?, lightweight_max_iterations: max_iterations.min(5), // cap at 5 }) } diff --git a/src/config/safety.rs b/src/config/safety.rs index edeceee0..953256d8 100644 --- a/src/config/safety.rs +++ b/src/config/safety.rs @@ -1,4 +1,4 @@ -use crate::config::helpers::{parse_bool_env, parse_optional_env}; +use crate::config::helpers::{db_first_bool, db_first_or_default}; use crate::error::ConfigError; pub use ironclaw_safety::SafetyConfig; @@ -7,11 +7,17 @@ pub(crate) fn resolve_safety_config( settings: &crate::settings::Settings, ) -> Result { let ss = &settings.safety; + let defaults = crate::settings::SafetySettings::default(); Ok(SafetyConfig { - max_output_length: parse_optional_env("SAFETY_MAX_OUTPUT_LENGTH", ss.max_output_length)?, - injection_check_enabled: parse_bool_env( - "SAFETY_INJECTION_CHECK_ENABLED", + max_output_length: db_first_or_default( + &ss.max_output_length, + &defaults.max_output_length, + "SAFETY_MAX_OUTPUT_LENGTH", + )?, + injection_check_enabled: db_first_bool( ss.injection_check_enabled, + defaults.injection_check_enabled, + "SAFETY_INJECTION_CHECK_ENABLED", )?, }) } @@ -35,9 +41,10 @@ mod tests { } #[test] - fn env_overrides_settings() { + fn db_settings_override_env() { let _guard = lock_env(); let mut settings = Settings::default(); + // Non-default value simulates an explicit DB/TOML setting settings.safety.max_output_length = 42; // SAFETY: Under ENV_MUTEX, no concurrent env access. @@ -45,6 +52,25 @@ mod tests { let cfg = resolve_safety_config(&settings).expect("resolve"); unsafe { std::env::remove_var("SAFETY_MAX_OUTPUT_LENGTH") }; + // DB value (42) wins over env value (7) + assert_eq!(cfg.max_output_length, 42); + } + + #[test] + fn env_used_when_no_db_setting() { + let _guard = lock_env(); + // Settings left at defaults — no explicit DB/TOML override + let settings = Settings::default(); + + // SAFETY: Under ENV_MUTEX, no concurrent env access. + unsafe { std::env::set_var("SAFETY_MAX_OUTPUT_LENGTH", "7") }; + unsafe { std::env::set_var("SAFETY_INJECTION_CHECK_ENABLED", "false") }; + let cfg = resolve_safety_config(&settings).expect("resolve"); + unsafe { std::env::remove_var("SAFETY_MAX_OUTPUT_LENGTH") }; + unsafe { std::env::remove_var("SAFETY_INJECTION_CHECK_ENABLED") }; + + // Env values win when settings are at their defaults assert_eq!(cfg.max_output_length, 7); + assert!(!cfg.injection_check_enabled); } } diff --git a/src/config/sandbox.rs b/src/config/sandbox.rs index 01a8c327..7f5adb7a 100644 --- a/src/config/sandbox.rs +++ b/src/config/sandbox.rs @@ -1,4 +1,7 @@ -use crate::config::helpers::{optional_env, parse_bool_env, parse_optional_env, parse_string_env}; +use crate::config::helpers::{ + db_first_bool, db_first_or_default, optional_env, parse_bool_env, parse_optional_env, + parse_string_env, +}; use crate::error::ConfigError; /// Docker sandbox configuration. @@ -54,16 +57,16 @@ impl Default for SandboxModeConfig { impl SandboxModeConfig { pub(crate) fn resolve(settings: &crate::settings::Settings) -> Result { let ss = &settings.sandbox; + let defaults = crate::settings::SandboxSettings::default(); - let extra_domains = optional_env("SANDBOX_EXTRA_DOMAINS")? - .map(|s| s.split(',').map(|d| d.trim().to_string()).collect()) - .unwrap_or_else(|| { - if ss.extra_allowed_domains.is_empty() { - Vec::new() - } else { - ss.extra_allowed_domains.clone() - } - }); + // extra_allowed_domains: DB wins if non-empty, otherwise env, otherwise empty. + let extra_domains = if !ss.extra_allowed_domains.is_empty() { + ss.extra_allowed_domains.clone() + } else { + optional_env("SANDBOX_EXTRA_DOMAINS")? + .map(|s| s.split(',').map(|d| d.trim().to_string()).collect()) + .unwrap_or_default() + }; // reaper/orphan fields have no Settings counterpart — env > default only. let reaper_interval_secs: u64 = parse_optional_env("SANDBOX_REAPER_INTERVAL_SECS", 300)?; @@ -85,15 +88,31 @@ impl SandboxModeConfig { } Ok(Self { - enabled: parse_bool_env("SANDBOX_ENABLED", ss.enabled)?, - policy: parse_string_env("SANDBOX_POLICY", ss.policy.clone())?, - // allow_full_access has no Settings counterpart — env > default only. + enabled: db_first_bool(ss.enabled, defaults.enabled, "SANDBOX_ENABLED")?, + policy: db_first_or_default(&ss.policy, &defaults.policy, "SANDBOX_POLICY")?, + // allow_full_access has no Settings counterpart — env > default only (security). allow_full_access: parse_bool_env("SANDBOX_ALLOW_FULL_ACCESS", false)?, - timeout_secs: parse_optional_env("SANDBOX_TIMEOUT_SECS", ss.timeout_secs)?, - memory_limit_mb: parse_optional_env("SANDBOX_MEMORY_LIMIT_MB", ss.memory_limit_mb)?, - cpu_shares: parse_optional_env("SANDBOX_CPU_SHARES", ss.cpu_shares)?, - image: parse_string_env("SANDBOX_IMAGE", ss.image.clone())?, - auto_pull_image: parse_bool_env("SANDBOX_AUTO_PULL", ss.auto_pull_image)?, + timeout_secs: db_first_or_default( + &ss.timeout_secs, + &defaults.timeout_secs, + "SANDBOX_TIMEOUT_SECS", + )?, + memory_limit_mb: db_first_or_default( + &ss.memory_limit_mb, + &defaults.memory_limit_mb, + "SANDBOX_MEMORY_LIMIT_MB", + )?, + cpu_shares: db_first_or_default( + &ss.cpu_shares, + &defaults.cpu_shares, + "SANDBOX_CPU_SHARES", + )?, + image: db_first_or_default(&ss.image, &defaults.image, "SANDBOX_IMAGE")?, + auto_pull_image: db_first_bool( + ss.auto_pull_image, + defaults.auto_pull_image, + "SANDBOX_AUTO_PULL", + )?, extra_allowed_domains: extra_domains, reaper_interval_secs, orphan_threshold_secs, @@ -264,19 +283,28 @@ impl ClaudeCodeConfig { } pub(crate) fn resolve(settings: &crate::settings::Settings) -> Result { + let ss = &settings.sandbox; let defaults = Self::default(); Ok(Self { - // Use settings.sandbox.claude_code_enabled as fallback (written by setup wizard). - enabled: parse_bool_env("CLAUDE_CODE_ENABLED", settings.sandbox.claude_code_enabled)?, + enabled: db_first_bool( + ss.claude_code_enabled, + defaults.enabled, + "CLAUDE_CODE_ENABLED", + )?, + // config_dir has no Settings counterpart — env > default only. config_dir: optional_env("CLAUDE_CONFIG_DIR")? .map(std::path::PathBuf::from) .unwrap_or(defaults.config_dir), + // model has no Settings counterpart — env > default only. model: parse_string_env("CLAUDE_CODE_MODEL", defaults.model)?, + // max_turns has no Settings counterpart — env > default only. max_turns: parse_optional_env("CLAUDE_CODE_MAX_TURNS", defaults.max_turns)?, + // memory_limit_mb has no Settings counterpart — env > default only. memory_limit_mb: parse_optional_env( "CLAUDE_CODE_MEMORY_LIMIT_MB", defaults.memory_limit_mb, )?, + // allowed_tools has no Settings counterpart — env > default only. allowed_tools: optional_env("CLAUDE_CODE_ALLOWED_TOOLS")? .map(|s| { s.split(',') @@ -607,7 +635,7 @@ mod tests { } #[test] - fn sandbox_env_overrides_settings() { + fn sandbox_db_settings_override_env() { let _guard = crate::config::helpers::lock_env(); let mut settings = crate::settings::Settings::default(); settings.sandbox.timeout_secs = 999; @@ -617,7 +645,26 @@ mod tests { let cfg = SandboxModeConfig::resolve(&settings).expect("resolve"); unsafe { std::env::remove_var("SANDBOX_TIMEOUT_SECS") }; - assert_eq!(cfg.timeout_secs, 5); + // DB value (999) wins over env (5) under DB-first priority. + assert_eq!(cfg.timeout_secs, 999); + } + + #[test] + fn sandbox_env_used_when_no_db_setting() { + let _guard = crate::config::helpers::lock_env(); + // Default settings — all fields at their defaults, so DB is "unset". + let settings = crate::settings::Settings::default(); + + // SAFETY: Under ENV_MUTEX, no concurrent env access. + unsafe { std::env::set_var("SANDBOX_TIMEOUT_SECS", "42") }; + unsafe { std::env::set_var("SANDBOX_MEMORY_LIMIT_MB", "512") }; + let cfg = SandboxModeConfig::resolve(&settings).expect("resolve"); + unsafe { std::env::remove_var("SANDBOX_TIMEOUT_SECS") }; + unsafe { std::env::remove_var("SANDBOX_MEMORY_LIMIT_MB") }; + + // Env values win when settings are at their defaults. + assert_eq!(cfg.timeout_secs, 42); + assert_eq!(cfg.memory_limit_mb, 512); } // ── ClaudeCodeConfig settings fallback tests ──────────────────── @@ -641,7 +688,7 @@ mod tests { } #[test] - fn claude_code_env_overrides_settings() { + fn claude_code_db_settings_override_env() { let _guard = crate::config::helpers::lock_env(); let mut settings = crate::settings::Settings::default(); settings.sandbox.claude_code_enabled = true; @@ -651,7 +698,8 @@ mod tests { let cfg = ClaudeCodeConfig::resolve(&settings).expect("resolve"); unsafe { std::env::remove_var("CLAUDE_CODE_ENABLED") }; - assert!(!cfg.enabled); + // DB value (true) wins over env (false) under DB-first priority. + assert!(cfg.enabled); } #[test] diff --git a/src/config/search.rs b/src/config/search.rs index e6b663cf..bc101f7a 100644 --- a/src/config/search.rs +++ b/src/config/search.rs @@ -1,5 +1,6 @@ -use crate::config::helpers::{optional_env, parse_optional_env}; +use crate::config::helpers::{db_first_or_default, parse_optional_env}; use crate::error::ConfigError; +use crate::settings::Settings; use crate::workspace::FusionStrategy; /// Workspace search configuration resolved from environment variables. @@ -33,30 +34,48 @@ impl Default for WorkspaceSearchConfig { } impl WorkspaceSearchConfig { - pub(crate) fn resolve() -> Result { - let fusion_strategy = match optional_env("SEARCH_FUSION_STRATEGY")? { - Some(s) => match s.to_lowercase().as_str() { - "rrf" => FusionStrategy::Rrf, - "weighted" => FusionStrategy::WeightedScore, - other => { - return Err(ConfigError::InvalidValue { - key: "SEARCH_FUSION_STRATEGY".to_string(), - message: format!("must be 'rrf' or 'weighted', got '{other}'"), - }); - } - }, - None => FusionStrategy::default(), + pub(crate) fn resolve(settings: &Settings) -> Result { + let defaults = crate::settings::SearchSettings::default(); + let ss = &settings.search; + + // Resolve fusion_strategy string via DB-first, then parse into enum. + let strategy_str = db_first_or_default( + &ss.fusion_strategy, + &defaults.fusion_strategy, + "SEARCH_FUSION_STRATEGY", + )?; + let fusion_strategy = match strategy_str.to_lowercase().as_str() { + "rrf" => FusionStrategy::Rrf, + "weighted" => FusionStrategy::WeightedScore, + other => { + return Err(ConfigError::InvalidValue { + key: "SEARCH_FUSION_STRATEGY".to_string(), + message: format!("must be 'rrf' or 'weighted', got '{other}'"), + }); + } }; - let rrf_k = parse_optional_env("SEARCH_RRF_K", 60u32)?; + let rrf_k = db_first_or_default(&ss.rrf_k, &defaults.rrf_k, "SEARCH_RRF_K")?; // Per-strategy weight defaults: RRF uses 0.5/0.5, weighted uses 0.3/0.7 (vector-biased). let (default_fts, default_vec) = match fusion_strategy { FusionStrategy::Rrf => (0.5f32, 0.5f32), FusionStrategy::WeightedScore => (0.3f32, 0.7f32), }; - let fts_weight = parse_optional_env("SEARCH_FTS_WEIGHT", default_fts)?; - let vector_weight = parse_optional_env("SEARCH_VECTOR_WEIGHT", default_vec)?; + + // For weights, we need to check whether the settings value differs from + // the *static* default (0.5) to detect DB overrides. If it does, use it; + // otherwise fall back to env, then per-strategy default. + let fts_weight = if (ss.fts_weight - defaults.fts_weight).abs() > f32::EPSILON { + ss.fts_weight + } else { + parse_optional_env("SEARCH_FTS_WEIGHT", default_fts)? + }; + let vector_weight = if (ss.vector_weight - defaults.vector_weight).abs() > f32::EPSILON { + ss.vector_weight + } else { + parse_optional_env("SEARCH_VECTOR_WEIGHT", default_vec)? + }; if !fts_weight.is_finite() || fts_weight < 0.0 { return Err(ConfigError::InvalidValue { @@ -109,7 +128,8 @@ mod tests { let _guard = lock_env(); clear_search_env(); - let config = WorkspaceSearchConfig::resolve().expect("should resolve"); + let settings = Settings::default(); + let config = WorkspaceSearchConfig::resolve(&settings).expect("should resolve"); assert_eq!(config.fusion_strategy, FusionStrategy::Rrf); assert_eq!(config.rrf_k, 60); assert!((config.fts_weight - 0.5).abs() < 0.001); @@ -117,7 +137,35 @@ mod tests { } #[test] - fn env_overrides() { + fn db_settings_override_env() { + let _guard = lock_env(); + clear_search_env(); + + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::set_var("SEARCH_FUSION_STRATEGY", "rrf"); + std::env::set_var("SEARCH_RRF_K", "30"); + std::env::set_var("SEARCH_FTS_WEIGHT", "0.9"); + std::env::set_var("SEARCH_VECTOR_WEIGHT", "0.1"); + } + + let mut settings = Settings::default(); + settings.search.fusion_strategy = "weighted".to_string(); + settings.search.rrf_k = 42; + settings.search.fts_weight = 0.4; + settings.search.vector_weight = 0.6; + + let config = WorkspaceSearchConfig::resolve(&settings).expect("should resolve"); + assert_eq!(config.fusion_strategy, FusionStrategy::WeightedScore); + assert_eq!(config.rrf_k, 42); + assert!((config.fts_weight - 0.4).abs() < 0.001); + assert!((config.vector_weight - 0.6).abs() < 0.001); + + clear_search_env(); + } + + #[test] + fn env_fallback_when_settings_at_default() { let _guard = lock_env(); clear_search_env(); @@ -129,7 +177,8 @@ mod tests { std::env::set_var("SEARCH_VECTOR_WEIGHT", "0.1"); } - let config = WorkspaceSearchConfig::resolve().expect("should resolve"); + let settings = Settings::default(); + let config = WorkspaceSearchConfig::resolve(&settings).expect("should resolve"); assert_eq!(config.fusion_strategy, FusionStrategy::WeightedScore); assert_eq!(config.rrf_k, 30); assert!((config.fts_weight - 0.9).abs() < 0.001); @@ -148,7 +197,8 @@ mod tests { std::env::set_var("SEARCH_FUSION_STRATEGY", "bm25"); } - let result = WorkspaceSearchConfig::resolve(); + let settings = Settings::default(); + let result = WorkspaceSearchConfig::resolve(&settings); assert!(result.is_err()); clear_search_env(); @@ -164,7 +214,8 @@ mod tests { std::env::set_var("SEARCH_FUSION_STRATEGY", "weighted"); } - let config = WorkspaceSearchConfig::resolve().expect("should resolve"); + let settings = Settings::default(); + let config = WorkspaceSearchConfig::resolve(&settings).expect("should resolve"); assert_eq!(config.fusion_strategy, FusionStrategy::WeightedScore); // Weighted mode should default to 0.3 FTS / 0.7 vector assert!((config.fts_weight - 0.3).abs() < 0.001); @@ -185,7 +236,8 @@ mod tests { std::env::set_var("SEARCH_VECTOR_WEIGHT", "0.0"); } - let result = WorkspaceSearchConfig::resolve(); + let settings = Settings::default(); + let result = WorkspaceSearchConfig::resolve(&settings); assert!(result.is_err()); clear_search_env(); @@ -203,7 +255,8 @@ mod tests { } // RRF ignores weights, so both=0 is fine - let config = WorkspaceSearchConfig::resolve().expect("should resolve"); + let settings = Settings::default(); + let config = WorkspaceSearchConfig::resolve(&settings).expect("should resolve"); assert_eq!(config.fusion_strategy, FusionStrategy::Rrf); clear_search_env(); diff --git a/src/config/skills.rs b/src/config/skills.rs index 97970784..e1a7b8d5 100644 --- a/src/config/skills.rs +++ b/src/config/skills.rs @@ -1,8 +1,9 @@ use std::path::PathBuf; use crate::bootstrap::ironclaw_base_dir; -use crate::config::helpers::{optional_env, parse_bool_env, parse_optional_env}; +use crate::config::helpers::{db_first_bool, db_first_or_default, optional_env}; use crate::error::ConfigError; +use crate::settings::Settings; /// Skills system configuration. #[derive(Debug, Clone)] @@ -44,17 +45,29 @@ fn default_installed_skills_dir() -> PathBuf { } impl SkillsConfig { - pub(crate) fn resolve() -> Result { + pub(crate) fn resolve(settings: &Settings) -> Result { + let defaults = crate::settings::SkillsSettings::default(); + let ss = &settings.skills; + Ok(Self { - enabled: parse_bool_env("SKILLS_ENABLED", true)?, + enabled: db_first_bool(ss.enabled, defaults.enabled, "SKILLS_ENABLED")?, + // local_dir and installed_dir are env-only (filesystem paths, no settings counterpart) local_dir: optional_env("SKILLS_DIR")? .map(PathBuf::from) .unwrap_or_else(default_skills_dir), installed_dir: optional_env("SKILLS_INSTALLED_DIR")? .map(PathBuf::from) .unwrap_or_else(default_installed_skills_dir), - max_active_skills: parse_optional_env("SKILLS_MAX_ACTIVE", 3)?, - max_context_tokens: parse_optional_env("SKILLS_MAX_CONTEXT_TOKENS", 4000)?, + max_active_skills: db_first_or_default( + &ss.max_active_skills, + &defaults.max_active_skills, + "SKILLS_MAX_ACTIVE", + )?, + max_context_tokens: db_first_or_default( + &ss.max_context_tokens, + &defaults.max_context_tokens, + "SKILLS_MAX_CONTEXT_TOKENS", + )?, }) } } diff --git a/src/config/transcription.rs b/src/config/transcription.rs index 191d2a02..c066ffbf 100644 --- a/src/config/transcription.rs +++ b/src/config/transcription.rs @@ -1,6 +1,6 @@ use secrecy::SecretString; -use crate::config::helpers::{optional_env, parse_bool_env, validate_base_url}; +use crate::config::helpers::{db_first_bool, optional_env, validate_base_url}; use crate::error::ConfigError; use crate::settings::Settings; @@ -39,10 +39,8 @@ impl Default for TranscriptionConfig { impl TranscriptionConfig { pub(crate) fn resolve(settings: &Settings) -> Result { - let enabled = parse_bool_env( - "TRANSCRIPTION_ENABLED", - settings.transcription.as_ref().is_some_and(|t| t.enabled), - )?; + let settings_enabled = settings.transcription.as_ref().is_some_and(|t| t.enabled); + let enabled = db_first_bool(settings_enabled, false, "TRANSCRIPTION_ENABLED")?; let provider = optional_env("TRANSCRIPTION_PROVIDER")?.unwrap_or_else(|| "openai".to_string()); diff --git a/src/config/tunnel.rs b/src/config/tunnel.rs index 1481a773..87ec27a5 100644 --- a/src/config/tunnel.rs +++ b/src/config/tunnel.rs @@ -1,12 +1,14 @@ -use crate::config::helpers::optional_env; +use crate::config::helpers::{db_first_bool, db_first_optional_string}; use crate::error::ConfigError; -use crate::settings::Settings; +use crate::settings::{Settings, TunnelSettings}; /// Tunnel configuration for exposing the agent to the internet. /// /// Used by channels and tools that need public webhook endpoints. /// The tunnel URL is shared across all channels (Telegram, Slack, etc.). /// +/// Resolution priority: DB/settings > env var > default. +/// /// Two modes: /// - **Static URL** (`TUNNEL_URL`): set the public URL directly (manual tunnel) /// - **Managed provider** (`TUNNEL_PROVIDER`): lifecycle-managed tunnel process @@ -25,8 +27,10 @@ pub struct TunnelConfig { impl TunnelConfig { pub(crate) fn resolve(settings: &Settings) -> Result { - let public_url = optional_env("TUNNEL_URL")? - .or_else(|| settings.tunnel.public_url.clone().filter(|s| !s.is_empty())); + let defaults = TunnelSettings::default(); + + // Priority: DB/settings > env > default. + let public_url = db_first_optional_string(&settings.tunnel.public_url, "TUNNEL_URL")?; if let Some(ref url) = public_url && !url.starts_with("https://") @@ -38,9 +42,8 @@ impl TunnelConfig { } // Resolve managed tunnel provider config. - // Priority: env var > settings > default (none). - let provider_name = optional_env("TUNNEL_PROVIDER")? - .or_else(|| settings.tunnel.provider.clone()) + // Priority: DB/settings > env > default (none). + let provider_name = db_first_optional_string(&settings.tunnel.provider, "TUNNEL_PROVIDER")? .unwrap_or_default(); let provider = if provider_name.is_empty() || provider_name == "none" { @@ -48,38 +51,48 @@ impl TunnelConfig { } else { Some(crate::tunnel::TunnelProviderConfig { provider: provider_name.clone(), - cloudflare: optional_env("TUNNEL_CF_TOKEN")? - .or_else(|| settings.tunnel.cf_token.clone()) + cloudflare: db_first_optional_string(&settings.tunnel.cf_token, "TUNNEL_CF_TOKEN")? .map(|token| crate::tunnel::CloudflareTunnelConfig { token }), tailscale: Some(crate::tunnel::TailscaleTunnelConfig { - funnel: optional_env("TUNNEL_TS_FUNNEL")? - .map(|s| s == "true" || s == "1") - .unwrap_or(settings.tunnel.ts_funnel), - hostname: optional_env("TUNNEL_TS_HOSTNAME")? - .or_else(|| settings.tunnel.ts_hostname.clone()), + funnel: db_first_bool( + settings.tunnel.ts_funnel, + defaults.ts_funnel, + "TUNNEL_TS_FUNNEL", + )?, + hostname: db_first_optional_string( + &settings.tunnel.ts_hostname, + "TUNNEL_TS_HOSTNAME", + )?, }), ngrok: { - let ngrok_domain = optional_env("TUNNEL_NGROK_DOMAIN")? - .or_else(|| settings.tunnel.ngrok_domain.clone()); - optional_env("TUNNEL_NGROK_TOKEN")? - .or_else(|| settings.tunnel.ngrok_token.clone()) + let ngrok_domain = db_first_optional_string( + &settings.tunnel.ngrok_domain, + "TUNNEL_NGROK_DOMAIN", + )?; + db_first_optional_string(&settings.tunnel.ngrok_token, "TUNNEL_NGROK_TOKEN")? .map(|auth_token| crate::tunnel::NgrokTunnelConfig { auth_token, domain: ngrok_domain, }) }, custom: { - let health_url = optional_env("TUNNEL_CUSTOM_HEALTH_URL")? - .or_else(|| settings.tunnel.custom_health_url.clone()); - let url_pattern = optional_env("TUNNEL_CUSTOM_URL_PATTERN")? - .or_else(|| settings.tunnel.custom_url_pattern.clone()); - optional_env("TUNNEL_CUSTOM_COMMAND")? - .or_else(|| settings.tunnel.custom_command.clone()) - .map(|start_command| crate::tunnel::CustomTunnelConfig { - start_command, - health_url, - url_pattern, - }) + let health_url = db_first_optional_string( + &settings.tunnel.custom_health_url, + "TUNNEL_CUSTOM_HEALTH_URL", + )?; + let url_pattern = db_first_optional_string( + &settings.tunnel.custom_url_pattern, + "TUNNEL_CUSTOM_URL_PATTERN", + )?; + db_first_optional_string( + &settings.tunnel.custom_command, + "TUNNEL_CUSTOM_COMMAND", + )? + .map(|start_command| crate::tunnel::CustomTunnelConfig { + start_command, + health_url, + url_pattern, + }) }, }) }; diff --git a/src/config/wasm.rs b/src/config/wasm.rs index 4c494a38..7db3160a 100644 --- a/src/config/wasm.rs +++ b/src/config/wasm.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; use std::time::Duration; use crate::bootstrap::ironclaw_base_dir; -use crate::config::helpers::{optional_env, parse_bool_env, parse_optional_env}; +use crate::config::helpers::{db_first_bool, db_first_or_default, optional_env}; use crate::error::ConfigError; /// WASM sandbox configuration. @@ -46,28 +46,41 @@ fn default_tools_dir() -> PathBuf { impl WasmConfig { pub(crate) fn resolve(settings: &crate::settings::Settings) -> Result { let ws = &settings.wasm; + let defaults = crate::settings::WasmSettings::default(); Ok(Self { - enabled: parse_bool_env("WASM_ENABLED", ws.enabled)?, - tools_dir: optional_env("WASM_TOOLS_DIR")? - .map(PathBuf::from) - .or_else(|| ws.tools_dir.clone()) - .unwrap_or_else(default_tools_dir), - default_memory_limit: parse_optional_env( + enabled: db_first_bool(ws.enabled, defaults.enabled, "WASM_ENABLED")?, + tools_dir: if let Some(ref dir) = ws.tools_dir { + dir.clone() + } else { + optional_env("WASM_TOOLS_DIR")? + .map(PathBuf::from) + .unwrap_or_else(default_tools_dir) + }, + default_memory_limit: db_first_or_default( + &ws.default_memory_limit, + &defaults.default_memory_limit, "WASM_DEFAULT_MEMORY_LIMIT", - ws.default_memory_limit, )?, - default_timeout_secs: parse_optional_env( + default_timeout_secs: db_first_or_default( + &ws.default_timeout_secs, + &defaults.default_timeout_secs, "WASM_DEFAULT_TIMEOUT_SECS", - ws.default_timeout_secs, )?, - default_fuel_limit: parse_optional_env( + default_fuel_limit: db_first_or_default( + &ws.default_fuel_limit, + &defaults.default_fuel_limit, "WASM_DEFAULT_FUEL_LIMIT", - ws.default_fuel_limit, )?, - cache_compiled: parse_bool_env("WASM_CACHE_COMPILED", ws.cache_compiled)?, - cache_dir: optional_env("WASM_CACHE_DIR")? - .map(PathBuf::from) - .or_else(|| ws.cache_dir.clone()), + cache_compiled: db_first_bool( + ws.cache_compiled, + defaults.cache_compiled, + "WASM_CACHE_COMPILED", + )?, + cache_dir: if let Some(ref dir) = ws.cache_dir { + Some(dir.clone()) + } else { + optional_env("WASM_CACHE_DIR")?.map(PathBuf::from) + }, }) } @@ -111,7 +124,7 @@ mod tests { } #[test] - fn env_overrides_settings() { + fn db_settings_override_env() { let _guard = lock_env(); let mut settings = Settings::default(); settings.wasm.default_fuel_limit = 42; @@ -121,6 +134,19 @@ mod tests { let cfg = WasmConfig::resolve(&settings).expect("resolve"); unsafe { std::env::remove_var("WASM_DEFAULT_FUEL_LIMIT") }; + assert_eq!(cfg.default_fuel_limit, 42); + } + + #[test] + fn env_used_when_no_db_setting() { + let _guard = lock_env(); + let settings = Settings::default(); + + // SAFETY: Under ENV_MUTEX, no concurrent env access. + unsafe { std::env::set_var("WASM_DEFAULT_FUEL_LIMIT", "7") }; + let cfg = WasmConfig::resolve(&settings).expect("resolve"); + unsafe { std::env::remove_var("WASM_DEFAULT_FUEL_LIMIT") }; + assert_eq!(cfg.default_fuel_limit, 7); } } diff --git a/src/settings.rs b/src/settings.rs index 46390f9d..9523785b 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -191,6 +191,22 @@ pub struct Settings { #[serde(default)] pub builder: BuilderSettings, + /// Routine scheduling and execution configuration. + #[serde(default)] + pub routines: RoutineSettings, + + /// Skills system configuration. + #[serde(default)] + pub skills: SkillsSettings, + + /// Memory hygiene configuration. + #[serde(default)] + pub hygiene: HygieneSettings, + + /// Workspace search fusion configuration. + #[serde(default)] + pub search: SearchSettings, + /// Transcription configuration. #[serde(default)] pub transcription: Option, @@ -786,6 +802,196 @@ impl Default for BuilderSettings { } } +/// Routine scheduling and execution configuration. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct RoutineSettings { + /// Whether the routines system is enabled. + #[serde(default = "default_true")] + pub enabled: bool, + + /// How often (seconds) to poll for cron routines that need firing. + #[serde(default = "default_routine_cron_interval")] + pub cron_check_interval_secs: u64, + + /// Max routines executing concurrently. + #[serde(default = "default_routine_max_concurrent")] + pub max_concurrent_routines: usize, + + /// Default cooldown between fires (seconds). + #[serde(default = "default_routine_cooldown")] + pub default_cooldown_secs: u64, + + /// Max output tokens for lightweight routine LLM calls. + #[serde(default = "default_routine_max_tokens")] + pub max_lightweight_tokens: u32, + + /// Enable tool execution in lightweight routines. + #[serde(default = "default_true")] + pub lightweight_tools_enabled: bool, + + /// Max tool iterations for lightweight routines. + #[serde(default = "default_routine_max_iterations")] + pub lightweight_max_iterations: u32, +} + +fn default_routine_cron_interval() -> u64 { + 15 +} + +fn default_routine_max_concurrent() -> usize { + 10 +} + +fn default_routine_cooldown() -> u64 { + 300 +} + +fn default_routine_max_tokens() -> u32 { + 4096 +} + +fn default_routine_max_iterations() -> u32 { + 3 +} + +impl Default for RoutineSettings { + fn default() -> Self { + Self { + enabled: true, + cron_check_interval_secs: default_routine_cron_interval(), + max_concurrent_routines: default_routine_max_concurrent(), + default_cooldown_secs: default_routine_cooldown(), + max_lightweight_tokens: default_routine_max_tokens(), + lightweight_tools_enabled: true, + lightweight_max_iterations: default_routine_max_iterations(), + } + } +} + +/// Skills system configuration. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SkillsSettings { + /// Whether the skills system is enabled. + #[serde(default = "default_true")] + pub enabled: bool, + + /// Maximum number of skills that can be active simultaneously. + #[serde(default = "default_skills_max_active")] + pub max_active_skills: usize, + + /// Maximum total context tokens allocated to skill prompts. + #[serde(default = "default_skills_max_context_tokens")] + pub max_context_tokens: usize, +} + +fn default_skills_max_active() -> usize { + 3 +} + +fn default_skills_max_context_tokens() -> usize { + 4000 +} + +impl Default for SkillsSettings { + fn default() -> Self { + Self { + enabled: true, + max_active_skills: default_skills_max_active(), + max_context_tokens: default_skills_max_context_tokens(), + } + } +} + +/// Memory hygiene configuration. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct HygieneSettings { + /// Whether hygiene is enabled. + #[serde(default = "default_true")] + pub enabled: bool, + + /// Days before `daily/` documents are deleted. + #[serde(default = "default_hygiene_daily_retention")] + pub daily_retention_days: u32, + + /// Days before `conversations/` documents are deleted. + #[serde(default = "default_hygiene_conversation_retention")] + pub conversation_retention_days: u32, + + /// Minimum hours between hygiene passes. + #[serde(default = "default_hygiene_cadence_hours")] + pub cadence_hours: u32, +} + +fn default_hygiene_daily_retention() -> u32 { + 30 +} + +fn default_hygiene_conversation_retention() -> u32 { + 7 +} + +fn default_hygiene_cadence_hours() -> u32 { + 12 +} + +impl Default for HygieneSettings { + fn default() -> Self { + Self { + enabled: true, + daily_retention_days: default_hygiene_daily_retention(), + conversation_retention_days: default_hygiene_conversation_retention(), + cadence_hours: default_hygiene_cadence_hours(), + } + } +} + +/// Workspace search fusion configuration. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SearchSettings { + /// Fusion strategy: "rrf" or "weighted". + #[serde(default = "default_search_fusion_strategy")] + pub fusion_strategy: String, + + /// RRF constant k. + #[serde(default = "default_search_rrf_k")] + pub rrf_k: u32, + + /// FTS weight for fusion. + #[serde(default = "default_search_fts_weight")] + pub fts_weight: f32, + + /// Vector weight for fusion. + #[serde(default = "default_search_vector_weight")] + pub vector_weight: f32, +} + +fn default_search_fusion_strategy() -> String { + "rrf".to_string() +} + +fn default_search_rrf_k() -> u32 { + 60 +} + +fn default_search_fts_weight() -> f32 { + 0.5 +} + +fn default_search_vector_weight() -> f32 { + 0.5 +} + +impl Default for SearchSettings { + fn default() -> Self { + Self { + fusion_strategy: default_search_fusion_strategy(), + rrf_k: default_search_rrf_k(), + fts_weight: default_search_fts_weight(), + vector_weight: default_search_vector_weight(), + } + } +} + /// Transcription pipeline settings. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct TranscriptionSettings { @@ -902,8 +1108,9 @@ impl Settings { let content = format!( "# IronClaw configuration file.\n\ #\n\ - # Priority varies by subsystem. LLM: DB > env > this file > defaults.\n\ - # Most others: env > DB > this file > defaults.\n\ + # Priority: DB settings > env vars > this file > defaults.\n\ + # Exceptions: bootstrap fields (DATABASE_URL, etc.) and\n\ + # security-sensitive fields are env-only.\n\ # Uncomment and edit values to override defaults.\n\ # Run `ironclaw config init` to regenerate this file.\n\ #\n\