Skip to content

feat!: add OPA Rego v1 compatibility check (INT-331)#35

Merged
glaracuente merged 6 commits intomainfrom
feature/INT-331/add_opa_rego_v1_check
Apr 27, 2026
Merged

feat!: add OPA Rego v1 compatibility check (INT-331)#35
glaracuente merged 6 commits intomainfrom
feature/INT-331/add_opa_rego_v1_check

Conversation

@glaracuente
Copy link
Copy Markdown
Contributor

@glaracuente glaracuente commented Apr 17, 2026

Summary

This PR adds a Rego v1 compatibility check to the action, ensuring policy files conform to OPA v1 / Rego v1 syntax before tests are run.

What changed

  • New input v1_compatible_check (default: true) — when enabled, runs opa check --v1-compatible against all Rego files in path before executing tests. If any files use v0-only syntax (missing if, contains, or import rego.v1), the action fails immediately with OPA's error output identifying the offending files and line numbers.

  • Test runner flag is now driven by the same input — when v1_compatible_check: true, opa test also runs with --v1-compatible so the syntax validation and test execution are consistent. When false, both revert to --v0-compatible.

  • All examples/ updated to Rego v1 syntax — replaced import future.keywords.* with import rego.v1, added if to rule bodies, and added contains to partial set rules across all 8 policies and 7 test files. All 46 tests still pass.

  • README updated — new input documented in the inputs table and How It Works section.

If the new V1 check fails, the report output will display as:

Screenshot 2026-04-20 at 11 25 28 AM

⚠️ Breaking change

This will be a major version bump. The v1_compatible_check input defaults to true, which means existing users with v0 syntax policies will see their workflows fail. To preserve previous behavior, set:

- uses: masterpointio/github-action-opa-rego-test@main
  with:
    path: ./policies
    v1_compatible_check: false
