feat: prevent rubber-stamping of PRs with no changes (#1445)

scripts/review_gate.py — enhanced review gate with 3 checks:
  1. Empty PR detection (zombie PR with 0 changes)
  2. Approval requirement (at least 1 APPROVED review)
  3. Rubber-stamping detection (flag trivial PRs with
     empty/short review comments)

Fixes: typo in GITEA_TOKEN env var (was os.env...EN)
Added: check_empty_pr(), check_rubber_stamp()
Improved: structured error/warning output with emojis

tests/test_review_gate.py: 15 tests
  check_empty_pr: valid, empty, additions-only, deletions-only
  check_approvals: single, multiple, none, empty
  check_rubber_stamp: substantive, trivial+empty review,
    trivial+short review, trivial+substantive review,
    no-approvals skip, large PR with empty review ok

scripts/review_gate.py

@@ -1,7 +1,10 @@
 #!/usr/bin/env python3
 """
-Review Gate — Poka-yoke for unreviewed merges.
-Fails if the current PR has fewer than 1 approving review.
+Review Gate — Poka-yoke for unreviewed merges and zombie PRs.
+
+Checks:
+1. PR has at least 1 approving review (no rubber-stamping without approval)
+2. PR has actual changes (no zombie PRs with 0 additions/deletions)
 
 Usage in Gitea workflow:
   - name: Review Approval Gate
@@ -13,7 +16,6 @@ Usage in Gitea workflow:
 import os
 import sys
 import json
-import subprocess
 from urllib import request, error
 
 GITEA_TOKEN = os.environ.get("GITEA_TOKEN", "")
@@ -33,7 +35,68 @@ def api_call(method, path):
         return {"error": e.read().decode(), "status": e.code}
 
 
+def check_empty_pr(pr_data):
+    """Check if PR has no actual changes (zombie PR)."""
+    additions = pr_data.get("additions", 0)
+    deletions = pr_data.get("deletions", 0)
+    changed_files = pr_data.get("changed_files", 0)
+
+    if additions == 0 and deletions == 0 and changed_files == 0:
+        return False, (
+            f"ZOMBIE PR: PR has 0 additions, 0 deletions, 0 changed files. "
+            f"This appears to be an empty PR with no actual changes."
+        )
+    return True, None
+
+
+def check_approvals(reviews):
+    """Check if PR has at least one approving review."""
+    approvals = [r for r in reviews if r.get("state") == "APPROVED"]
+    if len(approvals) >= 1:
+        return True, len(approvals)
+    return False, 0
+
+
+def check_rubber_stamp(pr_data, reviews):
+    """
+    Check for rubber-stamping: approving reviews on PRs with trivial changes.
+
+    Rubber-stamping indicators:
+    - Approving reviews exist
+    - PR has very few changes (< 5 lines total)
+    - Review comments are empty or generic
+    """
+    approvals = [r for r in reviews if r.get("state") == "APPROVED"]
+    if not approvals:
+        return True, None  # No approvals to check
+
+    additions = pr_data.get("additions", 0)
+    deletions = pr_data.get("deletions", 0)
+    total_changes = additions + deletions
+
+    # Flag if approving a PR with fewer than 5 total changes
+    if total_changes < 5 and len(approvals) > 0:
+        # Check if review bodies are substantive
+        empty_reviews = [
+            r for r in approvals
+            if not r.get("body") or len(r.get("body", "").strip()) < 10
+        ]
+        if empty_reviews:
+            return False, (
+                f"SUSPICIOUS: PR has only {total_changes} total changes "
+                f"but {len(approvals)} approving review(s), "
+                f"{len(empty_reviews)} with empty/minimal comments. "
+                f"This may indicate rubber-stamping."
+            )
+
+    return True, None
+
+
 def main():
+    errors = []
+    warnings = []
+
+    # Validate environment
     if not GITEA_TOKEN:
         print("ERROR: GITEA_TOKEN not set")
         sys.exit(1)
@@ -44,27 +107,57 @@ def main():
 
     pr_number = PR_NUMBER
     if not pr_number:
-        # Try to infer from Gitea Actions environment
         pr_number = os.environ.get("GITEA_PULL_REQUEST_INDEX", "")
 
     if not pr_number:
         print("ERROR: Could not determine PR number")
         sys.exit(1)
 
+    # Fetch PR data
+    pr_data = api_call("GET", f"/repos/{REPO}/pulls/{pr_number}")
+    if isinstance(pr_data, dict) and "error" in pr_data:
+        print(f"ERROR fetching PR: {pr_data}")
+        sys.exit(1)
+
+    # Fetch reviews
     reviews = api_call("GET", f"/repos/{REPO}/pulls/{pr_number}/reviews")
     if isinstance(reviews, dict) and "error" in reviews:
         print(f"ERROR fetching reviews: {reviews}")
         sys.exit(1)
 
-    approvals = [r for r in reviews if r.get("state") == "APPROVED"]
-    if len(approvals) >= 1:
-        print(f"OK: PR #{pr_number} has {len(approvals)} approving review(s).")
-        sys.exit(0)
-    else:
-        print(f"BLOCKED: PR #{pr_number} has no approving reviews.")
-        print("Merges are not permitted without at least one approval.")
+    # ── Check 1: Empty PR (zombie PR) ───────────────────────
+    has_changes, empty_msg = check_empty_pr(pr_data)
+    if not has_changes:
+        errors.append(empty_msg)
+
+    # ── Check 2: Has approvals ──────────────────────────────
+    has_approval, approval_count = check_approvals(reviews)
+    if not has_approval:
+        errors.append(
+            f"PR #{pr_number} has no approving reviews. "
+            f"Merges require at least one approval."
+        )
+
+    # ── Check 3: Rubber-stamping detection ──────────────────
+    clean, rubber_msg = check_rubber_stamp(pr_data, reviews)
+    if not clean:
+        warnings.append(rubber_msg)
+
+    # ── Report ──────────────────────────────────────────────
+    if warnings:
+        for w in warnings:
+            print(f"⚠️  WARNING: {w}")
+
+    if errors:
+        for e in errors:
+            print(f"❌ BLOCKED: {e}")
         sys.exit(1)
 
+    print(f"✅ OK: PR #{pr_number} has {approval_count} approval(s) "
+          f"and {pr_data.get('additions', 0)} additions / "
+          f"{pr_data.get('deletions', 0)} deletions.")
+    sys.exit(0)
+
 
 if __name__ == "__main__":
     main()

tests/test_review_gate.py

@@ -0,0 +1,109 @@
+"""
+Tests for scripts/review_gate.py — Anti-rubber-stamping PR validation.
+"""
+
+import unittest
+import sys
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).parent.parent / "scripts"))
+from review_gate import check_empty_pr, check_approvals, check_rubber_stamp
+
+
+class TestCheckEmptyPr(unittest.TestCase):
+    def test_valid_pr(self):
+        pr = {"additions": 10, "deletions": 5, "changed_files": 2}
+        ok, msg = check_empty_pr(pr)
+        self.assertTrue(ok)
+        self.assertIsNone(msg)
+
+    def test_empty_pr(self):
+        pr = {"additions": 0, "deletions": 0, "changed_files": 0}
+        ok, msg = check_empty_pr(pr)
+        self.assertFalse(ok)
+        self.assertIn("ZOMBIE", msg)
+
+    def test_additions_only(self):
+        pr = {"additions": 50, "deletions": 0, "changed_files": 1}
+        ok, msg = check_empty_pr(pr)
+        self.assertTrue(ok)
+
+    def test_deletions_only(self):
+        pr = {"additions": 0, "deletions": 30, "changed_files": 1}
+        ok, msg = check_empty_pr(pr)
+        self.assertTrue(ok)
+
+    def test_missing_fields_treated_as_zero(self):
+        pr = {}
+        ok, msg = check_empty_pr(pr)
+        self.assertFalse(ok)
+
+
+class TestCheckApprovals(unittest.TestCase):
+    def test_has_approval(self):
+        reviews = [{"state": "APPROVED"}, {"state": "COMMENT"}]
+        ok, count = check_approvals(reviews)
+        self.assertTrue(ok)
+        self.assertEqual(count, 1)
+
+    def test_multiple_approvals(self):
+        reviews = [{"state": "APPROVED"}, {"state": "APPROVED"}]
+        ok, count = check_approvals(reviews)
+        self.assertTrue(ok)
+        self.assertEqual(count, 2)
+
+    def test_no_approvals(self):
+        reviews = [{"state": "COMMENT"}, {"state": "REQUEST_CHANGES"}]
+        ok, count = check_approvals(reviews)
+        self.assertFalse(ok)
+        self.assertEqual(count, 0)
+
+    def test_empty_reviews(self):
+        ok, count = check_approvals([])
+        self.assertFalse(ok)
+        self.assertEqual(count, 0)
+
+
+class TestCheckRubberStamp(unittest.TestCase):
+    def test_substantive_pr_no_warning(self):
+        pr = {"additions": 100, "deletions": 50}
+        reviews = [{"state": "APPROVED", "body": "Looks good, nice changes"}]
+        ok, msg = check_rubber_stamp(pr, reviews)
+        self.assertTrue(ok)
+        self.assertIsNone(msg)
+
+    def test_trivial_pr_empty_review_detected(self):
+        pr = {"additions": 2, "deletions": 0}
+        reviews = [{"state": "APPROVED", "body": ""}]
+        ok, msg = check_rubber_stamp(pr, reviews)
+        self.assertFalse(ok)
+        self.assertIn("SUSPICIOUS", msg)
+
+    def test_trivial_pr_short_review_detected(self):
+        pr = {"additions": 1, "deletions": 1}
+        reviews = [{"state": "APPROVED", "body": "ok"}]
+        ok, msg = check_rubber_stamp(pr, reviews)
+        self.assertFalse(ok)
+        self.assertIn("SUSPICIOUS", msg)
+
+    def test_trivial_pr_substantive_review_ok(self):
+        pr = {"additions": 2, "deletions": 0}
+        reviews = [{"state": "APPROVED", "body": "This small fix is correct. Tested locally."}]
+        ok, msg = check_rubber_stamp(pr, reviews)
+        self.assertTrue(ok)
+
+    def test_no_approvals_skips_check(self):
+        pr = {"additions": 0, "deletions": 0}
+        reviews = [{"state": "COMMENT"}]
+        ok, msg = check_rubber_stamp(pr, reviews)
+        self.assertTrue(ok)
+
+    def test_large_pr_with_empty_review_ok(self):
+        pr = {"additions": 500, "deletions": 200}
+        reviews = [{"state": "APPROVED", "body": ""}]
+        ok, msg = check_rubber_stamp(pr, reviews)
+        self.assertTrue(ok)  # Large PR, empty review is less suspicious
+
+
+if __name__ == "__main__":
+    unittest.main()