feat: add eval_rule example demonstrating recommended API - #38
Conversation
Add a complete Copilot configuration for regorus with layered architecture: instructions, knowledge files, skills, role-based agents, and CI validation. ## Instructions (auto-loaded) - copilot-instructions.md: lean 5KB orientation with knowledge file references - copilot-code-review-instructions.md: "think freely" review guide with severity categories and domain-specific context (Undefined, FFI, telemetry) ## Knowledge Files (20 files, ~70KB) Deep institutional knowledge organized by subsystem: - Core: value-semantics, engine-api, error-handling-migration - Execution: interpreter-architecture, rvm-architecture, compilation-pipeline - Rego: rego-semantics, rego-compiler, builtin-system - Azure: azure-policy-language, azure-policy-aliases, azure-rbac-language - Safety: policy-evaluation-security, ffi-boundary, feature-composition - Diagnostics: telemetry-and-diagnostics, causality-and-partial-eval - Extensibility: language-extension-guide, tooling-architecture, time-builtins-compat ## Agents (16 role-based personas) Each agent brings a distinct thinking mode for review and planning: - Core engineering: red-teamer, semantics-expert, architect, performance-engineer - Quality: test-engineer, verification-engineer, security-auditor - Operations: reliability-engineer, support-engineer, ci-engineer - Evolution: refactorer, api-steward - Product: program-manager, demo-engineer, dx-engineer - Leadership: tech-lead (with 9 constitutional rules and cross-agent synthesis) ## Skills (6 task workflows) - thorough-review: multi-agent parallel review with cross-agent context protocol - design-alternatives: multi-approach evaluation against 9 dimensions - add-builtin, opa-conformance, security-review, verification ## CI & Infrastructure - copilot-setup-steps.yml: Rust 1.92.0 + clippy + rustfmt + cargo cache - copilot-config-validation.yml: YAML syntax, reference checking, staleness detection ## Architecture Documentation - docs/copilot-architecture.md: how all layers connect, extension guide Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add 'Review Perspectives' section to copilot-code-review-instructions.md that inlines key thinking modes from all 10 relevant agents so the single-pass GitHub PR reviewer adopts multiple roles automatically - Fix copilot-config-validation.yml grep pattern to only match knowledge file references in the table (not docs/rvm/*.md references) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
feat: add comprehensive GitHub Copilot configuration
Each review comment now includes: - Perspective tag identifying which role raised the finding - Severity indicator (critical/important/suggestion) - Issue-ready summary in blockquote for easy GitHub issue creation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit 2225c7e.
Move comment format from a subsection hint to a top-level REQUIRED section with concrete examples and explicit 'no exceptions' language. This should improve compliance from the PR review bot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds a GitHub Actions workflow that: - Triggers on PRs via pull_request_target (fork-safe, API-only diff) - Selects relevant perspectives based on changed file paths - Calls GitHub Models API with agent instructions + file context - Requests structured JSON output, renders perspective-tagged markdown - Upserts a single consolidated review comment (idempotent) Security: never checks out PR head code; pins actions by SHA; uses concurrency groups to cancel stale runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The 'models: read' permission may not be recognized by GitHub Actions yet, potentially causing silent workflow failures. Remove it and rely on GITHUB_TOKEN's default access. Add workflow_dispatch for manual testing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Testing both triggers to determine which fires for same-repo PRs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move all review logic from inline YAML to .github/scripts/perspective-review.sh. Workflow YAML is now minimal — just triggers, permissions, and script invocation. This fixes YAML parsing issues with complex heredocs and inline bash. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The perspective-review workflow needs the models:read permission on GITHUB_TOKEN to call the GitHub Models API endpoint. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Redesigned the perspective-review script to use the PR Review API for inline code comments instead of issue comments. Each perspective posts a separate review with findings anchored to specific diff lines. Key improvements: - Uses PR Review API for inline comments with code snippets - One review per perspective for clear attribution - Parses diff to extract valid line anchors with code content - LLM picks from exact anchor list (100% inline rate) - Two-bucket fallback: inline for anchored, body for unanchored - Locks reviews to analyzed commit SHA Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add an example showing how to use Engine::eval_rule() for direct rule evaluation. The existing regorus example only uses eval_query, but eval_rule is recommended for most applications as it is faster (no query parsing) and returns the value directly. The example: - Loads the server policy and evaluates allow, violation, and public_server rules using eval_rule - Demonstrates engine cloning for re-evaluation with different input - Shows both non-compliant and compliant input scenarios Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| @@ -0,0 +1,68 @@ | |||
| // Copyright (c) Microsoft Corporation. | |||
There was a problem hiding this comment.
🟠 Important: New example added without tests
The new example eval_rule demonstrates the usage of the API but lacks accompanying tests. It is crucial to ensure that the example behaves as expected and to prevent regressions in the future. Consider adding unit tests that validate the functionality showcased in this example.
|
|
||
| // --- eval_rule: get the value of a rule directly --- | ||
| // The path uses dotted notation: data.<package>.<rule> | ||
| let allow = engine.eval_rule("data.example.allow".to_string())?; |
There was a problem hiding this comment.
🟠 Important: Potential for API misuse
The example directly evaluates rules using eval_rule, which could lead to silent failures if the rule does not exist or if there are issues with the input data. It is advisable to handle potential errors gracefully and provide feedback to the user, ensuring that they are aware of any issues during evaluation.
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| //! Example: Using `eval_rule` for direct rule evaluation. |
There was a problem hiding this comment.
🔵 Suggestion: Documentation clarity
While the example provides a good overview of using eval_rule, additional comments explaining the purpose of each section could enhance clarity. This would help users understand the flow of the example and the significance of each step in the evaluation process.
| doctest=false | ||
|
|
||
| [[example]] | ||
| name="eval_rule" |
There was a problem hiding this comment.
🔵 Suggestion: Feature gating for example
The new example is added without any feature gating. It is recommended to ensure that examples are gated under appropriate features to avoid unnecessary compilation for users who do not require this functionality. This will help maintain a leaner build for users who do not need the example.
|
|
||
| // --- eval_rule: get the value of a rule directly --- | ||
| // The path uses dotted notation: data.<package>.<rule> | ||
| let allow = engine.eval_rule("data.example.allow".to_string())?; |
There was a problem hiding this comment.
🟠 Important: Potential for Non-Deterministic Behavior
The evaluation of rules using eval_rule may introduce non-deterministic behavior if the underlying policy or input data changes. It is crucial to ensure that the same input consistently yields the same output to maintain reliability. Consider implementing checks or logging to verify that the inputs and policies remain unchanged across evaluations.
| let mut engine = Engine::new(); | ||
|
|
||
| // Load the server policy. | ||
| engine.add_policy_from_file("examples/server/allowed_server.rego")?; |
There was a problem hiding this comment.
🟠 Important: Error Handling for Policy Loading
The call to engine.add_policy_from_file does not handle potential errors that may arise from loading the policy file. If the file is missing or contains invalid syntax, the application could panic. It is advisable to implement error handling to provide informative feedback to operators and prevent crashes.
| engine.add_policy_from_file("examples/server/allowed_server.rego")?; | ||
|
|
||
| // Set input data. | ||
| engine.set_input(Value::from_json_file("examples/server/input.json")?); |
There was a problem hiding this comment.
🟠 Important: Resource Management for Input Data
The input data is set using engine.set_input, but there is no validation or resource limit enforcement on the input size. This could lead to resource exhaustion if large or malicious input is provided. Implementing checks on the input size and structure can help mitigate potential Denial of Service (DoS) attacks.
| // --- eval_rule: get the value of a rule directly --- | ||
| // The path uses dotted notation: data.<package>.<rule> | ||
| let allow = engine.eval_rule("data.example.allow".to_string())?; | ||
| println!("allow = {allow}"); |
There was a problem hiding this comment.
🔵 Suggestion: Graceful Degradation on Rule Evaluation
When evaluating rules, consider implementing a mechanism for graceful degradation in case of failures. If a rule evaluation fails, it should return a clear error message rather than causing the entire application to fail. This will improve the user experience and operational reliability.
| engine.add_policy_from_file("examples/server/allowed_server.rego")?; | ||
|
|
||
| // Set input data. | ||
| engine.set_input(Value::from_json_file("examples/server/input.json")?); |
There was a problem hiding this comment.
🔵 Suggestion: Missing Input Validation for External Data
The input data is set using engine.set_input(Value::from_json_file(...)), which may not validate the structure or content of the JSON file. It is advisable to implement validation checks to ensure that the input data conforms to expected formats and types to prevent potential security vulnerabilities.
| let mut engine = Engine::new(); | ||
|
|
||
| // Load the server policy. | ||
| engine.add_policy_from_file("examples/server/allowed_server.rego")?; |
There was a problem hiding this comment.
🔵 Suggestion: Lack of Error Handling for Policy Loading
The call to engine.add_policy_from_file(...) does not have error handling beyond the ? operator. While this propagates errors, it may be beneficial to log or handle specific errors to provide better context for failures, especially in production environments.
| // --- eval_rule: get the value of a rule directly --- | ||
| // The path uses dotted notation: data.<package>.<rule> | ||
| let allow = engine.eval_rule("data.example.allow".to_string())?; | ||
| println!("allow = {allow}"); |
There was a problem hiding this comment.
🔵 Suggestion: Potential Information Disclosure in Output
The output of the evaluated rule is printed directly to the console. If the evaluated rule contains sensitive information, this could lead to information disclosure. Consider sanitizing or restricting the output to avoid leaking sensitive data.
| @@ -0,0 +1,68 @@ | |||
| // Copyright (c) Microsoft Corporation. | |||
There was a problem hiding this comment.
🟠 Important: Missing Tests for New Example Code
The newly added example in eval_rule.rs demonstrates the usage of the eval_rule API but lacks corresponding test cases. It is crucial to create tests that validate the behavior of this example, ensuring that it functions correctly under various scenarios, including edge cases and error conditions.
| // Set input data. | ||
| engine.set_input(Value::from_json_file("examples/server/input.json")?); | ||
|
|
||
| // --- eval_rule: get the value of a rule directly --- |
There was a problem hiding this comment.
🟠 Important: No Tests for Policy Evaluation Paths
The example includes multiple calls to eval_rule with different inputs, but there are no tests to verify these evaluations. It is essential to implement tests that cover both successful evaluations and potential failure modes, especially considering the different paths that may arise from varying input data.
| let mut engine2 = engine.clone(); | ||
|
|
||
| // Create a compliant input: all servers use HTTPS on private networks. | ||
| let compliant_input = Value::from_json_str( |
There was a problem hiding this comment.
🔵 Suggestion: Consider Edge Case Testing
The example demonstrates a compliant input scenario, but it would benefit from additional tests that cover edge cases such as empty inputs, invalid data formats, and unexpected values. Testing these edge cases will help ensure robustness and prevent potential runtime errors.
b3cc8ab to
369c82a
Compare
Summary
Add an example showing how to use
Engine::eval_rule()for direct rule evaluation. The existingregorusexample only useseval_query, but the code itself recommendseval_rulefor most applications:Changes
examples/eval_rule.rs— new example demonstrating:eval_ruledata.example.allow)Cargo.toml— register the new exampleTesting