diff --git a/devtools/command_catalog.py b/devtools/command_catalog.py index d483254b11..5c18691f95 100644 --- a/devtools/command_catalog.py +++ b/devtools/command_catalog.py @@ -679,6 +679,36 @@ def to_dict(self) -> dict[str, object]: "devtools workspace verify-worktree /realm/worktrees/lane-x --json --strict", ), ), + CommandSpec( + "workspace merge-gate", + "workspace", + "Structural pre-merge safety check: fresh local-verification receipt + no late review comments.", + "devtools.merge_gate", + use_when=( + "Immediately before squash-merging any PR in a merge train, replacing coordinator memory " + "(grace-period comment polling, remembering to run the broader local test suite CI skips " + 'per-PR) with a check that fails closed. `record --command "..."` requires the current ' + "checkout to already be the PR's exact head commit with a clean tree (it refuses otherwise), " + "runs a local verification " + "command, and persists a receipt flagging commands that look like they skip tests (e.g. " + "`verify --quick`). `check ` polls review comments across a real grace window (default " + "3x20s, covering CodeRabbit's 30-60s late-arrival window) and BLOCKs unless a receipt exists " + "for the CURRENT head sha within a freshness window with exit_code 0, and no review comment's " + "created_at is newer than the head commit's timestamp unless explicitly `ack`'d for that exact " + "head sha. Motivated by two 2026-08-01 incidents: PR #3502 merged before CodeRabbit's findings " + "posted, and PR #3517 nearly merged with a 43-test regression no CI check or review comment " + "ever flagged -- plus review findings on this tool itself (recording from an unrelated " + "checkout, a --quick example that would have missed its own motivating regression, a single " + "comment snapshot instead of a grace-period poll, and no way to triage a false-positive late " + "comment without an empty commit)." + ), + examples=( + 'devtools workspace merge-gate record 3517 --command "devtools verify"', + "devtools workspace merge-gate check 3517", + "devtools workspace merge-gate check 3517 --json --max-age-s 7200 --poll-rounds 1", + 'devtools workspace merge-gate ack 3517 123456789 --reason "already fixed upstream, false positive"', + ), + ), CommandSpec( "workspace merge-conductor", "workspace", diff --git a/devtools/merge_gate.py b/devtools/merge_gate.py new file mode 100644 index 0000000000..7ab0e0aa41 --- /dev/null +++ b/devtools/merge_gate.py @@ -0,0 +1,462 @@ +"""merge-gate: make "is this PR actually safe to merge" a structural check, not a memory habit. + +Two real incidents from a single ~28-PR merge-train session (2026-08-01) motivate +this: + + 1. PR #3502 was squash-merged with 0 review comments showing at check time; + CodeRabbit posted 3 real findings 30-60s later. The fix (an explicit + "poll comments a few times before merging" habit) worked for the rest of + that session, but it lived entirely in the coordinator's own discipline. + 2. PR #3517 nearly merged carrying a 43-test regression that no CI check and + no review comment ever flagged -- per-PR CI deliberately skips the heavy + test suite (see CLAUDE.md), so nothing but a coordinator choosing, from + memory, to run the broader local suite before merging would have caught + it. It was caught, that time. + +A coordinator merging dozens of PRs across a few hours cannot reliably repeat +either habit purely from memory every single time. This command turns both +into something that fails closed instead of silently not happening: + + - ``record``: run a local verification command against a PR branch's + current HEAD commit, and persist a receipt keyed to that exact sha under + ``.cache/verify/merge-gate/pr-.json``. Refuses to record unless the + CURRENT git checkout is clean and its ``HEAD`` matches the PR's fetched + ``headRefOid`` -- otherwise the receipt would attest to code that was + never actually tested (review-caught gap: recording from an unrelated + checkout, e.g. master or a stale worktree, previously produced a receipt + that ``check`` would accept). + - ``check``: polls PR review comments across a real grace window (default + 3 rounds x 20s, covering the 30-60s late-arrival window from incident 1) + before deciding. BLOCKs unless a receipt exists for the PR's *current* + head sha (not a stale one from an earlier push), was recorded within a + freshness window, had exit code 0, and its command actually looks like it + ran tests (a bare ``--quick`` profile is flagged, not silently accepted -- + review-caught gap: the documented example used exactly the profile that + would have missed the PR #3517 regression). No review comment's + ``created_at`` may be newer than the head commit's ``committedDate`` + unless it has been explicitly acknowledged via ``ack`` for this exact + head sha (review-caught gap: without ``ack``, a stale-forever comparison + made even a reviewed false positive permanently unmergeable without an + empty commit). + +This does not replace judgment about *what* a late comment means -- ``ack`` +still requires a human/agent to have actually read it and decided it's not +actionable. It makes the presence of an unverified late signal impossible to +merge past silently, and impossible to permanently paper over without an +explicit, current-head-scoped decision. + +Usage: + devtools workspace merge-gate record 3517 --command "devtools verify" + devtools workspace merge-gate check 3517 + devtools workspace merge-gate check 3517 --json --max-age-s 7200 --poll-rounds 1 + devtools workspace merge-gate ack 3517 --reason "false positive, already fixed upstream" +""" + +from __future__ import annotations + +import argparse +import json +import shlex +import subprocess +import sys +import time +from dataclasses import asdict, dataclass, field +from pathlib import Path +from typing import Any + +_RECEIPT_DIR = Path(".cache/verify/merge-gate") +_DEFAULT_MAX_AGE_S = 3600 +_DEFAULT_POLL_ROUNDS = 3 +_DEFAULT_POLL_INTERVAL_S = 20 +# Heuristic: profiles that explicitly skip tests (see CLAUDE.md -- `devtools +# verify --quick` is format+lint+mypy+render, no pytest). Not exhaustive; a +# command containing neither this nor an obvious test-runner name still gets +# flagged as an advisory, since the whole point is not trusting a plausible- +# looking command string without comment. +_TEST_SKIPPING_MARKERS: tuple[str, ...] = ("verify --quick", "verify --lab") +_LOOKS_LIKE_TESTS_MARKERS: tuple[str, ...] = ("test", "pytest", "verify --all", "devtools verify") + + +def _gh_json(args: list[str]) -> Any: + result = subprocess.run(["gh", *args], capture_output=True, text=True, timeout=60) + if result.returncode != 0: + raise RuntimeError(result.stderr.strip()[:300] or f"gh {' '.join(args)} failed") + return json.loads(result.stdout) + + +def _gh_json_paginated(args: list[str]) -> list[Any]: + """Like ``_gh_json`` but follows pagination -- the GitHub REST list + endpoints cap at 30-100 items per page, and a PR with more review comments + than that would otherwise silently hide later (possibly late-arriving) + ones from the late-comment check. ``--slurp`` wraps every page's own JSON + array into one outer array, which this then flattens.""" + result = subprocess.run(["gh", *args, "--paginate", "--slurp"], capture_output=True, text=True, timeout=120) + if result.returncode != 0: + raise RuntimeError(result.stderr.strip()[:300] or f"gh {' '.join(args)} --paginate failed") + pages = json.loads(result.stdout) + items: list[Any] = [] + for page in pages: + if isinstance(page, list): + items.extend(page) + else: + items.append(page) + return items + + +def _read_json_object(path: Path) -> dict[str, Any] | None: + """Return the parsed object, or None when the file is absent, unreadable, + truncated, or not a JSON object (a hand edit or an interrupted write must + not raise a traceback out of a merge-safety check).""" + if not path.exists(): + return None + try: + loaded = json.loads(path.read_text()) + except (OSError, json.JSONDecodeError): + return None + return loaded if isinstance(loaded, dict) else None + + +def _git_head_sha() -> str | None: + result = subprocess.run(["git", "rev-parse", "HEAD"], capture_output=True, text=True, timeout=15) + if result.returncode != 0: + return None + return result.stdout.strip() + + +def _git_is_clean() -> bool: + result = subprocess.run(["git", "status", "--porcelain"], capture_output=True, text=True, timeout=15) + return result.returncode == 0 and not result.stdout.strip() + + +@dataclass +class GateVerdict: + pr: int + ok: bool + reasons: list[str] = field(default_factory=list) + head_sha: str = "" + receipt: dict[str, Any] | None = None + late_comments: list[dict[str, Any]] = field(default_factory=list) + + +def _receipt_path(pr: int) -> Path: + return _RECEIPT_DIR / f"pr-{pr}.json" + + +def _ack_path(pr: int) -> Path: + return _RECEIPT_DIR / f"pr-{pr}-acks.json" + + +def _command_skips_tests(command: str) -> bool: + lowered = command.lower() + if any(marker in lowered for marker in _TEST_SKIPPING_MARKERS): + return True + return not any(marker in lowered for marker in _LOOKS_LIKE_TESTS_MARKERS) + + +def cmd_record(pr: int, command: str) -> int: + info = _gh_json(["pr", "view", str(pr), "--json", "headRefOid,headRefName"]) + head_sha = info["headRefOid"] + + local_head = _git_head_sha() + if local_head != head_sha: + print( + f"REFUSING to record: current checkout HEAD ({local_head[:8] if local_head else '?'}) does not " + f"match PR #{pr}'s head ({head_sha[:8]}). Check out the PR's exact commit (in an isolated worktree " + "if other work is in progress elsewhere) before recording -- a receipt attesting to the wrong " + "checkout is worse than no receipt.", + file=sys.stderr, + ) + return 2 + if not _git_is_clean(): + print( + "REFUSING to record: current checkout has uncommitted changes. The receipt must attest to " + "exactly the PR's committed content, not a locally-modified tree.", + file=sys.stderr, + ) + return 2 + + argv = shlex.split(command) + if not argv: + print("REFUSING to record: --command is empty after shell splitting.", file=sys.stderr) + return 2 + started = time.time() + try: + result = subprocess.run(argv, capture_output=True, text=True) + except OSError as exc: + print(f"REFUSING to record: could not run {command!r}: {exc}", file=sys.stderr) + return 2 + duration_s = round(time.time() - started, 2) + + receipt = { + "pr": pr, + "head_sha": head_sha, + "branch": info["headRefName"], + "command": command, + "skips_tests": _command_skips_tests(command), + "exit_code": result.returncode, + "duration_s": duration_s, + "recorded_at": time.time(), + "stdout_tail": result.stdout[-4000:], + "stderr_tail": result.stderr[-4000:], + } + _RECEIPT_DIR.mkdir(parents=True, exist_ok=True) + _receipt_path(pr).write_text(json.dumps(receipt, indent=2)) + + print(f"recorded receipt for PR #{pr} @ {head_sha[:8]}: exit={result.returncode} ({duration_s}s)") + if receipt["skips_tests"]: + print( + f" advisory: command {command!r} does not look like it ran tests -- `check` will flag this", + file=sys.stderr, + ) + if result.returncode != 0: + print(result.stdout[-2000:]) + print(result.stderr[-2000:], file=sys.stderr) + return result.returncode + + +def cmd_ack(pr: int, comment_id: int, *, reason: str) -> int: + info = _gh_json(["pr", "view", str(pr), "--json", "headRefOid"]) + head_sha = info["headRefOid"] + + ack_path = _ack_path(pr) + if ack_path.exists(): + acks = _read_json_object(ack_path) + if acks is None: + print( + f"REFUSING to ack: {ack_path} exists but is unreadable/corrupt -- fix or remove it by hand " + "first, rather than silently losing prior acknowledgements.", + file=sys.stderr, + ) + return 2 + else: + acks = {} + acks[str(comment_id)] = {"head_sha": head_sha, "reason": reason, "acked_at": time.time()} + _RECEIPT_DIR.mkdir(parents=True, exist_ok=True) + ack_path.write_text(json.dumps(acks, indent=2)) + print(f"acknowledged comment {comment_id} on PR #{pr} @ {head_sha[:8]}: {reason}") + return 0 + + +def _fetch_review_comments(pr: int) -> list[dict[str, Any]] | None: + """Combine every top-level review signal the late-comment check should + see: inline diff comments, issue-level PR comments, and review bodies + (a review's own summary text is a separate object from its line + comments -- see GitHub's REST API docs). All three are normalized to a + common shape (id, created_at, path, line, body) and empty-bodied entries + (e.g. an APPROVE review with no summary text) are dropped -- they carry + no signal for triage.""" + normalized: list[dict[str, Any]] = [] + try: + inline = _gh_json_paginated(["api", f"repos/{{owner}}/{{repo}}/pulls/{pr}/comments"]) + for item in inline: + normalized.append( + { + "id": item.get("id"), + "created_at": item.get("created_at", ""), + "path": item.get("path"), + "line": item.get("line"), + "body": item.get("body") or "", + } + ) + issue_comments = _gh_json_paginated(["api", f"repos/{{owner}}/{{repo}}/issues/{pr}/comments"]) + for item in issue_comments: + normalized.append( + { + "id": item.get("id"), + "created_at": item.get("created_at", ""), + "path": None, + "line": None, + "body": item.get("body") or "", + } + ) + reviews = _gh_json_paginated(["api", f"repos/{{owner}}/{{repo}}/pulls/{pr}/reviews"]) + for item in reviews: + normalized.append( + { + "id": item.get("id"), + "created_at": item.get("submitted_at", ""), + "path": None, + "line": None, + "body": item.get("body") or "", + } + ) + except (RuntimeError, json.JSONDecodeError, OSError, subprocess.SubprocessError): + return None + return [comment for comment in normalized if comment["body"].strip()] + + +def _poll_stable_comments(pr: int, *, rounds: int, interval_s: int) -> list[dict[str, Any]] | None: + """Poll review comments repeatedly so a comment posted 30-60s after CI + goes green (the PR #3502 incident) is observed rather than missed by a + single snapshot taken too early.""" + last: list[dict[str, Any]] | None = None + for round_index in range(max(1, rounds)): + comments = _fetch_review_comments(pr) + if comments is None: + return None + last = comments + if round_index < rounds - 1: + time.sleep(interval_s) + return last + + +def cmd_check(pr: int, *, max_age_s: int, poll_rounds: int, poll_interval_s: int, as_json: bool) -> int: + verdict = GateVerdict(pr=pr, ok=True) + + try: + info = _gh_json( + [ + "pr", + "view", + str(pr), + "--json", + "headRefOid,mergeStateStatus,state,commits", + ] + ) + except (RuntimeError, json.JSONDecodeError, OSError, subprocess.SubprocessError) as exc: + verdict.ok = False + verdict.reasons.append(f"gh pr view failed: {exc}") + _emit(verdict, as_json) + return 1 + + if info.get("state") != "OPEN": + verdict.ok = False + verdict.reasons.append(f"PR state is {info.get('state')!r}, not OPEN") + _emit(verdict, as_json) + return 1 + + head_sha = info["headRefOid"] + verdict.head_sha = head_sha + + mss = info.get("mergeStateStatus", "") + if mss not in {"CLEAN", "UNSTABLE", "UNKNOWN"}: + verdict.ok = False + verdict.reasons.append(f"mergeStateStatus is {mss!r} (expected CLEAN/UNSTABLE/UNKNOWN)") + + commits = info.get("commits") or [] + head_commit = next((commit for commit in commits if commit.get("oid") == head_sha), None) + head_committed_at = head_commit.get("committedDate") if head_commit else None + + receipt_path = _receipt_path(pr) + receipt = _read_json_object(receipt_path) + if receipt is None: + verdict.ok = False + verdict.reasons.append( + f"no local verification receipt found (or it is unreadable) at {receipt_path} -- run " + f'`devtools workspace merge-gate record {pr} --command "..."` against the current head first' + ) + else: + verdict.receipt = receipt + if receipt.get("head_sha") != head_sha: + verdict.ok = False + verdict.reasons.append( + f"receipt is for sha {receipt.get('head_sha', '')[:8]} but PR head is now {head_sha[:8]} " + "-- a new commit landed since the receipt was recorded; re-record before merging" + ) + else: + age_s = time.time() - receipt.get("recorded_at", 0) + if age_s > max_age_s: + verdict.ok = False + verdict.reasons.append(f"receipt is {int(age_s)}s old (max {max_age_s}s) -- re-record") + if receipt.get("exit_code", 1) != 0: + verdict.ok = False + verdict.reasons.append(f"receipt exit_code is {receipt.get('exit_code')}, not 0") + if receipt.get("skips_tests"): + verdict.reasons.append( + f"advisory: receipt command {receipt.get('command')!r} does not look like it ran tests " + "-- confirm this PR genuinely needs no test coverage before merging" + ) + + review_comments = _poll_stable_comments(pr, rounds=poll_rounds, interval_s=poll_interval_s) + if review_comments is None: + verdict.ok = False + verdict.reasons.append("could not fetch review comments after polling") + review_comments = [] + + acks = _read_json_object(_ack_path(pr)) or {} + + if head_committed_at: + for comment in review_comments: + created_at = comment.get("created_at", "") + if created_at <= head_committed_at: + continue + comment_id = comment.get("id") + ack = acks.get(str(comment_id)) + if ack is not None and ack.get("head_sha") == head_sha: + continue # explicitly triaged for this exact head sha + verdict.late_comments.append( + { + "id": comment_id, + "path": comment.get("path"), + "line": comment.get("line"), + "created_at": created_at, + "body_head": (comment.get("body") or "")[:200], + } + ) + if verdict.late_comments: + verdict.ok = False + verdict.reasons.append( + f"{len(verdict.late_comments)} unacknowledged review comment(s) posted after the head commit " + f"({head_committed_at}) -- read and `ack` (if not actionable) or fix before merging" + ) + else: + verdict.ok = False + verdict.reasons.append( + "could not determine the head commit timestamp, so the late-comment check cannot run -- " + "refusing to report OK" + ) + + _emit(verdict, as_json) + return 0 if verdict.ok else 1 + + +def _emit(verdict: GateVerdict, as_json: bool) -> None: + if as_json: + print(json.dumps(asdict(verdict), indent=2)) + return + print(f"PR #{verdict.pr} @ {verdict.head_sha[:8] if verdict.head_sha else '?'}: {'OK' if verdict.ok else 'BLOCK'}") + for reason in verdict.reasons: + print(f" - {reason}") + for late in verdict.late_comments: + print( + f" late comment id={late['id']} [{late['path']}:{late['line']}] {late['created_at']}: {late['body_head']}" + ) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + sub = parser.add_subparsers(dest="action", required=True) + + record_p = sub.add_parser("record", help="Run local verification against a PR's current head and persist a receipt") + record_p.add_argument("pr", type=int) + record_p.add_argument("--command", required=True, help="Local verification command to run, e.g. 'devtools verify'") + + check_p = sub.add_parser("check", help="Decide whether a PR is safe to merge right now") + check_p.add_argument("pr", type=int) + check_p.add_argument("--max-age-s", type=int, default=_DEFAULT_MAX_AGE_S) + check_p.add_argument("--poll-rounds", type=int, default=_DEFAULT_POLL_ROUNDS) + check_p.add_argument("--poll-interval-s", type=int, default=_DEFAULT_POLL_INTERVAL_S) + check_p.add_argument("--json", action="store_true", dest="as_json") + + ack_p = sub.add_parser("ack", help="Acknowledge a specific review comment as triaged for the current head sha") + ack_p.add_argument("pr", type=int) + ack_p.add_argument("comment_id", type=int) + ack_p.add_argument("--reason", required=True, help="Why this comment does not block merging") + + args = parser.parse_args(argv) + + if args.action == "record": + return cmd_record(args.pr, args.command) + if args.action == "ack": + return cmd_ack(args.pr, args.comment_id, reason=args.reason) + return cmd_check( + args.pr, + max_age_s=args.max_age_s, + poll_rounds=args.poll_rounds, + poll_interval_s=args.poll_interval_s, + as_json=args.as_json, + ) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/docs/devtools.md b/docs/devtools.md index 145b763a6e..ffcfc579bb 100644 --- a/docs/devtools.md +++ b/docs/devtools.md @@ -233,6 +233,7 @@ These are the commands worth remembering during normal repo work: | `devtools workspace lineage-validation` | Validate lineage-count evidence before citing archive counts externally. | | `devtools workspace mandate-continuity-replay` | Wire t8t continuity scenarios + work-evidence effects + discovery into one mandate artifact. | | `devtools workspace merge-conductor` | Mechanical-conflict triage for the PR merge train (dry-run by default). | +| `devtools workspace merge-gate` | Structural pre-merge safety check: fresh local-verification receipt + no late review comments. | | `devtools workspace raw-authority-daemon-health-proof` | Prove daemon status/health HTTP responsiveness during a real raw-authority drain. | | `devtools workspace raw-authority-restart-proof` | Prove raw-authority crash recovery and conserved fixed-point convergence. | | `devtools workspace raw-authority-scale-proof` | Run bounded raw-authority replay to a two-census fixed point. | diff --git a/tests/unit/devtools/test_merge_gate.py b/tests/unit/devtools/test_merge_gate.py new file mode 100644 index 0000000000..ee6fc77e9e --- /dev/null +++ b/tests/unit/devtools/test_merge_gate.py @@ -0,0 +1,393 @@ +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from devtools import merge_gate +from tests.infra.frozen_clock import FrozenClock + + +def _fake_run( + pr_view: dict[str, object], + comments: list[dict[str, object]], + local_exit: int = 0, + local_head_sha: str = "abc123", + dirty: bool = False, + poll_rounds: list[list[dict[str, object]]] | None = None, +) -> object: + """``comments`` (inline review comments) is returned on every poll round + unless ``poll_rounds`` gives an explicit per-round sequence (for testing + the multi-round poll itself). Issue comments and review bodies are always + empty here -- covered separately in the normalization tests below.""" + comment_rounds: list[list[dict[str, object]]] = poll_rounds if poll_rounds is not None else [comments] + call_count = {"round": 0} + + def _run(cmd: list[str], **kwargs: object) -> MagicMock: + joined = " ".join(cmd) + if cmd[:3] == ["gh", "pr", "view"]: + return MagicMock(returncode=0, stdout=json.dumps(pr_view), stderr="") + if "/issues/" in joined and "/comments" in joined: + return MagicMock(returncode=0, stdout=json.dumps([[]]), stderr="") + if "/pulls/" in joined and "/reviews" in joined: + return MagicMock(returncode=0, stdout=json.dumps([[]]), stderr="") + if "/pulls/" in joined and "/comments" in joined: + round_index = min(call_count["round"], len(comment_rounds) - 1) + call_count["round"] += 1 + return MagicMock(returncode=0, stdout=json.dumps([comment_rounds[round_index]]), stderr="") + if cmd[:2] == ["git", "rev-parse"]: + return MagicMock(returncode=0, stdout=local_head_sha + "\n", stderr="") + if cmd[:2] == ["git", "status"]: + return MagicMock(returncode=0, stdout=" M dirty.py\n" if dirty else "", stderr="") + return MagicMock(returncode=local_exit, stdout="ok\n", stderr="") + + return _run + + +def test_record_persists_receipt_keyed_to_current_head_sha(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + monkeypatch.setattr( + subprocess, + "run", + _fake_run({"headRefOid": "abc123", "headRefName": "feature/x"}, [], local_head_sha="abc123"), + ) + + exit_code = merge_gate.cmd_record(42, "devtools test tests/unit/foo.py") + + assert exit_code == 0 + receipt = json.loads(merge_gate._receipt_path(42).read_text()) + assert receipt["head_sha"] == "abc123" + assert receipt["exit_code"] == 0 + assert receipt["skips_tests"] is False + + +def test_record_captures_nonzero_local_command_exit(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + monkeypatch.setattr( + subprocess, + "run", + _fake_run({"headRefOid": "abc123", "headRefName": "feature/x"}, [], local_exit=1, local_head_sha="abc123"), + ) + + exit_code = merge_gate.cmd_record(42, "devtools test somefile") + + assert exit_code == 1 + receipt = json.loads(merge_gate._receipt_path(42).read_text()) + assert receipt["exit_code"] == 1 + + +def test_record_refuses_when_local_checkout_does_not_match_pr_head( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.chdir(tmp_path) + monkeypatch.setattr( + subprocess, + "run", + _fake_run({"headRefOid": "abc123", "headRefName": "feature/x"}, [], local_head_sha="deadbeef"), + ) + + exit_code = merge_gate.cmd_record(42, "devtools test somefile") + + assert exit_code == 2 + assert not merge_gate._receipt_path(42).exists() + + +def test_record_refuses_when_checkout_is_dirty(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + monkeypatch.setattr( + subprocess, + "run", + _fake_run({"headRefOid": "abc123", "headRefName": "feature/x"}, [], local_head_sha="abc123", dirty=True), + ) + + exit_code = merge_gate.cmd_record(42, "devtools test somefile") + + assert exit_code == 2 + assert not merge_gate._receipt_path(42).exists() + + +def test_record_flags_a_test_skipping_command(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + monkeypatch.setattr( + subprocess, + "run", + _fake_run({"headRefOid": "abc123", "headRefName": "feature/x"}, [], local_head_sha="abc123"), + ) + + merge_gate.cmd_record(42, "devtools verify --quick") + + receipt = json.loads(merge_gate._receipt_path(42).read_text()) + assert receipt["skips_tests"] is True + + +def _base_pr_view(head_sha: str = "abc123", committed_date: str = "2026-08-01T12:00:00Z") -> dict[str, object]: + return { + "headRefOid": head_sha, + "headRefName": "feature/x", + "state": "OPEN", + "mergeStateStatus": "CLEAN", + "commits": [{"oid": head_sha, "committedDate": committed_date}], + } + + +def _record(monkeypatch: pytest.MonkeyPatch, pr_view: dict[str, object], command: str = "devtools test x") -> None: + monkeypatch.setattr(subprocess, "run", _fake_run(pr_view, [], local_head_sha=str(pr_view["headRefOid"]))) + merge_gate.cmd_record(42, command) + + +def test_check_blocks_when_no_receipt_exists(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(subprocess, "run", _fake_run(_base_pr_view(), [])) + + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + assert exit_code == 1 + + +def test_check_ok_when_receipt_fresh_and_matches_head_with_no_late_comments( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.chdir(tmp_path) + _record(monkeypatch, _base_pr_view()) + + monkeypatch.setattr(subprocess, "run", _fake_run(_base_pr_view(), [])) + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + assert exit_code == 0 + + +def test_check_blocks_when_receipt_is_for_a_stale_sha(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + _record(monkeypatch, _base_pr_view(head_sha="abc123")) + + # A new commit landed after the receipt was recorded. + monkeypatch.setattr(subprocess, "run", _fake_run(_base_pr_view(head_sha="def456"), [])) + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + assert exit_code == 1 + + +def test_check_blocks_on_review_comment_newer_than_head_commit(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + pr_view = _base_pr_view(committed_date="2026-08-01T12:00:00Z") + _record(monkeypatch, pr_view) + + late_comment = [ + { + "id": 111, + "path": "polylogue/foo.py", + "line": 10, + "created_at": "2026-08-01T12:05:00Z", + "body": "this is a real finding", + } + ] + monkeypatch.setattr(subprocess, "run", _fake_run(pr_view, late_comment)) + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + assert exit_code == 1 + + +def test_check_catches_a_comment_that_arrives_only_on_a_later_poll_round( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """The PR #3502 incident: CodeRabbit posts 30-60s after the first snapshot.""" + monkeypatch.chdir(tmp_path) + pr_view = _base_pr_view(committed_date="2026-08-01T12:00:00Z") + _record(monkeypatch, pr_view) + + late_comment = { + "id": 222, + "path": "polylogue/foo.py", + "line": 10, + "created_at": "2026-08-01T12:05:00Z", + "body": "arrived late", + } + # Round 1: empty. Round 2: the comment has landed. + monkeypatch.setattr(subprocess, "run", _fake_run(pr_view, [], poll_rounds=[[], [late_comment]])) + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=2, poll_interval_s=0, as_json=False) + + assert exit_code == 1 + + +def test_check_ignores_comment_older_than_head_commit(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + pr_view = _base_pr_view(committed_date="2026-08-01T12:00:00Z") + _record(monkeypatch, pr_view) + + stale_comment = [ + { + "id": 333, + "path": "polylogue/foo.py", + "line": 10, + "created_at": "2026-08-01T11:55:00Z", + "body": "already addressed by the fix commit", + } + ] + monkeypatch.setattr(subprocess, "run", _fake_run(pr_view, stale_comment)) + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + assert exit_code == 0 + + +def test_check_allows_an_acknowledged_late_comment_for_the_same_head_sha( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.chdir(tmp_path) + pr_view = _base_pr_view(committed_date="2026-08-01T12:00:00Z") + _record(monkeypatch, pr_view) + + late_comment = [ + { + "id": 444, + "path": "polylogue/foo.py", + "line": 10, + "created_at": "2026-08-01T12:05:00Z", + "body": "false positive, already fine", + } + ] + monkeypatch.setattr(subprocess, "run", _fake_run(pr_view, [])) + merge_gate.cmd_ack(42, 444, reason="false positive") + + monkeypatch.setattr(subprocess, "run", _fake_run(pr_view, late_comment)) + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + assert exit_code == 0 + + +def test_check_ignores_an_ack_recorded_for_a_different_head_sha( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """A new push must invalidate old acks -- otherwise a stale triage silently covers new code.""" + monkeypatch.chdir(tmp_path) + old_pr_view = _base_pr_view(head_sha="abc123", committed_date="2026-08-01T12:00:00Z") + monkeypatch.setattr(subprocess, "run", _fake_run(old_pr_view, [])) + merge_gate.cmd_ack(42, 555, reason="false positive on the old commit") + + new_pr_view = _base_pr_view(head_sha="def456", committed_date="2026-08-01T13:00:00Z") + _record(monkeypatch, new_pr_view) + late_comment = [ + { + "id": 555, + "path": "polylogue/foo.py", + "line": 10, + "created_at": "2026-08-01T13:05:00Z", + "body": "same comment id, but this is a new push", + } + ] + monkeypatch.setattr(subprocess, "run", _fake_run(new_pr_view, late_comment)) + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + assert exit_code == 1 + + +def test_check_blocks_when_pr_is_not_open(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + pr_view = _base_pr_view() + pr_view["state"] = "MERGED" + monkeypatch.setattr(subprocess, "run", _fake_run(pr_view, [])) + + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + assert exit_code == 1 + + +def test_check_blocks_when_receipt_older_than_max_age( + frozen_clock: FrozenClock, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.chdir(tmp_path) + _record(monkeypatch, _base_pr_view()) + + frozen_clock.advance(7200) + monkeypatch.setattr(subprocess, "run", _fake_run(_base_pr_view(), [])) + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + assert exit_code == 1 + + +def test_check_blocks_when_receipt_exit_code_is_nonzero(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + pr_view = _base_pr_view() + monkeypatch.setattr(subprocess, "run", _fake_run(pr_view, [], local_exit=1, local_head_sha="abc123")) + merge_gate.cmd_record(42, "devtools test x") + + monkeypatch.setattr(subprocess, "run", _fake_run(pr_view, [])) + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + assert exit_code == 1 + + +def test_check_blocks_when_merge_state_status_is_dirty(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + pr_view = _base_pr_view() + _record(monkeypatch, pr_view) + + pr_view["mergeStateStatus"] = "DIRTY" + monkeypatch.setattr(subprocess, "run", _fake_run(pr_view, [])) + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + assert exit_code == 1 + + +def test_check_blocks_when_comment_polling_fails(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + pr_view = _base_pr_view() + _record(monkeypatch, pr_view) + + def _run_with_broken_api(cmd: list[str], **kwargs: object) -> MagicMock: + if cmd[:3] == ["gh", "pr", "view"]: + return MagicMock(returncode=0, stdout=json.dumps(pr_view), stderr="") + if cmd[:2] == ["gh", "api"]: + return MagicMock(returncode=1, stdout="", stderr="rate limited") + return MagicMock(returncode=0, stdout="", stderr="") + + monkeypatch.setattr(subprocess, "run", _run_with_broken_api) + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + assert exit_code == 1 + + +def test_check_catches_a_late_review_body_not_just_inline_comments( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Review summaries and issue-level comments carry findings too -- a gate + that only watches inline diff comments misses them.""" + monkeypatch.chdir(tmp_path) + pr_view = _base_pr_view(committed_date="2026-08-01T12:00:00Z") + _record(monkeypatch, pr_view) + + def _run_with_late_review(cmd: list[str], **kwargs: object) -> MagicMock: + joined = " ".join(cmd) + if cmd[:3] == ["gh", "pr", "view"]: + return MagicMock(returncode=0, stdout=json.dumps(pr_view), stderr="") + if "/pulls/" in joined and "/reviews" in joined: + late_review = [{"id": 999, "submitted_at": "2026-08-01T12:10:00Z", "body": "Request changes: real bug"}] + return MagicMock(returncode=0, stdout=json.dumps([late_review]), stderr="") + if "/issues/" in joined and "/comments" in joined: + return MagicMock(returncode=0, stdout=json.dumps([[]]), stderr="") + if "/pulls/" in joined and "/comments" in joined: + return MagicMock(returncode=0, stdout=json.dumps([[]]), stderr="") + return MagicMock(returncode=0, stdout="", stderr="") + + monkeypatch.setattr(subprocess, "run", _run_with_late_review) + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + assert exit_code == 1 + + +def test_check_reports_advisory_when_receipt_command_skips_tests( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.chdir(tmp_path) + pr_view = _base_pr_view() + _record(monkeypatch, pr_view, command="devtools verify --quick") + + monkeypatch.setattr(subprocess, "run", _fake_run(pr_view, [])) + exit_code = merge_gate.cmd_check(42, max_age_s=3600, poll_rounds=1, poll_interval_s=0, as_json=False) + + # Advisory only -- does not block by itself, but is reported. + assert exit_code == 0 + receipt = json.loads(merge_gate._receipt_path(42).read_text()) + assert receipt["skips_tests"] is True