From 58e5ceb99ac0a99c8268491c12c00596e037da08 Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Mon, 3 Aug 2026 09:44:55 -0600 Subject: [PATCH 1/3] fix: honor programmatic component enablement Signed-off-by: Bryan Bednarski --- crates/core/src/plugin.rs | 171 ++++++++++++++---- crates/core/src/plugin/dynamic/host.rs | 11 +- crates/core/tests/unit/plugin_tests.rs | 78 +++++++- docs/about-nemo-relay/release-notes/index.mdx | 4 + .../plugin-configuration-files.mdx | 4 + 5 files changed, 220 insertions(+), 48 deletions(-) diff --git a/crates/core/src/plugin.rs b/crates/core/src/plugin.rs index b46d4c8d0..f938a7429 100644 --- a/crates/core/src/plugin.rs +++ b/crates/core/src/plugin.rs @@ -1355,12 +1355,20 @@ pub fn plugin_config_schema() -> Json { /// is removed before the new configuration is activated. #[doc(hidden)] pub async fn initialize_plugins_exact(config: PluginConfig) -> Result { + initialize_plugins_with_diagnostics(config, Vec::new()).await +} + +async fn initialize_plugins_with_diagnostics( + config: PluginConfig, + diagnostics: Vec, +) -> Result { run_owned_plugin_mutation("plugin initialization", move || async move { let lease = LegacyPluginMutationLease::acquire()?; let rollback_failures = Arc::new(Mutex::new(Vec::new())); let initialization = tokio::spawn(initialize_plugins_exact_inner( config, Some(Arc::clone(&rollback_failures)), + diagnostics, )) .await .map_err(|error| { @@ -1477,14 +1485,16 @@ pub(crate) async fn initialize_plugins_exact_for_host( config: PluginConfig, owner_id: u64, rollback_failures: Arc>>, + diagnostics: Vec, ) -> Result { verify_plugin_host_owner(owner_id)?; - initialize_plugins_exact_inner(config, Some(rollback_failures)).await + initialize_plugins_exact_inner(config, Some(rollback_failures), diagnostics).await } async fn initialize_plugins_exact_inner( config: PluginConfig, rollback_failures: Option>>>, + diagnostics: Vec, ) -> Result { let enabled_component_count = config .components @@ -1497,7 +1507,10 @@ async fn initialize_plugins_exact_inner( component_count = enabled_component_count; "Plugin configuration activation started" ); - let report = validate_plugin_config(&config); + let mut report = ConfigReport { diagnostics }; + report + .diagnostics + .extend(validate_plugin_config(&config).diagnostics); if report.has_errors() { return Err(PluginError::InvalidConfig(join_error_messages(&report))); } @@ -1541,10 +1554,9 @@ async fn initialize_plugins_exact_inner( .await { Ok(registrations) => { - let previous_report = validate_plugin_config(&previous_state.config); store_active_plugin_configuration( previous_state.config, - previous_report, + previous_state.report, registrations, )?; log::warn!( @@ -1598,32 +1610,48 @@ async fn initialize_plugin_components_catching_panics( /// Validates and activates `config` layered on top of the discovered /// `plugins.toml` configuration, so a direct integration sees the same file /// layering as the gateway. Each file's schema version is validated before -/// layering. Non-default values in `config` win on conflicts, while default -/// `policy` and `enabled` values inherit from a matching file entry and -/// component `config` bodies merge field-by-field. Delegates to -/// [`initialize_plugins_exact`]. Call that function directly when `config` -/// is already fully resolved and every value must be applied exactly. +/// layering. Declaring a component in `config` applies its `enabled` value, +/// while default policy values inherit from discovered files and component +/// `config` bodies merge field-by-field. Delegates to +/// [`initialize_plugins_exact`]. Call that function directly when `config` is +/// already fully resolved and every value must be applied exactly. pub async fn initialize_plugins(config: PluginConfig) -> Result { - let config = resolve_plugin_config(config)?; - initialize_plugins_exact(config).await + let resolved = resolve_plugin_config(config)?; + initialize_plugins_with_diagnostics(resolved.config, resolved.diagnostics).await } /// Layers `config` over the default discovered `plugins.toml` files. /// /// This is crate-visible so owned dynamic-plugin activation can use the same /// one-time configuration resolution as regular harness-native initialization. -pub(crate) fn resolve_plugin_config(config: PluginConfig) -> Result { - let mut base = resolve_default_file_plugin_config()?; +pub(crate) fn resolve_plugin_config(config: PluginConfig) -> Result { + let discovered = resolve_default_file_plugin_config()?; + let diagnostics = programmatic_enable_override_diagnostics( + &discovered.value, + &discovered.enabled_sources, + &config, + ); + let mut base = discovered.value; layer_config(&mut base, plugin_config_overlay_value(&config)?); - Ok(serde_json::from_value(base)?) + Ok(ResolvedPluginConfig { + config: serde_json::from_value(base)?, + diagnostics, + }) +} + +pub(crate) struct ResolvedPluginConfig { + pub(crate) config: PluginConfig, + pub(crate) diagnostics: Vec, } /// Serializes a typed configuration as a discovery overlay. /// /// A [`PluginConfig`] cannot record whether a default-valued field was supplied /// explicitly or filled by serde. Treating those defaults as overlay values -/// would mask discovered file settings on every library initialization. Exact -/// callers bypass discovery through [`initialize_plugins_exact`]. +/// would mask discovered file settings on every library initialization. A +/// declared component is an activation intent, so its `enabled` value remains +/// in the overlay. Exact callers bypass discovery through +/// [`initialize_plugins_exact`]. fn plugin_config_overlay_value(config: &PluginConfig) -> Result { let mut overlay = serde_json::to_value(config)?; let Json::Object(root) = &mut overlay else { @@ -1635,7 +1663,6 @@ fn plugin_config_overlay_value(config: &PluginConfig) -> Result { } remove_default_policy_overlay(root, &config.policy); - remove_default_component_enabled_overlays(root, &config.components); Ok(overlay) } @@ -1668,30 +1695,25 @@ fn remove_default_policy_overlay(root: &mut Map, config: &ConfigPo } } -fn remove_default_component_enabled_overlays( - root: &mut Map, - configured: &[PluginComponentSpec], -) { - let Some(Json::Array(components)) = root.get_mut("components") else { - return; - }; - for (component, typed) in components.iter_mut().zip(configured) { - if typed.enabled == default_enabled() - && let Json::Object(component) = component - { - component.remove("enabled"); - } - } -} - /// Resolves the default `plugins.toml` layering into one JSON document, or an /// empty object when no plugin file exists. -fn resolve_default_file_plugin_config() -> Result { +fn resolve_default_file_plugin_config() -> Result { let paths = default_plugin_config_paths(std::env::current_dir().ok().as_deref(), user_config_dir()); - Ok(load_plugin_config_files(paths)? + let documents = read_plugin_config_files(paths)?; + let enabled_sources = component_enabled_sources(&documents); + let value = merge_plugin_config_documents(documents)? .map(|(value, _sources)| value) - .unwrap_or_else(|| Json::Object(Map::new()))) + .unwrap_or_else(|| Json::Object(Map::new())); + Ok(DiscoveredPluginConfig { + value, + enabled_sources, + }) +} + +struct DiscoveredPluginConfig { + value: Json, + enabled_sources: HashMap, } use std::path::{Path, PathBuf}; @@ -1701,6 +1723,13 @@ use std::path::{Path, PathBuf}; /// when none exist. Internal: `pub` only for cross-crate reuse by the gateway. #[doc(hidden)] pub fn load_plugin_config_files(paths: I) -> Result)>> +where + I: IntoIterator, +{ + merge_plugin_config_documents(read_plugin_config_files(paths)?) +} + +fn read_plugin_config_files(paths: I) -> Result> where I: IntoIterator, { @@ -1717,7 +1746,75 @@ where })?; documents.push((path, serde_json::to_value(parsed)?)); } - merge_plugin_config_documents(documents) + Ok(documents) +} + +fn component_enabled_sources(documents: &[(PathBuf, Json)]) -> HashMap { + let mut sources = HashMap::new(); + for (path, document) in documents { + let Some(components) = document.get("components").and_then(Json::as_array) else { + continue; + }; + for component in components { + let Some(kind) = component_kind(component) else { + continue; + }; + if component.get("enabled").and_then(Json::as_bool).is_some() { + sources.insert(kind.to_string(), path.clone()); + } + } + } + sources +} + +fn programmatic_enable_override_diagnostics( + discovered: &Json, + enabled_sources: &HashMap, + programmatic: &PluginConfig, +) -> Vec { + let Some(discovered_components) = discovered.get("components").and_then(Json::as_array) else { + return Vec::new(); + }; + let mut discovered_slots: HashMap<&str, Vec<&Json>> = HashMap::new(); + for component in discovered_components { + if let Some(kind) = component_kind(component) { + discovered_slots.entry(kind).or_default().push(component); + } + } + + let mut consumed = HashMap::new(); + let mut diagnostics = Vec::new(); + for component in &programmatic.components { + let nth = consumed.entry(component.kind.as_str()).or_insert(0usize); + let discovered_component = discovered_slots + .get(component.kind.as_str()) + .and_then(|slots| slots.get(*nth)); + *nth += 1; + if !component.enabled + || discovered_component + .and_then(|component| component.get("enabled")) + .and_then(Json::as_bool) + != Some(false) + { + continue; + } + + let source = enabled_sources + .get(&component.kind) + .map(|path| format!(" from {}", path.display())) + .unwrap_or_default(); + diagnostics.push(ConfigDiagnostic { + level: DiagnosticLevel::Warning, + code: "plugin.component_reenabled".to_string(), + component: Some(component.kind.clone()), + field: Some("enabled".to_string()), + message: format!( + "programmatic configuration enabled plugin component '{}' and overrode enabled = false{source}", + component.kind + ), + }); + } + diagnostics } /// Removes physical duplicates while preserving the highest-precedence path. diff --git a/crates/core/src/plugin/dynamic/host.rs b/crates/core/src/plugin/dynamic/host.rs index 8b3f3e2df..49eb57579 100644 --- a/crates/core/src/plugin/dynamic/host.rs +++ b/crates/core/src/plugin/dynamic/host.rs @@ -79,7 +79,7 @@ impl PluginHostActivation { { let dynamic_plugins = dynamic_plugins.into_iter().collect::>(); validate_dynamic_plugin_specs(&dynamic_plugins)?; - Self::activate_validated(config, dynamic_plugins).await + Self::activate_validated(config, dynamic_plugins, Vec::new()).await } /// Load dynamic plugins after layering `config` over discovered `plugins.toml` files. @@ -97,16 +97,17 @@ impl PluginHostActivation { { let dynamic_plugins = dynamic_plugins.into_iter().collect::>(); validate_dynamic_plugin_specs(&dynamic_plugins)?; - let config = resolve_plugin_config(config)?; - Self::activate_validated(config, dynamic_plugins).await + let resolved = resolve_plugin_config(config)?; + Self::activate_validated(resolved.config, dynamic_plugins, resolved.diagnostics).await } async fn activate_validated( config: PluginConfig, dynamic_plugins: Vec, + diagnostics: Vec, ) -> Result<(Self, ConfigReport)> { run_owned_plugin_mutation("dynamic plugin activation", move || async move { - Self::activate_inner(config, dynamic_plugins).await + Self::activate_inner(config, dynamic_plugins, diagnostics).await }) .await } @@ -114,6 +115,7 @@ impl PluginHostActivation { async fn activate_inner( mut config: PluginConfig, dynamic_plugins: Vec, + diagnostics: Vec, ) -> Result<(Self, ConfigReport)> { let dynamic_plugin_count = dynamic_plugins.len(); log::info!( @@ -190,6 +192,7 @@ impl PluginHostActivation { config, owner_id, Arc::clone(&rollback_failures), + diagnostics, )) .await .map_err(|error| { diff --git a/crates/core/tests/unit/plugin_tests.rs b/crates/core/tests/unit/plugin_tests.rs index 4e79c9e45..039a7bc7b 100644 --- a/crates/core/tests/unit/plugin_tests.rs +++ b/crates/core/tests/unit/plugin_tests.rs @@ -2132,6 +2132,41 @@ fn test_initialize_plugins_replaces_previous_configuration_on_success() { reset_global(); } +#[test] +fn test_initialize_plugins_preserves_resolution_diagnostics() { + let _guard = lock_runtime_owner(); + reset_global(); + register_plugin(Arc::new(RecordingPlugin)).unwrap(); + + let diagnostic = ConfigDiagnostic { + level: DiagnosticLevel::Warning, + code: "plugin.component_reenabled".to_string(), + component: Some("recording.plugin".to_string()), + field: Some("enabled".to_string()), + message: "programmatic configuration re-enabled the component".to_string(), + }; + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + let report = runtime + .block_on(initialize_plugins_with_diagnostics( + PluginConfig { + components: vec![PluginComponentSpec::new("recording.plugin")], + ..PluginConfig::default() + }, + vec![diagnostic.clone()], + )) + .unwrap(); + + assert_eq!(report.diagnostics, vec![diagnostic.clone()]); + assert_eq!( + active_plugin_report().unwrap().diagnostics, + vec![diagnostic] + ); + reset_global(); +} + #[test] fn test_initialize_plugins_reports_failed_restore_when_previous_configuration_cannot_be_restored() { let _guard = lock_runtime_owner(); @@ -2372,10 +2407,10 @@ fn test_load_plugin_config_files_deduplicates_aliases_at_highest_precedence() { } #[test] -fn test_plugin_config_overlay_inherits_file_values_for_typed_defaults() { +fn test_plugin_config_overlay_enables_programmatically_declared_components() { // After each file's schema version is validated, a typed `PluginConfig` is layered over - // the discovered file base. Default-valued `policy`/`enabled` fields inherit the file, - // the free-form `config` body merges, and an undeclared component kind is inherited. + // the discovered file base. Default-valued policy fields inherit the file, a declared + // component applies its enabled value, and the free-form `config` body merges. let file_base = json!({ "version": 1, "components": [ @@ -2384,7 +2419,7 @@ fn test_plugin_config_overlay_inherits_file_values_for_typed_defaults() { "enabled": false, "config": { "output_directory": "/var/log", "mode": "append" } }, - { "kind": "adaptive", "config": { "ttl": 60 } } + { "kind": "adaptive", "enabled": false, "config": { "ttl": 60 } } ], "policy": { "unknown_component": "error", @@ -2404,7 +2439,7 @@ fn test_plugin_config_overlay_inherits_file_values_for_typed_defaults() { layer_config(&mut merged, plugin_config_overlay_value(&code).unwrap()); let typed: PluginConfig = serde_json::from_value(merged).unwrap(); - // Typed defaults do not mask the file base. + // Typed policy defaults do not mask the file base. assert_eq!(typed.version, 1); assert_eq!( typed.policy.unknown_component, @@ -2414,8 +2449,8 @@ fn test_plugin_config_overlay_inherits_file_values_for_typed_defaults() { let observability = &typed.components[0]; assert_eq!(observability.kind, "observability"); assert!( - !observability.enabled, - "typed default enabled=true inherits the file's false" + observability.enabled, + "a declared component is enabled by code" ); // The component config body merges: code's `mode` wins, the file's `output_directory` // is inherited. @@ -2423,6 +2458,35 @@ fn test_plugin_config_overlay_inherits_file_values_for_typed_defaults() { assert_eq!(observability.config["output_directory"], json!("/var/log")); // A kind the code config does not declare is inherited from the file. assert_eq!(typed.components[1].kind, "adaptive"); + assert!(!typed.components[1].enabled); +} + +#[test] +fn test_programmatic_enable_override_diagnostic_names_discovered_source() { + let source = PathBuf::from("/etc/nemo-relay/plugins.toml"); + let discovered = json!({ + "components": [{ "kind": "observability", "enabled": false }] + }); + let enabled_sources = HashMap::from([("observability".to_string(), source.clone())]); + let programmatic = PluginConfig { + components: vec![PluginComponentSpec::new("observability")], + ..PluginConfig::default() + }; + + let diagnostics = + programmatic_enable_override_diagnostics(&discovered, &enabled_sources, &programmatic); + + assert_eq!(diagnostics.len(), 1); + let diagnostic = &diagnostics[0]; + assert_eq!(diagnostic.level, DiagnosticLevel::Warning); + assert_eq!(diagnostic.code, "plugin.component_reenabled"); + assert_eq!(diagnostic.component.as_deref(), Some("observability")); + assert_eq!(diagnostic.field.as_deref(), Some("enabled")); + assert!( + diagnostic.message.contains(&source.display().to_string()), + "{}", + diagnostic.message + ); } #[test] diff --git a/docs/about-nemo-relay/release-notes/index.mdx b/docs/about-nemo-relay/release-notes/index.mdx index 20813e32c..8f80bf24f 100644 --- a/docs/about-nemo-relay/release-notes/index.mdx +++ b/docs/about-nemo-relay/release-notes/index.mdx @@ -98,6 +98,10 @@ their values cannot be isolated between endpoints. ### Fixed Known Issues in 0.7 +- Programmatically declared plugin components now apply their `enabled` value + over discovered file configuration. When code re-enables a component that a + discovered file disabled, initialization reports a warning that names the + component and source file. - LLM payload redaction now follows the codec active for each call instead of a codec captured from plugin configuration. Codec-dependent policies omit the observability payload and annotation when Relay cannot safely normalize the diff --git a/docs/configure-plugins/plugin-configuration-files.mdx b/docs/configure-plugins/plugin-configuration-files.mdx index 80b459e32..fb086efa0 100644 --- a/docs/configure-plugins/plugin-configuration-files.mdx +++ b/docs/configure-plugins/plugin-configuration-files.mdx @@ -366,6 +366,10 @@ Lower-precedence files fill fields that higher-precedence files omit. Typed code fields always have values, so they override file values. Only component selection and keys inside component `config` merge with files. +When a component declared in code resolves `enabled = true` over a discovered +`enabled = false`, initialization and the active plugin report include a +`plugin.component_reenabled` warning that names the contributing file. + Without filesystem access, no files are read, so the base is empty and only your `initialize` config applies. From f898d345578222d62274895422b85e3819e54304 Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Mon, 3 Aug 2026 10:34:22 -0600 Subject: [PATCH 2/3] docs: correct plugin initialization call path Signed-off-by: Bryan Bednarski --- crates/core/src/plugin.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/core/src/plugin.rs b/crates/core/src/plugin.rs index f938a7429..df814bf56 100644 --- a/crates/core/src/plugin.rs +++ b/crates/core/src/plugin.rs @@ -1612,9 +1612,10 @@ async fn initialize_plugin_components_catching_panics( /// layering as the gateway. Each file's schema version is validated before /// layering. Declaring a component in `config` applies its `enabled` value, /// while default policy values inherit from discovered files and component -/// `config` bodies merge field-by-field. Delegates to -/// [`initialize_plugins_exact`]. Call that function directly when `config` is -/// already fully resolved and every value must be applied exactly. +/// `config` bodies merge field-by-field. The resolved configuration and +/// diagnostics are passed to the shared `initialize_plugins_with_diagnostics` +/// helper. Call [`initialize_plugins_exact`] directly when `config` is already +/// fully resolved and every value must be applied exactly. pub async fn initialize_plugins(config: PluginConfig) -> Result { let resolved = resolve_plugin_config(config)?; initialize_plugins_with_diagnostics(resolved.config, resolved.diagnostics).await From 1587a0cf16a1e8733f44cb0e1c202863f4f05987 Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Mon, 3 Aug 2026 11:55:12 -0600 Subject: [PATCH 3/3] refactor: share positional plugin matching Signed-off-by: Bryan Bednarski --- crates/core/src/plugin.rs | 34 +++++++++++--------------- crates/core/tests/unit/plugin_tests.rs | 12 ++++++--- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/crates/core/src/plugin.rs b/crates/core/src/plugin.rs index df814bf56..5bcb380dc 100644 --- a/crates/core/src/plugin.rs +++ b/crates/core/src/plugin.rs @@ -1205,12 +1205,7 @@ fn merge_plugin_components(left: &mut Json, right: Json) { *left = right; return; }; - let mut base_slots: HashMap> = HashMap::new(); - for (index, component) in left_components.iter().enumerate() { - if let Some(kind) = component_kind(component) { - base_slots.entry(kind.to_string()).or_default().push(index); - } - } + let base_component_count = left_components.len(); let mut consumed: HashMap = HashMap::new(); for component in right_components { let Some(kind) = component_kind(&component).map(str::to_owned) else { @@ -1218,10 +1213,7 @@ fn merge_plugin_components(left: &mut Json, right: Json) { continue; }; let nth = consumed.entry(kind.clone()).or_insert(0); - let slot = base_slots - .get(&kind) - .and_then(|slots| slots.get(*nth)) - .copied(); + let slot = nth_component_by_kind(&left_components[..base_component_count], &kind, *nth); *nth += 1; match slot { Some(index) => merge_plugin_component(&mut left_components[index], component), @@ -1326,6 +1318,15 @@ fn component_kind(component: &Json) -> Option<&str> { component.get("kind").and_then(Json::as_str) } +fn nth_component_by_kind(components: &[Json], kind: &str, nth: usize) -> Option { + components + .iter() + .enumerate() + .filter(|(_index, component)| component_kind(component) == Some(kind)) + .nth(nth) + .map(|(index, _component)| index) +} + /// Returns the JSON Schema for the canonical plugin configuration document. #[cfg(feature = "schema")] pub fn plugin_config_schema() -> Json { @@ -1776,20 +1777,13 @@ fn programmatic_enable_override_diagnostics( let Some(discovered_components) = discovered.get("components").and_then(Json::as_array) else { return Vec::new(); }; - let mut discovered_slots: HashMap<&str, Vec<&Json>> = HashMap::new(); - for component in discovered_components { - if let Some(kind) = component_kind(component) { - discovered_slots.entry(kind).or_default().push(component); - } - } - let mut consumed = HashMap::new(); let mut diagnostics = Vec::new(); for component in &programmatic.components { let nth = consumed.entry(component.kind.as_str()).or_insert(0usize); - let discovered_component = discovered_slots - .get(component.kind.as_str()) - .and_then(|slots| slots.get(*nth)); + let discovered_component = + nth_component_by_kind(discovered_components, &component.kind, *nth) + .and_then(|index| discovered_components.get(index)); *nth += 1; if !component.enabled || discovered_component diff --git a/crates/core/tests/unit/plugin_tests.rs b/crates/core/tests/unit/plugin_tests.rs index 039a7bc7b..5be548cbb 100644 --- a/crates/core/tests/unit/plugin_tests.rs +++ b/crates/core/tests/unit/plugin_tests.rs @@ -2462,14 +2462,20 @@ fn test_plugin_config_overlay_enables_programmatically_declared_components() { } #[test] -fn test_programmatic_enable_override_diagnostic_names_discovered_source() { +fn test_programmatic_enable_override_diagnostic_matches_positionally_and_names_source() { let source = PathBuf::from("/etc/nemo-relay/plugins.toml"); let discovered = json!({ - "components": [{ "kind": "observability", "enabled": false }] + "components": [ + { "kind": "observability", "enabled": true }, + { "kind": "observability", "enabled": false } + ] }); let enabled_sources = HashMap::from([("observability".to_string(), source.clone())]); let programmatic = PluginConfig { - components: vec![PluginComponentSpec::new("observability")], + components: vec![ + PluginComponentSpec::new("observability"), + PluginComponentSpec::new("observability"), + ], ..PluginConfig::default() };