feat(webhook): gate on .pr-review.json:enabled, drop labels
This commit is contained in:
+20
-40
@@ -2,13 +2,15 @@
|
||||
"""pragent pilot — central webhook receiver.
|
||||
|
||||
A stdlib-only HTTP server that Gitea posts user-webhook events to. It gates on
|
||||
the `AI-REVIEW` PR label, then runs the same review core (`ai_review.review_pr`)
|
||||
the CI-step pilot uses, posting findings back as `pragent-bot`.
|
||||
the PR's base ref having `.pr-review.json` with `"enabled": true`, then runs
|
||||
the same review core (`ai_review.review_pr`) the CI-step pilot uses, posting
|
||||
findings back as `pragent-bot`.
|
||||
|
||||
Per-owner setup: one Gitea **user-level webhook** per repo-owner fires for every
|
||||
repo that owner has; this service filters to labeled PRs. (Gitea 1.26.1 system
|
||||
repo that owner has; this service filters to opted-in PRs. (Gitea 1.26.1 system
|
||||
webhooks are broken — see pilot/README-webhook.md.) Onboarding a repo = add the
|
||||
bot as a Write collaborator + create the label + label a PR.
|
||||
bot as a Write collaborator + commit a `.pr-review.json` with `"enabled": true`
|
||||
on the base ref.
|
||||
|
||||
Stdlib only — no pip install, runs on python:3-slim with the scripts mounted.
|
||||
|
||||
@@ -53,15 +55,13 @@ except ImportError:
|
||||
feedback_harvest = None
|
||||
|
||||
# Pull-request webhook `action` values. We fire on EVERY pull_request action
|
||||
# except `closed` (no point reviewing a closed/merged PR) — the AI-REVIEW label
|
||||
# gate + sha dedupe downstream make broadening safe: a same-sha re-fire (title
|
||||
# edit, assignee, milestone, label toggle of another label…) is skipped by
|
||||
# `review_pr`'s dedupe, and an `unlabeled` event that removed AI-REVIEW fails
|
||||
# the label gate (payload `labels` reflect current state). Gitea emits
|
||||
# GitHub-style `action` names (`labeled`, `synchronize`) even though the
|
||||
# `X-Gitea-Event-Type` header uses `label_updated` / `synchronized`.
|
||||
# except `closed` (no point reviewing a closed/merged PR) — the
|
||||
# `.pr-review.json:enabled` gate + sha dedupe downstream make broadening safe:
|
||||
# a same-sha re-fire (title edit, assignee, milestone, label toggle…) is
|
||||
# skipped by `review_pr`'s dedupe. Gitea emits GitHub-style `action` names
|
||||
# (`labeled`, `synchronize`) even though the `X-Gitea-Event-Type` header uses
|
||||
# `label_updated` / `synchronized`.
|
||||
SKIP_ACTIONS = {"closed"}
|
||||
AI_REVIEW_LABEL = "AI-REVIEW"
|
||||
|
||||
GITEA_API = os.environ.get("GITEA_API", "http://gitea-http.gitea.svc.cluster.local:3000")
|
||||
BOT_TOKEN = os.environ.get("PRAGENT_BOT_TOKEN", "")
|
||||
@@ -87,30 +87,13 @@ _review_slots = threading.Semaphore(MAX_CONCURRENT)
|
||||
# Reviews currently accepted or running, keyed (repo, index, sha). The
|
||||
# sha-marker dedupe in `review_pr` reads Gitea *before* posting, so two
|
||||
# deliveries for the same commit in flight together both see "not yet reviewed"
|
||||
# and both post — the classic check-then-act race, and label-toggling is exactly
|
||||
# the kind of thing that fires two deliveries a second apart. This set closes
|
||||
# the window inside one process.
|
||||
# and both post — the classic check-then-act race. Common triggers are Gitea
|
||||
# retries after a slow 202 response and bursty re-fires from a rapid title /
|
||||
# assign / label toggle. This set closes the window inside one process.
|
||||
_inflight: set[tuple[str, str, str]] = set()
|
||||
_inflight_lock = threading.Lock()
|
||||
|
||||
|
||||
def _labels_have(labels, name: str) -> bool:
|
||||
"""True if the Gitea PR `labels` list (dicts with `name`, or bare strings)
|
||||
contains `name`."""
|
||||
if not isinstance(labels, list):
|
||||
return False
|
||||
for lab in labels:
|
||||
if isinstance(lab, dict) and lab.get("name") == name:
|
||||
return True
|
||||
if isinstance(lab, str) and lab == name:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _labels_have_ai_review(labels) -> bool:
|
||||
return _labels_have(labels, AI_REVIEW_LABEL)
|
||||
|
||||
|
||||
def is_repo_enabled(api: str, repo: str, ref: str, token: str) -> bool:
|
||||
"""True iff `.pr-review.json` on `ref` has `"enabled": true`.
|
||||
|
||||
@@ -162,10 +145,6 @@ def _handle_pull_request(payload: dict) -> tuple[int, str]:
|
||||
if not repo:
|
||||
return 400, "no repository.full_name"
|
||||
|
||||
labels = pr.get("labels")
|
||||
if not _labels_have_ai_review(labels):
|
||||
return 200, f"ignore (no {AI_REVIEW_LABEL} label) action={action}"
|
||||
|
||||
index = pr.get("number")
|
||||
if index is None:
|
||||
return 400, "no pull_request.number"
|
||||
@@ -176,6 +155,9 @@ def _handle_pull_request(payload: dict) -> tuple[int, str]:
|
||||
|
||||
base_ref = (pr.get("base") or {}).get("ref", "") or ""
|
||||
|
||||
if not is_repo_enabled(GITEA_API, repo, base_ref or "", BOT_TOKEN):
|
||||
return 200, f"skip (repo not opted in) action={action}"
|
||||
|
||||
if not BOT_TOKEN:
|
||||
return 500, "PRAGENT_BOT_TOKEN not set"
|
||||
|
||||
@@ -304,11 +286,9 @@ class Handler(BaseHTTPRequestHandler):
|
||||
self._send(200, f"ignore event={event}")
|
||||
return
|
||||
|
||||
pr0 = payload.get("pull_request") or {}
|
||||
repo_full = (payload.get("repository") or {}).get("full_name")
|
||||
print(
|
||||
f"pragent-webhook: pull_request action={payload.get('action')} "
|
||||
f"repo={(payload.get('repository') or {}).get('full_name')} "
|
||||
f"ai_review={_labels_have_ai_review(pr0.get('labels'))}",
|
||||
f"pragent-webhook: pull_request action={payload.get('action')} repo={repo_full}",
|
||||
flush=True,
|
||||
)
|
||||
status, msg = _handle_pull_request(payload)
|
||||
|
||||
@@ -16,7 +16,6 @@ def _payload(**over):
|
||||
"number": 7,
|
||||
"title": "t",
|
||||
"body": "b",
|
||||
"labels": [{"name": "AI-REVIEW"}],
|
||||
"head": {"sha": "a" * 40},
|
||||
"base": {"ref": "main"},
|
||||
}
|
||||
@@ -26,16 +25,11 @@ def _payload(**over):
|
||||
return p
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# label gating
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_labels_have_matches_dicts_and_strings():
|
||||
assert ws._labels_have([{"name": "AI-REVIEW"}], "AI-REVIEW")
|
||||
assert ws._labels_have(["AI-REVIEW"], "AI-REVIEW")
|
||||
assert not ws._labels_have([{"name": "other"}], "AI-REVIEW")
|
||||
assert not ws._labels_have(None, "AI-REVIEW")
|
||||
def _enable_repo(monkeypatch, enabled: bool = True):
|
||||
"""Patch `is_repo_enabled` to the given bool for handler tests."""
|
||||
monkeypatch.setattr(
|
||||
"webhook_server.is_repo_enabled", lambda *a, **kw: enabled
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -76,6 +70,7 @@ def test_claim_is_thread_safe():
|
||||
def test_duplicate_delivery_for_same_sha_is_not_reviewed_twice(monkeypatch):
|
||||
started = []
|
||||
monkeypatch.setattr(ws, "BOT_TOKEN", "tok")
|
||||
_enable_repo(monkeypatch)
|
||||
|
||||
class FakeThread:
|
||||
def __init__(self, target, args, daemon):
|
||||
@@ -98,6 +93,7 @@ def test_duplicate_delivery_for_same_sha_is_not_reviewed_twice(monkeypatch):
|
||||
def test_base_ref_is_passed_to_the_review_thread(monkeypatch):
|
||||
started = []
|
||||
monkeypatch.setattr(ws, "BOT_TOKEN", "tok")
|
||||
_enable_repo(monkeypatch)
|
||||
|
||||
class FakeThread:
|
||||
def __init__(self, target, args, daemon):
|
||||
@@ -115,16 +111,33 @@ def test_base_ref_is_passed_to_the_review_thread(monkeypatch):
|
||||
|
||||
def test_closed_action_is_ignored(monkeypatch):
|
||||
monkeypatch.setattr(ws, "BOT_TOKEN", "tok")
|
||||
_enable_repo(monkeypatch)
|
||||
status, msg = ws._handle_pull_request(_payload(action="closed"))
|
||||
assert status == 200
|
||||
assert "ignore" in msg
|
||||
|
||||
|
||||
def test_missing_label_is_ignored(monkeypatch):
|
||||
def test_handle_pull_request_skips_when_repo_not_enabled(monkeypatch):
|
||||
_enable_repo(monkeypatch, enabled=False)
|
||||
started = []
|
||||
monkeypatch.setattr(ws, "BOT_TOKEN", "tok")
|
||||
status, msg = ws._handle_pull_request(_payload(pr={"labels": [{"name": "wip"}]}))
|
||||
|
||||
class FakeThread:
|
||||
def __init__(self, target, args, daemon):
|
||||
self.args = args
|
||||
|
||||
def start(self):
|
||||
started.append(self.args)
|
||||
|
||||
monkeypatch.setattr(ws.threading, "Thread", FakeThread)
|
||||
ws._release(("o/r", "7", "a" * 40))
|
||||
|
||||
status, msg = ws._handle_pull_request(_payload())
|
||||
assert status == 200
|
||||
assert "AI-REVIEW" in msg
|
||||
assert "skip" in msg and "repo not opted in" in msg
|
||||
assert "opened" in msg
|
||||
assert started == []
|
||||
ws._release(("o/r", "7", "a" * 40))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user