` `` 

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

* **New Features**
  * Added optional Rego v1 compatibility check to the action (enabled by default). When enabled the action validates all policy files and fails early on v0-only syntax.
  * Action now surfaces a dedicated v1-compatibility failure section in its output.

* **Documentation**
  * README updated with a new `v1_compatible_check` input to control this behavior and adjusted workflow step ordering.

* **Examples**
  * Updated example policies and tests to Rego v1 syntax.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 17, 2026

📝 Walkthrough

Walkthrough

Adds an optional Rego v1 compatibility check to the action (new input v1_compatible_check) which runs opa check --v1-compatible before tests. Migrates example policies and tests to Rego v1 syntax and threads a v1/v0 compatibility flag through OPA command execution.

Changes

Cohort / File(s) Summary
Action config & docs
README.md, action.yml
Added v1_compatible_check input (default true); documentation updated and input passed into the Node runtime environment.
Core action flow
src/index.ts
Added useV1Compatible env flag, runs executeOpaV1CompatibilityCheck(path) when enabled, short-circuits further test/coverage processing on failure, appends v1-check failure output, and marks action failed on v1-check errors; threads useV1Compatible into test functions.
OPA command utilities
src/opaCommands.ts
Added executeOpaV1CompatibilityCheck(); extended executeOpaTestByDirectory() and executeIndividualOpaTests() with useV1Compatible to select --v1-compatible vs --v0-compatible for OPA invocations.
Example policies (imports + rule-head updates)
examples/cancel-in-progress-runs.rego, examples/do-not-delete-stateful-resources.rego, examples/drift-detection.rego, examples/enforce-module-use-policy.rego, examples/enforce-password-length.rego, examples/track-using-labels.rego
Replaced future.keywords.* imports with rego.v1; converted indexed rule heads to contains ... if { ... } or rule if { ... } forms preserving logic semantics.
Example policies (simple import-only changes)
examples/notification-stack-failure-origins.rego, examples/readers-writers-admins-teams.rego
Swapped future.keywords.* imports for rego.v1 with no other logic changes.
Example tests
examples/tests/*_test.rego (all files under examples/tests/)
Added import rego.v1 and converted test rule declarations from body-only form to if { ... } guarded form; assertions unchanged.

Sequence Diagram

sequenceDiagram
    participant GHA as GitHub Actions
    participant Node as Node Script
    participant OPA as OPA CLI
    participant FS as File System

    GHA->>Node: Start action (inputs incl. v1_compatible_check)
    Node->>Node: determine useV1Compatible flag
    alt v1_compatible_check = true
        Node->>OPA: opa check <path> --v1-compatible
        OPA->>FS: read Rego files
        FS-->>OPA: file contents
        OPA-->>Node: exit code + output
        alt exit code != 0
            Node-->>GHA: setFailed() with v1 check output
        else
            Node->>OPA: opa test <path> --v1-compatible --format=json
            OPA->>FS: read tests
            OPA-->>Node: test results
            Node-->>GHA: report test results
        end
    else v1_compatible_check = false
        Node->>OPA: opa test <path> --v0-compatible --format=json
        OPA-->>Node: test results
        Node-->>GHA: report test results
    end
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

Poem

🐰
Rego v1 hops, soft and spry,
contains if beneath the sky,
Old future gone, checks now aligned,
OPA inspects each Rego line,
Rabbits cheer — policies fly! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: adding a Rego v1 compatibility check feature with a breaking change marker (!) indicating the major version bump required.
Description check ✅ Passed The description is comprehensive and well-structured, covering what changed, the breaking change implications, and includes context with examples of the new input parameter and its default behavior.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/INT-331/add_opa_rego_v1_check

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@glaracuente glaracuente marked this pull request as ready for review April 17, 2026 20:30
@glaracuente glaracuente requested a review from oycyc as a code owner April 17, 2026 20:30
@glaracuente glaracuente requested a review from a team April 17, 2026 20:30
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (4)
examples/enforce-module-use-policy.rego (1)

37-48: Verify intended set membership semantics for this rule.

With deny contains failed_reasons if { ... failed_reasons := [...][_] }, the trailing [_] causes each comprehension element to become its own entry in deny (one membership per reason). That matches the prior deny[failed_reasons] { ... failed_reasons := [...][_] } behavior, so this is a faithful migration — just calling it out since the rule head now reads a bit ambiguously (looks like a single value is being added). A small readability improvement would be to replace the inner indexing with a direct iteration so the rule produces one reason per match without the [_] trick:

♻️ Optional readability refactor
-deny contains failed_reasons if {
-	# Did any of the resources fail to pass?
-	count(invalid_resources[_]) > 0
-
-	# Build a list of reasons for each failure
-	failed_reasons := [sprintf(
-		"Resource '%s' in top level module named '%s' is being created with the incorrect terraform module '%s'. Module(s) '%s' must be used instead.",
-		[reason.resource_type, reason.top_level_module_name, reason.resource_module_name, concat("', '", controlled_resource_types[reason.resource_type])],
-	) |
-		reason := invalid_resources[_]
-	][_]
-}
+deny contains failed_reason if {
+	some reason in invalid_resources
+	failed_reason := sprintf(
+		"Resource '%s' in top level module named '%s' is being created with the incorrect terraform module '%s'. Module(s) '%s' must be used instead.",
+		[reason.resource_type, reason.top_level_module_name, reason.resource_module_name, concat("', '", controlled_resource_types[reason.resource_type])],
+	)
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/enforce-module-use-policy.rego` around lines 37 - 48, The rule head
deny contains failed_reasons combined with the comprehension failed_reasons :=
[...][_] uses the trailing [_] to emit one deny entry per reason but reads
ambiguously; change the comprehension to iterate directly and bind each reason
per rule evaluation (e.g., remove the outer list indexing and yield reason in
the comprehension) so that deny produces one failed_reasons value per
invalid_resources item—update the comprehension that builds failed_reasons
(referencing invalid_resources, failed_reasons, and controlled_resource_types)
to a direct generator form instead of using the trailing [_] trick to improve
readability while preserving semantics.
src/opaCommands.ts (2)

