Skip to content

core: prefix internal helpers with an underscore so the public rules are clear - #98

Merged
jbpros merged 1 commit into
mainfrom
core/private-helpers
Oct 2, 2026
Merged

jbpros merged 1 commit into
mainfrom
core/private-helpers

Conversation

@jbpros

@jbpros jbpros commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Closes #27.

Eight rules keep their names, because the docs tell people to call or define them: report, report_with_params, violations, operators, op_passed, value_at, arg and leaf_passed. The other 227 now start with _.

This breaks any policy that calls a helper directly, like ergo.row_cause.

How the rename was done

OPA lets a local variable or parameter share a rule's name, so a plain find-and-replace wasn't safe. A script renamed identifiers outside strings and field reads (.x), and would have skipped any rule body that declared a local with the same name. It found none. Calls from other packages were renamed only where written as ergo.x.

Other changes

  • Four tests in ergo_test.rego call _is_violation, _definition_field and _rfc3339_shaped directly. Two pin that violations reads only the requirements, and two test the RFC 3339 gate on its own (compare_time accepts numbers through another branch), so neither can be checked through the report. They stay where they are: Regal's leaked-internal-reference doesn't apply to _test.rego files, and moving them into package ergo would put test rules into data.ergo.
  • REFERENCE.md now names exactly the rules a custom operator may call, says what each does, and says the rest can change. It also drops check_passed from the list of rules that cause a loop, since it's private now, and fixes "Two rules:" over a list of three.

Checks

  • opa check --strict . --ignore .github, opa fmt --list . (prints nothing), opa test . --ignore .github (642/642, the same count as main), and the --strict-builtin-errors run prints true.
  • The coverage command lists no unreached lines.
  • Only ergo_test.rego calls a _ rule from outside package ergo.
  • The baking example's report is byte-identical to main's.
  • regal lint --disable-all --enable leaked-internal-reference --ignore-files '.github/**' . finds nothing.
  • 1,000 random policies and inputs give byte-identical reports and violations on main and this branch.

#34 is the only open PR touching ergo.rego, so it will need its helper calls renamed when rebased.

🤖 Generated with Claude Code

@jbpros jbpros added the core Relates to the ergo library or format themselves. label Oct 2, 2026
…are clear

Only report, report_with_params, violations, operators, op_passed,
value_at, arg and leaf_passed keep their names. Every other rule now
starts with _, so a policy that calls one of them directly, like
ergo.row_cause, breaks and should use the report instead.

Closes #27

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jbpros
jbpros force-pushed the core/private-helpers branch from b587e6c to 6769820 Compare October 2, 2026 21:20
@jbpros
jbpros marked this pull request as ready for review October 2, 2026 21:22
@jbpros
jbpros merged commit 5a3b8be into main Oct 2, 2026
4 checks passed
@jbpros
jbpros deleted the core/private-helpers branch October 2, 2026 21:23
jbpros added a commit that referenced this pull request Oct 2, 2026
Follows #98. CI now runs
[Regal](https://www.openpolicyagent.org/projects/regal)'s `bugs` rules,
so it fails when a file outside `package ergo` calls a rule whose name
starts with `_` (`leaked-internal-reference`). Test files are exempt.

Only the `bugs` rules are on. All of Regal's rules together report about
400 findings, mostly line length and references it can't resolve across
the test and custom operator packages, and each would need its own
decision.

`redundant-existence-check` is off. Its one finding is
`_verdict_cause(passed) := "satisfied" if passed`, and that condition is
needed: removing it makes 46 tests error, because a failed check then
matches both `_verdict_cause` rules.

The flags are on the command line, not in `.regal/config.yaml`, because
OPA loads every YAML file it finds as data. AGENTS.md lists the command
with the other checks.

Regal is pinned to 0.43.0 through
`open-policy-agent/setup-regal@v2.0.0`. That action has no `v2` tag, so
Dependabot will bump it.

## Checks

- `regal lint --disable-all --enable-category bugs --disable
redundant-existence-check --ignore-files '.github/**' .` says `8 files
linted. No violations found.`
- On a scratch package where another package calls `ergo._x`, the same
rules exit 3 and flag the call.
- This PR's own CI run is the first time the workflow step runs.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
jbpros added a commit that referenced this pull request Oct 2, 2026
…rest (#103)

Follows #98. REFERENCE.md's "Policies" section now says a policy should
call only `ergo.report`, `ergo.report_with_params` and
`ergo.violations`, that `_` rules can change in any release, and that
Regal's `leaked-internal-reference` flags a call to one. Our CI can't
see users' repos, so this is how they find out.

I checked the Regal claim with Regal 0.43.0 on a scratch folder holding
`ergo.rego` and a `package policy` file that calls `ergo._row_cause` and
`ergo.report`. It flagged only the `_row_cause` call.

Custom operators are covered by the existing list in "Custom operators",
and #101 is about linting them.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Relates to the ergo library or format themselves.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

core: prefix internal helpers with an underscore so the public rules are clear

1 participant