From bb61fa6f39baf6e177ecece7312766e182fdf3d6 Mon Sep 17 00:00:00 2001 From: GitHub CI Date: Mon, 6 Jul 2026 16:46:21 -0700 Subject: [PATCH] =?UTF-8?q?feat(ci):=20canonical=20merge=20gate=20?= =?UTF-8?q?=E2=80=94=20line=20scope=20+=20waive=20rail=20(protoAgent#1874)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate was file-scoped with no waive: a PR touching a shared, debt-laden file inherited that file's pre-existing findings as blocking (protoContent#424 paid two extra fix rounds for debt it never touched). ci/protopatch-gate.py is now the canonical copy (the parser tracks reporting.ts's format, so it lives with it): line scope blocks only when evidence lines intersect the PR's changed hunks (+/-3 context margin; line-less evidence on a touched file still blocks — fail closed on format drift), and a protopatch-waive line in the PR body drops a specific finding to INFORM, logged loudly in the step summary. 13 stdlib tests wired into CI. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 3 + .gitignore | 1 + ci/protopatch-gate.py | 197 +++++++++++++++++++++++++++++++++++++ ci/test_protopatch_gate.py | 138 ++++++++++++++++++++++++++ docs/code-review.md | 27 +++++ 5 files changed, 366 insertions(+) create mode 100644 ci/protopatch-gate.py create mode 100644 ci/test_protopatch_gate.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 732508d..afd8f2e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,3 +28,6 @@ jobs: - run: pnpm test - run: pnpm build - run: pnpm pack:smoke + # The CI merge-gate script (ci/protopatch-gate.py) parses the report format + # reporting.ts renders — its tests pin that coupling (protoAgent#1874). + - run: python3 ci/test_protopatch_gate.py diff --git a/.gitignore b/.gitignore index 051726d..a6a65cd 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,4 @@ coverage/ .automaker/features/ .automaker/checkpoints/ .automaker/trajectory/ +__pycache__/ diff --git a/ci/protopatch-gate.py b/ci/protopatch-gate.py new file mode 100644 index 0000000..4f9f3ed --- /dev/null +++ b/ci/protopatch-gate.py @@ -0,0 +1,197 @@ +#!/usr/bin/env python3 +"""protoPatch merge gate — fail the `review` check ONLY on findings that actually gate. + +CANONICAL COPY. This script tracks the report format `clawpatch report` renders +(src/reporting.ts) — it lives here so the parser and the format evolve together. +Consuming repos vendor it into `.github/scripts/` (protoContent is the reference +integration); sync from here, don't fork the semantics (protoAgent#1874). + +protoPatch's raw report over-blocks in three ways, each filtered here: + + 1. CATEGORY — "no test coverage" escalates to HIGH, maintainability/style raise + MEDIUM. Inform-only categories never gate. + 2. FILE SCOPE — findings sometimes land in files the PR never touched. When the + changed-file set is known, out-of-diff findings inform only. + 3. LINE SCOPE (protoAgent#1874) — a PR touching a shared, debt-laden file used to + inherit that file's PRE-EXISTING findings as blocking. When the PR's unified + diff is provided, a finding blocks only if some evidence line range intersects + the changed hunks (±CONTEXT_MARGIN lines). Findings on untouched lines of a + touched file drop to INFORM. Evidence WITHOUT line info on a touched file + still blocks (conservative: format drift must not silently open the gate). + + WAIVE (protoAgent#1874) — the escape hatch for "yes it's real, but not this PR": + the PR body (passed via $PROTOPATCH_WAIVERS) may carry lines of the form + + protopatch-waive: — + + Each waived finding drops to INFORM and is logged LOUDLY in the step summary — + a waive is an audited decision, never a silent skip. + + BLOCK = severity in {critical, high, medium} AND category not inform-only AND + in-diff (file scope, and line scope when the diff is available) AND + not waived. + INFORM = everything else — still posted in the PR comment, doesn't fail the check. + +Usage: protopatch-gate.py [] [] +Env: PROTOPATCH_WAIVERS — text scanned for `protopatch-waive:` lines (pass the + PR body; anything else in it is ignored). +Exit 0 = pass (merge allowed), 1 = blocking findings present. +""" + +import os +import re +import sys + +INFORM_CATEGORIES = { + "test-gap", "test-coverage", "maintainability", "code-quality", "style", + "docs", "documentation", "performance", "perf", "nit", "readability", + "observability", "naming", +} +BLOCK_SEVERITIES = {"critical", "high", "medium"} + +# Evidence ranges within this many lines of a changed hunk still count as touched — +# the diff's own context width; an off-by-context miss must not false-INFORM. +CONTEXT_MARGIN = 3 + +# `- path:12-34 (symbol)` | `- path:42` | `- path` (reporting.ts evidenceLabel). +_EVIDENCE_RE = re.compile(r"(?m)^-\s+([^\s:]+)(?::(\d+)(?:-(\d+))?)?") +_WAIVE_RE = re.compile(r"(?m)^\s*protopatch-waive:\s*(\S+)\s*(?:[—–-]+\s*(.*\S))?\s*$") +_HUNK_RE = re.compile(r"(?m)^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@") +_DIFF_FILE_RE = re.compile(r'(?m)^diff --git a/.* b/"?([^"\n]+)"?$') + + +def parse_changed_ranges(diff_text: str) -> dict[str, list[tuple[int, int]]]: + """{new-file path: [(start, end), ...]} from a unified diff. A pure-deletion + hunk (+c,0) is kept as a point range at c — the seam is still 'touched'.""" + ranges: dict[str, list[tuple[int, int]]] = {} + current: str | None = None + for line in diff_text.splitlines(): + fm = _DIFF_FILE_RE.match(line) + if fm: + current = fm.group(1) + ranges.setdefault(current, []) + continue + hm = _HUNK_RE.match(line) + if hm and current is not None: + start = int(hm.group(1)) + count = int(hm.group(2)) if hm.group(2) is not None else 1 + end = start + max(count, 1) - 1 + ranges[current].append((start, end)) + return ranges + + +def parse_waivers(text: str) -> dict[str, str]: + """{findingId: reason} from `protopatch-waive:` lines (reason may be '').""" + return {m.group(1): (m.group(2) or "").strip() for m in _WAIVE_RE.finditer(text or "")} + + +def evidence_refs(block: str) -> list[tuple[str, int | None, int | None]]: + """[(path, start|None, end|None), ...] from a finding block's evidence list.""" + out = [] + for m in _EVIDENCE_RE.finditer(block): + path = m.group(1) + start = int(m.group(2)) if m.group(2) else None + end = int(m.group(3)) if m.group(3) else start + out.append((path, start, end)) + return out + + +def touches_changed_lines( + refs: list[tuple[str, int | None, int | None]], + changed_ranges: dict[str, list[tuple[int, int]]], +) -> bool: + """True when some evidence ref intersects a changed hunk (±margin) — or when a + ref on a changed file carries no line info (conservative: can't prove it's + outside the diff, so it gates).""" + for path, start, end in refs: + hunks = changed_ranges.get(path) + if hunks is None: + continue # ref on an unchanged file — the file-scope check handles it + if start is None: + return True # line-less evidence on a touched file: block conservatively + for h_start, h_end in hunks: + if start <= h_end + CONTEXT_MARGIN and end >= h_start - CONTEXT_MARGIN: + return True + return False + + +def main() -> int: + report_path = sys.argv[1] if len(sys.argv) > 1 else "clawpatch-report.md" + changed_path = sys.argv[2] if len(sys.argv) > 2 else "changed-files.txt" + diff_path = sys.argv[3] if len(sys.argv) > 3 else "pr.diff" + if not os.path.exists(report_path) or os.path.getsize(report_path) == 0: + print("protoPatch: no report — not blocking.") + return 0 + report = open(report_path).read() + changed = set() + if os.path.exists(changed_path): + changed = {ln.strip() for ln in open(changed_path) if ln.strip()} + changed_ranges: dict[str, list[tuple[int, int]]] = {} + if os.path.exists(diff_path): + changed_ranges = parse_changed_ranges(open(diff_path).read()) + waivers = parse_waivers(os.environ.get("PROTOPATCH_WAIVERS", "")) + + offenders, informed, waived = [], [], [] + for block in re.split(r"(?m)^##\s+", report): + m = re.match(r"(critical|high|medium|low|info)\s*:\s*(.+)", block, re.I) + if not m: + continue + sev, title = m.group(1).lower(), m.group(2).strip()[:100] + cat_m = re.search(r"(?m)^category:\s*(\S+)", block) + cat = cat_m.group(1).lower() if cat_m else "" + fid_m = re.search(r"(?m)^id:\s*(\S+)", block) + fid = fid_m.group(1) if fid_m else "" + label = f"{sev}/{cat or '?'}: {title}" + (f" ({fid})" if fid else "") + + if sev not in BLOCK_SEVERITIES or cat in INFORM_CATEGORIES: + informed.append(label) + continue + refs = evidence_refs(block) + # File scope: only gate if the finding touches a changed file (when known). + if changed: + files = [path for path, _s, _e in refs] + if files and not any(f in changed for f in files): + informed.append(label + " [outside diff]") + continue + # Line scope (#1874): with the diff in hand, findings whose evidence never + # intersects a changed hunk are pre-existing debt — inform, don't gate. + if changed_ranges and refs and not touches_changed_lines(refs, changed_ranges): + informed.append(label + " [untouched lines]") + continue + # Waive rail (#1874): an audited, per-PR acknowledgement. + if fid and fid in waivers: + reason = waivers[fid] or "(no reason given)" + waived.append(f"{label} — waived: {reason}") + continue + offenders.append(label) + + summary = os.environ.get("GITHUB_STEP_SUMMARY", os.devnull) + with open(summary, "a") as s: + if offenders: + s.write("### ❌ protoPatch gate: blocking correctness/security findings\n") + for o in offenders: + s.write(f"- {o}\n") + else: + s.write(f"protoPatch gate: pass — {len(informed)} informational finding(s), none gating.\n") + if waived: + s.write(f"\n### ⚠️ {len(waived)} finding(s) WAIVED on this PR (protopatch-waive)\n") + for w in waived: + s.write(f"- {w}\n") + if informed and offenders: + s.write(f"\n_{len(informed)} informational finding(s) not gating._\n") + + if waived: + print(f"protoPatch gate: {len(waived)} finding(s) waived on this PR:") + for w in waived: + print(" -", w) + if offenders: + print("::error::protoPatch found blocking correctness/security issues in the diff — fix or waive.") + for o in offenders: + print(" -", o) + return 1 + print(f"protoPatch gate: pass ({len(informed)} informational finding(s) don't block).") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/ci/test_protopatch_gate.py b/ci/test_protopatch_gate.py new file mode 100644 index 0000000..ebd4301 --- /dev/null +++ b/ci/test_protopatch_gate.py @@ -0,0 +1,138 @@ +#!/usr/bin/env python3 +"""The gate's scope + waive semantics (protoAgent#1874). Run: python3 ci/test_protopatch_gate.py""" + +import importlib.util +import os +import subprocess +import sys +import tempfile +import unittest + +HERE = os.path.dirname(os.path.abspath(__file__)) +GATE = os.path.join(HERE, "protopatch-gate.py") + +_spec = importlib.util.spec_from_file_location("gate", GATE) +gate = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(gate) + + +def report_block(sev="high", cat="bug", fid="f-1", title="Broken thing.", evidence=("lib/feed.ts:40-44",)): + ev = "\n".join(f"- {e}" for e in evidence) + return f"## {sev}: {title}\nid: {fid}\ncategory: {cat}\nevidence:\n{ev}\n" + + +DIFF = """diff --git a/lib/feed.ts b/lib/feed.ts +index 111..222 100644 +--- a/lib/feed.ts ++++ b/lib/feed.ts +@@ -10,4 +10,6 @@ + context ++added ++added + context +@@ -100,3 +102,0 @@ +-removed +-removed +-removed +""" + + +class ParseTests(unittest.TestCase): + def test_changed_ranges_from_unified_diff(self): + ranges = gate.parse_changed_ranges(DIFF) + self.assertEqual(ranges["lib/feed.ts"][0], (10, 15)) + # Pure deletion keeps a point range at the seam. + self.assertEqual(ranges["lib/feed.ts"][1], (102, 102)) + + def test_evidence_refs_all_label_shapes(self): + block = "evidence:\n- a.ts:12-34 (fn)\n- b.ts:42\n- c.ts\n" + self.assertEqual( + gate.evidence_refs(block), + [("a.ts", 12, 34), ("b.ts", 42, 42), ("c.ts", None, None)], + ) + + def test_waiver_lines_parse_with_and_without_reason(self): + body = "Some PR prose.\nprotopatch-waive: f-abc — pre-existing, tracked in #431\nprotopatch-waive: f-def\n" + self.assertEqual(gate.parse_waivers(body), {"f-abc": "pre-existing, tracked in #431", "f-def": ""}) + + +class ScopeTests(unittest.TestCase): + def test_evidence_inside_hunk_touches(self): + ranges = {"lib/feed.ts": [(10, 15)]} + self.assertTrue(gate.touches_changed_lines([("lib/feed.ts", 12, 13)], ranges)) + + def test_evidence_within_margin_touches(self): + ranges = {"lib/feed.ts": [(10, 15)]} + self.assertTrue(gate.touches_changed_lines([("lib/feed.ts", 17, 18)], ranges)) # 15+3 + + def test_evidence_far_away_does_not_touch(self): + ranges = {"lib/feed.ts": [(10, 15)]} + self.assertFalse(gate.touches_changed_lines([("lib/feed.ts", 40, 44)], ranges)) + + def test_lineless_evidence_on_touched_file_blocks_conservatively(self): + ranges = {"lib/feed.ts": [(10, 15)]} + self.assertTrue(gate.touches_changed_lines([("lib/feed.ts", None, None)], ranges)) + + +def run_gate(report, *, changed="", diff="", waivers=""): + with tempfile.TemporaryDirectory() as td: + rp, cp, dp = (os.path.join(td, n) for n in ("r.md", "c.txt", "p.diff")) + open(rp, "w").write(report) + args = [sys.executable, GATE, rp] + if changed: + open(cp, "w").write(changed) + args.append(cp) + if diff: + open(dp, "w").write(diff) + if len(args) == 3: + args.append(os.path.join(td, "absent.txt")) + args.append(dp) + env = {**os.environ, "PROTOPATCH_WAIVERS": waivers, "GITHUB_STEP_SUMMARY": os.path.join(td, "s.md")} + proc = subprocess.run(args, capture_output=True, text=True, env=env) + return proc.returncode, proc.stdout + + +class EndToEndTests(unittest.TestCase): + def test_finding_on_changed_lines_blocks(self): + rc, out = run_gate(report_block(evidence=("lib/feed.ts:11",)), changed="lib/feed.ts\n", diff=DIFF) + self.assertEqual(rc, 1) + self.assertIn("f-1", out) + + def test_preexisting_debt_on_untouched_lines_informs(self): + # The #1874 case: same file, lines nowhere near the hunks. + rc, out = run_gate(report_block(evidence=("lib/feed.ts:40-44",)), changed="lib/feed.ts\n", diff=DIFF) + self.assertEqual(rc, 0) + self.assertIn("pass", out) + + def test_no_diff_falls_back_to_file_scope(self): + rc, _ = run_gate(report_block(evidence=("lib/feed.ts:40-44",)), changed="lib/feed.ts\n") + self.assertEqual(rc, 1) # old behavior preserved without the diff input + + def test_waive_unblocks_and_is_loud(self): + rc, out = run_gate( + report_block(evidence=("lib/feed.ts:11",)), + changed="lib/feed.ts\n", + diff=DIFF, + waivers="protopatch-waive: f-1 — pre-existing, tracked in #431\n", + ) + self.assertEqual(rc, 0) + self.assertIn("waived", out) + self.assertIn("tracked in #431", out) + + def test_waive_of_a_different_finding_does_not_unblock(self): + rc, _ = run_gate( + report_block(evidence=("lib/feed.ts:11",)), + changed="lib/feed.ts\n", + diff=DIFF, + waivers="protopatch-waive: f-OTHER — nope\n", + ) + self.assertEqual(rc, 1) + + def test_inform_categories_and_out_of_diff_still_inform(self): + report = report_block(cat="test-gap") + report_block(fid="f-2", evidence=("other/file.ts:5",)) + rc, _ = run_gate(report, changed="lib/feed.ts\n", diff=DIFF) + self.assertEqual(rc, 0) + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/docs/code-review.md b/docs/code-review.md index 3ee65a4..957fa20 100644 --- a/docs/code-review.md +++ b/docs/code-review.md @@ -122,3 +122,30 @@ Categories requested from the provider: Review does not edit files. Use `clawpatch fix --finding ` for the explicit patch loop. + +## The merge gate (`ci/protopatch-gate.py`) + +The canonical CI gate over `clawpatch report` output — consuming repos vendor it +into `.github/scripts/` and sync from here (the parser tracks `reporting.ts`'s +format; protoAgent#1874 defined the semantics once to stop per-repo drift): + +- **Category filter** — inform-only categories (test-gap, maintainability, style, + docs, perf, …) never gate; correctness/security at critical/high/medium do. +- **File scope** — with `changed-files.txt`, findings outside the diff inform only. +- **Line scope** — with the PR's unified diff as the third arg, a finding blocks + only if evidence lines intersect a changed hunk (±3 context lines). Pre-existing + debt on untouched lines of a touched file informs instead of taxing the PR. + Line-less evidence on a touched file still blocks (fail closed on format drift). +- **Waive** — the PR body may carry `protopatch-waive: — ` + lines (pass the body via `$PROTOPATCH_WAIVERS`); waived findings inform and are + logged loudly in the step summary. An audited decision, never a silent skip. + +```yaml +- name: Gate on blocking findings + env: + PROTOPATCH_WAIVERS: ${{ github.event.pull_request.body }} + run: | + git diff --name-only "origin/${{ github.base_ref }}...HEAD" > changed-files.txt + git diff "origin/${{ github.base_ref }}...HEAD" > pr.diff + python3 .github/scripts/protopatch-gate.py clawpatch-report.md changed-files.txt pr.diff +```