From bc2f2cee826e30a28c90369efb17f33e30733e26 Mon Sep 17 00:00:00 2001 From: Felix Xia Date: Tue, 23 Jun 2026 20:07:16 +0100 Subject: [PATCH] Merge MCP matchers into server requirements --- codex-rs/config/src/config_requirements.rs | 164 ++++++++---- codex-rs/config/src/constraint.rs | 4 +- codex-rs/config/src/lib.rs | 1 - codex-rs/config/src/mcp_requirements.rs | 34 ++- codex-rs/config/src/mcp_requirements_tests.rs | 37 ++- .../config/src/requirements_layers/stack.rs | 122 ++++++++- .../src/requirements_layers/stack_tests.rs | 62 +++-- codex-rs/core/src/config/config_tests.rs | 234 ++++-------------- codex-rs/core/src/config/mod.rs | 120 ++------- codex-rs/tui/src/debug_config.rs | 6 +- 10 files changed, 407 insertions(+), 377 deletions(-) diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index 92950310aa..94ec1b9c0e 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -18,7 +18,8 @@ use super::requirements_exec_policy::RequirementsExecPolicyToml; use crate::Constrained; use crate::ConstraintError; use crate::ManagedHooksRequirementsToml; -use crate::McpServerMatcher; +use crate::McpServerCommandMatcher; +use crate::McpServerUrlMatcher; use crate::mcp_types::AppToolApproval; use crate::permissions_toml::PermissionProfileToml; use crate::types::WindowsSandboxModeToml; @@ -155,7 +156,6 @@ pub struct ConfigRequirements { pub feature_requirements: Option>, pub managed_hooks: Option>, pub mcp_servers: Option>>, - pub mcp_server_matchers: Option>>, pub plugins: Option>>, pub exec_policy: Option>, pub enforce_residency: ConstrainedWithSource>, @@ -197,7 +197,6 @@ impl Default for ConfigRequirements { feature_requirements: None, managed_hooks: None, mcp_servers: None, - mcp_server_matchers: None, plugins: None, exec_policy: None, enforce_residency: ConstrainedWithSource::new( @@ -224,9 +223,17 @@ pub enum McpServerIdentity { Url { url: String }, } +/// A requirement for one named MCP server. +/// +/// The `Identity` variant preserves the released exact-match contract. The +/// command and URL variants add matcher-based requirements under the same +/// `mcp_servers` namespace. #[derive(Deserialize, Debug, Clone, PartialEq, Eq)] -pub struct McpServerRequirement { - pub identity: McpServerIdentity, +#[serde(untagged, deny_unknown_fields)] +pub enum McpServerRequirement { + Identity { identity: McpServerIdentity }, + Command(McpServerCommandMatcher), + Url(McpServerUrlMatcher), } #[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)] @@ -844,7 +851,6 @@ pub struct ConfigRequirementsToml { pub feature_requirements: Option, pub hooks: Option, pub mcp_servers: Option>, - pub mcp_server_matchers: Option>, pub plugins: Option>, pub apps: Option, pub rules: Option, @@ -899,7 +905,6 @@ pub struct ConfigRequirementsWithSources { pub feature_requirements: Option>, pub hooks: Option>, pub mcp_servers: Option>>, - pub mcp_server_matchers: Option>>, pub plugins: Option>>, pub apps: Option>, pub rules: Option>, @@ -943,7 +948,6 @@ impl ConfigRequirementsWithSources { feature_requirements: _, hooks: _, mcp_servers: _, - mcp_server_matchers: _, plugins: _, apps: _, rules: _, @@ -980,7 +984,6 @@ impl ConfigRequirementsWithSources { feature_requirements, hooks, mcp_servers, - mcp_server_matchers, plugins, rules, enforce_residency, @@ -1015,7 +1018,6 @@ impl ConfigRequirementsWithSources { feature_requirements, hooks, mcp_servers, - mcp_server_matchers, plugins, apps, rules, @@ -1040,7 +1042,6 @@ impl ConfigRequirementsWithSources { feature_requirements: feature_requirements.map(|sourced| sourced.value), hooks: hooks.map(|sourced| sourced.value), mcp_servers: mcp_servers.map(|sourced| sourced.value), - mcp_server_matchers: mcp_server_matchers.map(|sourced| sourced.value), plugins: plugins.map(|sourced| sourced.value), apps: apps.map(|sourced| sourced.value), rules: rules.map(|sourced| sourced.value), @@ -1143,7 +1144,6 @@ impl ConfigRequirementsToml { .as_ref() .is_none_or(ManagedHooksRequirementsToml::is_empty) && self.mcp_servers.is_none() - && self.mcp_server_matchers.is_none() && self .plugins .as_ref() @@ -1163,6 +1163,25 @@ impl ConfigRequirementsToml { } } +fn validate_mcp_server_requirements( + requirements: &BTreeMap, + source: &RequirementSource, + plugin_name: Option<&str>, +) -> Result<(), ConstraintError> { + for (server_name, requirement) in requirements { + requirement + .validate() + .map_err(|reason| ConstraintError::McpServerRequirementParse { + server_name: plugin_name + .map(|plugin_name| format!("{plugin_name}/{server_name}")) + .unwrap_or_else(|| server_name.clone()), + requirement_source: source.clone(), + reason, + })?; + } + Ok(()) +} + impl TryFrom for ConfigRequirements { type Error = ConstraintError; @@ -1185,7 +1204,6 @@ impl TryFrom for ConfigRequirements { feature_requirements, hooks, mcp_servers, - mcp_server_matchers, plugins, apps: _apps, rules, @@ -1195,19 +1213,18 @@ impl TryFrom for ConfigRequirements { guardian_policy_config, } = toml; - if let Some(Sourced { - value: matchers, - source, - }) = &mcp_server_matchers - { - for (server_name, matcher) in matchers { - matcher - .validate() - .map_err(|reason| ConstraintError::McpServerMatcherParse { - server_name: server_name.clone(), - requirement_source: source.clone(), - reason, - })?; + if let Some(requirements) = &mcp_servers { + validate_mcp_server_requirements(&requirements.value, &requirements.source, None)?; + } + if let Some(plugin_requirements) = &plugins { + for (plugin_name, plugin) in &plugin_requirements.value { + if let Some(requirements) = &plugin.mcp_servers { + validate_mcp_server_requirements( + requirements, + &plugin_requirements.source, + Some(plugin_name), + )?; + } } } @@ -1482,7 +1499,6 @@ impl TryFrom for ConfigRequirements { feature_requirements, managed_hooks, mcp_servers, - mcp_server_matchers, plugins, exec_policy, enforce_residency, @@ -1578,7 +1594,6 @@ mod tests { feature_requirements, hooks, mcp_servers, - mcp_server_matchers, plugins, apps, rules, @@ -1612,8 +1627,6 @@ mod tests { .map(|value| Sourced::new(value, RequirementSource::Unknown)), hooks: hooks.map(|value| Sourced::new(value, RequirementSource::Unknown)), mcp_servers: mcp_servers.map(|value| Sourced::new(value, RequirementSource::Unknown)), - mcp_server_matchers: mcp_server_matchers - .map(|value| Sourced::new(value, RequirementSource::Unknown)), plugins: plugins.map(|value| Sourced::new(value, RequirementSource::Unknown)), apps: apps.map(|value| Sourced::new(value, RequirementSource::Unknown)), rules: rules.map(|value| Sourced::new(value, RequirementSource::Unknown)), @@ -1817,7 +1830,6 @@ mod tests { feature_requirements: Some(feature_requirements.clone()), hooks: None, mcp_servers: None, - mcp_server_matchers: None, plugins: None, apps: None, rules: None, @@ -1867,7 +1879,6 @@ mod tests { )), hooks: None, mcp_servers: None, - mcp_server_matchers: None, plugins: None, apps: None, rules: None, @@ -1914,7 +1925,6 @@ mod tests { feature_requirements: None, hooks: None, mcp_servers: None, - mcp_server_matchers: None, plugins: None, apps: None, rules: None, @@ -1969,7 +1979,6 @@ mod tests { feature_requirements: None, hooks: None, mcp_servers: None, - mcp_server_matchers: None, plugins: None, apps: None, rules: None, @@ -3461,7 +3470,7 @@ command = "python3 /enterprise/hooks/pre.py" BTreeMap::from([ ( "docs".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "codex-mcp".to_string(), }, @@ -3469,7 +3478,7 @@ command = "python3 /enterprise/hooks/pre.py" ), ( "remote".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Url { url: "https://example.com/mcp".to_string(), }, @@ -3483,9 +3492,9 @@ command = "python3 /enterprise/hooks/pre.py" } #[test] - fn deserialize_mcp_server_matchers() -> Result<()> { + fn deserialize_mcp_server_matcher_requirements() -> Result<()> { let toml_str = r#" - [mcp_server_matchers.internal_mcp_proxy] + [mcp_servers.internal_mcp_proxy] command = "company-cli" args = [ { match = "exact", value = "mcp" }, @@ -3498,11 +3507,11 @@ command = "python3 /enterprise/hooks/pre.py" with_unknown_source(from_str(toml_str)?).try_into()?; assert_eq!( - requirements.mcp_server_matchers, + requirements.mcp_servers, Some(Sourced::new( BTreeMap::from([( "internal_mcp_proxy".to_string(), - McpServerMatcher::Command(McpServerCommandMatcher { + McpServerRequirement::Command(McpServerCommandMatcher { command: "company-cli".to_string(), args: vec![ McpServerValueMatcher::Exact { @@ -3528,15 +3537,15 @@ command = "python3 /enterprise/hooks/pre.py" } #[test] - fn invalid_mcp_server_matcher_regex_reports_the_server_name_and_source() -> Result<()> { + fn invalid_mcp_server_requirement_regex_reports_the_server_name_and_source() -> Result<()> { let toml_str = r#" - [mcp_server_matchers.broken_rule] + [mcp_servers.broken_rule] url = { match = "regex", expression = "[" } "#; let err = ConfigRequirements::try_from(with_unknown_source(from_str(toml_str)?)) .expect_err("invalid matcher regex should fail requirements normalization"); - let ConstraintError::McpServerMatcherParse { + let ConstraintError::McpServerRequirementParse { server_name, requirement_source, reason, @@ -3572,7 +3581,7 @@ command = "python3 /enterprise/hooks/pre.py" PluginRequirementsToml { mcp_servers: Some(BTreeMap::from([( "remote".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Url { url: "https://example.com/mcp".to_string(), }, @@ -3585,7 +3594,7 @@ command = "python3 /enterprise/hooks/pre.py" PluginRequirementsToml { mcp_servers: Some(BTreeMap::from([( "sample".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "sample-mcp".to_string(), }, @@ -3600,6 +3609,71 @@ command = "python3 /enterprise/hooks/pre.py" Ok(()) } + #[test] + fn deserialize_plugin_mcp_server_matcher_requirement() -> Result<()> { + let toml_str = r#" + [plugins."sample@test".mcp_servers.internal_proxy] + command = "company-cli" + args = [ + { match = "exact", value = "mcp" }, + { match = "regex", expression = '^https://[a-z]+\.example\.com$' }, + ] + "#; + let requirements: ConfigRequirements = + with_unknown_source(from_str(toml_str)?).try_into()?; + + assert_eq!( + requirements.plugins, + Some(Sourced::new( + BTreeMap::from([( + "sample@test".to_string(), + PluginRequirementsToml { + mcp_servers: Some(BTreeMap::from([( + "internal_proxy".to_string(), + McpServerRequirement::Command(McpServerCommandMatcher { + command: "company-cli".to_string(), + args: vec![ + McpServerValueMatcher::Exact { + value: "mcp".to_string(), + }, + McpServerValueMatcher::Regex { + expression: r"^https://[a-z]+\.example\.com$".to_string(), + }, + ], + }), + )])), + }, + )]), + RequirementSource::Unknown, + )) + ); + Ok(()) + } + + #[test] + fn invalid_plugin_mcp_server_regex_reports_plugin_and_server_name() -> Result<()> { + let toml_str = r#" + [plugins."sample@test".mcp_servers.broken_rule] + url = { match = "regex", expression = "[" } + "#; + + let err = ConfigRequirements::try_from(with_unknown_source(from_str(toml_str)?)) + .expect_err("invalid plugin MCP regex should fail requirements normalization"); + let ConstraintError::McpServerRequirementParse { + server_name, + requirement_source, + reason, + } = err + else { + panic!("unexpected error: {err:?}"); + }; + + assert_eq!(server_name, "sample@test/broken_rule"); + assert_eq!(requirement_source, RequirementSource::Unknown); + assert!(reason.contains("invalid regex `[`"), "{reason}"); + Ok(()) + } + #[test] fn deserialize_exec_policy_requirements() -> Result<()> { let toml_str = r#" diff --git a/codex-rs/config/src/constraint.rs b/codex-rs/config/src/constraint.rs index 98250e1ee7..8628f3909c 100644 --- a/codex-rs/config/src/constraint.rs +++ b/codex-rs/config/src/constraint.rs @@ -26,9 +26,9 @@ pub enum ConstraintError { }, #[error( - "invalid matcher for MCP server `{server_name}` in requirements (set by {requirement_source}): {reason}" + "invalid requirement for MCP server `{server_name}` (set by {requirement_source}): {reason}" )] - McpServerMatcherParse { + McpServerRequirementParse { server_name: String, requirement_source: RequirementSource, reason: String, diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index fab0d9f5a8..4be07ece28 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -110,7 +110,6 @@ pub use marketplace_edit::remove_user_marketplace_config; pub use mcp_edit::ConfigEditsBuilder; pub use mcp_edit::load_global_mcp_servers; pub use mcp_requirements::McpServerCommandMatcher; -pub use mcp_requirements::McpServerMatcher; pub use mcp_requirements::McpServerUrlMatcher; pub use mcp_requirements::McpServerValueMatcher; pub use mcp_types::AppToolApproval; diff --git a/codex-rs/config/src/mcp_requirements.rs b/codex-rs/config/src/mcp_requirements.rs index a420e1de54..be49fd15ec 100644 --- a/codex-rs/config/src/mcp_requirements.rs +++ b/codex-rs/config/src/mcp_requirements.rs @@ -1,3 +1,5 @@ +use crate::McpServerIdentity; +use crate::McpServerRequirement; use crate::mcp_types::McpServerConfig; use crate::mcp_types::McpServerTransportConfig; use regex_lite::Regex; @@ -49,17 +51,10 @@ pub struct McpServerUrlMatcher { pub url: McpServerValueMatcher, } -/// A managed matcher for either a stdio command invocation or a direct MCP URL. -#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] -#[serde(untagged)] -pub enum McpServerMatcher { - Command(McpServerCommandMatcher), - Url(McpServerUrlMatcher), -} - -impl McpServerMatcher { +impl McpServerRequirement { pub(crate) fn validate(&self) -> Result<(), String> { match self { + Self::Identity { .. } => Ok(()), Self::Command(matcher) => { for (index, arg) in matcher.args.iter().enumerate() { arg.validate().map_err(|err| { @@ -74,6 +69,24 @@ impl McpServerMatcher { pub fn matches(&self, server: &McpServerConfig) -> bool { match (self, &server.transport) { + ( + Self::Identity { + identity: + McpServerIdentity::Command { + command: want_command, + }, + }, + McpServerTransportConfig::Stdio { + command: got_command, + .. + }, + ) => got_command == want_command, + ( + Self::Identity { + identity: McpServerIdentity::Url { url: want_url }, + }, + McpServerTransportConfig::StreamableHttp { url: got_url, .. }, + ) => got_url == want_url, (Self::Command(matcher), McpServerTransportConfig::Stdio { command, args, .. }) => { matcher.command == *command && matcher.args.len() == args.len() @@ -86,8 +99,7 @@ impl McpServerMatcher { (Self::Url(matcher), McpServerTransportConfig::StreamableHttp { url, .. }) => { matcher.url.matches(url) } - (Self::Command(_), McpServerTransportConfig::StreamableHttp { .. }) - | (Self::Url(_), McpServerTransportConfig::Stdio { .. }) => false, + _ => false, } } } diff --git a/codex-rs/config/src/mcp_requirements_tests.rs b/codex-rs/config/src/mcp_requirements_tests.rs index afcb4a8f20..f0b4b7b268 100644 --- a/codex-rs/config/src/mcp_requirements_tests.rs +++ b/codex-rs/config/src/mcp_requirements_tests.rs @@ -31,7 +31,7 @@ fn stdio_server(command: &str, args: &[&str]) -> McpServerConfig { #[test] fn command_matcher_matches_exact_positional_arguments() { - let matcher = McpServerMatcher::Command(McpServerCommandMatcher { + let requirement = McpServerRequirement::Command(McpServerCommandMatcher { command: "company-cli".to_string(), args: vec![ McpServerValueMatcher::Exact { @@ -43,19 +43,19 @@ fn command_matcher_matches_exact_positional_arguments() { ], }); - assert!(matcher.matches(&stdio_server( + assert!(requirement.matches(&stdio_server( "company-cli", &["mcp", "https://pricing.example.com"] ))); - assert!(!matcher.matches(&stdio_server( + assert!(!requirement.matches(&stdio_server( "company-cli", &["https://pricing.example.com", "mcp"] ))); - assert!(!matcher.matches(&stdio_server( + assert!(!requirement.matches(&stdio_server( "company-cli", &["mcp", "https://pricing.example.com", "--verbose"] ))); - assert!(!matcher.matches(&stdio_server( + assert!(!requirement.matches(&stdio_server( "/usr/local/bin/company-cli", &["mcp", "https://pricing.example.com"] ))); @@ -82,8 +82,25 @@ fn regex_matcher_allows_a_later_alternative_to_match_the_full_value() { } #[test] -fn matcher_deserializes_command_and_url_shapes() { - let command: McpServerMatcher = toml::from_str( +fn legacy_command_identity_keeps_ignoring_arguments() { + let requirement: McpServerRequirement = toml::from_str( + r#" +[identity] +command = "company-cli" +"#, + ) + .expect("legacy command identity"); + + assert!(requirement.matches(&stdio_server( + "company-cli", + &["any", "arguments", "remain", "allowed"] + ))); + assert!(!requirement.matches(&stdio_server("different-cli", &[]))); +} + +#[test] +fn requirement_deserializes_command_and_url_matcher_shapes() { + let command: McpServerRequirement = toml::from_str( r#" command = "company-cli" args = [ @@ -93,7 +110,7 @@ args = [ "#, ) .expect("command matcher"); - let url: McpServerMatcher = toml::from_str( + let url: McpServerRequirement = toml::from_str( r#" url = { match = "prefix", value = "https://mcp.example.com/" } "#, @@ -102,7 +119,7 @@ url = { match = "prefix", value = "https://mcp.example.com/" } assert_eq!( command, - McpServerMatcher::Command(McpServerCommandMatcher { + McpServerRequirement::Command(McpServerCommandMatcher { command: "company-cli".to_string(), args: vec![ McpServerValueMatcher::Exact { @@ -116,7 +133,7 @@ url = { match = "prefix", value = "https://mcp.example.com/" } ); assert_eq!( url, - McpServerMatcher::Url(McpServerUrlMatcher { + McpServerRequirement::Url(McpServerUrlMatcher { url: McpServerValueMatcher::Prefix { value: "https://mcp.example.com/".to_string(), }, diff --git a/codex-rs/config/src/requirements_layers/stack.rs b/codex-rs/config/src/requirements_layers/stack.rs index 0a5208ea80..2c441ca545 100644 --- a/codex-rs/config/src/requirements_layers/stack.rs +++ b/codex-rs/config/src/requirements_layers/stack.rs @@ -10,6 +10,8 @@ //! - `rules.prefix_rules` append high-priority rules first. //! - `hooks` append high-priority event groups first while failing closed on //! active managed-dir conflicts. +//! - Incompatible definitions of the same named MCP server fail composition +//! instead of silently combining legacy identities with matcher rules. //! - `permissions.filesystem.deny_read` is a high-priority-first union across //! layers. @@ -19,6 +21,7 @@ use crate::RequirementSource; use crate::Sourced; use crate::merge::merge_toml_values; use std::cell::OnceCell; +use std::collections::BTreeMap; use std::io; use thiserror::Error; use toml::Value as TomlValue; @@ -148,6 +151,7 @@ impl RequirementsLayerStack { hook_directory_field, } = self; + validate_mcp_server_requirement_shapes(&layers)?; let mut merged_toml = TomlValue::Table(toml::map::Map::new()); for layer in &layers { merge_toml_values(&mut merged_toml, &layer.regular_toml); @@ -187,6 +191,122 @@ impl RequirementsLayerStack { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum McpServerRequirementShape { + LegacyCommandIdentity, + LegacyUrlIdentity, + CommandMatcher, + ExactUrlMatcher, + PrefixUrlMatcher, + RegexUrlMatcher, +} + +impl std::fmt::Display for McpServerRequirementShape { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::LegacyCommandIdentity => f.write_str("legacy command identity"), + Self::LegacyUrlIdentity => f.write_str("legacy URL identity"), + Self::CommandMatcher => f.write_str("command matcher"), + Self::ExactUrlMatcher => f.write_str("exact URL matcher"), + Self::PrefixUrlMatcher => f.write_str("prefix URL matcher"), + Self::RegexUrlMatcher => f.write_str("regex URL matcher"), + } + } +} + +fn validate_mcp_server_requirement_shapes( + layers: &[ComposableRequirementsLayer], +) -> Result<(), RequirementsCompositionError> { + let mut seen = BTreeMap::::new(); + for layer in layers { + if let Some(requirements) = table_at_path(&layer.regular_toml, &["mcp_servers"]) { + validate_mcp_server_requirement_table( + requirements, + "mcp_servers", + &layer.source, + &mut seen, + )?; + } + + let Some(plugins) = table_at_path(&layer.regular_toml, &["plugins"]) else { + continue; + }; + for plugin_name in plugins.keys() { + let Some(requirements) = table_at_path( + &layer.regular_toml, + &["plugins", plugin_name, "mcp_servers"], + ) else { + continue; + }; + validate_mcp_server_requirement_table( + requirements, + &format!("plugins.\"{plugin_name}\".mcp_servers"), + &layer.source, + &mut seen, + )?; + } + } + Ok(()) +} + +fn validate_mcp_server_requirement_table( + requirements: &toml::Table, + table_path: &str, + source: &RequirementSource, + seen: &mut BTreeMap, +) -> Result<(), RequirementsCompositionError> { + for (server_name, requirement) in requirements { + let Some(shape) = mcp_server_requirement_shape(requirement) else { + continue; + }; + let field = format!("{table_path}.{server_name}"); + if let Some((existing_shape, existing_source)) = seen.get(&field) + && *existing_shape != shape + { + return Err(composition_conflict( + field, + existing_source.clone(), + source.clone(), + format!( + "cannot combine {existing_shape} with {shape}; define this MCP server using one requirement form" + ), + )); + } + seen.insert(field, (shape, source.clone())); + } + Ok(()) +} + +fn mcp_server_requirement_shape(value: &TomlValue) -> Option { + let requirement = value.as_table()?; + if let Some(identity) = requirement.get("identity").and_then(TomlValue::as_table) { + if identity.contains_key("command") { + return Some(McpServerRequirementShape::LegacyCommandIdentity); + } + if identity.contains_key("url") { + return Some(McpServerRequirementShape::LegacyUrlIdentity); + } + return None; + } + if requirement.contains_key("command") { + return Some(McpServerRequirementShape::CommandMatcher); + } + let matcher = requirement.get("url")?.as_table()?; + match matcher.get("match")?.as_str()? { + "exact" => Some(McpServerRequirementShape::ExactUrlMatcher), + "prefix" => Some(McpServerRequirementShape::PrefixUrlMatcher), + "regex" => Some(McpServerRequirementShape::RegexUrlMatcher), + _ => None, + } +} + +fn table_at_path<'a>(mut value: &'a TomlValue, path: &[&str]) -> Option<&'a toml::Table> { + for key in path { + value = value.as_table()?.get(*key)?; + } + value.as_table() +} + fn populate_merged_regular_fields_with_sources( output: &mut ConfigRequirementsWithSources, requirements: ConfigRequirementsToml, @@ -221,7 +341,6 @@ fn populate_merged_regular_fields_with_sources( feature_requirements, hooks: _, mcp_servers, - mcp_server_matchers, plugins, apps, rules: _, @@ -250,7 +369,6 @@ fn populate_merged_regular_fields_with_sources( set_sourced!(windows, &["windows"]); set_sourced!(feature_requirements, &["features", "feature_requirements"]); set_sourced!(mcp_servers, &["mcp_servers"]); - set_sourced!(mcp_server_matchers, &["mcp_server_matchers"]); set_sourced!(plugins, &["plugins"]); set_sourced!(apps, &["apps"]); set_sourced!(enforce_residency, &["enforce_residency"]); diff --git a/codex-rs/config/src/requirements_layers/stack_tests.rs b/codex-rs/config/src/requirements_layers/stack_tests.rs index 469eb7c8fc..8239f90982 100644 --- a/codex-rs/config/src/requirements_layers/stack_tests.rs +++ b/codex-rs/config/src/requirements_layers/stack_tests.rs @@ -390,43 +390,65 @@ command = "high-mcp" } #[test] -fn mcp_server_matchers_use_regular_toml_merge() { - let composed = compose(vec![ +fn mcp_requirements_reject_mixed_forms_across_layers() { + let err = compose(vec![ layer( "req_low", "Low", r#" -[mcp_server_matchers.shared] -url = { match = "prefix", value = "https://low.example.com/" } - -[mcp_server_matchers.low] -url = { match = "exact", value = "https://low.example.com/mcp" } +[mcp_servers.shared.identity] +command = "low-mcp" "#, ), layer( "req_high", "High", r#" -[mcp_server_matchers.shared] -url = { match = "prefix", value = "https://high.example.com/" } +[mcp_servers.shared] +command = "company-cli" +args = [{ match = "exact", value = "mcp" }] "#, ), ]) - .expect("compose requirements") - .expect("requirements present"); + .expect_err("mixed MCP requirement forms should fail composition"); - assert_eq!( - composed, - expected_requirements( + assert!(err.to_string().contains("mcp_servers.shared")); + assert!(err.to_string().contains("legacy command identity")); + assert!(err.to_string().contains("command matcher")); + assert!(err.to_string().contains("High (req_high)")); + assert!(err.to_string().contains("Low (req_low)")); +} + +#[test] +fn plugin_mcp_requirements_reject_mixed_forms_across_layers() { + let err = compose(vec![ + layer( + "req_low", + "Low", r#" -[mcp_server_matchers.low] -url = { match = "exact", value = "https://low.example.com/mcp" } +[plugins."sample@test".mcp_servers.shared.identity] +command = "low-plugin-mcp" +"#, + ), + layer( + "req_high", + "High", + r#" +[plugins."sample@test".mcp_servers.shared] +url = { match = "regex", expression = '^https://high\.example\.com/mcp$' } +"#, + ), + ]) + .expect_err("mixed plugin MCP requirement forms should fail composition"); -[mcp_server_matchers.shared] -url = { match = "prefix", value = "https://high.example.com/" } -"# - ) + assert!( + err.to_string() + .contains("plugins.\"sample@test\".mcp_servers.shared") ); + assert!(err.to_string().contains("legacy command identity")); + assert!(err.to_string().contains("regex URL matcher")); + assert!(err.to_string().contains("High (req_high)")); + assert!(err.to_string().contains("Low (req_low)")); } #[test] diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index b8db7d97cc..f775a6480d 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -6,7 +6,8 @@ use codex_config::CONFIG_TOML_FILE; use codex_config::ConfigLayerEntry; use codex_config::ConfigLayerStack; use codex_config::McpServerCommandMatcher; -use codex_config::McpServerMatcher; +use codex_config::McpServerIdentity; +use codex_config::McpServerRequirement; use codex_config::McpServerValueMatcher; use codex_config::ProfileV2Name; use codex_config::RequirementSource; @@ -4146,7 +4147,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() { BTreeMap::from([ ( MISMATCHED_URL_SERVER.to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Url { url: "https://example.com/other".to_string(), }, @@ -4154,7 +4155,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() { ), ( MISMATCHED_COMMAND_SERVER.to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "other-cmd".to_string(), }, @@ -4162,7 +4163,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() { ), ( MATCHED_URL_SERVER.to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Url { url: GOOD_URL.to_string(), }, @@ -4170,7 +4171,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() { ), ( MATCHED_COMMAND_SERVER.to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: GOOD_CMD.to_string(), }, @@ -4179,11 +4180,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() { ]), source.clone(), ); - filter_mcp_servers_by_requirements( - &mut servers, - Some(&requirements), - /*mcp_matchers*/ None, - ); + filter_mcp_servers_by_requirements(&mut servers, Some(&requirements)); let reason = Some(McpServerDisabledReason::Requirements { source }); assert_eq!( @@ -4214,11 +4211,7 @@ fn filter_mcp_servers_by_allowlist_allows_all_when_unset() { ("server-b".to_string(), http_mcp("https://example.com/b")), ]); - filter_mcp_servers_by_requirements( - &mut servers, - /*mcp_requirements*/ None, - /*mcp_matchers*/ None, - ); + filter_mcp_servers_by_requirements(&mut servers, /*mcp_requirements*/ None); assert_eq!( servers @@ -4296,7 +4289,7 @@ fn filter_mcp_servers_by_matchers_enforces_command_and_positional_args() { ), ]); let source = RequirementSource::LegacyManagedConfigTomlFromMdm; - let matcher = McpServerMatcher::Command(McpServerCommandMatcher { + let requirement = McpServerRequirement::Command(McpServerCommandMatcher { command: "company-cli".to_string(), args: vec![ McpServerValueMatcher::Exact { @@ -4315,21 +4308,17 @@ fn filter_mcp_servers_by_matchers_enforces_command_and_positional_args() { }, ], }); - let matchers = Sourced::new( + let requirements = Sourced::new( BTreeMap::from([ - ("internal_mcp_proxy".to_string(), matcher.clone()), - ("wrong-order".to_string(), matcher.clone()), - ("trailing-arg".to_string(), matcher.clone()), - ("wrong-host".to_string(), matcher), + ("internal_mcp_proxy".to_string(), requirement.clone()), + ("wrong-order".to_string(), requirement.clone()), + ("trailing-arg".to_string(), requirement.clone()), + ("wrong-host".to_string(), requirement), ]), source.clone(), ); - filter_mcp_servers_by_requirements( - &mut servers, - /*mcp_requirements*/ None, - Some(&matchers), - ); + filter_mcp_servers_by_requirements(&mut servers, Some(&requirements)); let reason = Some(McpServerDisabledReason::Requirements { source }); assert_eq!( @@ -4350,90 +4339,6 @@ fn filter_mcp_servers_by_matchers_enforces_command_and_positional_args() { ); } -#[test] -fn filter_mcp_servers_by_requirements_enforces_all_same_name_rules() { - const BOTH_MATCH: &str = "both-match"; - const EXACT_ONLY_MATCHES: &str = "exact-only-matches"; - const MATCHER_ONLY_MATCHES: &str = "matcher-only-matches"; - - let mut servers = HashMap::from([ - ( - BOTH_MATCH.to_string(), - stdio_mcp_with_args("company-cli", &["approved"]), - ), - ( - EXACT_ONLY_MATCHES.to_string(), - stdio_mcp_with_args("company-cli", &["rejected"]), - ), - ( - MATCHER_ONLY_MATCHES.to_string(), - stdio_mcp_with_args("company-cli", &["approved"]), - ), - ]); - let source = RequirementSource::LegacyManagedConfigTomlFromMdm; - let requirements = Sourced::new( - BTreeMap::from([ - ( - BOTH_MATCH.to_string(), - McpServerRequirement { - identity: McpServerIdentity::Command { - command: "company-cli".to_string(), - }, - }, - ), - ( - EXACT_ONLY_MATCHES.to_string(), - McpServerRequirement { - identity: McpServerIdentity::Command { - command: "company-cli".to_string(), - }, - }, - ), - ( - MATCHER_ONLY_MATCHES.to_string(), - McpServerRequirement { - identity: McpServerIdentity::Command { - command: "different-command".to_string(), - }, - }, - ), - ]), - source.clone(), - ); - let matcher = McpServerMatcher::Command(McpServerCommandMatcher { - command: "company-cli".to_string(), - args: vec![McpServerValueMatcher::Exact { - value: "approved".to_string(), - }], - }); - let matchers = Sourced::new( - BTreeMap::from([ - (BOTH_MATCH.to_string(), matcher.clone()), - (EXACT_ONLY_MATCHES.to_string(), matcher.clone()), - (MATCHER_ONLY_MATCHES.to_string(), matcher), - ]), - source.clone(), - ); - - filter_mcp_servers_by_requirements(&mut servers, Some(&requirements), Some(&matchers)); - - let reason = Some(McpServerDisabledReason::Requirements { source }); - assert_eq!( - servers - .iter() - .map(|(name, server)| ( - name.clone(), - (server.enabled, server.disabled_reason.clone()) - )) - .collect::)>>(), - HashMap::from([ - (BOTH_MATCH.to_string(), (true, None)), - (EXACT_ONLY_MATCHES.to_string(), (false, reason.clone())), - (MATCHER_ONLY_MATCHES.to_string(), (false, reason)), - ]) - ); -} - #[test] fn filter_mcp_servers_by_allowlist_blocks_all_when_empty() { let mut servers = HashMap::from([ @@ -4443,11 +4348,7 @@ fn filter_mcp_servers_by_allowlist_blocks_all_when_empty() { let source = RequirementSource::LegacyManagedConfigTomlFromMdm; let requirements = Sourced::new(BTreeMap::new(), source.clone()); - filter_mcp_servers_by_requirements( - &mut servers, - Some(&requirements), - /*mcp_matchers*/ None, - ); + filter_mcp_servers_by_requirements(&mut servers, Some(&requirements)); let reason = Some(McpServerDisabledReason::Requirements { source }); assert_eq!( @@ -4493,66 +4394,31 @@ fn filter_plugin_mcp_servers_by_allowlist_enforces_plugin_and_matcher_rules() { mcp_servers: Some(BTreeMap::from([ ( MATCHED_SERVER.to_string(), - McpServerRequirement { - identity: McpServerIdentity::Command { - command: GOOD_CMD.to_string(), - }, - }, + McpServerRequirement::Command(McpServerCommandMatcher { + command: GOOD_CMD.to_string(), + args: Vec::new(), + }), ), ( MISMATCHED_SERVER.to_string(), - McpServerRequirement { - identity: McpServerIdentity::Command { - command: GOOD_CMD.to_string(), - }, - }, + McpServerRequirement::Command(McpServerCommandMatcher { + command: GOOD_CMD.to_string(), + args: Vec::new(), + }), ), ( MATCHER_MISMATCHED_SERVER.to_string(), - McpServerRequirement { - identity: McpServerIdentity::Command { - command: GOOD_CMD.to_string(), - }, - }, + McpServerRequirement::Command(McpServerCommandMatcher { + command: GOOD_CMD.to_string(), + args: Vec::new(), + }), ), ])), }, )]), source.clone(), ); - let matchers = Sourced::new( - BTreeMap::from([ - ( - MATCHED_SERVER.to_string(), - McpServerMatcher::Command(McpServerCommandMatcher { - command: GOOD_CMD.to_string(), - args: Vec::new(), - }), - ), - ( - MISMATCHED_SERVER.to_string(), - McpServerMatcher::Command(McpServerCommandMatcher { - command: "bad-cmd".to_string(), - args: Vec::new(), - }), - ), - ( - MATCHER_MISMATCHED_SERVER.to_string(), - McpServerMatcher::Command(McpServerCommandMatcher { - command: GOOD_CMD.to_string(), - args: Vec::new(), - }), - ), - ]), - source.clone(), - ); - - filter_plugin_mcp_servers_by_requirements( - "sample@test", - &mut servers, - Some(&requirements), - Some(&matchers), - ); + filter_plugin_mcp_servers_by_requirements("sample@test", &mut servers, Some(&requirements)); let reason = Some(McpServerDisabledReason::Requirements { source }); assert_eq!( @@ -4585,7 +4451,7 @@ fn filter_plugin_mcp_servers_by_allowlist_blocks_unlisted_plugin() { codex_config::PluginRequirementsToml { mcp_servers: Some(BTreeMap::from([( "server-a".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "cmd-a".to_string(), }, @@ -4596,12 +4462,7 @@ fn filter_plugin_mcp_servers_by_allowlist_blocks_unlisted_plugin() { source.clone(), ); - filter_plugin_mcp_servers_by_requirements( - "sample@test", - &mut servers, - Some(&requirements), - /*mcp_matchers*/ None, - ); + filter_plugin_mcp_servers_by_requirements("sample@test", &mut servers, Some(&requirements)); assert_eq!( servers @@ -4642,26 +4503,26 @@ fn filter_plugin_mcp_servers_by_matchers_enforces_name_and_invocation() { ), ]); let source = RequirementSource::LegacyManagedConfigTomlFromMdm; - let matcher = McpServerMatcher::Command(McpServerCommandMatcher { + let requirement = McpServerRequirement::Command(McpServerCommandMatcher { command: "company-cli".to_string(), args: vec![McpServerValueMatcher::Exact { value: "approved".to_string(), }], }); - let matchers = Sourced::new( - BTreeMap::from([ - (MATCHED_SERVER.to_string(), matcher.clone()), - (MISMATCHED_SERVER.to_string(), matcher), - ]), + let requirements = Sourced::new( + BTreeMap::from([( + "sample@test".to_string(), + codex_config::PluginRequirementsToml { + mcp_servers: Some(BTreeMap::from([ + (MATCHED_SERVER.to_string(), requirement.clone()), + (MISMATCHED_SERVER.to_string(), requirement), + ])), + }, + )]), source.clone(), ); - filter_plugin_mcp_servers_by_requirements( - "sample@test", - &mut servers, - /*plugin_requirements*/ None, - Some(&matchers), - ); + filter_plugin_mcp_servers_by_requirements("sample@test", &mut servers, Some(&requirements)); let reason = Some(McpServerDisabledReason::Requirements { source }); assert_eq!( @@ -4689,7 +4550,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io:: let mcp_requirements = BTreeMap::from([ ( "session_overrides_user".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "session-command".to_string(), }, @@ -4697,7 +4558,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io:: ), ( "managed_overrides_session".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "managed-command".to_string(), }, @@ -4705,7 +4566,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io:: ), ( "fresh_global".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "fresh-global-command".to_string(), }, @@ -4713,7 +4574,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io:: ), ( "fresh_project".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "fresh-project-command".to_string(), }, @@ -9008,7 +8869,6 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset() feature_requirements: None, hooks: None, mcp_servers: None, - mcp_server_matchers: None, plugins: None, apps: None, rules: None, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index a2041bbb66..1fb3f32842 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -14,12 +14,9 @@ use codex_config::ConfigRequirements; use codex_config::ConfigRequirementsToml; use codex_config::ConstrainedWithSource; use codex_config::FeatureRequirementsToml; -use codex_config::McpServerIdentity; -use codex_config::McpServerMatcher; use codex_config::McpServerRequirement; use codex_config::PluginRequirementsToml; use codex_config::ProfileV2Name; -use codex_config::RequirementSource; use codex_config::ResidencyRequirement; use codex_config::SandboxModeRequirement; use codex_config::Sourced; @@ -42,7 +39,6 @@ use codex_config::types::AuthKeyringBackendKind; use codex_config::types::History; use codex_config::types::McpServerConfig; use codex_config::types::McpServerDisabledReason; -use codex_config::types::McpServerTransportConfig; use codex_config::types::MemoriesConfig; use codex_config::types::ModelAvailabilityNuxConfig; use codex_config::types::Notice; @@ -1489,22 +1485,18 @@ impl Config { plugin_id: &str, mcp_servers: &mut HashMap, ) { - let requirements = self.config_layer_stack.requirements(); filter_plugin_mcp_servers_by_requirements( plugin_id, mcp_servers, - requirements.plugins.as_ref(), - requirements.mcp_server_matchers.as_ref(), + self.config_layer_stack.requirements().plugins.as_ref(), ); - let empty_mcp_allowlist = requirements + let empty_mcp_allowlist = self + .config_layer_stack + .requirements() .mcp_servers .as_ref() .filter(|requirements| requirements.value.is_empty()); - filter_mcp_servers_by_requirements( - mcp_servers, - empty_mcp_allowlist, - /*mcp_matchers*/ None, - ); + filter_mcp_servers_by_requirements(mcp_servers, empty_mcp_allowlist); } pub async fn to_mcp_config( @@ -1874,30 +1866,17 @@ fn load_model_catalog( fn filter_mcp_servers_by_requirements( mcp_servers: &mut HashMap, mcp_requirements: Option<&Sourced>>, - mcp_matchers: Option<&Sourced>>, ) { - if mcp_requirements.is_none() && mcp_matchers.is_none() { + let Some(allowlist) = mcp_requirements else { return; - } + }; - let source = RequirementSource::composite( - mcp_requirements - .iter() - .map(|requirements| requirements.source.clone()) - .chain( - mcp_matchers - .iter() - .map(|requirements| requirements.source.clone()), - ), - ); + let source = allowlist.source.clone(); for (name, server) in mcp_servers.iter_mut() { - let identity_requirement = - mcp_requirements.and_then(|requirements| requirements.value.get(name)); - let matcher = mcp_matchers.and_then(|matchers| matchers.value.get(name)); - let allowed = (identity_requirement.is_some() || matcher.is_some()) - && identity_requirement - .is_none_or(|requirement| mcp_server_matches_requirement(requirement, server)) - && matcher.is_none_or(|matcher| matcher.matches(server)); + let allowed = allowlist + .value + .get(name) + .is_some_and(|requirement| requirement.matches(server)); if allowed { server.disabled_reason = None; } else { @@ -1913,38 +1892,20 @@ fn filter_plugin_mcp_servers_by_requirements( plugin_config_name: &str, mcp_servers: &mut HashMap, plugin_requirements: Option<&Sourced>>, - mcp_matchers: Option<&Sourced>>, ) { - if plugin_requirements.is_none() && mcp_matchers.is_none() { + let Some(requirements) = plugin_requirements else { return; - } - - let source = RequirementSource::composite( - plugin_requirements - .iter() - .map(|requirements| requirements.source.clone()) - .chain( - mcp_matchers - .iter() - .map(|requirements| requirements.source.clone()), - ), - ); - let plugin_mcp_requirements = plugin_requirements - .and_then(|requirements| requirements.value.get(plugin_config_name)) + }; + let source = requirements.source.clone(); + let plugin_mcp_requirements = requirements + .value + .get(plugin_config_name) .and_then(|plugin| plugin.mcp_servers.as_ref()); for (name, server) in mcp_servers.iter_mut() { - let allowed_by_plugin_requirement = plugin_requirements.is_none() - || plugin_mcp_requirements - .and_then(|mcp_requirements| mcp_requirements.get(name)) - .is_some_and(|requirement| mcp_server_matches_requirement(requirement, server)); - let allowed_by_matcher = mcp_matchers.is_none_or(|matchers| { - matchers - .value - .get(name) - .is_some_and(|matcher| matcher.matches(server)) - }); - let allowed = allowed_by_plugin_requirement && allowed_by_matcher; + let allowed = plugin_mcp_requirements + .and_then(|mcp_requirements| mcp_requirements.get(name)) + .is_some_and(|requirement| requirement.matches(server)); if allowed { server.disabled_reason = None; } else { @@ -1959,20 +1920,14 @@ fn filter_plugin_mcp_servers_by_requirements( fn constrain_mcp_servers( mcp_servers: HashMap, mcp_requirements: Option<&Sourced>>, - mcp_matchers: Option<&Sourced>>, ) -> ConstraintResult>> { - if mcp_requirements.is_none() && mcp_matchers.is_none() { + if mcp_requirements.is_none() { return Ok(Constrained::allow_any(mcp_servers)); } let mcp_requirements = mcp_requirements.cloned(); - let mcp_matchers = mcp_matchers.cloned(); Constrained::normalized(mcp_servers, move |mut servers| { - filter_mcp_servers_by_requirements( - &mut servers, - mcp_requirements.as_ref(), - mcp_matchers.as_ref(), - ); + filter_mcp_servers_by_requirements(&mut servers, mcp_requirements.as_ref()); servers }) } @@ -2013,26 +1968,6 @@ where Ok(false) } -fn mcp_server_matches_requirement( - requirement: &McpServerRequirement, - server: &McpServerConfig, -) -> bool { - match &requirement.identity { - McpServerIdentity::Command { - command: want_command, - } => matches!( - &server.transport, - McpServerTransportConfig::Stdio { command: got_command, .. } - if got_command == want_command - ), - McpServerIdentity::Url { url: want_url } => matches!( - &server.transport, - McpServerTransportConfig::StreamableHttp { url: got_url, .. } - if got_url == want_url - ), - } -} - pub async fn load_global_mcp_servers( codex_home: &Path, ) -> std::io::Result> { @@ -2982,7 +2917,6 @@ impl Config { feature_requirements, managed_hooks: _, mcp_servers, - mcp_server_matchers, plugins: _, exec_policy: _, enforce_residency, @@ -3738,12 +3672,8 @@ impl Config { &mut startup_warnings, )?; - let mcp_servers = constrain_mcp_servers( - cfg.mcp_servers.clone(), - mcp_servers.as_ref(), - mcp_server_matchers.as_ref(), - ) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{e}")))?; + let mcp_servers = constrain_mcp_servers(cfg.mcp_servers.clone(), mcp_servers.as_ref()) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{e}")))?; let network_permission_profile = constrained_permission_profile.get().clone(); let network = build_network_proxy_spec( diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index 26d78d3974..fcc6364791 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -699,7 +699,7 @@ mod tests { mcp_servers: Some(Sourced::new( BTreeMap::from([( "docs".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "codex-mcp".to_string(), }, @@ -778,13 +778,12 @@ mod tests { hooks: None, mcp_servers: Some(BTreeMap::from([( "docs".to_string(), - McpServerRequirement { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: "codex-mcp".to_string(), }, }, )])), - mcp_server_matchers: None, plugins: None, apps: None, rules: None, @@ -1142,7 +1141,6 @@ approval_policy = "never" feature_requirements: None, hooks: None, mcp_servers: None, - mcp_server_matchers: None, plugins: None, apps: None, rules: None,