diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 18eb065c..4bb2b15e 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -1,11 +1,9 @@ # CodeRabbit configuration — https://docs.coderabbit.ai/guides/configure-coderabbit -# 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). +# CodeRabbit reviews every non-draft PR automatically (free for this public repo). +# its verdict is advisory: it gates nothing and closes nothing. the merge path is +# ci + trust-gate + CODEOWNERS, with the owner's auto-merge label as the go signal. +# request_changes_workflow stays on so its stance is legible at a glance, but a +# request-changes review no longer blocks or reaps a pr. language: "en-US" early_access: false reviews: diff --git a/.github/workflows/coderabbit-gate.yml b/.github/workflows/coderabbit-gate.yml deleted file mode 100644 index 0479b63d..00000000 --- a/.github/workflows/coderabbit-gate.yml +++ /dev/null @@ -1,87 +0,0 @@ -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_REPO: ${{ github.event.pull_request.head.repo.full_name }} - 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 - # the GITHUB_TOKEN can't write a commit status onto a fork's head sha — - # POST .../statuses returns 403 "resource not accessible by integration" - # for a pr opened from a fork. that's expected and unfixable with this - # token, so don't fail this infra job over it: the required status simply - # stays unset, native auto-merge waits, and a maintainer merges the fork - # pr by hand. same-repo prs must still publish, so only forks are tolerated. - if gh api --method POST "repos/$REPO/statuses/$HEAD_SHA" \ - -f state="$STATE" -f context="coderabbit-approved" -f description="$desc"; then - exit 0 - fi - if [ "$HEAD_REPO" != "$REPO" ]; then - echo "::warning::could not publish the coderabbit-approved status on a fork pr head ($HEAD_REPO); a maintainer must merge this pr manually (or wire a token with statuses:write for cross-fork writes)." - exit 0 - fi - echo "::error::failed to publish the coderabbit-approved status" - exit 1 - - 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/.github/workflows/stale-pr-reaper.yml b/.github/workflows/stale-pr-reaper.yml deleted file mode 100644 index 5ef2eeea..00000000 --- a/.github/workflows/stale-pr-reaper.yml +++ /dev/null @@ -1,51 +0,0 @@ -name: stale-pr-reaper -# daily reaper: closes a pr whose author left CodeRabbit's change request -# unaddressed (no new commit) for 2 days. complements the 3-strikes close in -# coderabbit-gate.yml — that fires when the author keeps pushing failing -# commits, this fires when they go silent. scheduled workflows only run from the -# default branch, so this activates once it is on `main`. it reads only api -# metadata and runs from the trusted default branch — no pr head code is run. -on: - schedule: - - cron: "17 6 * * *" # daily ~06:17 UTC - workflow_dispatch: {} -permissions: {} -concurrency: - group: stale-pr-reaper - cancel-in-progress: false -jobs: - reap: - runs-on: ubuntu-latest - permissions: - contents: read # checkout + run pr_bot - pull-requests: write # comment + close - steps: - - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - with: - persist-credentials: false - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 - with: - python-version: "3.12" - - name: close prs stale for 2 days after a change request - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - run: | - now=$(date -u +%s) - gh pr list --repo "$REPO" --state open --limit 100 \ - --json number,headRefOid,author,isDraft \ - --jq '.[] | select(.isDraft|not) | [.number, .headRefOid, .author.login] | @tsv' \ - > prs.tsv - while IFS=$'\t' read -r num sha author; do - [ -z "$num" ] && continue - gh api "repos/$REPO/pulls/$num/reviews?per_page=100" > reviews.json - if PYTHONPATH=src python -m vouch.pr_bot stale-check \ - --reviews-file reviews.json --head-sha "$sha" \ - --author "$author" --now-epoch "$now"; then - echo "reaping #$num (stale after change request)" - gh pr merge "$num" --repo "$REPO" --disable-auto || true - gh pr edit "$num" --repo "$REPO" --remove-label auto-merge || true - gh pr close "$num" --repo "$REPO" --comment \ - "closing automatically: CodeRabbit requested changes and this pr has had no new commits for 2 days. the feedback still stands — push a fix and reopen this pr (or open a fresh one) and it will be reviewed again." - fi - done < prs.tsv diff --git a/CHANGELOG.md b/CHANGELOG.md index 5141d497..f3cab509 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,16 @@ All notable changes to vouch are documented here. Format follows markers; absolute bench scores shift, paired comparisons were fair either way. the reference baseline table is refreshed. ### Changed +- **CodeRabbit's verdict no longer gates anything.** the + `coderabbit-approved` commit status, the 3-strike auto-close, and the + daily stale-pr reaper are removed, along with the `coderabbit-gate` and + `stale-check` pr_bot commands that computed them. the status had + already been dropped from the `test` ruleset's required checks, so this + removes the machinery that outlived it rather than lowering a live bar. + CodeRabbit still reviews every non-draft pr and still files formal + approve / request-changes reviews — they are advisory now. the merge + path is ci + trust-gate + CODEOWNERS, with the owner's auto-merge label + as the go signal. - **auto approval is the default** (`review.approver_role: trusted-agent` in the starter config): a fresh KB approves the capturing agent's proposals with no human step. nothing bypasses the gate — every write diff --git a/src/vouch/pr_bot.py b/src/vouch/pr_bot.py index d9035224..474141d5 100644 --- a/src/vouch/pr_bot.py +++ b/src/vouch/pr_bot.py @@ -4,9 +4,8 @@ call ``python -m vouch.pr_bot `` 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. 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. +native auto-merge. CodeRabbit runs as a GitHub App and still comments on PRs, +but its verdict no longer gates anything — nothing here reads it. """ from __future__ import annotations @@ -15,7 +14,6 @@ import re import sys from collections.abc import Iterable, Mapping, Sequence -from datetime import UTC, datetime from typing import Any # the review-gate core: writes here are the north star. mirrored verbatim in @@ -48,19 +46,6 @@ _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 - -# a pr whose author leaves CodeRabbit's change request unaddressed (no new -# commit) for STALE_DAYS is auto-closed by the scheduled stale-pr-reaper. -STALE_DAYS = 2 -_EXEMPT_AUTHORS = frozenset({"plind-junior"}) | _BOT_ACTORS - def _match(path: str, glob: str) -> bool: g = glob.lstrip("/") @@ -115,91 +100,6 @@ 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 _iso_epoch(s: str) -> float: - """Epoch seconds for a github ISO8601 timestamp (e.g. 2026-07-15T10:20:30Z).""" - dt = datetime.fromisoformat(s.replace("Z", "+00:00")) - if dt.tzinfo is None: - dt = dt.replace(tzinfo=UTC) - return dt.timestamp() - - -def should_close_stale(reviews: Sequence[Mapping[str, Any]], *, head_sha: str, - now_epoch: float, author: str, days: int = STALE_DAYS, - login: str = CODERABBIT_LOGIN) -> bool: - """Auto-close a pr whose author left a CodeRabbit change request unaddressed. - - Fires only when CodeRabbit's latest verdict *on the current head* is - "changes requested" and that review is >= ``days`` old — i.e. no new commit - has landed since (a push would move ``head_sha`` off the review's - ``commit_id``). the owner and bots are exempt. - """ - if author_is_exempt(author): - return False - on_head = [r for r in reviews - if (r.get("user") or {}).get("login") == login - and r.get("commit_id") == head_sha - and str(r.get("state") or "").upper() in ("APPROVED", "CHANGES_REQUESTED")] - if not on_head or str(on_head[-1].get("state") or "").upper() != "CHANGES_REQUESTED": - return False - submitted = on_head[-1].get("submitted_at") - if not submitted: - return False - age_days = (now_epoch - _iso_epoch(str(submitted))) / 86400.0 - return age_days >= days - - 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()] @@ -338,17 +238,6 @@ 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) - - st = sub.add_parser("stale-check") - st.add_argument("--reviews-file", required=True) - st.add_argument("--head-sha", required=True) - st.add_argument("--author", required=True) - st.add_argument("--now-epoch", required=True, type=int) - ns = p.parse_args(argv) if ns.cmd == "classify": @@ -380,25 +269,6 @@ 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 - if ns.cmd == "stale-check": - with open(ns.reviews_file, encoding="utf-8") as fh: - loaded = json.load(fh) - reviews = loaded if isinstance(loaded, list) else [] - stale = should_close_stale(reviews, head_sha=ns.head_sha, - now_epoch=ns.now_epoch, author=ns.author) - return 0 if stale else 1 return 2 diff --git a/tests/test_pr_bot.py b/tests/test_pr_bot.py index 7941c310..951bd8da 100644 --- a/tests/test_pr_bot.py +++ b/tests/test_pr_bot.py @@ -1,6 +1,5 @@ import subprocess import sys -from datetime import UTC, datetime from pathlib import Path from vouch import pr_bot @@ -145,139 +144,3 @@ def test_codeowners_covers_every_core_glob(): 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 _iso(epoch): - return datetime.fromtimestamp(epoch, UTC).strftime("%Y-%m-%dT%H:%M:%SZ") - - -def _review_ts(state, sha, epoch, login="coderabbitai[bot]"): - return {"user": {"login": login}, "state": state, "commit_id": sha, - "submitted_at": _iso(epoch)} - - -_NOW = 1_700_000_000 - - -def test_stale_closes_after_two_days(): - reviews = [_review_ts("CHANGES_REQUESTED", "head", _NOW - 3 * 86400)] - assert pr_bot.should_close_stale( - reviews, head_sha="head", now_epoch=_NOW, author="rando") is True - - -def test_stale_not_within_two_days(): - reviews = [_review_ts("CHANGES_REQUESTED", "head", _NOW - 1 * 86400)] - assert pr_bot.should_close_stale( - reviews, head_sha="head", now_epoch=_NOW, author="rando") is False - - -def test_stale_ignored_when_author_pushed_a_new_commit(): - # the change request is on an old commit; head moved on and has no review. - reviews = [_review_ts("CHANGES_REQUESTED", "old", _NOW - 5 * 86400)] - assert pr_bot.should_close_stale( - reviews, head_sha="new", now_epoch=_NOW, author="rando") is False - - -def test_stale_never_when_approved_on_head(): - reviews = [_review_ts("APPROVED", "head", _NOW - 5 * 86400)] - assert pr_bot.should_close_stale( - reviews, head_sha="head", now_epoch=_NOW, author="rando") is False - - -def test_stale_exempts_owner_and_bots(): - reviews = [_review_ts("CHANGES_REQUESTED", "head", _NOW - 9 * 86400)] - assert pr_bot.should_close_stale( - reviews, head_sha="head", now_epoch=_NOW, author="plind-junior") is False - assert pr_bot.should_close_stale( - reviews, head_sha="head", now_epoch=_NOW, author="dependabot[bot]") is False - - -def test_cli_stale_check_exit_codes(tmp_path): - import json as _json - f = tmp_path / "reviews.json" - f.write_text(_json.dumps( - [_review_ts("CHANGES_REQUESTED", "head", _NOW - 3 * 86400)]), encoding="utf-8") - stale = subprocess.run( - [sys.executable, "-m", "vouch.pr_bot", "stale-check", "--reviews-file", str(f), - "--head-sha", "head", "--author", "rando", "--now-epoch", str(_NOW)]) - fresh = subprocess.run( - [sys.executable, "-m", "vouch.pr_bot", "stale-check", "--reviews-file", str(f), - "--head-sha", "head", "--author", "rando", "--now-epoch", str(_NOW - 3 * 86400)]) - assert stale.returncode == 0 and fresh.returncode == 1 - - -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