fix(config): unify ChannelsConfig resolution to env > settings > default (#1124)

ChannelsConfig::resolve() ignored most ChannelSettings fields, reading exclusively from env vars. This made `config set` ineffective for gateway, HTTP, CLI, and WASM channel settings — a prerequisite blocker for #86 (hot-reload) and CLI management commands. - Add gateway and CLI fields to ChannelSettings with correct defaults - Rewrite resolve() to fall back to settings when env var is unset - Keep strict boolean validation via parse_bool_env for all bool fields - Fix GATEWAY_PORT default divergence (3001 -> 3000) in extension manager - Export DEFAULT_GATEWAY_PORT constant as single source of truth - Add 8 tests: settings fallback, env override, DB roundtrip, invalid bool rejection Part of #1119 (Phase 1: Channels pilot) [skip-regression-check]
2026-08-25 14:53:34 +00:00 · 2026-03-15 05:59:08 +00:00
parent dac420840d
commit e74214dce8
4 changed files with 367 additions and 36 deletions
@@ -91,11 +91,20 @@ pub struct SignalConfig {
 }

 impl ChannelsConfig {
+    /// Resolve channels config following `env > settings > default` for every field.
    pub(crate) fn resolve(settings: &Settings) -> Result<Self, ConfigError> {
-        let http = if optional_env("HTTP_PORT")?.is_some() || optional_env("HTTP_HOST")?.is_some() {
+        let cs = &settings.channels;
+
+        // --- HTTP webhook ---
+        // HTTP is enabled when env vars are set OR settings has it enabled.
+        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 {
            Some(HttpConfig {
-                host: optional_env("HTTP_HOST")?.unwrap_or_else(|| "0.0.0.0".to_string()),
-                port: parse_optional_env("HTTP_PORT", 8080)?,
+                host: optional_env("HTTP_HOST")?
+                    .or_else(|| cs.http_host.clone())
+                    .unwrap_or_else(|| "0.0.0.0".to_string()),
+                port: parse_optional_env("HTTP_PORT", cs.http_port.unwrap_or(8080))?,
                webhook_secret: optional_env("HTTP_WEBHOOK_SECRET")?.map(SecretString::from),
                user_id: optional_env("HTTP_USER_ID")?.unwrap_or_else(|| "http".to_string()),
            })
@@ -103,42 +112,58 @@ impl ChannelsConfig {
            None
        };

-        let gateway_enabled = parse_bool_env("GATEWAY_ENABLED", true)?;
+        // --- Web gateway ---
+        let gateway_enabled = parse_bool_env("GATEWAY_ENABLED", cs.gateway_enabled)?;
        let gateway = if gateway_enabled {
            Some(GatewayConfig {
-                host: optional_env("GATEWAY_HOST")?.unwrap_or_else(|| "127.0.0.1".to_string()),
-                port: parse_optional_env("GATEWAY_PORT", 3000)?,
-                auth_token: optional_env("GATEWAY_AUTH_TOKEN")?,
-                user_id: optional_env("GATEWAY_USER_ID")?.unwrap_or_else(|| "default".to_string()),
+                host: optional_env("GATEWAY_HOST")?
+                    .or_else(|| cs.gateway_host.clone())
+                    .unwrap_or_else(|| "127.0.0.1".to_string()),
+                port: parse_optional_env(
+                    "GATEWAY_PORT",
+                    cs.gateway_port.unwrap_or(DEFAULT_GATEWAY_PORT),
+                )?,
+                auth_token: optional_env("GATEWAY_AUTH_TOKEN")?
+                    .or_else(|| cs.gateway_auth_token.clone()),
+                user_id: optional_env("GATEWAY_USER_ID")?
+                    .or_else(|| cs.gateway_user_id.clone())
+                    .unwrap_or_else(|| "default".to_string()),
            })
        } else {
            None
        };

-        let signal = if let Some(http_url) = optional_env("SIGNAL_HTTP_URL")? {
-            let account = optional_env("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 std::env::var_os("SIGNAL_ALLOW_FROM") {
+        // --- Signal ---
+        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 {
+                    key: "SIGNAL_ACCOUNT".to_string(),
+                    message: "SIGNAL_ACCOUNT is required when Signal is enabled".to_string(),
+                })?;
+            let allow_from_str =
+                optional_env("SIGNAL_ALLOW_FROM")?.or_else(|| cs.signal_allow_from.clone());
+            let allow_from = match allow_from_str {
                None => vec![account.clone()],
-                Some(val) => {
-                    let s = val.to_string_lossy();
-                    s.split(',')
-                        .map(|e| e.trim().to_string())
-                        .filter(|s| !s.is_empty())
-                        .collect()
-                }
+                Some(s) => s
+                    .split(',')
+                    .map(|e| e.trim().to_string())
+                    .filter(|s| !s.is_empty())
+                    .collect(),
            };
-            let dm_policy =
-                optional_env("SIGNAL_DM_POLICY")?.unwrap_or_else(|| "pairing".to_string());
-            let group_policy =
-                optional_env("SIGNAL_GROUP_POLICY")?.unwrap_or_else(|| "allowlist".to_string());
+            let dm_policy = optional_env("SIGNAL_DM_POLICY")?
+                .or_else(|| cs.signal_dm_policy.clone())
+                .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());
            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())
@@ -149,6 +174,7 @@ impl ChannelsConfig {
                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())
@@ -167,9 +193,17 @@ impl ChannelsConfig {
            None
        };

-        let cli_enabled = optional_env("CLI_ENABLED")?
-            .map(|s| s.to_lowercase() != "false" && s != "0")
-            .unwrap_or(true);
+        // --- CLI ---
+        let cli_enabled = parse_bool_env("CLI_ENABLED", cs.cli_enabled)?;
+
+        // --- WASM channels ---
+        let wasm_channels_dir = optional_env("WASM_CHANNELS_DIR")?
+            .map(PathBuf::from)
+            .or_else(|| cs.wasm_channels_dir.clone())
+            .unwrap_or_else(default_channels_dir);
+
+        let wasm_channels_enabled =
+            parse_bool_env("WASM_CHANNELS_ENABLED", cs.wasm_channels_enabled)?;

        Ok(Self {
            cli: CliConfig {
@@ -178,12 +212,10 @@ impl ChannelsConfig {
            http,
            gateway,
            signal,
-            wasm_channels_dir: optional_env("WASM_CHANNELS_DIR")?
-                .map(PathBuf::from)
-                .unwrap_or_else(default_channels_dir),
-            wasm_channels_enabled: parse_bool_env("WASM_CHANNELS_ENABLED", true)?,
+            wasm_channels_dir,
+            wasm_channels_enabled,
            wasm_channel_owner_ids: {
-                let mut ids = settings.channels.wasm_channel_owner_ids.clone();
+                let mut ids = cs.wasm_channel_owner_ids.clone();
                // Backwards compat: TELEGRAM_OWNER_ID env var
                if let Some(id_str) = optional_env("TELEGRAM_OWNER_ID")? {
                    let id: i64 = id_str.parse().map_err(|e: std::num::ParseIntError| {
@@ -200,6 +232,10 @@ impl ChannelsConfig {
    }
 }

+/// Default gateway port — used both in `resolve()` and as the fallback in
+/// other modules that need to construct a gateway URL.
+pub const DEFAULT_GATEWAY_PORT: u16 = 3000;
+
 /// Get the default channels directory (~/.ironclaw/channels/).
 fn default_channels_dir() -> PathBuf {
    ironclaw_base_dir().join("channels")
@@ -362,4 +398,244 @@ mod tests {
            "expected path ending in 'channels', got: {dir:?}"
        );
    }
+
+    #[test]
+    fn default_gateway_port_constant() {
+        assert_eq!(DEFAULT_GATEWAY_PORT, 3000);
+    }
+
+    /// With default settings and no env vars, gateway should use defaults.
+    #[test]
+    fn resolve_gateway_defaults_from_settings() {
+        let _lock = crate::config::helpers::ENV_MUTEX.lock();
+        // Clear env vars that would interfere
+        unsafe {
+            std::env::remove_var("GATEWAY_ENABLED");
+            std::env::remove_var("GATEWAY_HOST");
+            std::env::remove_var("GATEWAY_PORT");
+            std::env::remove_var("GATEWAY_AUTH_TOKEN");
+            std::env::remove_var("GATEWAY_USER_ID");
+            std::env::remove_var("HTTP_PORT");
+            std::env::remove_var("HTTP_HOST");
+            std::env::remove_var("SIGNAL_HTTP_URL");
+            std::env::remove_var("CLI_ENABLED");
+            std::env::remove_var("WASM_CHANNELS_DIR");
+            std::env::remove_var("WASM_CHANNELS_ENABLED");
+            std::env::remove_var("TELEGRAM_OWNER_ID");
+        }
+
+        let settings = crate::settings::Settings::default();
+        let cfg = ChannelsConfig::resolve(&settings).unwrap();
+
+        let gw = cfg.gateway.expect("gateway should be enabled by default");
+        assert_eq!(gw.host, "127.0.0.1");
+        assert_eq!(gw.port, DEFAULT_GATEWAY_PORT);
+        assert!(gw.auth_token.is_none());
+        assert_eq!(gw.user_id, "default");
+    }
+
+    /// Settings values should be used when no env vars are set.
+    #[test]
+    fn resolve_gateway_from_settings() {
+        let _lock = crate::config::helpers::ENV_MUTEX.lock();
+        unsafe {
+            std::env::remove_var("GATEWAY_ENABLED");
+            std::env::remove_var("GATEWAY_HOST");
+            std::env::remove_var("GATEWAY_PORT");
+            std::env::remove_var("GATEWAY_AUTH_TOKEN");
+            std::env::remove_var("GATEWAY_USER_ID");
+            std::env::remove_var("HTTP_PORT");
+            std::env::remove_var("HTTP_HOST");
+            std::env::remove_var("SIGNAL_HTTP_URL");
+            std::env::remove_var("CLI_ENABLED");
+            std::env::remove_var("WASM_CHANNELS_DIR");
+            std::env::remove_var("WASM_CHANNELS_ENABLED");
+            std::env::remove_var("TELEGRAM_OWNER_ID");
+        }
+
+        let mut settings = crate::settings::Settings::default();
+        settings.channels.gateway_port = Some(4000);
+        settings.channels.gateway_host = Some("0.0.0.0".to_string());
+        settings.channels.gateway_auth_token = Some("db-token-123".to_string());
+        settings.channels.gateway_user_id = Some("myuser".to_string());
+
+        let cfg = ChannelsConfig::resolve(&settings).unwrap();
+        let gw = cfg.gateway.expect("gateway should be enabled");
+        assert_eq!(gw.port, 4000);
+        assert_eq!(gw.host, "0.0.0.0");
+        assert_eq!(gw.auth_token.as_deref(), Some("db-token-123"));
+        assert_eq!(gw.user_id, "myuser");
+    }
+
+    /// Env vars should override settings values.
+    #[test]
+    fn resolve_env_overrides_settings() {
+        let _lock = crate::config::helpers::ENV_MUTEX.lock();
+        unsafe {
+            std::env::set_var("GATEWAY_PORT", "5000");
+            std::env::set_var("GATEWAY_HOST", "10.0.0.1");
+            std::env::set_var("GATEWAY_AUTH_TOKEN", "env-token");
+            std::env::remove_var("GATEWAY_ENABLED");
+            std::env::remove_var("GATEWAY_USER_ID");
+            std::env::remove_var("HTTP_PORT");
+            std::env::remove_var("HTTP_HOST");
+            std::env::remove_var("SIGNAL_HTTP_URL");
+            std::env::remove_var("CLI_ENABLED");
+            std::env::remove_var("WASM_CHANNELS_DIR");
+            std::env::remove_var("WASM_CHANNELS_ENABLED");
+            std::env::remove_var("TELEGRAM_OWNER_ID");
+        }
+
+        let mut settings = crate::settings::Settings::default();
+        settings.channels.gateway_port = Some(4000);
+        settings.channels.gateway_host = Some("0.0.0.0".to_string());
+        settings.channels.gateway_auth_token = Some("db-token".to_string());
+
+        let cfg = ChannelsConfig::resolve(&settings).unwrap();
+        let gw = cfg.gateway.expect("gateway should be enabled");
+        assert_eq!(gw.port, 5000, "env should override settings");
+        assert_eq!(gw.host, "10.0.0.1", "env should override settings");
+        assert_eq!(
+            gw.auth_token.as_deref(),
+            Some("env-token"),
+            "env should override settings"
+        );
+
+        // Cleanup
+        unsafe {
+            std::env::remove_var("GATEWAY_PORT");
+            std::env::remove_var("GATEWAY_HOST");
+            std::env::remove_var("GATEWAY_AUTH_TOKEN");
+        }
+    }
+
+    /// CLI enabled should fall back to settings.
+    #[test]
+    fn resolve_cli_enabled_from_settings() {
+        let _lock = crate::config::helpers::ENV_MUTEX.lock();
+        unsafe {
+            std::env::remove_var("CLI_ENABLED");
+            std::env::remove_var("GATEWAY_ENABLED");
+            std::env::remove_var("GATEWAY_HOST");
+            std::env::remove_var("GATEWAY_PORT");
+            std::env::remove_var("GATEWAY_AUTH_TOKEN");
+            std::env::remove_var("GATEWAY_USER_ID");
+            std::env::remove_var("HTTP_PORT");
+            std::env::remove_var("HTTP_HOST");
+            std::env::remove_var("SIGNAL_HTTP_URL");
+            std::env::remove_var("WASM_CHANNELS_DIR");
+            std::env::remove_var("WASM_CHANNELS_ENABLED");
+            std::env::remove_var("TELEGRAM_OWNER_ID");
+        }
+
+        let mut settings = crate::settings::Settings::default();
+        settings.channels.cli_enabled = false;
+
+        let cfg = ChannelsConfig::resolve(&settings).unwrap();
+        assert!(!cfg.cli.enabled, "settings should disable CLI");
+    }
+
+    /// HTTP channel should activate when settings has it enabled.
+    #[test]
+    fn resolve_http_from_settings() {
+        let _lock = crate::config::helpers::ENV_MUTEX.lock();
+        unsafe {
+            std::env::remove_var("HTTP_PORT");
+            std::env::remove_var("HTTP_HOST");
+            std::env::remove_var("HTTP_WEBHOOK_SECRET");
+            std::env::remove_var("HTTP_USER_ID");
+            std::env::remove_var("GATEWAY_ENABLED");
+            std::env::remove_var("GATEWAY_HOST");
+            std::env::remove_var("GATEWAY_PORT");
+            std::env::remove_var("GATEWAY_AUTH_TOKEN");
+            std::env::remove_var("GATEWAY_USER_ID");
+            std::env::remove_var("SIGNAL_HTTP_URL");
+            std::env::remove_var("CLI_ENABLED");
+            std::env::remove_var("WASM_CHANNELS_DIR");
+            std::env::remove_var("WASM_CHANNELS_ENABLED");
+            std::env::remove_var("TELEGRAM_OWNER_ID");
+        }
+
+        let mut settings = crate::settings::Settings::default();
+        settings.channels.http_enabled = true;
+        settings.channels.http_port = Some(9090);
+        settings.channels.http_host = Some("10.0.0.1".to_string());
+
+        let cfg = ChannelsConfig::resolve(&settings).unwrap();
+        let http = cfg.http.expect("HTTP should be enabled from settings");
+        assert_eq!(http.port, 9090);
+        assert_eq!(http.host, "10.0.0.1");
+    }
+
+    /// Settings round-trip through DB map for new gateway fields.
+    #[test]
+    fn settings_gateway_fields_db_roundtrip() {
+        let mut settings = crate::settings::Settings::default();
+        settings.channels.gateway_port = Some(4000);
+        settings.channels.gateway_host = Some("0.0.0.0".to_string());
+        settings.channels.gateway_auth_token = Some("tok-abc".to_string());
+        settings.channels.gateway_user_id = Some("myuser".to_string());
+        settings.channels.cli_enabled = false;
+
+        let map = settings.to_db_map();
+        let restored = crate::settings::Settings::from_db_map(&map);
+
+        assert_eq!(restored.channels.gateway_port, Some(4000));
+        assert_eq!(restored.channels.gateway_host.as_deref(), Some("0.0.0.0"));
+        assert_eq!(
+            restored.channels.gateway_auth_token.as_deref(),
+            Some("tok-abc")
+        );
+        assert_eq!(restored.channels.gateway_user_id.as_deref(), Some("myuser"));
+        assert!(!restored.channels.cli_enabled);
+    }
+
+    /// Invalid boolean env values must produce errors, not silently degrade.
+    #[test]
+    fn resolve_rejects_invalid_bool_env() {
+        let _lock = crate::config::helpers::ENV_MUTEX.lock();
+        let settings = crate::settings::Settings::default();
+
+        // GATEWAY_ENABLED=maybe should error
+        unsafe {
+            std::env::set_var("GATEWAY_ENABLED", "maybe");
+            std::env::remove_var("HTTP_PORT");
+            std::env::remove_var("HTTP_HOST");
+            std::env::remove_var("SIGNAL_HTTP_URL");
+            std::env::remove_var("CLI_ENABLED");
+            std::env::remove_var("WASM_CHANNELS_ENABLED");
+            std::env::remove_var("GATEWAY_PORT");
+            std::env::remove_var("GATEWAY_HOST");
+            std::env::remove_var("GATEWAY_AUTH_TOKEN");
+            std::env::remove_var("GATEWAY_USER_ID");
+            std::env::remove_var("WASM_CHANNELS_DIR");
+            std::env::remove_var("TELEGRAM_OWNER_ID");
+        }
+        let result = ChannelsConfig::resolve(&settings);
+        assert!(result.is_err(), "GATEWAY_ENABLED=maybe should be rejected");
+
+        // CLI_ENABLED=on should error
+        unsafe {
+            std::env::remove_var("GATEWAY_ENABLED");
+            std::env::set_var("CLI_ENABLED", "on");
+        }
+        let result = ChannelsConfig::resolve(&settings);
+        assert!(result.is_err(), "CLI_ENABLED=on should be rejected");
+
+        // WASM_CHANNELS_ENABLED=yes should error
+        unsafe {
+            std::env::remove_var("CLI_ENABLED");
+            std::env::set_var("WASM_CHANNELS_ENABLED", "yes");
+        }
+        let result = ChannelsConfig::resolve(&settings);
+        assert!(
+            result.is_err(),
+            "WASM_CHANNELS_ENABLED=yes should be rejected"
+        );
+
+        // Cleanup
+        unsafe {
+            std::env::remove_var("WASM_CHANNELS_ENABLED");
+        }
+    }
 }
@@ -34,7 +34,9 @@ use crate::settings::Settings;
 // Re-export all public types so `crate::config::FooConfig` continues to work.
 pub use self::agent::AgentConfig;
 pub use self::builder::BuilderModeConfig;
-pub use self::channels::{ChannelsConfig, CliConfig, GatewayConfig, HttpConfig, SignalConfig};
+pub use self::channels::{
+    ChannelsConfig, CliConfig, DEFAULT_GATEWAY_PORT, GatewayConfig, HttpConfig, SignalConfig,
+};
 pub use self::database::{DatabaseBackend, DatabaseConfig, SslMode, default_libsql_path};
 pub use self::embeddings::EmbeddingsConfig;
 pub use self::heartbeat::HeartbeatConfig;
@@ -3451,7 +3451,8 @@ impl ExtensionManager {
            .or_else(|| relay_config.callback_url.clone())
            .unwrap_or_else(|| {
                let host = std::env::var("GATEWAY_HOST").unwrap_or_else(|_| "127.0.0.1".into());
-                let port = std::env::var("GATEWAY_PORT").unwrap_or_else(|_| "3001".into());
+                let port = std::env::var("GATEWAY_PORT")
+                    .unwrap_or_else(|_| crate::config::DEFAULT_GATEWAY_PORT.to_string());
                format!("http://{}:{}", host, port)
            });

@@ -220,7 +220,7 @@ pub struct TunnelSettings {
 }

 /// Channel-specific settings.
-#[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct ChannelSettings {
    /// Whether HTTP webhook channel is enabled.
    #[serde(default)]
@@ -234,6 +234,30 @@ pub struct ChannelSettings {
    #[serde(default)]
    pub http_host: Option<String>,

+    /// Whether the web gateway is enabled.
+    #[serde(default = "default_true")]
+    pub gateway_enabled: bool,
+
+    /// Web gateway listen host.
+    #[serde(default)]
+    pub gateway_host: Option<String>,
+
+    /// Web gateway listen port.
+    #[serde(default)]
+    pub gateway_port: Option<u16>,
+
+    /// Web gateway bearer auth token. Auto-generated at gateway startup if unset.
+    #[serde(default)]
+    pub gateway_auth_token: Option<String>,
+
+    /// Web gateway user ID.
+    #[serde(default)]
+    pub gateway_user_id: Option<String>,
+
+    /// Whether the CLI channel is enabled.
+    #[serde(default = "default_true")]
+    pub cli_enabled: bool,
+
    /// Whether Signal channel is enabled.
    #[serde(default)]
    pub signal_enabled: bool,
@@ -289,6 +313,34 @@ pub struct ChannelSettings {
    pub wasm_channels_dir: Option<PathBuf>,
 }

+impl Default for ChannelSettings {
+    fn default() -> Self {
+        Self {
+            http_enabled: false,
+            http_port: None,
+            http_host: None,
+            gateway_enabled: true,
+            gateway_host: None,
+            gateway_port: None,
+            gateway_auth_token: None,
+            gateway_user_id: None,
+            cli_enabled: true,
+            signal_enabled: false,
+            signal_http_url: None,
+            signal_account: None,
+            signal_allow_from: None,
+            signal_allow_from_groups: None,
+            signal_dm_policy: None,
+            signal_group_policy: None,
+            signal_group_allow_from: None,
+            wasm_channel_owner_ids: std::collections::HashMap::new(),
+            wasm_channels: Vec::new(),
+            wasm_channels_enabled: true,
+            wasm_channels_dir: None,
+        }
+    }
+}
+
 /// Heartbeat configuration.
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct HeartbeatSettings {