diff --git a/crates/core/src/plugin.rs b/crates/core/src/plugin.rs index 601cb5559..fe34673ae 100644 --- a/crates/core/src/plugin.rs +++ b/crates/core/src/plugin.rs @@ -1729,10 +1729,15 @@ fn resolve_discovered_plugin_config( struct DiscoveredPluginConfig { value: Json, - enabled_sources: HashMap, + enabled_sources: HashMap, sources: Vec, } +struct ComponentEnabledSource { + enabled: bool, + path: PathBuf, +} + use std::path::{Path, PathBuf}; /// Reads, parses, and merges the `plugins.toml` files at `paths` (lowest @@ -1766,7 +1771,9 @@ where Ok(documents) } -fn component_enabled_sources(documents: &[(PathBuf, Json)]) -> HashMap { +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 { @@ -1776,8 +1783,14 @@ fn component_enabled_sources(documents: &[(PathBuf, Json)]) -> HashMap Vec, + enabled_sources: &HashMap, programmatic: &PluginConfig, ) -> Vec { let Some(discovered_components) = discovered.get("components").and_then(Json::as_array) else { @@ -1822,18 +1835,21 @@ fn programmatic_enable_override_diagnostics( nth_component_by_kind(discovered_components, &component.kind, *nth) .and_then(|index| discovered_components.get(index)); *nth += 1; - if !component.enabled - || discovered_component - .and_then(|component| component.get("enabled")) - .and_then(Json::as_bool) - != Some(false) - { + let discovered_enabled = discovered_component + .and_then(|component| component.get("enabled")) + .and_then(Json::as_bool); + let file_disabled = discovered_enabled == Some(false) + || (discovered_enabled.is_none() + && enabled_sources + .get(&component.kind) + .is_some_and(|source| !source.enabled)); + if !component.enabled || !file_disabled { continue; } let source = enabled_sources .get(&component.kind) - .map(|path| format!(" from {}", path.display())) + .map(|source| format!(" from {}", source.path.display())) .unwrap_or_default(); diagnostics.push(ConfigDiagnostic { level: DiagnosticLevel::Warning, @@ -1879,15 +1895,25 @@ where { let mut merged = Json::Object(Map::new()); let mut sources = Vec::new(); - for (path, document) in documents { + for (path, mut document) in documents { validate_plugin_config_version(&path, &document)?; validate_unique_component_kinds(&path, &document)?; + + filter_disabled_plugin_components(&mut document); layer_config(&mut merged, document); sources.push(path); } Ok((!sources.is_empty()).then_some((merged, sources))) } +/// Removes disabled components from one discovered plugin document before layering. +fn filter_disabled_plugin_components(document: &mut Json) { + let Some(components) = document.get_mut("components").and_then(Json::as_array_mut) else { + return; + }; + components.retain(|component| component.get("enabled").and_then(Json::as_bool) != Some(false)); +} + /// Rejects a file with an unsupported top-level plugin config version before layering can /// overwrite it with a higher-precedence source or typed default. fn validate_plugin_config_version(path: &Path, document: &Json) -> Result<()> { diff --git a/crates/core/tests/unit/plugin_tests.rs b/crates/core/tests/unit/plugin_tests.rs index b39f6f934..d50cfbfd6 100644 --- a/crates/core/tests/unit/plugin_tests.rs +++ b/crates/core/tests/unit/plugin_tests.rs @@ -2279,8 +2279,8 @@ fn test_load_plugin_config_files_merges_files_by_precedence() { assert_eq!(observability["kind"], json!("observability")); assert_eq!( observability["enabled"], - json!(false), - "the system layer wins shared scalar fields" + json!(true), + "a disabled system component does not override lower layers" ); assert_eq!( observability["config"]["output_directory"], @@ -2289,13 +2289,13 @@ fn test_load_plugin_config_files_merges_files_by_precedence() { ); assert_eq!( observability["config"]["mode"], - json!("system"), - "the system layer wins recursively merged config fields" + json!("project"), + "a disabled system component does not contribute configuration" ); assert_eq!( observability["config"]["values"], - json!(["system", "project", "lower"]), - "list entries aggregate from highest to lowest precedence" + json!(["project", "lower"]), + "a disabled system component does not contribute list entries" ); assert_eq!( components[1]["kind"], @@ -2309,6 +2309,34 @@ fn test_load_plugin_config_files_merges_files_by_precedence() { ); } +#[test] +fn test_load_plugin_config_files_omits_components_disabled_in_every_file() { + let dir = tempfile::tempdir().unwrap(); + let lower = dir.path().join("lower.toml"); + let higher = dir.path().join("higher.toml"); + std::fs::write( + &lower, + "[[components]]\n\ + kind = \"observability\"\n\ + enabled = false\n", + ) + .unwrap(); + std::fs::write( + &higher, + "[[components]]\n\ + kind = \"observability\"\n\ + enabled = false\n", + ) + .unwrap(); + + let (merged, sources) = load_plugin_config_files([lower.clone(), higher.clone()]) + .unwrap() + .expect("the files exist"); + + assert_eq!(sources, vec![lower, higher]); + assert_eq!(merged["components"], json!([])); +} + #[test] fn test_load_plugin_config_files_rejects_version_before_layering() { let dir = tempfile::tempdir().unwrap(); @@ -2545,7 +2573,13 @@ fn test_programmatic_enable_override_diagnostic_matches_positionally_and_names_s { "kind": "observability", "enabled": false } ] }); - let enabled_sources = HashMap::from([("observability".to_string(), source.clone())]); + let enabled_sources = HashMap::from([( + "observability".to_string(), + ComponentEnabledSource { + enabled: false, + path: source.clone(), + }, + )]); let programmatic = PluginConfig { components: vec![ PluginComponentSpec::new("observability"), @@ -2570,6 +2604,39 @@ fn test_programmatic_enable_override_diagnostic_matches_positionally_and_names_s ); } +#[test] +fn test_programmatic_reenable_diagnostic_survives_disabled_component_normalization() { + let source = PathBuf::from("/etc/nemo-relay/plugins.toml"); + let documents = vec![( + source.clone(), + json!({ + "components": [{ "kind": "observability", "enabled": false }] + }), + )]; + let enabled_sources = component_enabled_sources(&documents); + let (discovered, _) = merge_plugin_config_documents(documents) + .unwrap() + .expect("the file-backed configuration exists"); + assert_eq!(discovered["components"], json!([])); + + let diagnostics = programmatic_enable_override_diagnostics( + &discovered, + &enabled_sources, + &PluginConfig { + components: vec![PluginComponentSpec::new("observability")], + ..PluginConfig::default() + }, + ); + + assert_eq!(diagnostics.len(), 1); + assert_eq!(diagnostics[0].code, "plugin.component_reenabled"); + assert!( + diagnostics[0] + .message + .contains(&source.display().to_string()) + ); +} + #[test] fn test_plugin_config_overlay_applies_non_default_values() { let mut file_base = json!({ diff --git a/docs/configure-plugins/plugin-configuration-files.mdx b/docs/configure-plugins/plugin-configuration-files.mdx index 73db0435c..e5a486c9f 100644 --- a/docs/configure-plugins/plugin-configuration-files.mdx +++ b/docs/configure-plugins/plugin-configuration-files.mdx @@ -319,10 +319,16 @@ The effective Agent Trajectory Observability Format (ATOF) configuration keeps `version` and `enabled` from the user file. Its `sinks` list contains the system sink first, followed by the user sink. -The top-level `components` array is special. Relay matches components by `kind` -across files. A higher-precedence component with the same `kind` merges into the -lower-precedence component. Relay adds a component with a different `kind` to -the effective configuration. +The top-level `components` array is special. Relay matches enabled components +by `kind` across files. A higher-precedence component with the same `kind` +merges into the lower-precedence component. Relay adds a component with a +different `kind` to the effective configuration. + +A component entry that explicitly sets `enabled = false` is skipped before +matching and merging. It does not change a lower-precedence component's enabled +state or contribute any `config` fields or list entries. If every layer for a +component kind sets `enabled = false`, that kind is absent from the effective +configuration. This behavior applies to list fields declared at the top level of a component's `config`. It also applies to the observability destination lists