You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
On PR #1787, the fullsend-ai-review bot flagged a medium-severity [missing-effective-on] finding on the new signature_verification warn rule at policy/release/sbom/sbom.rego. The bot cited AGENTS.md lines 92-98, which state: "New deny/warn rules MUST include an effective_on date in their METADATA annotation to provide a migration window before enforcement begins."
The PR author (st3penta) explained in the PR body and review responses that effective_on was deliberately omitted: the rule is opt-in (only fires when a user explicitly configures the sbom entry in signing_identities rule data) and fail-closed (produces no output when unconfigured). Builds without that configuration never trigger the rule, so there is no migration window to grant, and users who opt in want failures surfaced immediately.
The current AGENTS.md guidance was added via issue #1760, which originated from a retro on PR #1736 where effective_on was missing on mandatory enforcement rules. However, #1760's implementation applied the requirement as a blanket "MUST" with no exception for opt-in/fail-closed rules, creating a false positive on this PR.
What could go better
The AGENTS.md guidance at lines 92-98 is too broad. The effective_on convention exists to prevent new mandatory rules from breaking existing builds on deployment. But rules that are both opt-in (require explicit user configuration to activate) and fail-closed (produce no output when unconfigured) cannot break existing builds by definition — unconfigured builds are completely unaffected. Requiring effective_on on such rules provides no safety benefit and creates friction when authors must justify a deliberate omission against a blanket "MUST" in review.
Confidence: High. The PR author's reasoning is sound and well-documented. The signature_verification rule only fires when a user adds an sbom entry to signing_identities rule data AND a signature check then fails. Without that config, the rule is invisible. This pattern will recur as more opt-in verification rules are added (e.g., for other artifact types), so the guidance should be refined now rather than requiring case-by-case justification on each future PR.
Proposed change
Amend the effective_on checklist item in AGENTS.md (lines 92-98) to add an explicit exception clause for opt-in/fail-closed rules. The updated text should:
Keep the existing "MUST" for mandatory enforcement rules (deny/warn rules that fire by default or affect all builds without explicit opt-in).
Add an exception: Rules that are both opt-in (require explicit user configuration in rule data to activate) and fail-closed (produce no output when unconfigured) may omit effective_on, since unconfigured builds are unaffected and users who opt in expect immediate enforcement.
Exception: Rules that are both opt-in (require explicit user configuration to activate) and fail-closed (produce no output when unconfigured) may omit effective_on, since unconfigured builds are unaffected. When omitting effective_on for this reason, document the rationale in the PR description.
Validation criteria
The next review agent run on a PR that adds an opt-in/fail-closed rule without effective_on should not flag a missing-effective-on finding, provided the PR description documents the rationale.
The review agent should still flag missing effective_on on mandatory enforcement rules (deny/warn rules that fire by default).
Verify by checking the next 3 PRs that add new deny/warn rules to confirm the guidance produces correct review behavior for both mandatory and opt-in rules.
What happened
On PR #1787, the fullsend-ai-review bot flagged a medium-severity
[missing-effective-on]finding on the newsignature_verificationwarn rule atpolicy/release/sbom/sbom.rego. The bot cited AGENTS.md lines 92-98, which state: "New deny/warn rules MUST include an effective_on date in their METADATA annotation to provide a migration window before enforcement begins."The PR author (st3penta) explained in the PR body and review responses that
effective_onwas deliberately omitted: the rule is opt-in (only fires when a user explicitly configures thesbomentry insigning_identitiesrule data) and fail-closed (produces no output when unconfigured). Builds without that configuration never trigger the rule, so there is no migration window to grant, and users who opt in want failures surfaced immediately.The current AGENTS.md guidance was added via issue #1760, which originated from a retro on PR #1736 where
effective_onwas missing on mandatory enforcement rules. However, #1760's implementation applied the requirement as a blanket "MUST" with no exception for opt-in/fail-closed rules, creating a false positive on this PR.What could go better
The AGENTS.md guidance at lines 92-98 is too broad. The
effective_onconvention exists to prevent new mandatory rules from breaking existing builds on deployment. But rules that are both opt-in (require explicit user configuration to activate) and fail-closed (produce no output when unconfigured) cannot break existing builds by definition — unconfigured builds are completely unaffected. Requiringeffective_onon such rules provides no safety benefit and creates friction when authors must justify a deliberate omission against a blanket "MUST" in review.Confidence: High. The PR author's reasoning is sound and well-documented. The
signature_verificationrule only fires when a user adds ansbomentry tosigning_identitiesrule data AND a signature check then fails. Without that config, the rule is invisible. This pattern will recur as more opt-in verification rules are added (e.g., for other artifact types), so the guidance should be refined now rather than requiring case-by-case justification on each future PR.Proposed change
Amend the
effective_onchecklist item in AGENTS.md (lines 92-98) to add an explicit exception clause for opt-in/fail-closed rules. The updated text should:effective_on, since unconfigured builds are unaffected and users who opt in expect immediate enforcement.effective_onis omitted for this reason, the PR description documents the rationale (as was done on PR Verify SBOM signatures from OCI referrers and tag refs #1787).Example amended text after the existing guidance:
Validation criteria
effective_onshould not flag amissing-effective-onfinding, provided the PR description documents the rationale.effective_onon mandatory enforcement rules (deny/warn rules that fire by default).Generated by retro agent from #1787