docs(#1799): add CycloneDX/SPDX parity checklist item - #1800
Open
fullsend-ai-coder[bot] wants to merge 1 commit into
Open
docs(#1799): add CycloneDX/SPDX parity checklist item#1800fullsend-ai-coder[bot] wants to merge 1 commit into
fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
Add a new review checklist item to the "Review Checklist for New Policy Rules" section of AGENTS.md. The item instructs reviewers to verify equivalent behavior when a deny/warn rule is implemented in both sbom_cyclonedx and sbom_spdx paths. This addresses a gap identified by retro analysis of PR #1796, where reviewers did not verify parity between CycloneDX and SPDX implementations of the hermeto_attribution_required rule. Closes #1799
|
🤖 Finished Review · ✅ Success · Started 5:53 PM UTC · Completed 6:03 PM UTC Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
ReviewFindingsMedium
Low
Labels: PR is a documentation-only change to AGENTS.md |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| the pattern. | ||
| - **Test coverage:** Every new rule needs tests in a corresponding `_test.rego` file. CI enforces | ||
| 100% coverage. | ||
| - **CycloneDX/SPDX parity:** When a rule is implemented in both `sbom_cyclonedx` and `sbom_spdx`, |
There was a problem hiding this comment.
[low] placement coherence
Issue #1799 proposes adding the parity checklist item 'after the existing effective_on checklist item,' but the diff places it after the 'Test coverage' item (the last existing item). The issue was written when effective_on was the only checklist item — 'after it' effectively meant 'at the end of the list.' Two items (Collection membership, Test coverage) were added subsequently, so appending to the current end is consistent with the original intent. The placement is defensible.
fullsend-ai-review
Bot
added
requires-manual-review
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.
What:
Add a CycloneDX/SPDX parity checklist item to the "Review Checklist for New Policy Rules" section of AGENTS.md.
Why:
PR #1796 added a new deny rule to both
sbom_cyclonedxandsbom_spdxwith different levels of strictness in the helper functions used. Multiple reviewers (automated and human) did not catch the asymmetry because AGENTS.md lacked guidance to check for parity between CycloneDX and SPDX implementations. The new checklist item instructs reviewers to verify equivalent behavior, matching helper strictness, and test coverage across attribution sources when rules span both SBOM format paths.Tickets:
Closes #1799
Post-script verification
agent/1799-add-parity-checklist)7c9b0e0f9032fb1345ec191be9126963d5a92453..HEAD)