Add CI-ready entitlement smart diff - #74
Open
hosom wants to merge 10 commits into
Open
Conversation
Export deterministic desired memberships and compare PR base/head without provider reconciliation. Report incomplete results when arbitrary Ruby groups prevent a complete offline calculation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b96e14a4-88bb-45b9-9b94-a681a34d05bc
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Dynamic filter dependencies and LDAP snapshot assumptions can produce unsafe or falsely complete results.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (4)
| Severity | Finding |
|---|---|
 |
lib/entitlements.rb — This removes every extension-defined constant under Entitlements::Rule, not only classes loaded… |
 |
lib/entitlements/desired_groups.rb — skip_dynamic_groups is not propagated through calculated-group filters. Filters are instantiated… |
|
lib/entitlements/extras/ldap_group/rules/ldap_group.rb — Missing shellentitlements is treated as definite non-membership, although that attribute is… |
 |
README.md — Remove the stray -app suffix. This issue also appears on line 21 of the same file. |
What changed in this PR
Adds deterministic, CI-ready entitlement membership comparison using frozen people and time inputs.
Changes:
- Adds desired-membership export and bounded JSON/Markdown smart diff.
- Handles dynamic groups, mirrors, LDAP rules, and frozen expiration evaluation.
- Packages the new executable and adds comprehensive fixtures/specs.
| File | Description |
|---|---|
.gitignore |
Tracks the new executable. |
README.md |
Documents CI smart diff usage. |
Gemfile.lock |
Updates the gem version. |
entitlements-app.gemspec |
Packages the executable. |
bin/entitlements-smart-diff |
Adds the command entry point. |
lib/version.rb |
Bumps version to 1.2.2. |
lib/entitlements.rb |
Adds evaluation time, reset behavior, and requires. |
lib/entitlements/desired_groups.rb |
Implements deterministic membership export. |
lib/entitlements/smart_diff.rb |
Implements comparison and report rendering. |
lib/entitlements/smart_diff/cli.rb |
Implements CLI parsing and output. |
lib/entitlements/data/groups/calculated.rb |
Adds dynamic-group skipping and mirror warnings. |
lib/entitlements/data/groups/calculated/base.rb |
Cleans up failed dynamic dependencies. |
lib/entitlements/data/groups/calculated/ruby.rb |
Detects and reloads Ruby groups. |
lib/entitlements/data/groups/calculated/text.rb |
Uses frozen evaluation time. |
lib/entitlements/data/groups/calculated/modifiers/expiration.rb |
Freezes expiration evaluation. |
lib/entitlements/data/groups/calculated/rules/group.rb |
Resets caches and propagates options. |
lib/entitlements/extras/ldap_group/rules/ldap_group.rb |
Adds snapshot-based offline matching. |
spec/unit/entitlements/desired_groups_spec.rb |
Tests export behavior. |
spec/unit/entitlements/smart_diff_spec.rb |
Tests comparison and rendering. |
spec/unit/entitlements/extras/ldap_group/rules/ldap_group_spec.rb |
Tests offline LDAP matching. |
spec/unit/fixtures/smart-diff/config.yaml |
Configures smart-diff fixtures. |
spec/unit/fixtures/smart-diff/people.yaml |
Supplies frozen people data. |
spec/unit/fixtures/smart-diff/groups/internal/contractors.txt |
Defines contractor membership. |
spec/unit/fixtures/smart-diff/groups/internal/engineers.txt |
Defines engineer membership. |
spec/unit/fixtures/smart-diff/groups/teams/direct.txt |
Defines direct membership. |
spec/unit/fixtures/smart-diff/groups/teams/expiring.yaml |
Exercises expiration behavior. |
spec/unit/fixtures/smart-diff/groups/teams/filtered.txt |
Exercises filtering. |
spec/unit/fixtures/smart-diff/groups/teams/nested.yaml |
Exercises nested groups. |
spec/unit/fixtures/smart-diff/groups/teams/ruby-group.rb |
Exercises Ruby membership. |
spec/unit/fixtures/dynamic-groups/config.yaml |
Configures dynamic-group fixtures. |
spec/unit/fixtures/dynamic-groups/groups/teams/dependent.txt |
Tests dynamic dependencies. |
spec/unit/fixtures/dynamic-groups/groups/teams/dynamic.rb |
Defines unsafe dynamic behavior. |
spec/unit/fixtures/dynamic-groups/groups/teams/static-ruby.rb |
Tests conservative Ruby detection. |
spec/unit/fixtures/dynamic-groups/groups/teams/static.txt |
Provides exportable static membership. |
Suppressed comments (1)
README.md:21
- Remove the stray
-appsuffix.
Ruby entitlement groups are treated as dynamic by smart diff because arbitrary Ruby cannot be proven deterministic from frozen inputs. Groups that use or transitively depend on Ruby definitions are omitted from the membership comparison, and both JSON and Markdown report that the result is incomplete. Normal deployment behavior is unchanged.-app
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+107
to
+109
| Entitlements::Rule.constants(false).each do |constant| | ||
| Entitlements::Rule.send(:remove_const, constant) unless constant == :Base | ||
| end |
| Entitlements::Data::Groups::Calculated.read_all( | ||
| group_name, | ||
| group_config, | ||
| skip_dynamic_groups: allow_incomplete |
Comment on lines
+25
to
+31
| if Entitlements.cache[:desired_groups_export] | ||
| return Set.new(Entitlements.cache[:people_obj].read.values.select do |person| | ||
| begin | ||
| Array(person["shellentitlements"]).map(&:downcase).include?(value.downcase) | ||
| rescue KeyError | ||
| false | ||
| end |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b96e14a4-88bb-45b9-9b94-a681a34d05bc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b96e14a4-88bb-45b9-9b94-a681a34d05bc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b96e14a4-88bb-45b9-9b94-a681a34d05bc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b96e14a4-88bb-45b9-9b94-a681a34d05bc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b96e14a4-88bb-45b9-9b94-a681a34d05bc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b96e14a4-88bb-45b9-9b94-a681a34d05bc
hosom
commented
Sep 3, 2026
hosom
commented
Sep 3, 2026
|
|
||
| Entitlements.logger.debug "Calculating members from #{filename}" | ||
| result = members_from_rules(rules) | ||
| @members = result unless result == :calculating |
Contributor
Author
There was a problem hiding this comment.
This is both a general evaluator bug fix and a requirement for smart diff: :calculating is a transient in-progress sentinel, so caching it here can permanently poison the file object and prevent a later retry from producing the actual member set.
hosom
commented
Sep 3, 2026
|
|
||
| Entitlements.logger.debug "Calculating members from #{filename}" | ||
| result = members_from_rules(rules) | ||
| @members = result unless result == :calculating |
Contributor
Author
There was a problem hiding this comment.
This is both a general evaluator bug fix and a requirement for smart diff: :calculating is a transient in-progress sentinel, so caching it here can permanently poison the file object and prevent a later retry from producing the actual member set.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 38d1ec2b-befd-4535-8e8a-e32447bd9f1a
Track and unload only constants introduced by entitlement Ruby files. Propagate dynamic skipping through filters, omit dynamic dependency closures, and render collapsible backend-specific report tables without warning output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ee634509-607d-4e8e-8072-59a390fecedf
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
entitlements-smart-difffor base/head JSON and bounded Markdown reportsentitlements-appto 1.2.2 and package the new executableValidation
script/test(552 examples, 100% coverage)entitlements-app-1.2.2.gem