fix(webhook): re-read PR labels at review start so a late AI-USAGE counts #10
@@ -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(
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user