diff --git a/.coderabbit.yaml b/.coderabbit.yaml index c8f39e58..18eb065c 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -1,13 +1,16 @@ # CodeRabbit configuration — https://docs.coderabbit.ai/guides/configure-coderabbit -# CodeRabbit is the AI reviewer for vouch (free for this public repo). it reviews -# every non-draft PR automatically. it is advisory: it comments, it does not gate -# merges. the merge gate stays deterministic — ci + trust-gate + CODEOWNERS, with -# the owner's auto-merge label as the go signal. +# CodeRabbit is the required review gate for vouch (free for this public repo). it +# reviews every non-draft PR automatically. request_changes_workflow is on, so it +# submits a formal approve / request-changes review; the coderabbit-gate workflow +# turns that verdict into the required `coderabbit-approved` status check, so a pr +# only auto-merges once CodeRabbit approves (on top of ci + trust-gate + CODEOWNERS, +# with the owner's auto-merge label as the go signal). a pr CodeRabbit requests +# changes on 3 times is auto-closed (the owner and bots are exempt). language: "en-US" early_access: false reviews: profile: "chill" - request_changes_workflow: false + request_changes_workflow: true high_level_summary: true poem: false review_status: true diff --git a/.github/workflows/coderabbit-gate.yml b/.github/workflows/coderabbit-gate.yml new file mode 100644 index 00000000..8a091165 --- /dev/null +++ b/.github/workflows/coderabbit-gate.yml @@ -0,0 +1,72 @@ +name: coderabbit-gate +# CodeRabbit is the required review gate. this workflow turns CodeRabbit's review +# verdict into the `coderabbit-approved` commit status the branch ruleset +# requires (so native auto-merge waits for its approval), and auto-closes a +# contributor pr CodeRabbit has requested changes on 3 times. it reads only +# review metadata via the api and checks out the trusted base ref — no untrusted +# head code is ever checked out or run. +on: + pull_request_review: + types: [submitted, dismissed, edited] + pull_request_target: # zizmor: ignore[dangerous-triggers] no untrusted code runs here; only review metadata is read, a commit status is set, and a stale pr may be closed. + types: [opened, reopened, synchronize] +permissions: {} +# a newer event for the same pr supersedes an in-flight run (latest verdict wins). +concurrency: + group: coderabbit-gate-${{ github.event.pull_request.number }} + cancel-in-progress: true +jobs: + gate: + runs-on: ubuntu-latest + permissions: + contents: read # checkout base ref + run pr_bot + statuses: write # publish the coderabbit-approved commit status + pull-requests: write # comment + close after 3 rejected rounds + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + ref: ${{ github.event.pull_request.base.sha }} + persist-credentials: false + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: "3.12" + - name: evaluate coderabbit verdict + id: gate + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + AUTHOR: ${{ github.event.pull_request.user.login }} + run: | + gh api "repos/$REPO/pulls/$PR/reviews?per_page=100" > reviews.json + PYTHONPATH=src python -m vouch.pr_bot coderabbit-gate \ + --reviews-file reviews.json --head-sha "$HEAD_SHA" --author "$AUTHOR" \ + >> "$GITHUB_OUTPUT" + - name: publish coderabbit-approved status + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + STATE: ${{ steps.gate.outputs.state }} + VERDICT: ${{ steps.gate.outputs.verdict }} + run: | + case "$VERDICT" in + approved) desc="CodeRabbit approved this commit." ;; + changes) desc="CodeRabbit requested changes — resolve them to merge." ;; + *) desc="Waiting for CodeRabbit to review this commit." ;; + esac + gh api --method POST "repos/$REPO/statuses/$HEAD_SHA" \ + -f state="$STATE" -f context="coderabbit-approved" -f description="$desc" + - name: auto-close after 3 rejected rounds + if: steps.gate.outputs.close == 'true' + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR: ${{ github.event.pull_request.number }} + STRIKES: ${{ steps.gate.outputs.strikes }} + run: | + gh pr merge "$PR" --repo "$REPO" --disable-auto || true + gh pr edit "$PR" --repo "$REPO" --remove-label auto-merge || true + gh pr close "$PR" --repo "$REPO" --comment \ + "CodeRabbit requested changes on this pr $STRIKES times without an approval, so it is being closed automatically. the feedback still stands — address it and reopen this pr (or open a fresh one) and it will be reviewed again." diff --git a/CHANGELOG.md b/CHANGELOG.md index 85aea98a..d405b36e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,13 +26,17 @@ All notable changes to vouch are documented here. Format follows them "hybrid" (#476). - ai auto-merge bot: the owner arms auto-merge on a pr with the `auto-merge` label or by commenting `/auto-merge`; a non-core pr then merges once ci is - green. core paths always require owner review via `.github/CODEOWNERS`; ui - prs opened without before/after screenshots are auto-closed. review is done - by CodeRabbit (`.coderabbit.yaml`, free for this public repo) plus the free - `ci` + `trust-gate` gates — no paid model in the loop. deterministic - decision logic lives in `src/vouch/pr_bot.py` (pure stdlib). repo guards - (branch ruleset + labels) are configured by `scripts/setup_repo_guards.sh`. - workflow security is linted in ci with zizmor + actionlint. + green and CodeRabbit approves. core paths always require owner review via + `.github/CODEOWNERS`; ui prs opened without before/after screenshots are + auto-closed. review is the gate, not just advice: CodeRabbit + (`.coderabbit.yaml`, free for this public repo) submits a formal review and + the `coderabbit-gate` workflow turns its verdict into the required + `coderabbit-approved` status check — so a fresh push voids a prior approval, + and a pr CodeRabbit requests changes on 3 times is auto-closed (owner and + bots exempt). no paid model in the loop. deterministic decision logic lives + in `src/vouch/pr_bot.py` (pure stdlib). repo guards (branch ruleset + labels) + are configured by `scripts/setup_repo_guards.sh`. workflow security is linted + in ci with zizmor + actionlint. ### Changed - `kb.search` is one implementation (`context.search_kb`) across mcp and diff --git a/scripts/setup_repo_guards.sh b/scripts/setup_repo_guards.sh index d49dd50c..7a5a00a4 100644 --- a/scripts/setup_repo_guards.sh +++ b/scripts/setup_repo_guards.sh @@ -6,12 +6,15 @@ # - labels: auto-merge, ci: passing, ci: failing # - repo setting: allow auto-merge # - a branch RULESET requiring a PR, code-owner review, and the ci + -# trust-gate status checks — the versionable replacement for classic branch -# protection, with org admins allowed to bypass (so you can still merge your -# own core PRs, which you can't self-approve). +# trust-gate + coderabbit-approved status checks — the versionable +# replacement for classic branch protection, with org admins allowed to +# bypass (so you can still merge your own core PRs, which you can't +# self-approve, and override the coderabbit gate when you must). # -# review is done by CodeRabbit (install its GitHub App) + ci; this script does -# not configure any AI reviewer or secret. +# review is done by CodeRabbit (install its GitHub App). the coderabbit-gate +# workflow turns its verdict into the required `coderabbit-approved` check, so a +# pr only auto-merges once CodeRabbit approves; this script does not configure +# any AI reviewer or secret. # # VERIFY BEFORE RELYING ON IT (schema/ids are account-specific): # - the required check names below match .github/workflows/ci.yml job names. @@ -42,7 +45,7 @@ cat > "$rules_body" <`` for every decision that must be trustworthy: an author's trust tier, whether a PR touches core/ui paths, whether a UI PR carries before/after screenshots, and whether a labeled PR may arm -native auto-merge. Claude Code verification runs as a GitHub Action, not here — -this module only makes the deterministic calls that gate it. +native auto-merge. CodeRabbit is the review gate and runs as a GitHub App, not +here — this module only turns its verdict into the required `coderabbit-approved` +commit status and the deterministic calls that gate the merge. """ from __future__ import annotations @@ -13,7 +14,8 @@ import json import re import sys -from collections.abc import Iterable, Sequence +from collections.abc import Iterable, Mapping, Sequence +from typing import Any # the review-gate core: writes here are the north star. mirrored verbatim in # .github/CODEOWNERS (test_pr_bot asserts parity). a PR touching any of these @@ -45,6 +47,15 @@ _OWNER_ASSOCIATION = "OWNER" _BOT_ACTORS = frozenset({"dependabot[bot]"}) +# CodeRabbit is the required review gate (.coderabbit.yaml). only reviews it +# authors on github count; anyone else's approval never satisfies the gate. +CODERABBIT_LOGIN = "coderabbitai[bot]" + +# a contributor gets STRIKE_LIMIT rounds of "changes requested" from CodeRabbit +# before the pr is auto-closed. the owner and bots are exempt (author_is_exempt). +STRIKE_LIMIT = 3 +_EXEMPT_AUTHORS = frozenset({"plind-junior"}) | _BOT_ACTORS + def _match(path: str, glob: str) -> bool: g = glob.lstrip("/") @@ -99,6 +110,58 @@ def should_arm_automerge(*, is_core: bool, ci_passing: bool, return claude_verdict == "APPROVE" +def _cr_verdicts(reviews: Sequence[Mapping[str, Any]], *, + login: str) -> list[tuple[str, Any]]: + """(state, commit_id) for CodeRabbit reviews carrying a verdict. + + COMMENTED and DISMISSED reviews carry no verdict and are dropped. + """ + out: list[tuple[str, Any]] = [] + for r in reviews: + if (r.get("user") or {}).get("login") != login: + continue + state = str(r.get("state") or "").upper() + if state in ("APPROVED", "CHANGES_REQUESTED"): + out.append((state, r.get("commit_id"))) + return out + + +def coderabbit_verdict(reviews: Sequence[Mapping[str, Any]], *, + head_sha: str | None = None, + login: str = CODERABBIT_LOGIN) -> tuple[str, int]: + """CodeRabbit's (verdict, strikes) for a pr's review list. + + ``verdict`` is its stance on ``head_sha`` — 'approved', 'changes', or + 'pending' when it has not yet reviewed that commit (so a fresh push voids + a prior approval). ``strikes`` counts the distinct commits it has requested + changes on, i.e. failed review rounds, across the pr's whole history. + """ + verdicts = _cr_verdicts(reviews, login=login) + strikes = len({cid for state, cid in verdicts if state == "CHANGES_REQUESTED"}) + scoped = [v for v in verdicts if head_sha is None or v[1] == head_sha] + if not scoped: + return "pending", strikes + return ("approved" if scoped[-1][0] == "APPROVED" else "changes"), strikes + + +def gate_status(verdict: str) -> str: + """Commit-status state for the required `coderabbit-approved` check.""" + return {"approved": "success", "changes": "failure"}.get(verdict, "pending") + + +def author_is_exempt(author: str) -> bool: + """The owner and bots are never auto-closed for failed reviews.""" + return author in _EXEMPT_AUTHORS + + +def should_close(verdict: str, strikes: int, *, author: str, + limit: int = STRIKE_LIMIT) -> bool: + """Auto-close a contributor pr CodeRabbit has rejected `limit` rounds.""" + return (not author_is_exempt(author) + and verdict == "changes" + and strikes >= limit) + + def _read_lines(path: str) -> list[str]: with open(path, encoding="utf-8") as fh: return [ln.strip() for ln in fh if ln.strip()] @@ -129,6 +192,11 @@ def main(argv: Sequence[str] | None = None) -> int: a.add_argument("--verdict", required=True) a.add_argument("--draft", action="store_true") + g = sub.add_parser("coderabbit-gate") + g.add_argument("--reviews-file", required=True) + g.add_argument("--head-sha", required=True) + g.add_argument("--author", required=True) + ns = p.parse_args(argv) if ns.cmd == "classify": @@ -149,6 +217,18 @@ def main(argv: Sequence[str] | None = None) -> int: ok = should_arm_automerge(is_core=c2["is_core"], ci_passing=ns.ci == "passing", claude_verdict=ns.verdict, is_draft=ns.draft) return 0 if ok else 1 + if ns.cmd == "coderabbit-gate": + with open(ns.reviews_file, encoding="utf-8") as fh: + loaded = json.load(fh) + reviews = loaded if isinstance(loaded, list) else [] + verdict, strikes = coderabbit_verdict(reviews, head_sha=ns.head_sha) + close = should_close(verdict, strikes, author=ns.author) + sys.stdout.write( + f"state={gate_status(verdict)}\n" + f"verdict={verdict}\n" + f"strikes={strikes}\n" + f"close={'true' if close else 'false'}\n") + return 0 return 2 diff --git a/tests/test_pr_bot.py b/tests/test_pr_bot.py index 9d26baf8..34a2b9e8 100644 --- a/tests/test_pr_bot.py +++ b/tests/test_pr_bot.py @@ -102,3 +102,81 @@ def test_codeowners_covers_every_core_glob(): for glob in pr_bot.CORE_GLOBS: needle = "/" + glob.replace("/**", "/") assert needle in text, f"{glob} missing from .github/CODEOWNERS" + + +def _review(state, sha, login="coderabbitai[bot]"): + return {"user": {"login": login}, "state": state, "commit_id": sha} + + +def test_coderabbit_pending_when_no_review_on_head(): + # approved an earlier commit, but head has no coderabbit review yet. + reviews = [_review("APPROVED", "old")] + assert pr_bot.coderabbit_verdict(reviews, head_sha="new") == ("pending", 0) + + +def test_coderabbit_approved_on_head(): + reviews = [_review("CHANGES_REQUESTED", "c1"), _review("APPROVED", "c2")] + assert pr_bot.coderabbit_verdict(reviews, head_sha="c2") == ("approved", 1) + + +def test_coderabbit_changes_on_head(): + reviews = [_review("CHANGES_REQUESTED", "c1")] + assert pr_bot.coderabbit_verdict(reviews, head_sha="c1") == ("changes", 1) + + +def test_coderabbit_strikes_count_distinct_commits(): + reviews = [ + _review("CHANGES_REQUESTED", "c1"), + _review("CHANGES_REQUESTED", "c1"), # same commit, still one strike + _review("CHANGES_REQUESTED", "c2"), + _review("CHANGES_REQUESTED", "c3"), + ] + verdict, strikes = pr_bot.coderabbit_verdict(reviews, head_sha="c3") + assert (verdict, strikes) == ("changes", 3) + + +def test_coderabbit_ignores_other_reviewers_and_comments(): + reviews = [ + _review("APPROVED", "c1", login="rando"), # not coderabbit + _review("COMMENTED", "c1"), # no verdict + _review("CHANGES_REQUESTED", "c1"), + ] + assert pr_bot.coderabbit_verdict(reviews, head_sha="c1") == ("changes", 1) + + +def test_gate_status_maps_verdicts(): + assert pr_bot.gate_status("approved") == "success" + assert pr_bot.gate_status("changes") == "failure" + assert pr_bot.gate_status("pending") == "pending" + + +def test_should_close_after_three_strikes(): + assert pr_bot.should_close("changes", 3, author="rando") is True + assert pr_bot.should_close("changes", 2, author="rando") is False + + +def test_should_close_never_when_approved(): + assert pr_bot.should_close("approved", 5, author="rando") is False + + +def test_should_close_exempts_owner_and_bots(): + assert pr_bot.should_close("changes", 9, author="plind-junior") is False + assert pr_bot.should_close("changes", 9, author="dependabot[bot]") is False + + +def test_cli_coderabbit_gate_outputs(tmp_path): + import json as _json + f = tmp_path / "reviews.json" + f.write_text(_json.dumps([ + _review("CHANGES_REQUESTED", "c1"), + _review("CHANGES_REQUESTED", "c2"), + _review("CHANGES_REQUESTED", "head"), + ]), encoding="utf-8") + out = subprocess.run( + [sys.executable, "-m", "vouch.pr_bot", "coderabbit-gate", + "--reviews-file", str(f), "--head-sha", "head", "--author", "rando"], + capture_output=True, text=True, check=True) + assert "state=failure" in out.stdout + assert "verdict=changes" in out.stdout + assert "strikes=3" in out.stdout + assert "close=true" in out.stdout