Skip to content

docs(#1760): add effective_on review checklist to AGENTS.md - #1761

Open
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/1760-add-effective-on-checklist
Open

docs(#1760): add effective_on review checklist to AGENTS.md#1761
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/1760-add-effective-on-checklist

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Add a "Review Checklist for New Policy Rules" section to AGENTS.md with two items: (1) new deny/warn rules must include an effective_on date in their rule data to provide a migration window before enforcement, and (2) new rules must be added to the appropriate collection(s). This ensures the review agent flags missing effective_on dates on future PRs, addressing the gap identified when PR #1736 was merged without one.


Closes #1760

Post-script verification

  • Branch is not main/master (agent/1760-add-effective-on-checklist)
  • Secret scan passed (gitleaks — 97efaf301694a04d7c7b4355484cf9aa658a470b..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Add a "Review Checklist for New Policy Rules" section to AGENTS.md
with two items: (1) new deny/warn rules must include an effective_on
date in their rule data to provide a migration window before
enforcement, and (2) new rules must be added to the appropriate
collection(s). This ensures the review agent flags missing
effective_on dates on future PRs, addressing the gap identified
when PR #1736 was merged without one.

Closes #1760
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 24, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:46 PM UTC · Completed 2:57 PM UTC
Commit: 47d3320 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

  • [Technical accuracy in documentation] AGENTS.md:68 — The checklist states that new deny/warn rules must include an effective_on date "in their rule data entry (in example/data/)". This is incorrect for the most common category of new deny rules. In this codebase, the effective_on that controls whether a deny rule is treated as a warning (future date) vs. a failure (past date) comes from the rule's METADATA annotation (# custom: effective_on:) in the .rego file itself, processed via lib.time.when(chain) in policy/lib/time/time.rego (which reads metadata.annotations.custom.effective_on). Over 20 existing rules use this pattern (e.g., policy/release/buildah_build_task/buildah_build_task.rego, policy/release/sbom_spdx/sbom_spdx.rego). The effective_on fields in example/data/ files serve a different purpose: they control data versioning via lib.time.most_current() and rule-effectiveness filtering. The checklist as written will direct reviewers to check the wrong location, defeating the purpose of the guidance.
    Remediation: Rewrite the checklist item to distinguish the two mechanisms: (1) New .rego deny/warn rules must include a future effective_on date in their METADATA annotation (# custom: effective_on: <date>). (2) New data entries in example/data/ should include an appropriate effective_on date for versioning.

  • [protected-path] AGENTS.md — AGENTS.md is a protected governance file. The PR has a linked issue (Add effective_on review checklist item to AGENTS.md for new deny rules #1760) and the description explains the rationale for the change. Human approval is always required for protected-path changes, regardless of context.


Labels: PR modifies documentation (AGENTS.md) and requires manual review due to protected path

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread AGENTS.md
`effective_on` date in their rule data entry (in `example/data/`) to
provide a migration window. Rules without `effective_on` enforce
immediately on deployment, which can break existing builds without
warning. Check that the corresponding rule data file has a future

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] Technical accuracy in documentation

The checklist incorrectly directs reviewers to check the corresponding rule data file in example/data/ for the effective_on date. The effective_on that controls deny-vs-warn behavior comes from METADATA annotations in .rego files (via lib.time.when(chain)), not from data files. Over 20 existing rules use # custom: effective_on: in their METADATA block. The data-level effective_on in example/data/ serves a different purpose (data versioning). This will lead reviewers to check the wrong location.

Suggested fix: Rewrite to distinguish two mechanisms: (1) New .rego deny/warn rules must include a future effective_on in their METADATA annotation (# custom: effective_on: ). (2) New data entries in example/data/ should include effective_on for versioning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add effective_on review checklist item to AGENTS.md for new deny rules

0 participants