From cd41643f6f50f95435f41d376a81086a73c8f0cd Mon Sep 17 00:00:00 2001 From: Tom Wu Date: Fri, 8 May 2026 15:08:08 -0400 Subject: [PATCH 1/2] docs(claude.md): replace PR Review Gate with GitHub-enforced gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior `## PR Review Gate` had the agent read LGTM comments directly, which is brittle: it relies on the agent honoring a marker comment rather than GitHub branch protection actually blocking the merge. This change: - Renames the section to `## GitHub-Enforced PR Review Gate`. - Drops the "look at LGTM comment markers" semantics. - Names a specific required check, `codex-pr-review-gate`, as the authoritative signal for independent AI review approval. - Explicitly forbids the agent from posting `LGTM` or `` markers itself. - Explicitly forbids reading old LGTM comments to bypass the check. - Keeps the merge-conflict + Not-LGTM-yet response flows intact, but routes the "wait for re-review" step through the GitHub check rather than through agent-side comment polling. - Updates the `## PR rules` reference so its third bullet points at the new section name. No CI workflow, branch protection, or repository-settings change is made by this commit — those are owner-side actions. The `codex-pr-review-gate` check is referenced by name on the assumption that the repo owner will land it (or already has). Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 130 ++++++++++++++++++------------------------------------ 1 file changed, 42 insertions(+), 88 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 463f9114..038cc395 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -119,112 +119,66 @@ Pytest markers: `@pytest.mark.unit`, `@pytest.mark.integration`, `@pytest.mark.s 1. **Run tests after every code change.** After any edit to code or tests, run `make test-unit` (or `make test-unit-fast` during iteration). The change is not "done" until local tests pass. Run `make test-all` before pushing a PR. 2. **Commit and let CI run.** After local tests pass, commit and push. Do not declare a change shippable based on local results alone — wait for CI on the branch. -3. **Merge only when CI is green and the review gate has passed** (see next section). +3. **Merge only when CI is green and the GitHub-enforced review gate has passed** (see next section). -## PR Review Gate +## GitHub-Enforced PR Review Gate -You may write code, push branches, open PRs, respond to PR feedback, resolve -merge conflicts, and merge your PR after the review gate passes. +I am the builder/coding agent for this repository. -Do not merge before the review gate passes. +I may write code, push branches, open PRs, respond to PR feedback, resolve merge +conflicts, and merge my PR only after GitHub branch protection passes. -After opening or updating a PR: -1. Push the branch. -2. Wait for CI to finish. -3. Wait for independent review. -4. Treat "Not LGTM yet" as blocking feedback. +Do not merge before the review gate passes. -A PR is mergeable only when all of these are true: +A PR may merge only when all of these are true: 1. The PR is not draft. -2. CI/checks are green: no failing, cancelled, or pending required checks. -3. An independent reviewer comment says: - -``` -LGTM - -``` - -4. The `` marker exactly matches the current PR head SHA. -5. No newer comment, review, or review thread after that LGTM contains blocking - feedback. -6. No new commit was pushed after the LGTM marker. -7. GitHub reports the PR can merge cleanly, with no merge conflicts. - -Important rules: -- Do not post LGTM yourself. -- Do not treat an LGTM for an older commit as valid. -- If you push a new commit, the prior LGTM is stale; wait for review again. -- If the reviewer comments "Not LGTM yet", fix the issue, run relevant checks, - push a follow-up commit, reply on the PR with what changed, and wait for - review again. -- Keep review and fix discussion visible on the PR. - -### Merge conflict responsibility - -You are responsible for keeping your PR mergeable. - -Before waiting for review, before merging, and after any base branch update, -check whether the PR has merge conflicts or is behind the base branch. - -Use commands such as: - -```bash -git fetch origin -gh pr view --json number,url,headRefName,baseRefName,headRefOid,mergeStateStatus,statusCheckRollup -gh pr checks -``` - -If the PR has merge conflicts, a dirty merge state, or GitHub reports it cannot -be merged cleanly: -1. Do not ask the reviewer to fix it. -2. Update your branch against the latest base branch using the repo's normal - workflow. If no workflow is specified, prefer: - - ```bash - git fetch origin - git checkout - git merge origin/ - ``` - -3. Resolve conflicts carefully. Preserve both the requested branch changes and - the current base branch behavior unless the conflict makes that impossible. +2. GitHub says the PR is mergeable. +3. Required CI checks are green. +4. The required `codex-pr-review-gate` check is green for the current head SHA. +5. There are no merge conflicts. +6. There is no newer blocking review feedback. + +The `codex-pr-review-gate` check is the source of truth for independent AI +review approval. Do not bypass it by reading old LGTM comments directly. + +Do not post `LGTM` yourself. +Do not post or edit `` markers yourself. +Do not treat an old approval as valid after pushing a new commit. + +If review feedback says `Not LGTM yet`: +1. Treat it as blocking. +2. Fix the issue. +3. Run relevant local checks. +4. Push a follow-up commit. +5. Reply on the PR with what changed. +6. Wait for CI and `codex-pr-review-gate` again. + +If the PR has merge conflicts: +1. Update the branch against the latest base branch. +2. Resolve conflicts carefully. +3. Preserve requested changes and current base behavior when possible. 4. Run relevant local checks. -5. Commit the conflict resolution. -6. Push the branch. -7. Reply on the PR with a short summary of the conflict resolution. -8. Wait for CI and independent review again. - -### Before merging - -Re-fetch PR state immediately: -- current head SHA -- CI/check status -- comments -- reviews -- review threads if available -- draft state -- mergeability / merge conflict state +5. Commit and push the resolution. +6. Wait for CI and `codex-pr-review-gate` again. -Use GitHub CLI when available, for example: +Before merging, re-fetch PR state: ```bash -gh pr view --json number,url,isDraft,headRefName,headRefOid,mergeStateStatus,statusCheckRollup,comments,reviews -gh pr checks -gh pr merge +gh pr view --json number,url,isDraft,headRefName,headRefOid,mergeStateStatus,statusCheckRollup,comments,reviews +gh pr checks ``` -Only merge if the fresh state still satisfies the gate. +Merge only if GitHub reports the PR is mergeable and all required checks, +including `codex-pr-review-gate`, are passing for the current head SHA. -If the gate passes, merge the PR. Use the repository's normal merge method if it -is clear from repo policy or branch protection. Otherwise prefer squash merge: +Use the repository's normal merge method. If unclear, prefer: ```bash -gh pr merge --squash --delete-branch +gh pr merge --squash --delete-branch ``` If GitHub blocks the merge, report the exact blocker and leave the PR unmerged. -If CI is pending, review is missing, LGTM is stale, merge conflicts exist, or -blocking feedback exists, do not merge; report what is still needed. +Do not work around branch protection. ## Common Gotchas From 0b4366c970a074ae51b3d277eb0d42f65c6c8f53 Mon Sep 17 00:00:00 2001 From: Tom Wu Date: Fri, 8 May 2026 15:41:33 -0400 Subject: [PATCH 2/2] docs(claude.md): switch to local Codex review via openai/codex-plugin-cc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the GitHub-enforced `codex-pr-review-gate` section with a `## PR Workflow With Codex Review` section that points the agent at the local Codex review plugin (`openai/codex-plugin-cc`). Why: rather than building/maintaining a server-enforced GitHub Action + branch protection rule for AI review, run Codex locally from Claude Code via `/codex:review --base main`. Removes the GitHub action dependency, removes the comment-marker polling logic, and keeps the review fully agent-side. What changed: - Section renamed: GitHub-Enforced PR Review Gate → PR Workflow With Codex Review. - Merge criteria: CI green + GitHub mergeable + Codex local review passes + no unresolved comments / conflicts. - Explicit prohibitions: don't post LGTM yourself; don't require or wait for the old codex-pr-review-gate GitHub check. - Documents `/codex:review --base main` (sync) and the `/codex:review --background` + `/codex:status` + `/codex:result` flow for larger PRs. - `## PR rules` bullet 3 now references the new section name. What this PR does NOT do: - Does NOT change branch protection (this repo has none on main). - Does NOT remove any required check (no rulesets exist; no CI check named codex-pr-review-gate exists in workflows). - Does NOT delete any workflow file (no codex-pr-review-gate.yml was ever created — this branch was the prior PR's content; the workflow itself was deferred). If branch protection is added later, the only required check should be the existing `Lint, type-check, and test` (and any new CI tier the team adds). The codex review is intentionally local-only. Supersedes the prior version of this branch (was titled "GitHub-enforced PR review gate"); the substantive content is now the local-Codex flow. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 79 +++++++++++++++++++++++++------------------------------ 1 file changed, 36 insertions(+), 43 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 038cc395..07e10802 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -119,66 +119,59 @@ Pytest markers: `@pytest.mark.unit`, `@pytest.mark.integration`, `@pytest.mark.s 1. **Run tests after every code change.** After any edit to code or tests, run `make test-unit` (or `make test-unit-fast` during iteration). The change is not "done" until local tests pass. Run `make test-all` before pushing a PR. 2. **Commit and let CI run.** After local tests pass, commit and push. Do not declare a change shippable based on local results alone — wait for CI on the branch. -3. **Merge only when CI is green and the GitHub-enforced review gate has passed** (see next section). +3. **Merge only when CI is green and Codex local review reports no blocking issues** (see next section). -## GitHub-Enforced PR Review Gate +## PR Workflow With Codex Review -I am the builder/coding agent for this repository. +This repository uses local Codex review through `openai/codex-plugin-cc`. -I may write code, push branches, open PRs, respond to PR feedback, resolve merge -conflicts, and merge my PR only after GitHub branch protection passes. +Before shipping or merging a PR, run Codex review from Claude Code: -Do not merge before the review gate passes. +``` +/codex:review --base main +``` -A PR may merge only when all of these are true: -1. The PR is not draft. -2. GitHub says the PR is mergeable. -3. Required CI checks are green. -4. The required `codex-pr-review-gate` check is green for the current head SHA. -5. There are no merge conflicts. -6. There is no newer blocking review feedback. +For larger changes, prefer background review: + +``` +/codex:review --base main --background +/codex:status +/codex:result +``` -The `codex-pr-review-gate` check is the source of truth for independent AI -review approval. Do not bypass it by reading old LGTM comments directly. +Do not self-approve by posting `LGTM` markers. +Do not require or wait for the old GitHub `codex-pr-review-gate` check. -Do not post `LGTM` yourself. -Do not post or edit `` markers yourself. -Do not treat an old approval as valid after pushing a new commit. +A PR may merge only when: +1. CI is green. +2. GitHub says the PR is mergeable. +3. Codex local review reports no blocking issues. +4. There are no unresolved review comments or merge conflicts. -If review feedback says `Not LGTM yet`: -1. Treat it as blocking. -2. Fix the issue. +If Codex review reports blockers: +1. Keep the PR open. +2. Fix the issues. 3. Run relevant local checks. 4. Push a follow-up commit. -5. Reply on the PR with what changed. -6. Wait for CI and `codex-pr-review-gate` again. +5. Run Codex review again. If the PR has merge conflicts: 1. Update the branch against the latest base branch. 2. Resolve conflicts carefully. -3. Preserve requested changes and current base behavior when possible. -4. Run relevant local checks. -5. Commit and push the resolution. -6. Wait for CI and `codex-pr-review-gate` again. - -Before merging, re-fetch PR state: - -```bash -gh pr view --json number,url,isDraft,headRefName,headRefOid,mergeStateStatus,statusCheckRollup,comments,reviews -gh pr checks -``` - -Merge only if GitHub reports the PR is mergeable and all required checks, -including `codex-pr-review-gate`, are passing for the current head SHA. +3. Run relevant local checks. +4. Push the resolution. +5. Run Codex review again. -Use the repository's normal merge method. If unclear, prefer: +If CI passes and Codex review passes: +- Merge the PR using the repository's normal merge method. +- Do not manually close the PR as the success path. -```bash -gh pr merge --squash --delete-branch -``` +If GitHub blocks the merge: +- Report the exact blocker. +- Leave the PR open. -If GitHub blocks the merge, report the exact blocker and leave the PR unmerged. -Do not work around branch protection. +Only close without merging if the work is abandoned, duplicated, or superseded, +and leave a PR comment explaining why. ## Common Gotchas