What happened
PR #1796 added a new hermeto_attribution_required deny rule to both policy/release/sbom_cyclonedx/sbom_cyclonedx.rego and policy/release/sbom_spdx/sbom_spdx.rego. The CycloneDX implementation uses component_found_by_hermeto which does exact property matching on {"name": "hermeto:found_by", "value": "hermeto"}, while the SPDX implementation uses package_found_by_hermeto which does broad regex matching on the annotator field containing "hermeto" or "cachi2". Three automated reviewers (qodo, CodeRabbit, fullsend-ai-review) and one human reviewer (simonbaird) reviewed the PR. None of them verified that the two format implementations had equivalent strictness in attribution checking. The rule-level tests also lacked explicit cachi2:found_by acceptance path coverage for the new deny rule (library-level tests cover cachi2, but the deny rule tests do not). The AGENTS.md review checklist currently has only one item (effective_on date requirement) and does not mention CycloneDX/SPDX parity.
What could go better
When deny rules are added to both SBOM format paths (a common pattern in this codebase — most rules exist in parallel across CycloneDX and SPDX), reviewers should verify that the implementations have equivalent behavior, not just equivalent structure. The asymmetry in this PR may be intentional (SPDX annotator matching may be the idiomatic approach for that format), but without explicit guidance, reviewers — both human and automated — lack the context to evaluate whether the difference is a deliberate design choice or an oversight. This gap is structural: every new dual-format rule inherits the asymmetry of the shared helpers unless reviewers know to check for it. Confidence: high that the gap exists and is not currently covered. Uncertainty: moderate about whether the CycloneDX/SPDX helper asymmetry in this specific case is intentional or an oversight — the author's dismissal of qodo's related finding ("pre-existing behavior") suggests awareness but not explicit endorsement.
Proposed change
Add a CycloneDX/SPDX parity checklist item to the "Review Checklist for New Policy Rules" section of AGENTS.md. Specifically, after the existing effective_on checklist item, add guidance such as: "When a rule is implemented in both sbom_cyclonedx and sbom_spdx, verify that the two implementations enforce equivalent behavior. Check that shared helper functions (e.g., component_found_by_hermeto vs package_found_by_hermeto) apply the same level of strictness. Verify that test coverage for both format implementations covers all attribution sources (hermeto:found_by and cachi2:found_by) and edge cases (empty components/packages, mixed attribution, unconfigured rule data)." This gives both human reviewers and the fullsend review agent the context needed to catch implementation asymmetries in dual-format rules.
Validation criteria
The next PR that adds or modifies a deny rule in both sbom_cyclonedx and sbom_spdx should receive at least one review comment (human or automated) that explicitly verifies parity between the two implementations. The AGENTS.md file should contain the new checklist item. The fullsend review agent, when given this guidance, should flag any behavioral asymmetry between CycloneDX and SPDX implementations as a finding.
Generated by retro agent from #1796
What happened
PR #1796 added a new
hermeto_attribution_requireddeny rule to bothpolicy/release/sbom_cyclonedx/sbom_cyclonedx.regoandpolicy/release/sbom_spdx/sbom_spdx.rego. The CycloneDX implementation usescomponent_found_by_hermetowhich does exact property matching on{"name": "hermeto:found_by", "value": "hermeto"}, while the SPDX implementation usespackage_found_by_hermetowhich does broad regex matching on theannotatorfield containing "hermeto" or "cachi2". Three automated reviewers (qodo, CodeRabbit, fullsend-ai-review) and one human reviewer (simonbaird) reviewed the PR. None of them verified that the two format implementations had equivalent strictness in attribution checking. The rule-level tests also lacked explicitcachi2:found_byacceptance path coverage for the new deny rule (library-level tests cover cachi2, but the deny rule tests do not). The AGENTS.md review checklist currently has only one item (effective_ondate requirement) and does not mention CycloneDX/SPDX parity.What could go better
When deny rules are added to both SBOM format paths (a common pattern in this codebase — most rules exist in parallel across CycloneDX and SPDX), reviewers should verify that the implementations have equivalent behavior, not just equivalent structure. The asymmetry in this PR may be intentional (SPDX annotator matching may be the idiomatic approach for that format), but without explicit guidance, reviewers — both human and automated — lack the context to evaluate whether the difference is a deliberate design choice or an oversight. This gap is structural: every new dual-format rule inherits the asymmetry of the shared helpers unless reviewers know to check for it. Confidence: high that the gap exists and is not currently covered. Uncertainty: moderate about whether the CycloneDX/SPDX helper asymmetry in this specific case is intentional or an oversight — the author's dismissal of qodo's related finding ("pre-existing behavior") suggests awareness but not explicit endorsement.
Proposed change
Add a CycloneDX/SPDX parity checklist item to the "Review Checklist for New Policy Rules" section of
AGENTS.md. Specifically, after the existingeffective_onchecklist item, add guidance such as: "When a rule is implemented in bothsbom_cyclonedxandsbom_spdx, verify that the two implementations enforce equivalent behavior. Check that shared helper functions (e.g.,component_found_by_hermetovspackage_found_by_hermeto) apply the same level of strictness. Verify that test coverage for both format implementations covers all attribution sources (hermeto:found_byandcachi2:found_by) and edge cases (empty components/packages, mixed attribution, unconfigured rule data)." This gives both human reviewers and the fullsend review agent the context needed to catch implementation asymmetries in dual-format rules.Validation criteria
The next PR that adds or modifies a deny rule in both
sbom_cyclonedxandsbom_spdxshould receive at least one review comment (human or automated) that explicitly verifies parity between the two implementations. The AGENTS.md file should contain the new checklist item. The fullsend review agent, when given this guidance, should flag any behavioral asymmetry between CycloneDX and SPDX implementations as a finding.Generated by retro agent from #1796