diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 474c717..a5b206e 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -59,6 +59,24 @@ Follow-ups: PR titles under 70 characters. Detail goes in the body. +## Agentic PR review gate + +- If an AI agent opens or updates a PR, do not merge before the review gate passes. +- Wait for CI and an independent review signal tied to the current head SHA. +- Treat a reviewer comment of `Not LGTM yet` as blocking. +- Treat any new commit as invalidating older approval. +- A valid approval must include: + +```text +LGTM + +``` + +- Builder agents are authorized to merge after the review gate passes. + Immediately before merging, re-fetch PR state and merge only when CI is green, + the marker matches the current head SHA, and no newer blocking feedback + exists. + ## When to defer - If the request is ambiguous, ask a clarifying question or offer 2-3 differentiated options. diff --git a/AGENTS.md b/AGENTS.md index 84a71ba..a5e2a04 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -91,6 +91,44 @@ Follow-ups: PR titles should be under 70 characters. Use the description for detail. +## Agentic PR review gate + +Use this pattern when one agent writes code and another agent reviews the PR. +Approval is valid only for the exact PR head SHA that was reviewed. + +Builder prompt: + +```text +You may write code, push branches, and open PRs. Do not merge before the review +gate passes. After opening or updating a PR, push the branch, wait for CI, and +wait for independent review. Treat "Not LGTM yet" as blocking. A PR is mergeable +only when a reviewer comments: + +LGTM + + +The marker must match the current head SHA; any new commit makes prior approval +stale. You are authorized to merge after the review gate passes. Immediately +before merging, re-fetch the head SHA, CI state, comments, and reviews. Merge +only if CI is green, the marker matches the current head SHA, and no newer +blocking feedback exists. Otherwise report that the PR is ready but unmerged. +Keep review and fix discussion visible on the PR. +``` + +Reviewer prompt: + +```text +Review open PRs in the current repo. Fetch metadata, head SHA, diff, comments, +reviews, and CI. Never post LGTM while CI is pending or failing. If CI fails or +code review finds issues, comment "Not LGTM yet" with actionable findings and: + + + +If CI is green and no findings remain, post exactly "LGTM" plus the marker. +Before posting, re-fetch the head SHA and comments to avoid duplicate or stale +reviews. Do not merge PRs from the reviewer role. +``` + ## Agent instruction hierarchy When rules overlap, follow the more specific and safer one. Precedence: diff --git a/CLAUDE.md b/CLAUDE.md index 13cf2e7..d3b8df5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -47,6 +47,40 @@ The instruction files double as drop-in templates for new projects. When editing - Confine `GenAI_Common`-specific framing to the "Repository purpose" section, which `BOOTSTRAP.md` flags for replacement. - Don't reference paths or files that won't exist in a fresh project. +## Agentic PR review gate + +Use this builder/reviewer split when a coding agent opens or updates PRs. + +Builder prompt: + +```text +You may write code, push branches, and open PRs. Do not merge before the review +gate passes. After opening or updating a PR, push the branch, wait for CI, and +wait for independent review. Treat "Not LGTM yet" as blocking. A PR is mergeable +only when a reviewer comments LGTM with a marker matching the current head SHA. +You are authorized to merge after the review gate passes. Immediately before +merging, re-fetch PR state and merge only when CI is green, the marker matches +the current head SHA, and no newer blocking feedback exists. +``` + +Reviewer prompt: + +```text +Review open PRs in the current repo. Fetch metadata, head SHA, diff, comments, +reviews, and CI. Never post LGTM while CI is pending or failing. If CI fails or +code review finds issues, comment "Not LGTM yet" with actionable findings. If CI +is green and no findings remain, post exactly: + +LGTM + + +Before posting, re-fetch the head SHA and comments to avoid duplicate or stale +reviews. Do not merge PRs from the reviewer role. +``` + +See `docs/knowledge/agentic-pr-review-loop.md` for scheduler, lock, and state +guardrails. + ## Quality checks Run before declaring a doc change done. Tier 1 needs no install; tiers 2–4 are opt-in but recommended for ongoing maintenance. diff --git a/docs/knowledge/agentic-pr-review-loop.md b/docs/knowledge/agentic-pr-review-loop.md index fdc0709..efebf80 100644 --- a/docs/knowledge/agentic-pr-review-loop.md +++ b/docs/knowledge/agentic-pr-review-loop.md @@ -22,7 +22,7 @@ It fits best when: - CI exists and covers the important runtime paths. - PR comments are treated as the visible record of review decisions. -- The team wants builder momentum without giving the builder merge authority. +- The team wants builder momentum without letting the builder bypass review. - The reviewing agent can inspect diffs, CI state, logs, comments, and tests. ## Minimum viable workflow @@ -46,16 +46,15 @@ LGTM is attached to a commit, not to a feeling. Use this for Claude Code or any agent responsible for writing code: ```text -You may write code, push branches, and open PRs, but you must not merge unless -the user explicitly asks you to merge. +You may write code, push branches, and open PRs. Do not merge before the review +gate passes. After opening or updating a PR: 1. Push the branch. 2. Wait for CI to start and finish. -3. Do not enable auto-merge. -4. Wait for the independent PR-review automation. +3. Wait for the independent PR-review automation. -A PR is merge-ready only when an automation-authored comment on the PR says: +A PR is mergeable only when an automation-authored comment on the PR says: LGTM @@ -63,6 +62,11 @@ LGTM The marker must match the current PR head SHA. If a new commit is pushed, any older LGTM is stale. +You are authorized to merge after the review gate passes. Immediately before +merging, re-fetch the PR head SHA, CI state, comments, and reviews. Merge only +if CI is green, the marker matches the current head SHA, and no newer blocking +feedback exists. Otherwise report that the PR is ready but unmerged. + If the reviewer comments "Not LGTM yet": 1. Treat it as blocking feedback. 2. Fix the issue in code, tests, docs, or config. @@ -100,7 +104,8 @@ LGTM Before posting, re-fetch the PR head SHA, CI state, and recent comments. Only post if the head SHA is unchanged and no marker already exists for that SHA. -Do not merge PRs. Do not enable auto-merge. Do not post duplicate comments. +Do not merge PRs from the reviewer role. Do not enable auto-merge. Do not post +duplicate comments. ``` ## Scheduler and race controls @@ -139,7 +144,8 @@ repeated run summaries. ## Validation checklist -- [ ] The builder cannot merge without explicit user instruction. +- [ ] The builder can merge after the review gate passes. +- [ ] The builder cannot merge before the gate. - [ ] The reviewer never posts `LGTM` while CI is pending or failing. - [ ] The review marker includes the exact head SHA. - [ ] A new commit makes the prior marker stale.