From 9a88bba9ebe17d1ff9de8882cee3881c04a9ad7c Mon Sep 17 00:00:00 2001 From: Tom Wu Date: Thu, 7 May 2026 20:46:20 -0400 Subject: [PATCH 1/3] Add PR review gate to agent templates --- .github/copilot-instructions.md | 13 +++++++++++++ AGENTS.md | 34 +++++++++++++++++++++++++++++++++ CLAUDE.md | 32 +++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 474c717..c91bc30 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -59,6 +59,19 @@ 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 unless the user explicitly asks. +- 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 + +``` + ## 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..013b211 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -91,6 +91,40 @@ 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, but do not merge unless the +user explicitly asks. After opening or updating a PR, push the branch, wait for +CI, do not enable auto-merge, and wait for independent review. Treat "Not LGTM +yet" as blocking. A PR is ready only when a reviewer comments: + +LGTM + + +The marker must match the current head SHA; any new commit makes prior approval +stale. 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. +``` + ## Agent instruction hierarchy When rules overlap, follow the more specific and safer one. Precedence: diff --git a/CLAUDE.md b/CLAUDE.md index 13cf2e7..23586f6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -47,6 +47,38 @@ 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, but do not merge unless the +user explicitly asks. After opening or updating a PR, push the branch, wait for +CI, do not enable auto-merge, and wait for independent review. Treat "Not LGTM +yet" as blocking. A PR is ready only when a reviewer comments LGTM with a +marker matching the current head SHA. +``` + +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. +``` + +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. From cecf4b073ac83de8d27963f2bd9aa20c88f886f3 Mon Sep 17 00:00:00 2001 From: Tom Wu Date: Thu, 7 May 2026 20:51:00 -0400 Subject: [PATCH 2/3] Allow builder merge after PR review gate --- .github/copilot-instructions.md | 6 +++++- AGENTS.md | 16 ++++++++++------ CLAUDE.md | 14 ++++++++------ docs/knowledge/agentic-pr-review-loop.md | 22 ++++++++++++++-------- 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index c91bc30..d682e23 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -61,7 +61,7 @@ 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 unless the user explicitly asks. +- 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. @@ -72,6 +72,10 @@ LGTM ``` +- If the user or repo policy authorizes the agent to merge, re-fetch PR state + immediately before merging 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 013b211..7b77a88 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -99,16 +99,20 @@ 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, but do not merge unless the -user explicitly asks. After opening or updating a PR, push the branch, wait for -CI, do not enable auto-merge, and wait for independent review. Treat "Not LGTM -yet" as blocking. A PR is ready only when a reviewer comments: +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. Keep review and fix discussion visible on the PR. +stale. If the user or repo policy authorizes you to merge, re-fetch the head +SHA, CI state, comments, and reviews immediately before merging. 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: @@ -122,7 +126,7 @@ 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. +reviews. Do not merge PRs from the reviewer role. ``` ## Agent instruction hierarchy diff --git a/CLAUDE.md b/CLAUDE.md index 23586f6..996aa94 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,11 +54,13 @@ 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, but do not merge unless the -user explicitly asks. After opening or updating a PR, push the branch, wait for -CI, do not enable auto-merge, and wait for independent review. Treat "Not LGTM -yet" as blocking. A PR is ready only when a reviewer comments LGTM with a -marker matching the current head SHA. +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. +If the user or repo policy authorizes you to merge, re-fetch PR state +immediately before merging and merge only when CI is green, the marker matches +the current head SHA, and no newer blocking feedback exists. ``` Reviewer prompt: @@ -73,7 +75,7 @@ LGTM Before posting, re-fetch the head SHA and comments to avoid duplicate or stale -reviews. Do not merge PRs. +reviews. Do not merge PRs from the reviewer role. ``` See `docs/knowledge/agentic-pr-review-loop.md` for scheduler, lock, and state diff --git a/docs/knowledge/agentic-pr-review-loop.md b/docs/knowledge/agentic-pr-review-loop.md index fdc0709..b3bf556 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. +If the user or repo policy authorizes you to merge, re-fetch the PR head SHA, +CI state, comments, and reviews immediately before merging. 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 cannot merge unless user or repo policy grants merge authority. +- [ ] Even with merge authority, 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. From 41a662313eba16517452aaf65131679e3cf2a5e1 Mon Sep 17 00:00:00 2001 From: Tom Wu Date: Thu, 7 May 2026 20:53:00 -0400 Subject: [PATCH 3/3] Authorize builder merge after review gate --- .github/copilot-instructions.md | 7 ++++--- AGENTS.md | 10 +++++----- CLAUDE.md | 4 ++-- docs/knowledge/agentic-pr-review-loop.md | 12 ++++++------ 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d682e23..a5b206e 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -72,9 +72,10 @@ LGTM ``` -- If the user or repo policy authorizes the agent to merge, re-fetch PR state - immediately before merging and merge only when CI is green, the marker matches - the current head SHA, and no newer blocking feedback exists. +- 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 diff --git a/AGENTS.md b/AGENTS.md index 7b77a88..a5e2a04 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -108,11 +108,11 @@ LGTM The marker must match the current head SHA; any new commit makes prior approval -stale. If the user or repo policy authorizes you to merge, re-fetch the head -SHA, CI state, comments, and reviews immediately before merging. 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. +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: diff --git a/CLAUDE.md b/CLAUDE.md index 996aa94..d3b8df5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -58,8 +58,8 @@ 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. -If the user or repo policy authorizes you to merge, re-fetch PR state -immediately before merging and merge only when CI is green, the marker matches +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. ``` diff --git a/docs/knowledge/agentic-pr-review-loop.md b/docs/knowledge/agentic-pr-review-loop.md index b3bf556..efebf80 100644 --- a/docs/knowledge/agentic-pr-review-loop.md +++ b/docs/knowledge/agentic-pr-review-loop.md @@ -62,10 +62,10 @@ LGTM The marker must match the current PR head SHA. If a new commit is pushed, any older LGTM is stale. -If the user or repo policy authorizes you to merge, re-fetch the PR head SHA, -CI state, comments, and reviews immediately before merging. 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. +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. @@ -144,8 +144,8 @@ repeated run summaries. ## Validation checklist -- [ ] The builder cannot merge unless user or repo policy grants merge authority. -- [ ] Even with merge authority, the builder cannot merge before the gate. +- [ ] 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.