diff --git a/src/config.rs b/src/config.rs index 60b9eb74b..a88b85895 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1602,11 +1602,13 @@ maintenance_merge_similarity_threshold = 1.1 dm_allowed_users: vec![], }, ]; - assert!(validate_named_messaging_adapters(&messaging, &bindings).is_ok()); + let result = validate_named_messaging_adapters(&messaging, bindings, false) + .expect("bindings should be resolvable"); + assert_eq!(result.len(), 2); } #[test] - fn validate_named_adapters_missing_instance() { + fn validate_named_adapters_missing_instance_skipped() { let messaging = MessagingConfig { discord: None, slack: None, @@ -1632,7 +1634,9 @@ maintenance_merge_similarity_threshold = 1.1 require_mention: false, dm_allowed_users: vec![], }]; - assert!(validate_named_messaging_adapters(&messaging, &bindings).is_err()); + let result = validate_named_messaging_adapters(&messaging, bindings, false) + .expect("bindings should be resolvable"); + assert!(result.is_empty(), "unresolvable binding should be skipped"); } #[test] @@ -1695,11 +1699,16 @@ maintenance_merge_similarity_threshold = 1.1 require_mention: false, dm_allowed_users: vec![], }]; - assert!(validate_named_messaging_adapters(&messaging, &bindings).is_err()); + let result = validate_named_messaging_adapters(&messaging, bindings, false) + .expect("bindings should be resolvable"); + assert!( + result.is_empty(), + "unsupported platform binding should be skipped" + ); } #[test] - fn validate_binding_without_default_adapter_rejected() { + fn validate_binding_without_default_adapter_skipped() { let messaging = MessagingConfig { discord: None, slack: None, @@ -1731,7 +1740,228 @@ maintenance_merge_similarity_threshold = 1.1 require_mention: false, dm_allowed_users: vec![], }]; - assert!(validate_named_messaging_adapters(&messaging, &bindings).is_err()); + let result = validate_named_messaging_adapters(&messaging, bindings, false) + .expect("bindings should be resolvable"); + assert!( + result.is_empty(), + "binding without default adapter should be skipped" + ); + } + + #[test] + fn validate_mixed_valid_and_invalid_bindings_filters_correctly() { + let messaging = MessagingConfig { + discord: None, + slack: None, + telegram: Some(TelegramConfig { + enabled: true, + token: "tok".into(), + instances: vec![TelegramInstanceConfig { + name: "support".into(), + enabled: true, + token: "tok2".into(), + dm_allowed_users: vec![], + }], + dm_allowed_users: vec![], + }), + email: None, + webhook: None, + twitch: None, + signal: None, + }; + let bindings = vec![ + // Valid: default adapter with credentials + Binding { + agent_id: "agent-a".into(), + channel: "telegram".into(), + adapter: None, + guild_id: None, + workspace_id: None, + chat_id: None, + channel_ids: vec![], + require_mention: false, + dm_allowed_users: vec![], + }, + // Invalid: references a non-existent named adapter + Binding { + agent_id: "agent-b".into(), + channel: "telegram".into(), + adapter: Some("ghost".into()), + guild_id: None, + workspace_id: None, + chat_id: None, + channel_ids: vec![], + require_mention: false, + dm_allowed_users: vec![], + }, + // Valid: references an existing named adapter + Binding { + agent_id: "agent-c".into(), + channel: "telegram".into(), + adapter: Some("support".into()), + guild_id: None, + workspace_id: None, + chat_id: None, + channel_ids: vec![], + require_mention: false, + dm_allowed_users: vec![], + }, + // Invalid: no discord config at all + Binding { + agent_id: "agent-d".into(), + channel: "discord".into(), + adapter: None, + guild_id: None, + workspace_id: None, + chat_id: None, + channel_ids: vec![], + require_mention: false, + dm_allowed_users: vec![], + }, + ]; + let result = validate_named_messaging_adapters(&messaging, bindings, false) + .expect("bindings should be resolvable"); + assert_eq!( + result.len(), + 2, + "only the two valid bindings should survive" + ); + assert_eq!(result[0].agent_id, "agent-a"); + assert_eq!(result[1].agent_id, "agent-c"); + } + + #[test] + fn validate_missing_messaging_config_skipped() { + let messaging = MessagingConfig { + discord: None, + slack: None, + telegram: None, + email: None, + webhook: None, + twitch: None, + signal: None, + }; + let bindings = vec![Binding { + agent_id: "main".into(), + channel: "telegram".into(), + adapter: None, + guild_id: None, + workspace_id: None, + chat_id: None, + channel_ids: vec![], + require_mention: false, + dm_allowed_users: vec![], + }]; + let result = validate_named_messaging_adapters(&messaging, bindings, false) + .expect("bindings should be resolvable"); + assert!( + result.is_empty(), + "binding with no messaging config should be skipped" + ); + } + + #[test] + fn validate_strict_mode_rejects_missing_messaging_config() { + let messaging = MessagingConfig { + discord: None, + slack: None, + telegram: None, + email: None, + webhook: None, + twitch: None, + signal: None, + }; + let bindings = vec![Binding { + agent_id: "main".into(), + channel: "telegram".into(), + adapter: None, + guild_id: None, + workspace_id: None, + chat_id: None, + channel_ids: vec![], + require_mention: false, + dm_allowed_users: vec![], + }]; + let result = validate_named_messaging_adapters(&messaging, bindings, true); + assert!( + result.is_err(), + "strict mode should reject unresolvable bindings" + ); + } + + #[test] + fn validate_disabled_instance_is_filtered_out() { + let messaging = MessagingConfig { + discord: None, + slack: None, + telegram: Some(TelegramConfig { + enabled: true, + token: "tok".into(), + instances: vec![TelegramInstanceConfig { + name: "support".into(), + enabled: false, + token: "tok2".into(), + dm_allowed_users: vec![], + }], + dm_allowed_users: vec![], + }), + email: None, + webhook: None, + twitch: None, + signal: None, + }; + let bindings = vec![Binding { + agent_id: "main".into(), + channel: "telegram".into(), + adapter: Some("support".into()), + guild_id: None, + workspace_id: None, + chat_id: None, + channel_ids: vec![], + require_mention: false, + dm_allowed_users: vec![], + }]; + let result = validate_named_messaging_adapters(&messaging, bindings, false) + .expect("bindings should be resolvable"); + assert!( + result.is_empty(), + "binding to disabled instance should be skipped" + ); + } + + #[test] + fn validate_disabled_platform_default_is_filtered_out() { + let messaging = MessagingConfig { + discord: None, + slack: None, + telegram: Some(TelegramConfig { + enabled: false, + token: "tok".into(), + instances: vec![], + dm_allowed_users: vec![], + }), + email: None, + webhook: None, + twitch: None, + signal: None, + }; + let bindings = vec![Binding { + agent_id: "main".into(), + channel: "telegram".into(), + adapter: None, + guild_id: None, + workspace_id: None, + chat_id: None, + channel_ids: vec![], + require_mention: false, + dm_allowed_users: vec![], + }]; + let result = validate_named_messaging_adapters(&messaging, bindings, false) + .expect("bindings should be resolvable"); + assert!( + result.is_empty(), + "binding to disabled platform default should be skipped" + ); } #[test] diff --git a/src/config/load.rs b/src/config/load.rs index a4b44882c..cd85acb1c 100644 --- a/src/config/load.rs +++ b/src/config/load.rs @@ -905,13 +905,22 @@ impl Config { let toml_config: TomlConfig = toml::from_str(content).context("failed to parse config TOML")?; - // Run full conversion to catch semantic errors (env resolution, defaults, etc.) + // Run full conversion with strict binding validation so config + // authoring catches unresolvable bindings as errors. let instance_dir = Self::default_instance_dir(); - Self::from_toml(toml_config, instance_dir)?; + Self::from_toml_inner(toml_config, instance_dir, true)?; Ok(()) } pub(super) fn from_toml(toml: TomlConfig, instance_dir: PathBuf) -> Result { + Self::from_toml_inner(toml, instance_dir, false) + } + + fn from_toml_inner( + toml: TomlConfig, + instance_dir: PathBuf, + strict_bindings: bool, + ) -> Result { // Validate providers before processing for (provider_id, config) in &toml.llm.providers { // Validate provider_id @@ -2245,7 +2254,7 @@ impl Config { }) .collect(); - validate_named_messaging_adapters(&messaging, &bindings)?; + let bindings = validate_named_messaging_adapters(&messaging, bindings, strict_bindings)?; let api = ApiConfig { enabled: toml.api.enabled, diff --git a/src/config/types.rs b/src/config/types.rs index fe9d31463..72ed836e0 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -1674,55 +1674,107 @@ pub(super) fn is_named_adapter_platform(platform: &str) -> bool { ) } +/// Validate channel bindings against messaging config, returning only the +/// resolvable bindings. Unresolvable bindings (missing messaging config, +/// missing adapter, etc.) are logged as warnings and skipped instead of +/// causing a hard startup failure. +/// Validate channel bindings against messaging config, returning only the +/// resolvable bindings. When `strict` is true (config authoring/validation), +/// unresolvable bindings cause an error. When `strict` is false (startup), +/// they are logged as warnings and skipped so the agent can still boot. pub(super) fn validate_named_messaging_adapters( messaging: &MessagingConfig, - bindings: &[Binding], -) -> Result<()> { + bindings: Vec, + strict: bool, +) -> Result> { let adapter_states = build_adapter_validation_states(messaging)?; + let mut valid_bindings = Vec::with_capacity(bindings.len()); + for binding in bindings { if !is_named_adapter_platform(binding.channel.as_str()) { if binding.adapter.is_some() { - return Err(ConfigError::Invalid(format!( - "binding for channel '{}' can't set adapter: this platform does not support named adapters", - binding.channel - )) - .into()); + let msg = format!( + "binding for agent '{}' on channel '{}' can't set adapter: this platform does not support named adapters", + binding.agent_id, binding.channel + ); + if strict { + return Err(ConfigError::Invalid(msg).into()); + } + tracing::warn!( + agent_id = %binding.agent_id, + channel = %binding.channel, + adapter = %binding.adapter.as_deref().unwrap_or(""), + "skipping binding: this platform does not support named adapters" + ); + continue; } + valid_bindings.push(binding); continue; } - let state = adapter_states.get(binding.channel.as_str()).ok_or_else(|| { - ConfigError::Invalid(format!( - "binding for channel '{}' can't be resolved: no messaging config exists for that platform", - binding.channel - )) - })?; + let state = match adapter_states.get(binding.channel.as_str()) { + Some(s) => s, + None => { + let msg = format!( + "binding for agent '{}' on channel '{}' can't be resolved: no messaging config exists for that platform", + binding.agent_id, binding.channel + ); + if strict { + return Err(ConfigError::Invalid(msg).into()); + } + tracing::warn!( + agent_id = %binding.agent_id, + channel = %binding.channel, + "skipping binding: no messaging config exists for this platform" + ); + continue; + } + }; // adapter is already normalized at ingest time via normalize_adapter(). match binding.adapter.as_deref() { Some(adapter_name) => { if !state.named_instances.contains(adapter_name) { - return Err(ConfigError::Invalid(format!( - "binding for channel '{}' references missing adapter '{}'", - binding.channel, adapter_name - )) - .into()); + let msg = format!( + "binding for agent '{}' on channel '{}' references missing or disabled adapter '{}'", + binding.agent_id, binding.channel, adapter_name + ); + if strict { + return Err(ConfigError::Invalid(msg).into()); + } + tracing::warn!( + agent_id = %binding.agent_id, + channel = %binding.channel, + adapter = %adapter_name, + "skipping binding: references missing or disabled adapter" + ); + continue; } } None => { if !state.default_present { - return Err(ConfigError::Invalid(format!( - "binding for channel '{}' requires the default adapter, but no default credentials are configured", - binding.channel - )) - .into()); + let msg = format!( + "binding for agent '{}' on channel '{}' requires the default adapter, but it is disabled or has no credentials configured", + binding.agent_id, binding.channel + ); + if strict { + return Err(ConfigError::Invalid(msg).into()); + } + tracing::warn!( + agent_id = %binding.agent_id, + channel = %binding.channel, + "skipping binding: requires the default adapter, but it is disabled or has no credentials configured" + ); + continue; } } } + + valid_bindings.push(binding); } - Ok(()) + Ok(valid_bindings) } pub(super) fn build_adapter_validation_states( @@ -1731,37 +1783,49 @@ pub(super) fn build_adapter_validation_states( let mut states = std::collections::HashMap::new(); if let Some(discord) = &messaging.discord { - let named_instances = validate_instance_names( + // Validate ALL instance names for structural issues (duplicates, empty, etc.) + validate_instance_names( "discord", discord .instances .iter() .map(|instance| instance.name.as_str()), )?; - validate_runtime_keys( - "discord", - !discord.token.trim().is_empty(), - &named_instances, - )?; + // Only include enabled instances in the resolvable set + let named_instances: std::collections::HashSet = discord + .instances + .iter() + .filter(|i| i.enabled) + .map(|i| i.name.clone()) + .collect(); + let default_present = discord.enabled && !discord.token.trim().is_empty(); + validate_runtime_keys("discord", default_present, &named_instances)?; states.insert( "discord", AdapterValidationState { - default_present: !discord.token.trim().is_empty(), + default_present, named_instances, }, ); } if let Some(slack) = &messaging.slack { - let named_instances = validate_instance_names( + validate_instance_names( "slack", slack .instances .iter() .map(|instance| instance.name.as_str()), )?; - let default_present = - !slack.bot_token.trim().is_empty() && !slack.app_token.trim().is_empty(); + let named_instances: std::collections::HashSet = slack + .instances + .iter() + .filter(|i| i.enabled) + .map(|i| i.name.clone()) + .collect(); + let default_present = slack.enabled + && !slack.bot_token.trim().is_empty() + && !slack.app_token.trim().is_empty(); validate_runtime_keys("slack", default_present, &named_instances)?; states.insert( "slack", @@ -1773,14 +1837,20 @@ pub(super) fn build_adapter_validation_states( } if let Some(telegram) = &messaging.telegram { - let named_instances = validate_instance_names( + validate_instance_names( "telegram", telegram .instances .iter() .map(|instance| instance.name.as_str()), )?; - let default_present = !telegram.token.trim().is_empty(); + let named_instances: std::collections::HashSet = telegram + .instances + .iter() + .filter(|i| i.enabled) + .map(|i| i.name.clone()) + .collect(); + let default_present = telegram.enabled && !telegram.token.trim().is_empty(); validate_runtime_keys("telegram", default_present, &named_instances)?; states.insert( "telegram", @@ -1792,15 +1862,22 @@ pub(super) fn build_adapter_validation_states( } if let Some(twitch) = &messaging.twitch { - let named_instances = validate_instance_names( + validate_instance_names( "twitch", twitch .instances .iter() .map(|instance| instance.name.as_str()), )?; - let default_present = - !twitch.username.trim().is_empty() && !twitch.oauth_token.trim().is_empty(); + let named_instances: std::collections::HashSet = twitch + .instances + .iter() + .filter(|i| i.enabled) + .map(|i| i.name.clone()) + .collect(); + let default_present = twitch.enabled + && !twitch.username.trim().is_empty() + && !twitch.oauth_token.trim().is_empty(); validate_runtime_keys("twitch", default_present, &named_instances)?; states.insert( "twitch", @@ -1812,14 +1889,21 @@ pub(super) fn build_adapter_validation_states( } if let Some(email) = &messaging.email { - let named_instances = validate_instance_names( + validate_instance_names( "email", email .instances .iter() .map(|instance| instance.name.as_str()), )?; - let default_present = !email.imap_host.trim().is_empty() + let named_instances: std::collections::HashSet = email + .instances + .iter() + .filter(|i| i.enabled) + .map(|i| i.name.clone()) + .collect(); + let default_present = email.enabled + && !email.imap_host.trim().is_empty() && !email.imap_username.trim().is_empty() && !email.imap_password.trim().is_empty() && !email.smtp_host.trim().is_empty(); @@ -1834,15 +1918,22 @@ pub(super) fn build_adapter_validation_states( } if let Some(signal) = &messaging.signal { - let named_instances = validate_instance_names( + validate_instance_names( "signal", signal .instances .iter() .map(|instance| instance.name.as_str()), )?; - let default_present = - !signal.http_url.trim().is_empty() && !signal.account.trim().is_empty(); + let named_instances: std::collections::HashSet = signal + .instances + .iter() + .filter(|i| i.enabled) + .map(|i| i.name.clone()) + .collect(); + let default_present = signal.enabled + && !signal.http_url.trim().is_empty() + && !signal.account.trim().is_empty(); validate_runtime_keys("signal", default_present, &named_instances)?; states.insert( "signal",