From cd4df533169c19219d90c17132eb1f3bd822160b Mon Sep 17 00:00:00 2001 From: kevin zhao Date: Thu, 13 Nov 2025 17:12:21 -0500 Subject: [PATCH] parser state --- codex-rs/execpolicy2/src/main.rs | 10 ++--- codex-rs/execpolicy2/src/parser.rs | 61 ++++++++++++++--------------- codex-rs/execpolicy2/tests/basic.rs | 50 +++++++++++++++++------ 3 files changed, 73 insertions(+), 48 deletions(-) diff --git a/codex-rs/execpolicy2/src/main.rs b/codex-rs/execpolicy2/src/main.rs index 544c072bb7..68ee265f39 100644 --- a/codex-rs/execpolicy2/src/main.rs +++ b/codex-rs/execpolicy2/src/main.rs @@ -53,9 +53,9 @@ fn load_policies(policy_paths: &[PathBuf]) -> Result }) .collect::>() .context("failed to load policy files")?; - Ok(PolicyParser::parse_many(loaded_policies.iter().map( - |(policy_identifier, policy_file_contents)| { - (policy_identifier.as_str(), policy_file_contents.as_str()) - }, - ))?) + let mut parser = PolicyParser::new(); + for (policy_identifier, policy_file_contents) in &loaded_policies { + parser.parse(policy_identifier, policy_file_contents)?; + } + Ok(parser.build()) } diff --git a/codex-rs/execpolicy2/src/parser.rs b/codex-rs/execpolicy2/src/parser.rs index 5e7a078ef2..527c697a6d 100644 --- a/codex-rs/execpolicy2/src/parser.rs +++ b/codex-rs/execpolicy2/src/parser.rs @@ -25,45 +25,44 @@ use crate::rule::RuleRef; use crate::rule::validate_match_examples; use crate::rule::validate_not_match_examples; -pub struct PolicyParser; +pub struct PolicyParser { + builder: RefCell, +} + +impl Default for PolicyParser { + fn default() -> Self { + Self::new() + } +} impl PolicyParser { - /// Parses a policy, tagging parser errors with `policy_identifier` so failures include the - /// identifier alongside line numbers. - pub fn parse( - policy_identifier: &str, - policy_file_contents: &str, - ) -> Result { - Self::parse_many([(policy_identifier, policy_file_contents)]) + pub fn new() -> Self { + Self { + builder: RefCell::new(PolicyBuilder::new()), + } } - /// Parses multiple policy files and merges the resulting rules. - pub fn parse_many( - policies: Policies, - ) -> Result - where - Policies: IntoIterator, - Identifier: AsRef, - Contents: AsRef, - { + /// Parses a policy, tagging parser errors with `policy_identifier` so failures include the + /// identifier alongside line numbers. + pub fn parse(&mut self, policy_identifier: &str, policy_file_contents: &str) -> Result<()> { let mut dialect = Dialect::Extended.clone(); dialect.enable_f_strings = true; let globals = GlobalsBuilder::standard().with(policy_builtins).build(); + let ast = AstModule::parse( + policy_identifier, + policy_file_contents.to_string(), + &dialect, + ) + .map_err(Error::Starlark)?; + let module = Module::new(); + let mut eval = Evaluator::new(&module); + eval.extra = Some(&self.builder); + eval.eval_module(ast, &globals).map_err(Error::Starlark)?; + Ok(()) + } - let builder = RefCell::new(PolicyBuilder::new()); - for (policy_identifier, policy_file_contents) in policies { - let ast = AstModule::parse( - policy_identifier.as_ref(), - policy_file_contents.as_ref().to_string(), - &dialect, - ) - .map_err(Error::Starlark)?; - let module = Module::new(); - let mut eval = Evaluator::new(&module); - eval.extra = Some(&builder); - eval.eval_module(ast, &globals).map_err(Error::Starlark)?; - } - Ok(builder.into_inner().build()) + pub fn build(self) -> crate::policy::Policy { + self.builder.into_inner().build() } } diff --git a/codex-rs/execpolicy2/tests/basic.rs b/codex-rs/execpolicy2/tests/basic.rs index 67fd650c17..b58869ac9f 100644 --- a/codex-rs/execpolicy2/tests/basic.rs +++ b/codex-rs/execpolicy2/tests/basic.rs @@ -41,7 +41,11 @@ prefix_rule( pattern = ["git", "status"], ) "#; - let policy = PolicyParser::parse("test.codexpolicy", policy_src).expect("parse policy"); + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); let cmd = tokens(&["git", "status"]); let evaluation = policy.check(&cmd); assert_eq!( @@ -70,12 +74,14 @@ prefix_rule( decision = "forbidden", ) "#; - - let policy = PolicyParser::parse_many([ - ("first.codexpolicy", first_policy), - ("second.codexpolicy", second_policy), - ]) - .expect("parse policy"); + let mut parser = PolicyParser::new(); + parser + .parse("first.codexpolicy", first_policy) + .expect("parse policy"); + parser + .parse("second.codexpolicy", second_policy) + .expect("parse policy"); + let policy = parser.build(); let git_rules = rule_snapshots(policy.rules().get_vec("git").expect("git rules")); assert_eq!( @@ -136,7 +142,11 @@ prefix_rule( pattern = [["bash", "sh"], ["-c", "-l"]], ) "#; - let policy = PolicyParser::parse("test.codexpolicy", policy_src).expect("parse policy"); + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); let bash_rules = rule_snapshots(policy.rules().get_vec("bash").expect("bash rules")); let sh_rules = rule_snapshots(policy.rules().get_vec("sh").expect("sh rules")); @@ -193,7 +203,11 @@ prefix_rule( pattern = ["npm", ["i", "install"], ["--legacy-peer-deps", "--no-save"]], ) "#; - let policy = PolicyParser::parse("test.codexpolicy", policy_src).expect("parse policy"); + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); let rules = rule_snapshots(policy.rules().get_vec("npm").expect("npm rules")); assert_eq!( @@ -251,7 +265,11 @@ prefix_rule( ], ) "#; - let policy = PolicyParser::parse("test.codexpolicy", policy_src).expect("parse policy"); + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); let match_eval = policy.check(&tokens(&["git", "status"])); assert_eq!( Evaluation::Match { @@ -289,7 +307,11 @@ prefix_rule( decision = "forbidden", ) "#; - let policy = PolicyParser::parse("test.codexpolicy", policy_src).expect("parse policy"); + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); let status = policy.check(&tokens(&["git", "status"])); assert_eq!( @@ -344,7 +366,11 @@ prefix_rule( decision = "forbidden", ) "#; - let policy = PolicyParser::parse("test.codexpolicy", policy_src).expect("parse policy"); + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); let commands = vec![ tokens(&["git", "status"]),