Add tool registry collision policy configuration (#36954)

## What changed

- Add `features.tool_registry.error_on_tool_collisions`, defaulting to `false`, to the TOML model, generated schema, and resolved runtime configuration.
- Treat `tool_registry` as structured configuration rather than a feature toggle, including in strict config validation and profile configuration.
- Preserve the resolved collision policy in session config lockfiles when it is enabled or explicitly configured.

## Testing

- Cover deserialization, strict validation, default and enabled resolution, feature materialization, and config lockfile serialization.

GitOrigin-RevId: 2c27109dcf2a3d1e51064cc60088703e36a0f85a
This commit is contained in:
rka-oai
2026-08-04 19:28:37 +00:00
committed by copyberry
parent b87981a510
commit 9952933c1d
11 changed files with 151 additions and 6 deletions
+4
View File
@@ -114,6 +114,10 @@ pub fn features_schema(schema_gen: &mut SchemaGenerator) -> Schema {
.properties
.insert(legacy_key.to_string(), schema_gen.subschema_for::<bool>());
}
validation.properties.insert(
"tool_registry".to_string(),
schema_gen.subschema_for::<codex_features::ToolRegistryConfigToml>(),
);
validation.additional_properties = Some(Box::new(Schema::Bool(false)));
object.object = Some(Box::new(validation));
+1 -1
View File
@@ -159,7 +159,7 @@ fn push_unknown_feature_paths(
for feature_key in features
.keys()
.map(String::as_str)
.filter(|key| !is_known_feature_key(key))
.filter(|key| *key != "tool_registry" && !is_known_feature_key(key))
{
let mut path = prefix
.iter()
@@ -88,6 +88,29 @@ foo = true"#;
);
}
#[test]
fn strict_config_accepts_tool_registry_config() {
let path = Path::new("/tmp/config.toml");
for contents in [
"[features.tool_registry]\nerror_on_tool_collisions = true\n",
"[profiles.work.features.tool_registry]\nerror_on_tool_collisions = true\n",
] {
assert_eq!(
config_error_from_ignored_toml_fields::<ConfigToml>(path, contents),
None
);
}
assert!(
config_error_from_ignored_toml_fields::<ConfigToml>(
path,
"[features.tool_registry]\nunknown = true\n",
)
.is_some()
);
}
#[test]
fn strict_config_rejects_unknown_profile_feature_key() {
let path = Path::new("/tmp/config.toml");
+16
View File
@@ -726,6 +726,9 @@
"tool_call_mcp_elicitation": {
"type": "boolean"
},
"tool_registry": {
"$ref": "#/definitions/ToolRegistryConfigToml"
},
"tool_search": {
"type": "boolean"
},
@@ -3154,6 +3157,16 @@
}
]
},
"ToolRegistryConfigToml": {
"additionalProperties": false,
"properties": {
"error_on_tool_collisions": {
"description": "Fail the turn when multiple tools share the same effective name.",
"type": "boolean"
}
},
"type": "object"
},
"ToolSuggestConfig": {
"additionalProperties": false,
"properties": {
@@ -5322,6 +5335,9 @@
"tool_call_mcp_elicitation": {
"type": "boolean"
},
"tool_registry": {
"$ref": "#/definitions/ToolRegistryConfigToml"
},
"tool_search": {
"type": "boolean"
},
+30
View File
@@ -568,6 +568,36 @@ disable_in_process_fallback = true
Ok(())
}
#[tokio::test]
async fn load_config_resolves_tool_registry_config() -> std::io::Result<()> {
let codex_home = tempdir()?;
for (config_toml, error_on_tool_collisions) in [
("", false),
(
"[features.tool_registry]\nerror_on_tool_collisions = true\n",
true,
),
] {
let config_toml: ConfigToml =
toml::from_str(config_toml).expect("TOML deserialization should succeed");
let config = Config::load_from_base_config_with_overrides(
config_toml,
ConfigOverrides::default(),
codex_home.abs(),
)
.await?;
assert_eq!(
config.tool_registry.error_on_tool_collisions,
error_on_tool_collisions
);
assert!(!config.features.enabled(Feature::CodeMode));
}
Ok(())
}
#[tokio::test]
async fn load_config_resolves_token_budget_config() -> std::io::Result<()> {
for (config_toml, expected) in [
+18
View File
@@ -1030,6 +1030,9 @@ pub struct Config {
/// Whether to register the update_plan tool.
pub update_plan_enabled: bool,
/// Policy for collecting and validating tool runtimes.
pub tool_registry: ToolRegistryConfig,
/// Configuration for the experimental code-mode tool surface.
pub code_mode: CodeModeConfig,
@@ -1092,6 +1095,12 @@ pub struct Config {
pub otel: codex_config::types::OtelConfig,
}
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize)]
pub struct ToolRegistryConfig {
/// Fail the turn when multiple tools share the same effective name.
pub error_on_tool_collisions: bool,
}
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize)]
pub struct CodeModeConfig {
pub excluded_tool_namespaces: Vec<String>,
@@ -3640,6 +3649,14 @@ impl Config {
let experimental_request_user_input_enabled =
resolve_experimental_request_user_input_enabled(&cfg);
let update_plan_enabled = resolve_update_plan_enabled(&cfg);
let tool_registry = ToolRegistryConfig {
error_on_tool_collisions: cfg
.features
.as_ref()
.and_then(|features| features.tool_registry.as_ref())
.and_then(|config| config.error_on_tool_collisions)
.unwrap_or_default(),
};
let code_mode = resolve_code_mode_config(&cfg);
let multi_agent_v2 = resolve_multi_agent_v2_config(&cfg);
let token_budget = resolve_token_budget_config(&cfg, &features)?;
@@ -4172,6 +4189,7 @@ impl Config {
web_search_config,
experimental_request_user_input_enabled,
update_plan_enabled,
tool_registry,
code_mode,
use_experimental_unified_exec_tool,
background_terminal_max_timeout,
+20 -5
View File
@@ -11,6 +11,7 @@ use codex_features::FeaturesToml;
use codex_features::MultiAgentV2ConfigToml;
use codex_features::RolloutBudgetConfigToml;
use codex_features::TokenBudgetConfigToml;
use codex_features::ToolRegistryConfigToml;
use codex_protocol::ThreadId;
use crate::config::Config;
@@ -153,6 +154,11 @@ fn save_config_resolved_fields(
.features
.get_or_insert_with(FeaturesToml::default);
features.materialize_resolved_enabled(config.features.get());
if config.tool_registry.error_on_tool_collisions || features.tool_registry.is_some() {
features.tool_registry = Some(ToolRegistryConfigToml {
error_on_tool_collisions: Some(config.tool_registry.error_on_tool_collisions),
});
}
let mut multi_agent_v2: MultiAgentV2ConfigToml =
resolved_config_to_toml(&config.multi_agent_v2, "features.multi_agent_v2")?;
multi_agent_v2.enabled = Some(config.features.enabled(Feature::MultiAgentV2));
@@ -262,6 +268,7 @@ mod tests {
async fn lock_contains_prompts_and_materializes_features() {
let mut sc = crate::session::tests::make_session_configuration_for_tests().await;
let mut config = (*sc.original_config_do_not_use).clone();
config.tool_registry.error_on_tool_collisions = true;
config.multi_agent_v2.subagent_developer_instructions =
Some("Locked subagent developer instructions.".to_string());
config.token_budget = Some(crate::config::TokenBudgetConfig {
@@ -318,6 +325,12 @@ mod tests {
.features
.as_ref()
.expect("lock should materialize feature states");
assert_eq!(
features.tool_registry,
Some(ToolRegistryConfigToml {
error_on_tool_collisions: Some(true),
})
);
let feature_entries = features.entries();
for spec in codex_features::FEATURES {
assert_eq!(
@@ -404,15 +417,17 @@ mod tests {
sc.original_config_do_not_use = Arc::new(config);
let lockfile = sc.to_config_lockfile_toml().expect("lock should serialize");
let features = lockfile
.config
.features
.as_ref()
.expect("lock should materialize feature states");
assert_eq!(
lockfile
.config
.features
.as_ref()
.and_then(|features| features.token_budget.as_ref()),
features.token_budget.as_ref(),
Some(&FeatureToml::Enabled(true))
);
assert_eq!(features.tool_registry, None);
}
#[tokio::test]
+8
View File
@@ -4,6 +4,14 @@ use serde::Deserialize;
use serde::Serialize;
use std::collections::BTreeMap;
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)]
#[serde(deny_unknown_fields)]
pub struct ToolRegistryConfigToml {
/// Fail the turn when multiple tools share the same effective name.
#[serde(skip_serializing_if = "Option::is_none")]
pub error_on_tool_collisions: Option<bool>,
}
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)]
#[serde(deny_unknown_fields)]
pub struct CodeModeConfigToml {
+4
View File
@@ -30,6 +30,7 @@ pub use feature_configs::NonPrefixedMcpToolNamesConfigToml;
use feature_configs::RemovedAppsMcpPathOverrideConfigToml;
pub use feature_configs::RolloutBudgetConfigToml;
pub use feature_configs::TokenBudgetConfigToml;
pub use feature_configs::ToolRegistryConfigToml;
use legacy::LegacyFeatureToggles;
pub use legacy::legacy_feature_keys;
@@ -658,6 +659,8 @@ pub fn is_known_feature_key(key: &str) -> bool {
/// Deserializable features table for TOML.
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, JsonSchema)]
pub struct FeaturesToml {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub tool_registry: Option<ToolRegistryConfigToml>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub code_mode: Option<FeatureToml<CodeModeConfigToml>>,
#[serde(default, skip_serializing_if = "Option::is_none")]
@@ -736,6 +739,7 @@ impl FeaturesToml {
pub fn materialize_resolved_enabled(&mut self, features: &Features) {
self.clear_removed_compatibility_entries();
let Self {
tool_registry: _,
code_mode,
code_mode_host,
non_prefixed_mcp_tool_names,
+26
View File
@@ -27,6 +27,22 @@ fn under_development_features_are_disabled_by_default() {
}
}
#[test]
fn tool_registry_config_is_not_a_feature_toggle() {
let features: FeaturesToml =
toml::from_str("[tool_registry]\nerror_on_tool_collisions = true\n")
.expect("tool registry settings should deserialize");
assert_eq!(
features.tool_registry,
Some(crate::ToolRegistryConfigToml {
error_on_tool_collisions: Some(true),
})
);
assert!(features.entries().is_empty());
assert!(!crate::is_known_feature_key("tool_registry"));
}
#[test]
fn executor_capability_discovery_is_an_opt_in_map_feature() {
let mut features = Features::with_defaults();
@@ -533,6 +549,9 @@ fn materialize_resolved_enabled_writes_all_features_and_preserves_custom_config(
features.enable(Feature::RespectSystemProxy);
let mut features_toml = FeaturesToml {
tool_registry: Some(crate::ToolRegistryConfigToml {
error_on_tool_collisions: Some(true),
}),
code_mode_host: Some(FeatureToml::Config(crate::CodeModeHostConfigToml {
enabled: Some(false),
disable_in_process_fallback: Some(true),
@@ -560,7 +579,14 @@ fn materialize_resolved_enabled_writes_all_features_and_preserves_custom_config(
features_toml.materialize_resolved_enabled(&features);
assert_eq!(
features_toml.tool_registry,
Some(crate::ToolRegistryConfigToml {
error_on_tool_collisions: Some(true),
})
);
let entries = features_toml.entries();
assert!(!entries.contains_key("tool_registry"));
for spec in crate::FEATURES {
assert_eq!(
entries.get(spec.key),
@@ -295,6 +295,7 @@ fn new_config(model: Option<String>, arg0_paths: Arg0DispatchPaths) -> anyhow::R
web_search_config: None,
experimental_request_user_input_enabled: true,
update_plan_enabled: true,
tool_registry: Default::default(),
code_mode: Default::default(),
use_experimental_unified_exec_tool: false,
background_terminal_max_timeout: 300_000,