77-79: Nit: de-duplicate compatFlag derivation.

The same ternary appears in both executeOpaTestByDirectory and executeIndividualOpaTests. Extracting a tiny helper at module scope keeps the two call sites in lockstep if a third compat mode ever gets added.

♻️ Proposed refactor
 const opaV0CompatibleFlag = "--v0-compatible"; // https://www.openpolicyagent.org/docs/latest/v0-compatibility/
 const opaV1CompatibleFlag = "--v1-compatible";
+
+const getCompatFlag = (useV1Compatible: boolean): string =>
+  useV1Compatible ? opaV1CompatibleFlag : opaV0CompatibleFlag;
-  const compatFlag = useV1Compatible
-    ? opaV1CompatibleFlag
-    : opaV0CompatibleFlag;
+  const compatFlag = getCompatFlag(useV1Compatible);

Also applies to: 170-172

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/opaCommands.ts` around lines 77 - 79, Both executeOpaTestByDirectory and
executeIndividualOpaTests compute the same compatFlag ternary; extract that
logic into a small module‑scope helper (e.g., getCompatFlag or deriveCompatFlag)
that takes the useV1Compatible boolean and returns opaV1CompatibleFlag or
opaV0CompatibleFlag so both functions call the helper instead of duplicating the
ternary; update both call sites (executeOpaTestByDirectory and
executeIndividualOpaTests) to use the new helper and remove the inline ternary
expressions.

14-39: Optional: use JSON output and surface stdout.

opa check supports --format=json, which would let the PR comment render file/line offenders in a structured way instead of raw text. If you prefer to keep raw output, no change needed — but consider that the caller in src/index.ts currently drops the output field, so anything OPA writes to stdout is lost. Either drop output from the return shape, or have callers concatenate both (see related comment in src/index.ts).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/opaCommands.ts` around lines 14 - 39, The function
executeOpaV1CompatibilityCheck currently captures stdout but callers drop the
output; change the OPA invocation to use structured output by adding the
"--format=json" flag to the exec args (i.e., ["check", path,
opaV1CompatibleFlag, "--format=json"]) and keep returning output so callers can
render JSON file/line offenders, then update the caller in src/index.ts to
concatenate or use both return.output and return.error (or otherwise surface
output) instead of ignoring it; alternatively, if you prefer not to emit stdout,
remove the output field from executeOpaV1CompatibilityCheck's return type and
ensure callers only use error/exitCode.
README.md (1)

108-115: Minor: step sequence framing.

