Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions crates/cli/tests/coverage/shared/config_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2048,8 +2048,16 @@ fn plugin_config_path_overrides_sibling_plugin_file() {
let sibling_path = temp.path().join("plugins.toml");
let override_path = temp.path().join("override.toml");
std::fs::write(&config_path, "").unwrap();
std::fs::write(&sibling_path, "version = 1\n").unwrap();
std::fs::write(&override_path, "version = 2\n").unwrap();
std::fs::write(
&sibling_path,
"version = 1\n[policy]\nunknown_field = \"warn\"\n",
)
.unwrap();
std::fs::write(
&override_path,
"version = 1\n[policy]\nunknown_field = \"error\"\n",
)
.unwrap();
let command = RunOverrides {
agent: Some(CodingAgent::Codex),
config: Some(config_path),
Expand All @@ -2066,7 +2074,10 @@ fn plugin_config_path_overrides_sibling_plugin_file() {

assert_eq!(
resolved.gateway.plugin_config,
Some(json!({ "version": 2 }))
Some(json!({
"version": 1,
"policy": { "unknown_field": "error" }
}))
);
}

Expand Down
79 changes: 75 additions & 4 deletions crates/core/src/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1597,9 +1597,12 @@ 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. `config` wins on conflicts; as a typed document its
/// default `version`/`policy`/`enabled` override the file, while `config` bodies
/// merge field-by-field. Delegates to [`initialize_plugins_exact`].
/// 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.
pub async fn initialize_plugins(config: PluginConfig) -> Result<ConfigReport> {
let config = resolve_plugin_config(config)?;
initialize_plugins_exact(config).await
Expand All @@ -1611,10 +1614,55 @@ pub async fn initialize_plugins(config: PluginConfig) -> Result<ConfigReport> {
/// one-time configuration resolution as regular harness-native initialization.
pub(crate) fn resolve_plugin_config(config: PluginConfig) -> Result<PluginConfig> {
let mut base = resolve_default_file_plugin_config()?;
layer_config(&mut base, serde_json::to_value(config)?);
layer_config(&mut base, plugin_config_overlay_value(&config)?);
Ok(serde_json::from_value(base)?)
}

/// 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`].
fn plugin_config_overlay_value(config: &PluginConfig) -> Result<Json> {
let mut overlay = serde_json::to_value(config)?;
let Json::Object(root) = &mut overlay else {
return Ok(overlay);
};

if config.version == default_plugin_config_version() {
root.remove("version");
}

if let Some(Json::Object(policy)) = root.get_mut("policy") {
let defaults = ConfigPolicy::default();
if config.policy.unknown_component == defaults.unknown_component {
policy.remove("unknown_component");
}
if config.policy.unknown_field == defaults.unknown_field {
policy.remove("unknown_field");
}
if config.policy.unsupported_value == defaults.unsupported_value {
policy.remove("unsupported_value");
}
if policy.is_empty() {
root.remove("policy");
}
}

if let Some(Json::Array(components)) = root.get_mut("components") {
for (component, typed) in components.iter_mut().zip(&config.components) {
if typed.enabled == default_enabled()
&& let Json::Object(component) = component
{
component.remove("enabled");
}
}
}

Ok(overlay)
}

/// 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<Json> {
Expand Down Expand Up @@ -1682,13 +1730,36 @@ where
let mut merged = Json::Object(Map::new());
let mut sources = Vec::new();
for (path, document) in documents {
validate_plugin_config_version(&path, &document)?;
validate_unique_component_kinds(&path, &document)?;
layer_config(&mut merged, document);
sources.push(path);
}
Ok((!sources.is_empty()).then_some((merged, sources)))
}

/// 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<()> {
let Some(raw_version) = document.get("version") else {
return Ok(());
};
let version = serde_json::from_value::<u32>(raw_version.clone()).map_err(|error| {
PluginError::InvalidConfig(format!(
"invalid plugin config version in {}: {error}",
path.display()
))
})?;
if version == default_plugin_config_version() {
return Ok(());
}
Err(PluginError::InvalidConfig(format!(
"plugin config version {version} in {} is unsupported; expected {}",
path.display(),
default_plugin_config_version()
)))
}

/// Rejects a single file that declares the same component `kind` more than once.
fn validate_unique_component_kinds(path: &Path, document: &Json) -> Result<()> {
let Some(components) = document.get("components").and_then(Json::as_array) else {
Expand Down
94 changes: 81 additions & 13 deletions crates/core/tests/unit/plugin_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2227,6 +2227,36 @@ fn test_load_plugin_config_files_merges_files_by_precedence() {
);
}

#[test]
fn test_load_plugin_config_files_rejects_version_before_layering() {
let dir = tempfile::tempdir().unwrap();
let invalid = dir.path().join("invalid.toml");
let higher = dir.path().join("higher.toml");
std::fs::write(
&invalid,
"version = 2\n\
[[components]]\n\
kind = \"observability\"\n",
)
.unwrap();
std::fs::write(&higher, "version = 1\n").unwrap();

let error = load_plugin_config_files([invalid.clone(), higher])
.expect_err("a higher-precedence version must not mask an invalid source version");

match error {
PluginError::InvalidConfig(message) => {
assert!(message.contains("plugin config version 2"), "{message}");
assert!(
message.contains(&invalid.display().to_string()),
"{message}"
);
assert!(message.contains("expected 1"), "{message}");
}
other => panic!("unexpected plugin config version error: {other}"),
}
}

#[test]
fn test_default_plugin_config_paths_order_user_project_system() {
let dir = tempfile::tempdir().unwrap();
Expand Down Expand Up @@ -2284,13 +2314,12 @@ fn test_load_plugin_config_files_deduplicates_aliases_at_highest_precedence() {
}

#[test]
fn test_layer_config_applies_typed_overlay_defaults_over_file_base() {
// The code-vs-file path `initialize_plugins` takes: a typed `PluginConfig` is layered
// over the discovered file base. Its serde defaults (`version`/`policy`/`enabled`)
// override the file, the free-form `config` body merges, and an undeclared component
// kind is inherited from the file.
fn test_plugin_config_overlay_inherits_file_values_for_typed_defaults() {
// 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.
let file_base = json!({
"version": 2,
"version": 1,
"components": [
{
"kind": "observability",
Expand All @@ -2314,21 +2343,21 @@ fn test_layer_config_applies_typed_overlay_defaults_over_file_base() {
};

let mut merged = file_base;
layer_config(&mut merged, serde_json::to_value(code).unwrap());
layer_config(&mut merged, plugin_config_overlay_value(&code).unwrap());
let typed: PluginConfig = serde_json::from_value(merged).unwrap();

// Typed defaults override the file base.
assert_eq!(typed.version, 1, "typed default version overrides the file");
// Typed defaults do not mask the file base.
assert_eq!(typed.version, 1);
assert_eq!(
typed.policy.unknown_component,
UnsupportedBehavior::Warn,
"typed default policy overrides the file"
UnsupportedBehavior::Error,
"typed default policy inherits the file value"
);
let observability = &typed.components[0];
assert_eq!(observability.kind, "observability");
assert!(
observability.enabled,
"typed default enabled=true overrides the file's false"
!observability.enabled,
"typed default enabled=true inherits the file's false"
);
// The component config body merges: code's `mode` wins, the file's `output_directory`
// is inherited.
Expand All @@ -2337,3 +2366,42 @@ fn test_layer_config_applies_typed_overlay_defaults_over_file_base() {
// A kind the code config does not declare is inherited from the file.
assert_eq!(typed.components[1].kind, "adaptive");
}

#[test]
fn test_plugin_config_overlay_applies_non_default_values() {
let mut file_base = json!({
"version": 1,
"components": [{ "kind": "observability", "enabled": true }],
"policy": {
"unknown_component": "error",
"unknown_field": "warn",
"unsupported_value": "error"
}
});
let code = PluginConfig {
components: vec![PluginComponentSpec {
enabled: false,
..PluginComponentSpec::new("observability")
}],
policy: ConfigPolicy {
unknown_field: UnsupportedBehavior::Ignore,
..ConfigPolicy::default()
},
..PluginConfig::default()
};

layer_config(&mut file_base, plugin_config_overlay_value(&code).unwrap());
let typed: PluginConfig = serde_json::from_value(file_base).unwrap();

assert!(!typed.components[0].enabled);
assert_eq!(
typed.policy.unknown_component,
UnsupportedBehavior::Error,
"a default-valued field inherits the file"
);
assert_eq!(
typed.policy.unknown_field,
UnsupportedBehavior::Ignore,
"a non-default field overrides the file"
);
}
5 changes: 2 additions & 3 deletions crates/ffi/tests/integration/plugin_activation_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ fn run_discovered_config_activation_test() {
std::fs::write(
&plugins_toml,
format!(
r#"version = 999
r#"version = 1
Comment thread
coderabbitai[bot] marked this conversation as resolved.

[[components]]
kind = {DISCOVERED_STATIC_PLUGIN_KIND:?}
Expand Down Expand Up @@ -158,8 +158,7 @@ source = "project-file"
"config": {}
}]));

// The explicit version 1 must override the discovered version 999. The
// file-only component and its config must still survive the merge.
// The file-only component and its config must survive the merge.
assert_eq!(report["diagnostics"], json!([]));
assert_eq!(DISCOVERED_STATIC_REGISTRATIONS.load(Ordering::SeqCst), 1);
assert_eq!(
Expand Down
2 changes: 1 addition & 1 deletion go/nemo_relay/plugin_activation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ func configureNativePluginProject(t *testing.T) string {
}
pluginsTOML := filepath.Join(projectConfigDir, "plugins.toml")
const staticKind = "go.fixture.static_base"
fileConfig := fmt.Sprintf(`version = 999
fileConfig := fmt.Sprintf(`version = 1

[[components]]
kind = %q
Expand Down
Loading