From 4ef62f28bbb5d05575829f7e3e65a96b95ec74c3 Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 20 Aug 2026 23:24:54 +0000 Subject: [PATCH] fix(webhook): re-read PR labels at review start so a late AI-USAGE counts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Labelling a PR is two webhook events. AI-REVIEW arrives first, the review claims (repo, index, sha) and starts; the AI-USAGE event that follows a moment later hits the in-flight dedupe and is dropped. `report_usage` was snapshotted from the first payload, which had not seen AI-USAGE yet, so the review posted without its usage block even though the label was on the PR by the time it finished — observed on gitea_admin/pragent#9 (`usage=False` in the pod log, no
section in the posted review). Re-read the labels from the API at review start and upgrade the flag. The read is best-effort: any failure logs and returns [], leaving the payload's verdict intact, because a usage section is not worth failing a review over. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01B11e8TZZxJyzHW7jj7KWUN --- pilot/webhook_server.py | 37 +++++++++++++++++++++++++++ tests/pilot/test_webhook_server.py | 40 ++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/pilot/webhook_server.py b/pilot/webhook_server.py index 7ccbdb1..5a7ecfd 100644 --- a/pilot/webhook_server.py +++ b/pilot/webhook_server.py @@ -39,6 +39,7 @@ import hmac import json import os import threading +import urllib.request from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from ai_review import review_pr @@ -176,10 +177,46 @@ def _release(key: tuple[str, str, str]) -> None: _inflight.discard(key) +def _fetch_current_labels(repo: str, index: str) -> list: + """Re-read the PR's labels from the API. [] on any failure. + + The webhook payload is a snapshot from the instant the event fired. When a + PR is labelled AI-REVIEW first and AI-USAGE a moment later, the review is + already claimed and running, so the second event is deduped and the usage + opt-in is lost — the review posts without its usage block. Reading the + labels again at review start closes that window. + """ + url = f"{GITEA_API}/repos/{repo}/issues/{index}/labels" + req = urllib.request.Request(url, headers={ + "Authorization": f"token {BOT_TOKEN}", + "Accept": "application/json", + }) + try: + with urllib.request.urlopen(req, timeout=15) as resp: + return json.loads(resp.read().decode() or "[]") + except Exception as e: + print(f"pragent-webhook: could not re-read labels for {repo}#{index}: {e}", flush=True) + return [] + + def _run_review( key: tuple[str, str, str], title: str, body: str, report_usage: bool, base_ref: str ) -> None: repo, index, sha = key + # The AI-USAGE opt-in may have been applied *after* the label event that + # triggered this review (labelling is two events, the review claims on the + # first). Re-read the labels now so the later opt-in still counts. + if not report_usage: + report_usage = _labels_have( + _fetch_current_labels(repo, index), AI_USAGE_LABEL + ) + if report_usage: + print( + f"pragent-webhook: {repo}#{index} AI-USAGE found on re-read " + f"(added after the trigger event)", + flush=True, + ) + try: with _review_slots: ok = review_pr( diff --git a/tests/pilot/test_webhook_server.py b/tests/pilot/test_webhook_server.py index 27d87b5..33d3461 100644 --- a/tests/pilot/test_webhook_server.py +++ b/tests/pilot/test_webhook_server.py @@ -134,3 +134,43 @@ def test_missing_label_is_ignored(monkeypatch): def test_review_slots_bound_matches_config(): assert ws.MAX_CONCURRENT >= 1 assert ws._review_slots._value <= ws.MAX_CONCURRENT + + +# --------------------------------------------------------------------------- +# AI-USAGE opt-in applied after the triggering label event +# --------------------------------------------------------------------------- + + +def test_run_review_rereads_labels_for_late_ai_usage(monkeypatch): + # Labelling a PR AI-REVIEW then AI-USAGE fires two events; the review + # claims on the first, so the second is deduped and the payload snapshot + # never sees AI-USAGE. The re-read at review start must recover it. + seen = {} + monkeypatch.setattr(ws, "_fetch_current_labels", + lambda repo, index: [{"name": "AI-REVIEW"}, {"name": "AI-USAGE"}]) + monkeypatch.setattr(ws, "review_pr", lambda **kw: seen.update(kw) or True) + ws._run_review(("o/r", "9", "abc1234"), "t", "b", False, "main") + assert seen["report_usage"] is True + + +def test_run_review_keeps_usage_off_when_label_absent(monkeypatch): + seen = {} + monkeypatch.setattr(ws, "_fetch_current_labels", + lambda repo, index: [{"name": "AI-REVIEW"}]) + monkeypatch.setattr(ws, "review_pr", lambda **kw: seen.update(kw) or True) + ws._run_review(("o/r", "9", "abc1234"), "t", "b", False, "main") + assert seen["report_usage"] is False + + +def test_label_reread_failure_is_not_fatal(monkeypatch): + # A dead API must not take the review down with it — the re-read is a + # best-effort upgrade of an opt-in flag, nothing more. + def _boom(*a, **k): + raise OSError("api down") + monkeypatch.setattr(ws.urllib.request, "urlopen", _boom) + assert ws._fetch_current_labels("o/r", "9") == [] + + seen = {} + monkeypatch.setattr(ws, "review_pr", lambda **kw: seen.update(kw) or True) + ws._run_review(("o/r", "9", "abc1234"), "t", "b", False, "main") + assert seen["report_usage"] is False