Step 3 reads as a distinct action step, but the v1 compatibility check is actually executed inside the existing Execute and Parse Results from Tests node step (in src/index.ts), not as its own composite step in action.yml. The current wording is fine for users thinking in logical stages, but if you want literal fidelity with action.yml, consider grouping steps 3–6 under the single "Execute and Parse Results" step or noting that compatibility check, tests, coverage, and parsing all run in one node step.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 108 - 115, Update README.md to accurately reflect the
implementation: change the framing so that steps 3–6 (the Rego v1 compatibility
check, running opa test, coverage tests, and parsing/formatting) are described
as part of the single "Execute and Parse Results from Tests" node step rather
than separate action steps; reference that the v1 compatibility check is
performed inside src/index.ts and that action.yml defines one composite node
step that runs those tasks, or alternatively note that steps 3–6 are logical
sub-steps executed by that single node step.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@action.yml`:
- Around line 48-51: Update consumer-facing communications and the failure
message for the OPA v1 compatibility check: add a prominent note to the release
notes / CHANGELOG calling out that v1_compatible_check defaults to "true" (or
that it will change) and how to opt out during migration; in code, update the
error path in src/index.ts that currently fails the action on OPA compatibility
errors to append a clear hint telling users they can set v1_compatible_check:
false to restore prior behavior while they migrate (refer to the
v1_compatible_check action input and the error/exit branch that reports opa
check failures). Optionally, consider changing the action.yml default for
v1_compatible_check from "true" to "false" for this release to avoid breaking
main consumers and document that plan in the release notes.

In `@src/index.ts`:
- Around line 35-48: When useV1Compatible is true and
executeOpaV1CompatibilityCheck(path) returns a non-zero exit code, don't
early-return after core.setFailed; instead assemble a full diagnostic string
that includes both v1Error (stderr) and the captured output (stdout) (use the
variables returned by executeOpaV1CompatibilityCheck such as error and output or
build a `details` string), call core.setOutput("parsed_results", commentBody)
and core.setOutput("tests_failed", "true") with that diagnostic/commentBody,
then call core.setFailed with the combined `details` message; update the failure
branch in the if (v1ExitCode !== 0) block (references: useV1Compatible,
executeOpaV1CompatibilityCheck, core.setOutput("parsed_results"),
core.setOutput("tests_failed"), core.setFailed) so the PR comment step receives
the parsed_results even on failure.

---

Nitpick comments:
In `@examples/enforce-module-use-policy.rego`:
- Around line 37-48: The rule head deny contains failed_reasons combined with
the comprehension failed_reasons := [...][_] uses the trailing [_] to emit one
deny entry per reason but reads ambiguously; change the comprehension to iterate
directly and bind each reason per rule evaluation (e.g., remove the outer list
indexing and yield reason in the comprehension) so that deny produces one
failed_reasons value per invalid_resources item—update the comprehension that
builds failed_reasons (referencing invalid_resources, failed_reasons, and
controlled_resource_types) to a direct generator form instead of using the
trailing [_] trick to improve readability while preserving semantics.

In `@README.md`:
- Around line 108-115: Update README.md to accurately reflect the
implementation: change the framing so that steps 3–6 (the Rego v1 compatibility
check, running opa test, coverage tests, and parsing/formatting) are described
as part of the single "Execute and Parse Results from Tests" node step rather
than separate action steps; reference that the v1 compatibility check is
performed inside src/index.ts and that action.yml defines one composite node
step that runs those tasks, or alternatively note that steps 3–6 are logical
sub-steps executed by that single node step.

In `@src/opaCommands.ts`:
- Around line 77-79: Both executeOpaTestByDirectory and
executeIndividualOpaTests compute the same compatFlag ternary; extract that
logic into a small module‑scope helper (e.g., getCompatFlag or deriveCompatFlag)
that takes the useV1Compatible boolean and returns opaV1CompatibleFlag or
opaV0CompatibleFlag so both functions call the helper instead of duplicating the
ternary; update both call sites (executeOpaTestByDirectory and
executeIndividualOpaTests) to use the new helper and remove the inline ternary
expressions.
- Around line 14-39: The function executeOpaV1CompatibilityCheck currently
captures stdout but callers drop the output; change the OPA invocation to use
structured output by adding the "--format=json" flag to the exec args (i.e.,
["check", path, opaV1CompatibleFlag, "--format=json"]) and keep returning output
so callers can render JSON file/line offenders, then update the caller in
src/index.ts to concatenate or use both return.output and return.error (or
otherwise surface output) instead of ignoring it; alternatively, if you prefer
not to emit stdout, remove the output field from
executeOpaV1CompatibilityCheck's return type and ensure callers only use
error/exitCode.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9b8bcb89-6cb4-4e7f-b1c4-03c664adacc0

📥 Commits

Reviewing files that changed from the base of the PR and between ec618c3 and 62710f8.

⛔ Files ignored due to path filters (2)
  • dist/index.js is excluded by !**/dist/**
  • dist/index.js.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (19)
  • README.md
  • action.yml
  • examples/cancel-in-progress-runs.rego
  • examples/do-not-delete-stateful-resources.rego
  • examples/drift-detection.rego
  • examples/enforce-module-use-policy.rego
  • examples/enforce-password-length.rego
  • examples/notification-stack-failure-origins.rego
  • examples/readers-writers-admins-teams.rego
  • examples/tests/cancel-in-progress-runs_test.rego
  • examples/tests/do-not-delete-stateful-resources_test.rego
  • examples/tests/enforce-module-use-policy_test.rego
  • examples/tests/enforce-password-length_test.rego
  • examples/tests/ignore-changes-outside-root_test.rego
  • examples/tests/readers-writers-admins-teams_test.rego
  • examples/tests/track-using-labels_test.rego
  • examples/track-using-labels.rego
  • src/index.ts
  • src/opaCommands.ts

Comment thread action.yml
Comment thread src/index.ts
@gberenice
Copy link
Copy Markdown
Member

@glaracuente since it's a breaking change, change the PR name from feat: X to feat!: X. This would tell the googleapis/release-please GHA we use to release a new major version.

@glaracuente glaracuente marked this pull request as draft April 20, 2026 14:08
@glaracuente glaracuente changed the title feat: add OPA Rego v1 compatibility check (INT-331) feat!: add OPA Rego v1 compatibility check (INT-331) Apr 20, 2026
@masterpointio masterpointio deleted a comment from github-actions Bot Apr 20, 2026
@masterpointio masterpointio deleted a comment from github-actions Bot Apr 20, 2026
@masterpointio masterpointio deleted a comment from github-actions Bot Apr 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 20, 2026

Below is the Action testing on itself with this PR's source code against policies in /examples file by file. Confirm it is as expected.

File Status Passed Total Coverage Details
./examples/tests/do-not-delete-stateful-resources_test.rego ✅ PASS 5 5 85.71%
Uncovered Lines34
Show Details✅ test_deny_s3_bucket_deletion
✅ test_deny_db_instance_deletion
✅ test_deny_efs_file_system_deletion
✅ test_deny_dynamodb_table_deletion
✅ test_allow_instance_deletion
./examples/tests/track-using-labels_test.rego ✅ PASS 8 8 45.45%
Uncovered Lines5, 7, 14-15, 25-28, 37, 39-40, 43
Show Details✅ test_track_different_branches
✅ test_propose_non_empty_branch
✅ test_propose_empty_branch
✅ test_affected_directory
✅ test_affected_extension
✅ test_not_affected_directory
✅ test_not_affected_extension
✅ test_ignore_not_affected
./examples/tests/enforce-password-length_test.rego ✅ PASS 3 3 90.91%
Uncovered Lines29
Show Details✅ test_deny_creation_of_password_with_less_than_16_characters
✅ test_warn_creation_of_password_between_16_and_20_characters
✅ test_allow_creation_of_password_longer_than_20_characters
./examples/tests/enforce-module-use-policy_test.rego ✅ PASS 3 3 47.83%
Uncovered Lines37, 42, 46, 52, 54, 57, 60-61, 64, 68, 78, 80
Show Details✅ test_deny_creation_of_controlled_resource_type
✅ test_deny_update_of_controlled_resource_type
✅ test_allow_creation_of_uncontrolled_resource_type
./examples/tests/readers-writers-admins-teams_test.rego ✅ PASS 6 6 83.33%
Uncovered Lines14, 22, 26
Show Details✅ test_allow_writers
✅ test_allow_admins
✅ test_allow_readers
✅ test_space_admin_access
✅ test_space_write_access
✅ test_space_read_access
./examples/tests/ignore-changes-outside-root_test.rego ✅ PASS 12 12 92.86%
Uncovered Lines42
Show Details✅ test_affected_no_files
✅ test_affected_tf_files
✅ test_affected_no_tf_files
✅ test_affected_outside_project_root
✅ test_ignore_affected
✅ test_ignore_not_affected
✅ test_ignore_tag
✅ test_propose_affected
✅ test_propose_not_affected
✅ test_track_affected
✅ test_track_not_affected
✅ test_track_not_stack_branch
./examples/tests/notification-stack-failure-origins_test.rego ✅ PASS 7 7 96.67%
Uncovered Lines79
Show Details✅ test_slack_notification_for_tracked_failed_run
✅ test_no_slack_notification_for_non_tracked_run
✅ test_no_slack_notification_for_successful_run
✅ test_slack_notification_with_unknown_github_user
✅ test_pr_comment_for_tracked_failed_run
✅ test_no_pr_comment_for_non_tracked_run
✅ test_no_pr_comment_for_successful_run
./examples/tests/cancel-in-progress-runs_test.rego ✅ PASS 2 2 83.33%
Uncovered Lines18
Show Details✅ test_cancel_runs_allowed
✅ test_cancel_runs_denied
./examples/drift-detection.rego ⚠️ NO TESTS 0 0 N/A
Show DetailsNo test file found

Report generated by 🧪 GitHub Actions for OPA Rego Test

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 20, 2026

Below is the Action testing on itself with this PR's source code against /examples entire package directory. Confirm it is as expected.

File Status Passed Total Coverage Details
examples/tests/cancel-in-progress-runs_test.rego ✅ PASS 2 2 83.33%
Uncovered Lines18
Show Details✅ test_cancel_runs_allowed
✅ test_cancel_runs_denied
examples/tests/do-not-delete-stateful-resources_test.rego ✅ PASS 5 5 85.71%
Uncovered Lines34
Show Details✅ test_deny_s3_bucket_deletion
✅ test_deny_db_instance_deletion
✅ test_deny_efs_file_system_deletion
✅ test_deny_dynamodb_table_deletion
✅ test_allow_instance_deletion
examples/tests/enforce-module-use-policy_test.rego ✅ PASS 3 3 47.83%
Uncovered Lines37, 42, 46, 52, 54, 57, 60-61, 64, 68, 78, 80
Show Details✅ test_deny_creation_of_controlled_resource_type
✅ test_deny_update_of_controlled_resource_type
✅ test_allow_creation_of_uncontrolled_resource_type
examples/tests/enforce-password-length_test.rego ✅ PASS 3 3 90.91%
Uncovered Lines29
Show Details✅ test_deny_creation_of_password_with_less_than_16_characters
✅ test_warn_creation_of_password_between_16_and_20_characters
✅ test_allow_creation_of_password_longer_than_20_characters
examples/tests/ignore-changes-outside-root_test.rego ✅ PASS 12 12 92.86%
Uncovered Lines42
Show Details✅ test_affected_no_files
✅ test_affected_tf_files
✅ test_affected_no_tf_files
✅ test_affected_outside_project_root
✅ test_ignore_affected
✅ test_ignore_not_affected#01
✅ test_ignore_tag
✅ test_propose_affected
✅ test_propose_not_affected
✅ test_track_affected
✅ test_track_not_affected
✅ test_track_not_stack_branch
examples/tests/notification-stack-failure-origins_test.rego ✅ PASS 7 7 96.67%
Uncovered Lines79
Show Details✅ test_slack_notification_for_tracked_failed_run
✅ test_no_slack_notification_for_non_tracked_run
✅ test_no_slack_notification_for_successful_run
✅ test_slack_notification_with_unknown_github_user
✅ test_pr_comment_for_tracked_failed_run
✅ test_no_pr_comment_for_non_tracked_run
✅ test_no_pr_comment_for_successful_run
examples/tests/readers-writers-admins-teams_test.rego ✅ PASS 6 6 83.33%
Uncovered Lines14, 22, 26
Show Details✅ test_allow_writers
✅ test_allow_admins
✅ test_allow_readers
✅ test_space_admin_access
✅ test_space_write_access
✅ test_space_read_access
examples/tests/track-using-labels_test.rego ✅ PASS 8 8 86.36%
Uncovered Lines5, 14, 43
Show Details✅ test_track_different_branches
✅ test_propose_non_empty_branch
✅ test_propose_empty_branch
✅ test_affected_directory
✅ test_affected_extension
✅ test_not_affected_directory
✅ test_not_affected_extension
✅ test_ignore_not_affected
./examples/drift-detection.rego ⚠️ NO TESTS 0 0 N/A
Show DetailsNo test file found

Report generated by 🧪 GitHub Actions for OPA Rego Test

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 20, 2026

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements 95.56% 86/90
🟢 Branches 93.1% 27/29
🟢 Functions 100% 8/8
🟢 Lines 96.63% 86/89

Test suite run success

15 tests passing in 1 suite.

Report generated by 🧪jest coverage report action from 716c4c7

@glaracuente glaracuente marked this pull request as ready for review April 20, 2026 15:26
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/index.ts (1)

114-128: ⚠️ Potential issue | 🟡 Minor

The misleading errorString fallback concern is incorrect; tests_failed output issue is valid but with clarification.

The first concern about formatResults([], [], runCoverageReport) returning "" and triggering the errorString fallback is not accurate. The formatResults function always initializes output with a title and table headers (lines 15–28 of src/formatResults.ts), so it never returns an empty string—even with empty inputs. The condition on line 114 of src/index.ts (if (formattedOutput === "") { ... }) never executes in practice, and errorString is not appended on v1 failure.

The second concern about tests_failed is valid: when v1CheckFailed is true, parsedResults remains empty (test execution is skipped at line 63), so testsFailed is false, and core.setOutput("tests_failed", "false") is set even though the action failed. This is problematic for any external process gating on the output (e.g., in the action's own PR comment step or downstream workflows). However, note that the PR comment step in action.yml has no condition on tests_failed—it posts the v1 diagnostic regardless, guarded only by write_pr_comment and success() || failure().

Recommendation: Either include v1 failure in the tests_failed flag (or add a separate v1_check_failed output) so that callers can properly gate on the action's true failure state. The current tests_failed = parsedResults.some(...) approach leaves v1 failures invisible to output consumers.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/index.ts` around lines 114 - 128, formattedOutput never becomes empty so
the errorString fallback branch is dead; the real issue is that when
v1CheckFailed is true parsedResults is empty and testsFailed computes false,
hiding the failure. Update the logic around
v1CheckFailed/parsedResults/testsFailed: either (A) set
core.setOutput("tests_failed", "true") when v1CheckFailed is true before
computing testsFailed, or (B) add a new output like
core.setOutput("v1_check_failed", v1CheckFailed.toString()) and keep
tests_failed as-is; make the change where formattedOutput, v1CheckFailed,
parsedResults and testsFailed are handled (symbols: formattedOutput,
errorString, v1CheckFailed, parsedResults, testsFailed, core.setOutput) so
downstream callers can correctly detect v1 compatibility failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/index.ts`:
- Around line 114-128: formattedOutput never becomes empty so the errorString
fallback branch is dead; the real issue is that when v1CheckFailed is true
parsedResults is empty and testsFailed computes false, hiding the failure.
Update the logic around v1CheckFailed/parsedResults/testsFailed: either (A) set
core.setOutput("tests_failed", "true") when v1CheckFailed is true before
computing testsFailed, or (B) add a new output like
core.setOutput("v1_check_failed", v1CheckFailed.toString()) and keep
tests_failed as-is; make the change where formattedOutput, v1CheckFailed,
parsedResults and testsFailed are handled (symbols: formattedOutput,
errorString, v1CheckFailed, parsedResults, testsFailed, core.setOutput) so
downstream callers can correctly detect v1 compatibility failures.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 056c0cd0-dfcc-48b4-8b6a-3778bf23d07f

📥 Commits

Reviewing files that changed from the base of the PR and between 62710f8 and 716c4c7.

⛔ Files ignored due to path filters (2)
  • dist/index.js is excluded by !**/dist/**
  • dist/index.js.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (1)
  • src/index.ts

Copy link
Copy Markdown
Member

@gberenice gberenice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks 🙌

@glaracuente glaracuente merged commit e245317 into main Apr 27, 2026
8 checks passed
@glaracuente glaracuente deleted the feature/INT-331/add_opa_rego_v1_check branch April 27, 2026 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants