diff --git a/codex-rs/execpolicy2/src/parser.rs b/codex-rs/execpolicy2/src/parser.rs index d74be3e8f8..6654e27429 100644 --- a/codex-rs/execpolicy2/src/parser.rs +++ b/codex-rs/execpolicy2/src/parser.rs @@ -18,6 +18,7 @@ use crate::decision::Decision; use crate::error::Error; use crate::error::Result; use crate::policy::validate_match_examples; +use crate::policy::validate_not_match_examples; use crate::rule::PatternToken; use crate::rule::PrefixPattern; use crate::rule::PrefixRule; @@ -228,10 +229,7 @@ fn policy_builtins(builder: &mut GlobalsBuilder) { }) .collect(); - rules - .iter() - .try_for_each(|rule| rule.validate_not_matches(¬_matches))?; - + validate_not_match_examples(&rules, ¬_matches)?; validate_match_examples(&rules, &matches)?; rules.into_iter().for_each(|rule| builder.add_rule(rule)); diff --git a/codex-rs/execpolicy2/src/policy.rs b/codex-rs/execpolicy2/src/policy.rs index 2e5b03385a..5977d1f4c9 100644 --- a/codex-rs/execpolicy2/src/policy.rs +++ b/codex-rs/execpolicy2/src/policy.rs @@ -61,28 +61,18 @@ impl Evaluation { /// Count how many rules match each provided example and error if any example is unmatched. pub(crate) fn validate_match_examples(rules: &[RuleRef], matches: &[Vec]) -> Result<()> { - let match_counts = rules.iter().fold(vec![0; matches.len()], |counts, rule| { - counts - .iter() - .zip(rule.validate_matches(matches)) - .map(|(count, matched)| if matched { count + 1 } else { *count }) - .collect() - }); + let mut unmatched_examples = Vec::new(); - let unmatched_examples: Vec = matches - .iter() - .zip(&match_counts) - .filter_map(|(example, count)| { - if *count == 0 { - Some( - try_join(example.iter().map(String::as_str)) - .unwrap_or_else(|_| "unable to render example".to_string()), - ) - } else { - None - } - }) - .collect(); + for example in matches { + if rules.iter().any(|rule| rule.matches(example).is_some()) { + continue; + } + + unmatched_examples.push( + try_join(example.iter().map(String::as_str)) + .unwrap_or_else(|_| "unable to render example".to_string()), + ); + } if unmatched_examples.is_empty() { Ok(()) @@ -93,3 +83,21 @@ pub(crate) fn validate_match_examples(rules: &[RuleRef], matches: &[Vec] }) } } + +/// Ensure that no rule matches any provided negative example. +pub(crate) fn validate_not_match_examples( + rules: &[RuleRef], + not_matches: &[Vec], +) -> Result<()> { + for example in not_matches { + if let Some(rule) = rules.iter().find(|rule| rule.matches(example).is_some()) { + return Err(Error::ExampleDidMatch { + rule: format!("{rule:?}"), + example: try_join(example.iter().map(String::as_str)) + .unwrap_or_else(|_| "unable to render example".to_string()), + }); + } + } + + Ok(()) +} diff --git a/codex-rs/execpolicy2/src/rule.rs b/codex-rs/execpolicy2/src/rule.rs index f46332f8a8..0336c3159e 100644 --- a/codex-rs/execpolicy2/src/rule.rs +++ b/codex-rs/execpolicy2/src/rule.rs @@ -1,9 +1,6 @@ use crate::decision::Decision; -use crate::error::Error; -use crate::error::Result; use serde::Deserialize; use serde::Serialize; -use shlex::try_join; use std::any::Any; use std::fmt::Debug; use std::sync::Arc; @@ -83,27 +80,6 @@ pub trait Rule: Any + Debug + Send + Sync { fn program(&self) -> &str; fn matches(&self, cmd: &[String]) -> Option; - - /// Return a boolean for each example indicating whether this rule matches it. - fn validate_matches(&self, matches: &[Vec]) -> Vec { - matches - .iter() - .map(|example| self.matches(example).is_some()) - .collect() - } - - fn validate_not_matches(&self, not_matches: &[Vec]) -> Result<()> { - for example in not_matches { - if self.matches(example).is_some() { - return Err(Error::ExampleDidMatch { - rule: format!("{self:?}"), - example: try_join(example.iter().map(String::as_str)) - .unwrap_or_else(|_| "unable to render example".to_string()), - }); - } - } - Ok(()) - } } pub type RuleRef = Arc;