From 133508d9ce643d2b2e27d7f99c201e13df68898f Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 21 Sep 2026 17:18:21 -0700 Subject: [PATCH 1/3] ci: add provider-neutral review fabric receipts --- .github/review-fabric-policy.json | 15 ++ .github/review-fabric.md | 72 +++++++ .github/scripts/review_fabric.py | 345 ++++++++++++++++++++++++++++++ tests/test_review_fabric.py | 256 ++++++++++++++++++++++ 4 files changed, 688 insertions(+) create mode 100644 .github/review-fabric-policy.json create mode 100644 .github/review-fabric.md create mode 100644 .github/scripts/review_fabric.py create mode 100644 tests/test_review_fabric.py diff --git a/.github/review-fabric-policy.json b/.github/review-fabric-policy.json new file mode 100644 index 000000000000..b9a11672a963 --- /dev/null +++ b/.github/review-fabric-policy.json @@ -0,0 +1,15 @@ +{ + "schema": "cmux.review-fabric-policy/v1", + "minimum_independent_runs": 2, + "quorum_roles": ["reviewer"], + "required_capability_classes": { + "frontier": 1 + }, + "blocking_run_dispositions": ["hold", "execute", "reject"], + "satisfied_finding_dispositions": [ + "fixed", + "declined_with_rationale", + "not_actionable" + ], + "blocking_severities": ["P0", "P1"] +} diff --git a/.github/review-fabric.md b/.github/review-fabric.md new file mode 100644 index 000000000000..fe8db42fa253 --- /dev/null +++ b/.github/review-fabric.md @@ -0,0 +1,72 @@ +# Review fabric receipts + +The review fabric owns review state independently of any one provider. + +A receipt is bound to one exact pull-request head and records independent review runs plus the findings they published. `.github/scripts/review_fabric.py` evaluates those receipts against `.github/review-fabric-policy.json`. + +## Identity + +Reviewer independence is keyed by `session_id`, not GitHub login, provider, or model name. One session invoking multiple models still contributes one quorum vote. + +Every run records: + +- worker/session provenance; +- provider, harness, model, and capability class; +- role; +- exact head SHA; +- rules version; +- status and disposition; +- optional evidence class. + +Old-head runs remain auditable but never count toward a current-head quorum. + +## Findings + +Published actionable findings must have an accepted disposition and a reply after the latest reviewer message. A `declined_with_rationale` finding must include a rationale. + +The schema can also retain truthful nonterminal/audit states such as `answered_unverified`, `resolved_unverified`, `resolved_unanswered`, `outdated`, and `unavailable`. Those states remain visible without pretending that a reply or GitHub thread resolution proved the finding was fixed. + +The v1 evidence classes follow the Fieldwork distinction: + +- `source-read` +- `model-executed` +- `target-test-prepared` +- `target-executed` +- `integration-executed` +- `full-gate` + +Synthesis must not silently upgrade one evidence class into another. A finding may use severity `unknown` when a provider does not expose enough structured evidence to classify it without guessing. + +## Default policy + +The repository policy currently requires: + +- two independent completed `reviewer` sessions; +- at least one `frontier` capability-class session; +- no completed `hold`, `execute`, or `reject` run; +- all published actionable findings disposed and answered; +- complete capture. + +The policy intentionally names no providers or models. Greptile, CodeRabbit, Claude Code, Codex, local GPU agents, or future reviewers can all produce the same receipt shape. + +## CLI + +Evaluate a receipt from a file: + +```sh +python3 .github/scripts/review_fabric.py \ + --input /path/to/receipt.json \ + --json +``` + +Or pipe the receipt on stdin: + +```sh +cat receipt.json | python3 .github/scripts/review_fabric.py +``` + +Exit status is zero only when the policy passes. + +## Next integration + +This is the provider-neutral contract layer. Listener/dispatcher work should create these receipts from exact-head review runs. The existing agent PR obligation ledger can then consume the evaluation instead of treating any named review provider as the source of truth. diff --git a/.github/scripts/review_fabric.py b/.github/scripts/review_fabric.py new file mode 100644 index 000000000000..1b9dbd673c81 --- /dev/null +++ b/.github/scripts/review_fabric.py @@ -0,0 +1,345 @@ +#!/usr/bin/env python3 +"""Evaluate provider-neutral review receipts for one exact pull-request head.""" +from __future__ import annotations + +import argparse +import datetime as dt +import json +import sys +from collections import Counter +from pathlib import Path +from typing import Any + +RECEIPT_SCHEMA = "cmux.review-fabric/v1" +POLICY_SCHEMA = "cmux.review-fabric-policy/v1" +EVALUATION_SCHEMA = "cmux.review-fabric-evaluation/v1" + +RUN_STATUSES = {"queued", "running", "completed", "failed", "cancelled", "unavailable"} +RUN_DISPOSITIONS = {"accept", "repair", "hold", "execute", "reject", "unavailable"} +FINDING_DISPOSITIONS = { + "pending", + "fixed", + "declined_with_rationale", + "not_actionable", + "answered_unverified", + "resolved_unverified", + "resolved_unanswered", + "outdated", + "unavailable", + "waiting_on_reviewer", + "blocked_on_human", +} +SEVERITIES = {"P0", "P1", "P2", "P3", "info", "unknown"} +EVIDENCE_CLASSES = { + "source-read", + "model-executed", + "target-test-prepared", + "target-executed", + "integration-executed", + "full-gate", +} + + +def parse_time(value: str | None) -> dt.datetime | None: + if not value: + return None + return dt.datetime.fromisoformat(value.replace("Z", "+00:00")) + + +def _required_string(item: dict[str, Any], key: str, kind: str, errors: list[str]) -> str: + value = item.get(key) + if not isinstance(value, str) or not value.strip(): + errors.append(f"{kind} requires non-empty {key}") + return "" + return value + + +def validate_document(document: dict[str, Any], policy: dict[str, Any]) -> list[str]: + errors: list[str] = [] + if document.get("schema") != RECEIPT_SCHEMA: + errors.append(f"receipt schema must be {RECEIPT_SCHEMA}") + if policy.get("schema") != POLICY_SCHEMA: + errors.append(f"policy schema must be {POLICY_SCHEMA}") + + head = _required_string(document, "head_sha", "receipt", errors) + if head and len(head) != 40: + errors.append("receipt head_sha must be a full 40-character commit SHA") + if document.get("capture_complete") is not True: + errors.append("review receipt capture is incomplete") + + runs = document.get("runs") + if not isinstance(runs, list): + errors.append("receipt runs must be a list") + runs = [] + findings = document.get("findings") + if not isinstance(findings, list): + errors.append("receipt findings must be a list") + findings = [] + + run_ids: set[str] = set() + for index, run in enumerate(runs): + if not isinstance(run, dict): + errors.append(f"run[{index}] must be an object") + continue + run_id = _required_string(run, "id", f"run[{index}]", errors) + if run_id in run_ids: + errors.append(f"duplicate review run id: {run_id}") + run_ids.add(run_id) + for key in ( + "worker_id", + "session_id", + "provider", + "harness", + "model", + "role", + "capability_class", + "head_sha", + "status", + "disposition", + "rules_version", + ): + _required_string(run, key, f"run {run_id or index}", errors) + if run.get("status") not in RUN_STATUSES: + errors.append(f"run {run_id or index} has unknown status {run.get('status')!r}") + if run.get("disposition") not in RUN_DISPOSITIONS: + errors.append(f"run {run_id or index} has unknown disposition {run.get('disposition')!r}") + evidence_class = run.get("evidence_class") + if evidence_class is not None and evidence_class not in EVIDENCE_CLASSES: + errors.append(f"run {run_id or index} has unknown evidence_class {evidence_class!r}") + + finding_ids: set[str] = set() + for index, finding in enumerate(findings): + if not isinstance(finding, dict): + errors.append(f"finding[{index}] must be an object") + continue + finding_id = _required_string(finding, "id", f"finding[{index}]", errors) + if finding_id in finding_ids: + errors.append(f"duplicate finding id: {finding_id}") + finding_ids.add(finding_id) + run_id = _required_string(finding, "run_id", f"finding {finding_id or index}", errors) + if run_id and run_id not in run_ids: + errors.append(f"finding {finding_id or index} references unknown run {run_id}") + finding_head = _required_string(finding, "head_sha", f"finding {finding_id or index}", errors) + if finding_head and len(finding_head) != 40: + errors.append(f"finding {finding_id or index} head_sha must be a full commit SHA") + if finding.get("severity") not in SEVERITIES: + errors.append(f"finding {finding_id or index} has unknown severity {finding.get('severity')!r}") + if finding.get("disposition") not in FINDING_DISPOSITIONS: + errors.append( + f"finding {finding_id or index} has unknown disposition {finding.get('disposition')!r}" + ) + if not isinstance(finding.get("actionable"), bool): + errors.append(f"finding {finding_id or index} requires boolean actionable") + if not isinstance(finding.get("published"), bool): + errors.append(f"finding {finding_id or index} requires boolean published") + if not isinstance(finding.get("verified"), bool): + errors.append(f"finding {finding_id or index} requires boolean verified") + + minimum = policy.get("minimum_independent_runs") + if not isinstance(minimum, int) or minimum < 1: + errors.append("policy minimum_independent_runs must be a positive integer") + quorum_roles = policy.get("quorum_roles") + if not isinstance(quorum_roles, list) or not quorum_roles or not all( + isinstance(role, str) and role for role in quorum_roles + ): + errors.append("policy quorum_roles must be a non-empty string list") + required_classes = policy.get("required_capability_classes") + if not isinstance(required_classes, dict) or not all( + isinstance(name, str) + and name + and isinstance(count, int) + and count >= 0 + for name, count in (required_classes or {}).items() + ): + errors.append("policy required_capability_classes must map names to non-negative integers") + blocking_dispositions = policy.get("blocking_run_dispositions") + if not isinstance(blocking_dispositions, list) or not all( + disposition in RUN_DISPOSITIONS for disposition in (blocking_dispositions or []) + ): + errors.append("policy blocking_run_dispositions contains an unknown disposition") + satisfied = policy.get("satisfied_finding_dispositions") + if not isinstance(satisfied, list) or not all( + disposition in FINDING_DISPOSITIONS for disposition in (satisfied or []) + ): + errors.append("policy satisfied_finding_dispositions contains an unknown disposition") + blocking_severities = policy.get("blocking_severities") + if not isinstance(blocking_severities, list) or not all( + severity in SEVERITIES for severity in (blocking_severities or []) + ): + errors.append("policy blocking_severities contains an unknown severity") + return errors + + +def evaluate(document: dict[str, Any], policy: dict[str, Any]) -> dict[str, Any]: + validation_errors = validate_document(document, policy) + head = str(document.get("head_sha") or "") + reasons = list(validation_errors) + + runs = document.get("runs") if isinstance(document.get("runs"), list) else [] + current_runs = [ + run + for run in runs + if isinstance(run, dict) and run.get("head_sha") == head + ] + stale_run_ids = [ + str(run.get("id") or "") + for run in runs + if isinstance(run, dict) and run.get("head_sha") != head + ] + + blocking_run_dispositions = set(policy.get("blocking_run_dispositions") or []) + quorum_roles = set(policy.get("quorum_roles") or []) + blocked_runs = [ + run + for run in current_runs + if run.get("status") == "completed" + and run.get("disposition") in blocking_run_dispositions + ] + for run in blocked_runs: + reasons.append( + f"review run {run.get('id')} ended with blocking disposition {run.get('disposition')}" + ) + + eligible_runs = [ + run + for run in current_runs + if run.get("status") == "completed" + and run.get("role") in quorum_roles + and run.get("disposition") not in blocking_run_dispositions + ] + + # One worker/session gets one vote even if it invokes several providers/models. + sessions: dict[str, dict[str, Any]] = {} + for run in eligible_runs: + session_id = str(run.get("session_id") or "") + if session_id and session_id not in sessions: + sessions[session_id] = run + + minimum = int(policy.get("minimum_independent_runs") or 0) + if len(sessions) < minimum: + reasons.append( + f"independent review quorum is {len(sessions)}/{minimum} for head {head}" + ) + + capability_counts = Counter( + str(run.get("capability_class") or "") + for run in sessions.values() + ) + for capability, required in (policy.get("required_capability_classes") or {}).items(): + have = capability_counts.get(capability, 0) + if have < required: + reasons.append( + f"capability quorum {capability} is {have}/{required}" + ) + + satisfied_dispositions = set(policy.get("satisfied_finding_dispositions") or []) + blocking_severities = set(policy.get("blocking_severities") or []) + findings = document.get("findings") if isinstance(document.get("findings"), list) else [] + current_findings = [ + finding + for finding in findings + if isinstance(finding, dict) and finding.get("head_sha") == head + ] + + active_findings = [] + for finding in current_findings: + if not finding.get("published") or not finding.get("actionable"): + continue + active_findings.append(finding) + finding_id = finding.get("id") + disposition = finding.get("disposition") + if ( + finding.get("verified") is True + and finding.get("severity") in blocking_severities + and disposition not in satisfied_dispositions + ): + reasons.append( + f"verified blocker {finding_id} remains {disposition}" + ) + if disposition not in satisfied_dispositions: + reasons.append( + f"actionable finding {finding_id} remains {disposition}" + ) + continue + + if disposition == "declined_with_rationale" and not str( + finding.get("rationale") or "" + ).strip(): + reasons.append( + f"declined finding {finding_id} requires a rationale" + ) + + latest_reviewer = parse_time(finding.get("latest_reviewer_at")) + latest_reply = parse_time(finding.get("latest_reply_at")) + if latest_reviewer is None or latest_reply is None or latest_reply <= latest_reviewer: + reasons.append( + f"finding {finding_id} lacks a reply after the latest reviewer message" + ) + + # Keep reasons stable for machine consumers and readable logs. + unique_reasons = list(dict.fromkeys(reasons)) + report = { + "schema": EVALUATION_SCHEMA, + "head_sha": head, + "passed": not unique_reasons, + "capture_complete": document.get("capture_complete") is True, + "counts": { + "current_runs": len(current_runs), + "eligible_runs": len(eligible_runs), + "independent_sessions": len(sessions), + "current_findings": len(current_findings), + "actionable_published_findings": len(active_findings), + }, + "capability_counts": dict(sorted(capability_counts.items())), + "stale_run_ids": stale_run_ids, + "reasons": unique_reasons, + } + return report + + +def load_json(path: str) -> dict[str, Any]: + if path == "-": + value = json.load(sys.stdin) + else: + value = json.loads(Path(path).read_text(encoding="utf-8")) + if not isinstance(value, dict): + raise ValueError("JSON document must be an object") + return value + + +def default_policy_path() -> Path: + return Path(__file__).resolve().parents[1] / "review-fabric-policy.json" + + +def main() -> int: + parser = argparse.ArgumentParser() + parser.add_argument("--input", default="-", help="review receipt JSON path, or - for stdin") + parser.add_argument( + "--policy", + default=str(default_policy_path()), + help="review fabric policy JSON path", + ) + parser.add_argument("--json", action="store_true", help="emit the full evaluation JSON") + args = parser.parse_args() + + try: + report = evaluate(load_json(args.input), load_json(args.policy)) + except Exception as error: + print(f"review-fabric: ERROR: {error}", file=sys.stderr) + return 1 + + if args.json: + print(json.dumps(report, indent=2, sort_keys=True)) + else: + print("review-fabric: " + ("PASS" if report["passed"] else "FAIL")) + print( + "- independent sessions: " + f"{report['counts']['independent_sessions']}" + ) + for reason in report["reasons"]: + print(f"- {reason}") + return 0 if report["passed"] else 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_review_fabric.py b/tests/test_review_fabric.py new file mode 100644 index 000000000000..cdeaa60053e9 --- /dev/null +++ b/tests/test_review_fabric.py @@ -0,0 +1,256 @@ +import importlib.util +import json +import sys +import unittest +from pathlib import Path + +ROOT = Path(__file__).parents[1] +SCRIPT = ROOT / ".github/scripts/review_fabric.py" +POLICY = ROOT / ".github/review-fabric-policy.json" + +spec = importlib.util.spec_from_file_location("review_fabric", SCRIPT) +review_fabric = importlib.util.module_from_spec(spec) +sys.modules[spec.name] = review_fabric +spec.loader.exec_module(review_fabric) + +HEAD = "a" * 40 +OLD_HEAD = "b" * 40 + + +def policy(): + return json.loads(POLICY.read_text(encoding="utf-8")) + + +def run( + run_id, + session_id, + *, + head=HEAD, + capability="frontier", + role="reviewer", + status="completed", + disposition="accept", +): + return { + "id": run_id, + "worker_id": f"worker-{session_id}", + "session_id": session_id, + "provider": "provider", + "harness": "agent-harness", + "model": "model", + "role": role, + "capability_class": capability, + "head_sha": head, + "status": status, + "disposition": disposition, + "rules_version": "rules-v1", + "evidence_class": "model-executed", + } + + +def finding( + finding_id, + run_id, + *, + head=HEAD, + severity="P2", + disposition="fixed", + actionable=True, + published=True, + verified=True, + reviewer_at="2026-09-21T12:00:00Z", + reply_at="2026-09-21T12:01:00Z", + rationale=None, +): + item = { + "id": finding_id, + "run_id": run_id, + "head_sha": head, + "severity": severity, + "disposition": disposition, + "actionable": actionable, + "published": published, + "verified": verified, + "latest_reviewer_at": reviewer_at, + "latest_reply_at": reply_at, + } + if rationale is not None: + item["rationale"] = rationale + return item + + +def document(runs, findings=None, *, capture_complete=True, head=HEAD): + return { + "schema": review_fabric.RECEIPT_SCHEMA, + "head_sha": head, + "capture_complete": capture_complete, + "runs": runs, + "findings": findings or [], + } + + +class ReviewFabricTests(unittest.TestCase): + def test_two_independent_sessions_with_frontier_lane_pass(self): + report = review_fabric.evaluate( + document([ + run("opus", "session-opus", capability="frontier"), + run("local", "session-local", capability="local"), + ]), + policy(), + ) + self.assertTrue(report["passed"]) + self.assertEqual(report["counts"]["independent_sessions"], 2) + self.assertEqual(report["capability_counts"], {"frontier": 1, "local": 1}) + + def test_same_session_using_two_models_gets_one_vote(self): + report = review_fabric.evaluate( + document([ + run("opus", "same-session", capability="frontier"), + run("sol", "same-session", capability="frontier"), + ]), + policy(), + ) + self.assertFalse(report["passed"]) + self.assertEqual(report["counts"]["independent_sessions"], 1) + self.assertTrue(any("1/2" in reason for reason in report["reasons"])) + + def test_frontier_requirement_is_independent_of_total_quorum(self): + report = review_fabric.evaluate( + document([ + run("local-a", "session-a", capability="local"), + run("local-b", "session-b", capability="local"), + ]), + policy(), + ) + self.assertFalse(report["passed"]) + self.assertIn("capability quorum frontier is 0/1", report["reasons"]) + + def test_stale_head_receipts_never_count(self): + report = review_fabric.evaluate( + document([ + run("old", "session-old", head=OLD_HEAD), + run("current", "session-current"), + ]), + policy(), + ) + self.assertFalse(report["passed"]) + self.assertEqual(report["stale_run_ids"], ["old"]) + self.assertEqual(report["counts"]["independent_sessions"], 1) + + def test_hold_blocks_even_when_another_lane_accepts(self): + report = review_fabric.evaluate( + document([ + run("accept", "session-a"), + run("hold", "session-b", capability="local", disposition="hold"), + ]), + policy(), + ) + self.assertFalse(report["passed"]) + self.assertTrue(any("blocking disposition hold" in reason for reason in report["reasons"])) + + def test_pending_verified_p1_is_reported_as_blocker(self): + report = review_fabric.evaluate( + document( + [ + run("opus", "session-opus"), + run("sol", "session-sol"), + ], + [finding("f1", "opus", severity="P1", disposition="pending", reply_at=None)], + ), + policy(), + ) + self.assertFalse(report["passed"]) + self.assertIn("verified blocker f1 remains pending", report["reasons"]) + self.assertIn("actionable finding f1 remains pending", report["reasons"]) + + def test_fixed_finding_requires_reply_after_latest_reviewer_message(self): + report = review_fabric.evaluate( + document( + [ + run("opus", "session-opus"), + run("sol", "session-sol"), + ], + [ + finding( + "f1", + "opus", + disposition="fixed", + reviewer_at="2026-09-21T12:02:00Z", + reply_at="2026-09-21T12:01:00Z", + ) + ], + ), + policy(), + ) + self.assertFalse(report["passed"]) + self.assertTrue(any("lacks a reply after" in reason for reason in report["reasons"])) + + def test_declined_finding_requires_rationale(self): + report = review_fabric.evaluate( + document( + [ + run("opus", "session-opus"), + run("sol", "session-sol"), + ], + [finding("f1", "opus", disposition="declined_with_rationale", rationale="")], + ), + policy(), + ) + self.assertFalse(report["passed"]) + self.assertIn("declined finding f1 requires a rationale", report["reasons"]) + + def test_non_actionable_or_unpublished_findings_do_not_block(self): + report = review_fabric.evaluate( + document( + [ + run("opus", "session-opus"), + run("sol", "session-sol"), + ], + [ + finding("internal", "opus", disposition="pending", published=False), + finding("info", "sol", disposition="pending", actionable=False), + ], + ), + policy(), + ) + self.assertTrue(report["passed"]) + + def test_incomplete_capture_fails_closed(self): + report = review_fabric.evaluate( + document( + [ + run("opus", "session-opus"), + run("sol", "session-sol"), + ], + capture_complete=False, + ), + policy(), + ) + self.assertFalse(report["passed"]) + self.assertIn("review receipt capture is incomplete", report["reasons"]) + + def test_unknown_finding_run_fails_closed(self): + report = review_fabric.evaluate( + document( + [ + run("opus", "session-opus"), + run("sol", "session-sol"), + ], + [finding("f1", "missing")], + ), + policy(), + ) + self.assertFalse(report["passed"]) + self.assertTrue(any("unknown run missing" in reason for reason in report["reasons"])) + + def test_repository_policy_is_provider_neutral(self): + raw = POLICY.read_text(encoding="utf-8").lower() + for provider_name in ("greptile", "coderabbit", "anthropic", "openai", "claude", "codex"): + self.assertNotIn(provider_name, raw) + configured = policy() + self.assertEqual(configured["minimum_independent_runs"], 2) + self.assertEqual(configured["required_capability_classes"], {"frontier": 1}) + + +if __name__ == "__main__": + unittest.main() From fcdffd02db9b0abcd2fc51cc7a623ea66a5ca1bb Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 21 Sep 2026 17:18:48 -0700 Subject: [PATCH 2/3] ci: adapt GitHub review data into review fabric receipts --- .github/review-fabric.md | 24 +++ .github/scripts/github_review_receipt.py | 223 +++++++++++++++++++++++ tests/test_github_review_receipt.py | 212 +++++++++++++++++++++ 3 files changed, 459 insertions(+) create mode 100644 .github/scripts/github_review_receipt.py create mode 100644 tests/test_github_review_receipt.py diff --git a/.github/review-fabric.md b/.github/review-fabric.md index fe8db42fa253..39a50f90d547 100644 --- a/.github/review-fabric.md +++ b/.github/review-fabric.md @@ -70,3 +70,27 @@ Exit status is zero only when the policy passes. ## Next integration This is the provider-neutral contract layer. Listener/dispatcher work should create these receipts from exact-head review runs. The existing agent PR obligation ledger can then consume the evaluation instead of treating any named review provider as the source of truth. + + +## GitHub review adapter + +`.github/scripts/github_review_receipt.py` converts the existing trusted GitHub review ledger into the same provider-neutral receipt format. + +The adapter deliberately stays conservative: + +- structured GitHub review submissions become `external` reviewer runs; +- review commit identities remain exact, so an old review never becomes current-head coverage; +- Greptile's mutable summary comment may provide an exact reviewed head through the trusted existing parser; +- active inline findings remain `pending`, `answered_unverified`, `resolved_unverified`, or `resolved_unanswered` according to the existing ledger; +- provider findings use severity `unknown` unless a later verifier supplies stronger evidence; +- a current inline thread without a structured review provenance makes capture incomplete instead of fabricating a review run. + +External review lanes are useful evidence but cannot satisfy the default fabric policy alone: the repository still requires two independent sessions and at least one frontier-class lane. + +The adapter can emit a receipt in a trusted GitHub Actions context with: + +```sh +python3 .github/scripts/github_review_receipt.py > github-review-receipt.json +``` + +It reuses the existing agent PR gate's GitHub collector and classification rules; it does not create a second source of truth for review-thread semantics. diff --git a/.github/scripts/github_review_receipt.py b/.github/scripts/github_review_receipt.py new file mode 100644 index 000000000000..cb6ee5ddf235 --- /dev/null +++ b/.github/scripts/github_review_receipt.py @@ -0,0 +1,223 @@ +#!/usr/bin/env python3 +"""Convert GitHub review-bot data into provider-neutral review-fabric receipts.""" +from __future__ import annotations + +import datetime as dt +import hashlib +import importlib.util +import json +import os +import sys +from pathlib import Path +from typing import Any + +SCRIPT_DIR = Path(__file__).resolve().parent +if str(SCRIPT_DIR) not in sys.path: + sys.path.insert(0, str(SCRIPT_DIR)) + +import review_fabric + +DEFAULT_RULES_VERSION = "github-review-adapter-v1" + + +def load_gate() -> Any: + path = SCRIPT_DIR / "agent-pr-review-gate.py" + spec = importlib.util.spec_from_file_location("agent_pr_review_gate_for_fabric", path) + if spec is None or spec.loader is None: + raise RuntimeError("unable to load agent PR review gate") + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +def parse_time(value: str | None) -> dt.datetime: + if not value: + return dt.datetime.min.replace(tzinfo=dt.timezone.utc) + return dt.datetime.fromisoformat(value.replace("Z", "+00:00")) + + +def stable_id(prefix: str, *parts: str) -> str: + digest = hashlib.sha256("\x00".join(parts).encode()).hexdigest()[:20] + return f"{prefix}:{digest}" + + +def _review_run( + *, + bot: str, + head: str, + submitted_at: str, + state: str, + rules_version: str, + harness: str = "github-review", +) -> dict[str, Any]: + run_id = stable_id("github-review", bot, head, submitted_at, harness) + session_id = stable_id("github-session", bot, head, submitted_at, harness) + disposition = "repair" if state == "CHANGES_REQUESTED" else "accept" + return { + "id": run_id, + "worker_id": bot, + "session_id": session_id, + "provider": bot, + "harness": harness, + "model": "external-opaque", + "role": "reviewer", + "capability_class": "external", + "head_sha": head, + "status": "completed", + "disposition": disposition, + "rules_version": rules_version, + "evidence_class": "source-read", + "submitted_at": submitted_at, + } + + +def _finding_disposition(item: Any) -> str: + mapping = { + "pending_reply": "pending", + "answered_unverified": "answered_unverified", + "resolved_unverified": "resolved_unverified", + "resolved_unanswered": "resolved_unanswered", + "outdated": "outdated", + "informational": "not_actionable", + "unavailable": "unavailable", + } + return mapping.get(str(item.disposition), "pending") + + +def receipt_from_pr( + pr: dict[str, Any], + *, + bots: tuple[str, ...], + reply_actors: tuple[str, ...], + gate: Any, + rules_version: str = DEFAULT_RULES_VERSION, +) -> dict[str, Any]: + head = str(pr.get("headRefOid") or "") + if len(head) != 40: + raise ValueError("pull request headRefOid must be a full commit SHA") + + runs: list[dict[str, Any]] = [] + review_records: list[tuple[str, dt.datetime, dict[str, Any]]] = [] + by_run_id: dict[str, dict[str, Any]] = {} + + for review in (pr.get("reviews") or {}).get("nodes") or []: + state = str(review.get("state") or "") + if state not in {"COMMENTED", "APPROVED", "CHANGES_REQUESTED"}: + continue + bot = gate.canonical_bot(gate.login(review.get("author")), bots) + review_head = str((review.get("commit") or {}).get("oid") or "") + if bot is None or len(review_head) != 40: + continue + submitted_at = str(review.get("submittedAt") or "") + run = _review_run( + bot=bot, + head=review_head, + submitted_at=submitted_at, + state=state, + rules_version=rules_version, + ) + if run["id"] not in by_run_id: + runs.append(run) + by_run_id[run["id"]] = run + review_records.append((bot, parse_time(submitted_at), run)) + + # Greptile reuses one summary comment across re-reviews. The trusted gate + # already extracts its exact "Last reviewed commit" identity. + if "greptile-apps" in bots: + summary_head = gate.greptile_summary_head(pr) + if summary_head: + run = _review_run( + bot="greptile-apps", + head=summary_head, + submitted_at=f"summary:{summary_head}", + state="COMMENTED", + rules_version=rules_version, + harness="github-summary", + ) + if run["id"] not in by_run_id: + runs.append(run) + by_run_id[run["id"]] = run + review_records.append( + ("greptile-apps", dt.datetime.max.replace(tzinfo=dt.timezone.utc), run) + ) + + review_records.sort(key=lambda row: row[1]) + capture_complete = pr.get("captureComplete", True) is not False + findings: list[dict[str, Any]] = [] + + for item in gate.review_ledger(pr, bots, reply_actors): + candidates = [ + (submitted_at, run) + for bot, submitted_at, run in review_records + if bot == item.bot and submitted_at <= parse_time(item.latest_bot_comment_at) + ] + if not candidates: + # Do not fabricate a current-head review session merely because an + # inline thread exists. + capture_complete = False + continue + + _, source_run = max(candidates, key=lambda row: row[0]) + disposition = _finding_disposition(item) + finding = { + "id": f"github-thread:{item.thread_id}", + "run_id": source_run["id"], + "head_sha": head, + "severity": "unknown", + "disposition": disposition, + "actionable": bool(item.active), + "published": True, + "verified": False, + "latest_reviewer_at": item.latest_bot_comment_at, + "latest_reply_at": item.latest_reply_at, + "path": item.path, + "line": item.line, + "reply_actor": item.reply_actor, + "source_disposition": item.disposition, + } + findings.append(finding) + + if item.active: + source_run["disposition"] = "repair" + + return { + "schema": review_fabric.RECEIPT_SCHEMA, + "head_sha": head, + "capture_complete": capture_complete, + "source": "github-review-ledger", + "pr_number": pr.get("number"), + "runs": runs, + "findings": findings, + } + + +def main() -> int: + gate = load_gate() + try: + pr = gate.fetch_pr() + bots = tuple( + bot.strip().lower() + for bot in os.environ.get( + "REVIEW_BOTS", + ",".join(gate.DEFAULT_REVIEW_BOTS), + ).split(",") + if bot.strip() + ) + actors = gate.configured_reply_actors(pr) + receipt = receipt_from_pr( + pr, + bots=bots, + reply_actors=actors, + gate=gate, + rules_version=os.environ.get("REVIEW_RULES_VERSION", DEFAULT_RULES_VERSION), + ) + print(json.dumps(receipt, indent=2, sort_keys=True)) + return 0 + except Exception as error: + print(f"github-review-receipt: ERROR: {error}", file=sys.stderr) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_github_review_receipt.py b/tests/test_github_review_receipt.py new file mode 100644 index 000000000000..a5ecf8a33823 --- /dev/null +++ b/tests/test_github_review_receipt.py @@ -0,0 +1,212 @@ +import importlib.util +import json +import sys +import unittest +from pathlib import Path + +ROOT = Path(__file__).parents[1] +SCRIPT = ROOT / ".github/scripts/github_review_receipt.py" +POLICY = ROOT / ".github/review-fabric-policy.json" + +spec = importlib.util.spec_from_file_location("github_review_receipt", SCRIPT) +github_review_receipt = importlib.util.module_from_spec(spec) +sys.modules[spec.name] = github_review_receipt +spec.loader.exec_module(github_review_receipt) + +gate = github_review_receipt.load_gate() +review_fabric = github_review_receipt.review_fabric + +HEAD = "a" * 40 +OLD_HEAD = "b" * 40 + + +def review(bot="coderabbitai[bot]", *, oid=HEAD, state="COMMENTED", at="2026-09-21T12:00:00Z"): + return { + "author": {"login": bot}, + "state": state, + "submittedAt": at, + "commit": {"oid": oid}, + } + + +def thread( + bot="coderabbitai[bot]", + *, + thread_id="thread-1", + body="Please fix this", + bot_at="2026-09-21T12:01:00Z", + reply=False, + reply_at="2026-09-21T12:02:00Z", + resolved=False, + outdated=False, +): + comments = [{"author": {"login": bot}, "body": body, "createdAt": bot_at}] + if reply: + comments.append( + { + "author": {"login": "agent-author"}, + "body": "Addressed", + "createdAt": reply_at, + } + ) + return { + "id": thread_id, + "isResolved": resolved, + "isOutdated": outdated, + "path": "Sources/Foo.swift", + "line": 10, + "comments": {"nodes": comments}, + } + + +def greptile_summary(oid=HEAD): + return { + "author": {"login": "greptile-apps[bot]"}, + "body": ( + "\n" + "Reviews (2) ยท Last reviewed commit: " + f"[reviewed](https://github.com/manaflow-ai/cmux/commit/{oid})" + ), + "createdAt": "2026-09-21T12:03:00Z", + "updatedAt": "2026-09-21T12:03:00Z", + } + + +def make_pr(*, reviews=None, threads=None, comments=None, capture_complete=True, head=HEAD): + return { + "number": 42, + "body": "", + "headRefOid": head, + "author": {"login": "agent-author"}, + "reviews": {"nodes": reviews or []}, + "reviewThreads": {"nodes": threads or []}, + "comments": {"nodes": comments or []}, + "captureComplete": capture_complete, + } + + +def receipt(pr): + return github_review_receipt.receipt_from_pr( + pr, + bots=gate.DEFAULT_REVIEW_BOTS, + reply_actors=("agent-author",), + gate=gate, + ) + + +class GitHubReviewReceiptTests(unittest.TestCase): + def test_current_head_review_becomes_external_review_run(self): + result = receipt(make_pr(reviews=[review()])) + self.assertTrue(result["capture_complete"]) + self.assertEqual(len(result["runs"]), 1) + run = result["runs"][0] + self.assertEqual(run["head_sha"], HEAD) + self.assertEqual(run["provider"], "coderabbitai") + self.assertEqual(run["harness"], "github-review") + self.assertEqual(run["capability_class"], "external") + self.assertEqual(run["disposition"], "accept") + + def test_external_review_alone_cannot_satisfy_default_fabric_policy(self): + result = receipt(make_pr(reviews=[review()])) + policy = json.loads(POLICY.read_text(encoding="utf-8")) + report = review_fabric.evaluate(result, policy) + self.assertFalse(report["passed"]) + self.assertTrue(any("independent review quorum" in reason for reason in report["reasons"])) + self.assertTrue(any("capability quorum frontier" in reason for reason in report["reasons"])) + + def test_old_review_can_source_current_actionable_thread_without_counting_as_current_review(self): + result = receipt( + make_pr( + reviews=[review(oid=OLD_HEAD)], + threads=[thread()], + ) + ) + self.assertTrue(result["capture_complete"]) + self.assertEqual(result["runs"][0]["head_sha"], OLD_HEAD) + self.assertEqual(result["runs"][0]["disposition"], "repair") + self.assertEqual(result["findings"][0]["head_sha"], HEAD) + self.assertEqual(result["findings"][0]["disposition"], "pending") + + policy = json.loads(POLICY.read_text(encoding="utf-8")) + report = review_fabric.evaluate(result, policy) + self.assertIn(result["runs"][0]["id"], report["stale_run_ids"]) + self.assertTrue(any("actionable finding" in reason for reason in report["reasons"])) + + def test_active_thread_without_structured_review_fails_capture_closed(self): + result = receipt(make_pr(threads=[thread()])) + self.assertFalse(result["capture_complete"]) + self.assertEqual(result["findings"], []) + + def test_replied_thread_remains_unverified(self): + result = receipt( + make_pr( + reviews=[review()], + threads=[thread(reply=True)], + ) + ) + finding = result["findings"][0] + self.assertEqual(finding["disposition"], "answered_unverified") + self.assertTrue(finding["actionable"]) + self.assertFalse(finding["verified"]) + + def test_resolved_thread_states_remain_distinct(self): + replied = receipt( + make_pr( + reviews=[review()], + threads=[thread(reply=True, resolved=True)], + ) + ) + self.assertEqual(replied["findings"][0]["disposition"], "resolved_unverified") + + unanswered = receipt( + make_pr( + reviews=[review()], + threads=[thread(resolved=True)], + ) + ) + self.assertEqual(unanswered["findings"][0]["disposition"], "resolved_unanswered") + + def test_outdated_thread_is_retained_but_inactive(self): + result = receipt( + make_pr( + reviews=[review(oid=OLD_HEAD)], + threads=[thread(outdated=True)], + ) + ) + finding = result["findings"][0] + self.assertEqual(finding["disposition"], "outdated") + self.assertFalse(finding["actionable"]) + + def test_changes_requested_marks_source_run_for_repair(self): + result = receipt(make_pr(reviews=[review(state="CHANGES_REQUESTED")])) + self.assertEqual(result["runs"][0]["disposition"], "repair") + + def test_greptile_summary_provides_exact_head_review_evidence(self): + result = receipt(make_pr(comments=[greptile_summary()])) + self.assertEqual(len(result["runs"]), 1) + run = result["runs"][0] + self.assertEqual(run["provider"], "greptile-apps") + self.assertEqual(run["head_sha"], HEAD) + self.assertEqual(run["harness"], "github-summary") + + def test_unconfigured_reviewers_are_ignored(self): + result = receipt(make_pr(reviews=[review(bot="some-other-bot[bot]")])) + self.assertEqual(result["runs"], []) + self.assertTrue(result["capture_complete"]) + + def test_dismissed_reviews_are_not_receipts(self): + result = receipt(make_pr(reviews=[review(state="DISMISSED")])) + self.assertEqual(result["runs"], []) + + def test_provider_thread_disposition_does_not_invent_severity(self): + result = receipt( + make_pr( + reviews=[review()], + threads=[thread(body="P1 maybe, please investigate")], + ) + ) + self.assertEqual(result["findings"][0]["severity"], "unknown") + + +if __name__ == "__main__": + unittest.main() From 034f44c5a4f6e1a83dcb63e03d2d40bb5d9a03b7 Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 21 Sep 2026 17:19:22 -0700 Subject: [PATCH 3/3] ci: observe review fabric on pull request events --- .github/workflows/review-fabric-observe.yml | 73 +++++++++++++++++++++ tests/test_review_fabric_observe.py | 34 ++++++++++ 2 files changed, 107 insertions(+) create mode 100644 .github/workflows/review-fabric-observe.yml create mode 100644 tests/test_review_fabric_observe.py diff --git a/.github/workflows/review-fabric-observe.yml b/.github/workflows/review-fabric-observe.yml new file mode 100644 index 000000000000..e43f68af1f48 --- /dev/null +++ b/.github/workflows/review-fabric-observe.yml @@ -0,0 +1,73 @@ +name: Review fabric observe + +on: + pull_request_target: + types: [opened, synchronize, reopened, ready_for_review, edited] + pull_request_review: + types: [submitted, dismissed] + pull_request_review_comment: + types: [created, edited, deleted] + issue_comment: + types: [created, edited, deleted] + +permissions: + contents: read + issues: read + pull-requests: read + +concurrency: + group: review-fabric-observe-${{ github.event.pull_request.number || github.event.issue.number }} + cancel-in-progress: true + +jobs: + observe: + name: review-fabric-observe + if: ${{ github.event_name != 'issue_comment' || github.event.issue.pull_request != null }} + runs-on: ${{ vars.LINUX_RUNNER || 'blacksmith-4vcpu-ubuntu-2404' }} + steps: + # This listener handles untrusted PR events with trusted repository code. + # It never checks out or executes the pull-request head. + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + ref: ${{ github.event.pull_request.base.sha || github.event.repository.default_branch }} + persist-credentials: false + + - name: Capture provider-neutral GitHub review receipt + env: + GH_TOKEN: ${{ github.token }} + GITHUB_EVENT_PATH: ${{ github.event_path }} + GITHUB_REPOSITORY: ${{ github.repository }} + REVIEW_BOTS: ${{ vars.AGENT_REVIEW_BOTS || 'coderabbitai,greptile-apps' }} + run: | + set -euo pipefail + mkdir -p artifacts/review-fabric + python3 .github/scripts/github_review_receipt.py > artifacts/review-fabric/github-review-receipt.json + + - name: Evaluate review fabric policy in shadow mode + id: evaluate + run: | + set -uo pipefail + set +e + python3 .github/scripts/review_fabric.py --input artifacts/review-fabric/github-review-receipt.json --json > artifacts/review-fabric/evaluation.json + policy_exit=$? + set -e + echo "policy_exit=$policy_exit" >> "$GITHUB_OUTPUT" + { + echo "## Review fabric shadow evaluation" + echo + echo "Policy exit: `$policy_exit`" + echo + echo "```json" + cat artifacts/review-fabric/evaluation.json + echo "```" + } >> "$GITHUB_STEP_SUMMARY" + # Shadow mode records the verdict without making it authoritative. + exit 0 + + - name: Preserve review fabric receipt + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: review-fabric-${{ github.event.pull_request.number || github.event.issue.number }}-${{ github.run_id }} + path: artifacts/review-fabric/ + if-no-files-found: error diff --git a/tests/test_review_fabric_observe.py b/tests/test_review_fabric_observe.py new file mode 100644 index 000000000000..bedf0cdb233e --- /dev/null +++ b/tests/test_review_fabric_observe.py @@ -0,0 +1,34 @@ +from pathlib import Path + +ROOT = Path(__file__).parents[1] +WORKFLOW = ROOT / ".github/workflows/review-fabric-observe.yml" + + +def test_review_fabric_observer_is_trusted_read_only_shadow_mode() -> None: + workflow = WORKFLOW.read_text(encoding="utf-8") + + assert " pull_request_target:" in workflow + assert " pull_request_review:" in workflow + assert " pull_request_review_comment:" in workflow + assert " issue_comment:" in workflow + + assert "permissions:\n contents: read\n issues: read\n pull-requests: read" in workflow + assert "actions: write" not in workflow + assert "contents: write" not in workflow + assert "pull-requests: write" not in workflow + assert "issues: write" not in workflow + + assert "github.event.pull_request.base.sha || github.event.repository.default_branch" in workflow + assert "persist-credentials: false" in workflow + assert "github.event.pull_request.head.sha" not in workflow + + assert "python3 .github/scripts/github_review_receipt.py" in workflow + assert "python3 .github/scripts/review_fabric.py" in workflow + assert "--input artifacts/review-fabric/github-review-receipt.json" in workflow + + assert "policy_exit=$?" in workflow + assert "Shadow mode records the verdict without making it authoritative." in workflow + assert "exit 0" in workflow + + assert "actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a" in workflow + assert "review-fabric-${{ github.event.pull_request.number || github.event.issue.number }}" in workflow