Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ coverage/
.automaker/features/
.automaker/checkpoints/
.automaker/trajectory/
__pycache__/
197 changes: 197 additions & 0 deletions ci/protopatch-gate.py
Original file line number Diff line number Diff line change
@@ -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: <findingId> — <reason>

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 <report.md> [<changed-files.txt>] [<pr.diff>]
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())
138 changes: 138 additions & 0 deletions ci/test_protopatch_gate.py
Original file line number Diff line number Diff line change
@@ -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)
27 changes: 27 additions & 0 deletions docs/code-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,30 @@ Categories requested from the provider:

Review does not edit files. Use `clawpatch fix --finding <id>` 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: <findingId> — <reason>`
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
```