Conversation
… `<var:name>` placeholder Pattern matching lacked a way to match command arguments against a typed list of allowed values. Long alternations (e.g., `a|b|c|d`) had to be inlined in patterns, and path arguments couldn't be canonicalized for robust matching across different path forms. Add `definitions.vars` with typed variable definitions: - `type: literal` (default): exact string match - `type: path`: canonicalize both sides before comparison, with normalize_path fallback when the file doesn't exist - `<var:name>` placeholder in rule patterns - Config validation, merge strategy, and JSON schema - `vars` context variable in CEL expressions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the rule engine's capabilities by introducing a new mechanism for defining and using typed variables within configuration files. This feature addresses the challenge of managing complex rule patterns that involve numerous allowed values or require specific matching logic for path-like arguments. By centralizing variable definitions, it improves the readability and maintainability of rules, making it easier to express sophisticated matching criteria without verbose pattern syntax. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #201 +/- ##
==========================================
+ Coverage 89.26% 89.56% +0.30%
==========================================
Files 47 47
Lines 9046 9364 +318
==========================================
+ Hits 8075 8387 +312
- Misses 971 977 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request introduces typed variable definitions (definitions.vars) to the configuration, enabling the use of <var:name> placeholders in rules for both literal and path-based matching. The changes include updates to the configuration model, JSON schema, pattern parsing, and matching logic, along with new documentation and integration tests. However, the vars field in ExprContext is currently unpopulated, which means matched variables are not exposed to CEL expressions in when clauses. Additionally, several new validation and negative test cases could be refactored into parameterized rstest functions to improve code maintainability and align with repository style guidelines.
Pattern matching with `<var:name>` placeholders only validated whether a command token was in the allowed values list, but did not capture the actual matched value. This meant `when` clauses could not inspect which specific value was matched, limiting conditional deny rules. Add `MatchCaptures` struct to carry both wildcard and var captures from pattern matching. Thread a `RefCell<HashMap>` through `match_engine` to accumulate var captures during recursive matching, then populate `ExprContext.vars` so CEL expressions can reference captured values.
…ameterized cases Separate test functions that only differed in input values made it harder to see that they tested the same invariant. Consolidate `validate_rejects_var_ref_in_definitions_vars` / `validate_rejects_path_ref_in_definitions_vars` into a single parameterized `validate_rejects_placeholder_in_definitions_vars`, and merge three `match_var_ref` negative tests into `match_var_ref_negative_cases`.
When pattern matching backtracked (e.g., Optional "with" branch fails and falls back to "without"), wildcard captures and `after_double_dash` were properly saved/restored, but `var_captures` was not. This caused `<var:name>` captures from failed branches to persist, contaminating the `vars` map passed to `when` clause CEL expressions. Save and restore `var_captures` at all six backtracking points in `match_engine`: Wildcard, Alternation (direct and `=`-joined), FlagWithValue (space-separated and `=`-joined), and Optional.
Why
Example
Current:
Desired:
What
definitions.vars, referenced from rule patterns with the<var:name>placeholdervarscontext variable in CEL expressions