diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 948c047fd..7f86ef328 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -85,6 +85,17 @@ OPEN_PRS_PAGE_SIZE = 25 DEFAULT_STALE_OPENCODE_MINUTES = 45 RUNNING_CHECK_STATES = {"PENDING", "EXPECTED", "QUEUED", "IN_PROGRESS", "WAITING", "REQUESTED"} +REST_MERGEABLE_STATE_MAP = { + "behind": "BEHIND", + "blocked": "BLOCKED", + "clean": "CLEAN", + "dirty": "DIRTY", + "draft": "DRAFT", + "has_hooks": "HAS_HOOKS", + "unknown": "UNKNOWN", + "unstable": "UNSTABLE", +} +REST_MERGEABLE_STATES = set(REST_MERGEABLE_STATE_MAP.values()) @dataclass @@ -265,9 +276,44 @@ def fetch_open_prs(repo: str, max_prs: int) -> list[dict[str, Any]]: break cursor = pr_page["pageInfo"]["endCursor"] + enrich_rest_mergeable_states(repo, prs) return prs +def fetch_rest_mergeable_state(repo: str, number: int) -> str: + """Fetch and normalize GitHub REST mergeable_state for one pull request.""" + raw_state = run( + [ + "gh", + "api", + f"repos/{repo}/pulls/{number}", + "--jq", + ".mergeable_state // \"\"", + ] + ).strip() + return REST_MERGEABLE_STATE_MAP.get(raw_state.lower(), raw_state.upper()) + + +def enrich_rest_mergeable_states(repo: str, prs: list[dict[str, Any]]) -> None: + """Attach REST mergeability evidence to GraphQL pull request payloads.""" + for pr in prs: + try: + pr["restMergeableState"] = fetch_rest_mergeable_state(repo, int(pr["number"])) + except RuntimeError as exc: + pr["restMergeableStateError"] = bounded_error_summary(str(exc)) + + +def effective_merge_state(pr: dict[str, Any]) -> str: + """Return the safest merge state from GraphQL plus REST mergeability evidence.""" + graph_state = (pr.get("mergeStateStatus") or "").upper() + rest_state = (pr.get("restMergeableState") or "").upper() + if rest_state in REST_MERGEABLE_STATES: + return rest_state + if graph_state in {"BEHIND", "DIRTY", "CONFLICTING", "UNKNOWN"}: + return graph_state + return rest_state or graph_state + + def context_nodes(pr: dict[str, Any]) -> list[dict[str, Any]]: """Return status rollup context nodes for a pull request payload.""" rollup = pr.get("statusCheckRollup") or {} @@ -576,7 +622,17 @@ def inspect_pr( if head_repo != repo: return Decision(number, "skip", f"fork or external head repo: {head_repo}") - merge_state = (pr.get("mergeStateStatus") or "").upper() + merge_state = effective_merge_state(pr) + if merge_state == "UNKNOWN": + if pr.get("autoMergeRequest"): + return disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason="mergeability is still being calculated; wait for GitHub mergeability evidence before re-enabling auto-merge", + ) + return Decision(number, "wait", "mergeability is still being calculated") + if merge_state in {"DIRTY", "CONFLICTING"}: if pr.get("autoMergeRequest"): return disable_auto_merge_decision( @@ -911,6 +967,7 @@ def self_test() -> None: "baseRefOid": "base", "headRefName": "feature", "mergeStateStatus": "CLEAN", + "restMergeableState": "CLEAN", "isDraft": False, "headRepository": {"nameWithOwner": "owner/repo"}, "reviewDecision": "REVIEW_REQUIRED", @@ -952,6 +1009,50 @@ def self_test() -> None: base_branch="main", ) assert decision.action == "auto_merge" + sample["restMergeableState"] = "BEHIND" + decision = inspect_pr( + "owner/repo", + sample, + dry_run=True, + trigger_reviews=True, + enable_auto_merge_flag=True, + update_branches=True, + workflow="OpenCode Review", + security_workflow="Strix Security Scan", + base_branch="main", + ) + assert decision.action == "update_branch" + sample["restMergeableState"] = "DIRTY" + sample["autoMergeRequest"] = {"enabledAt": "2026-01-01T00:02:00Z"} + decision = inspect_pr( + "owner/repo", + sample, + dry_run=True, + trigger_reviews=True, + enable_auto_merge_flag=True, + update_branches=True, + workflow="OpenCode Review", + security_workflow="Strix Security Scan", + base_branch="main", + ) + assert decision.action == "disable_auto_merge" + assert "merge conflict: DIRTY" in decision.reason + sample["restMergeableState"] = "UNKNOWN" + sample["autoMergeRequest"] = None + decision = inspect_pr( + "owner/repo", + sample, + dry_run=True, + trigger_reviews=True, + enable_auto_merge_flag=True, + update_branches=True, + workflow="OpenCode Review", + security_workflow="Strix Security Scan", + base_branch="main", + ) + assert decision.action == "wait" + assert "mergeability is still being calculated" in decision.reason + sample["restMergeableState"] = "CLEAN" sample["autoMergeRequest"] = {"enabledAt": "2026-01-01T00:02:00Z"} sample["statusCheckRollup"]["contexts"]["nodes"] = [ {"__typename": "CheckRun", "name": "strix", "status": "COMPLETED", "conclusion": "FAILURE"} @@ -1032,6 +1133,7 @@ def self_test() -> None: assert opencode_in_progress(sample) sample["statusCheckRollup"]["contexts"]["nodes"] = [] sample["mergeStateStatus"] = "BEHIND" + sample["restMergeableState"] = "" sample["reviews"]["nodes"] = [ { "state": "APPROVED", diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index cb999e01a..ed0250245 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -14,6 +14,7 @@ def make_pr(**overrides): "isDraft": False, "mergeable": "MERGEABLE", "mergeStateStatus": "CLEAN", + "restMergeableState": "", "reviewDecision": "REVIEW_REQUIRED", "baseRefName": "main", "baseRefOid": "base", @@ -163,6 +164,7 @@ def fake_graphql(query, **fields): return pages.pop(0) monkeypatch.setattr(sched, "gh_graphql", fake_graphql) + monkeypatch.setattr(sched, "enrich_rest_mergeable_states", lambda repo, prs: None) assert sched.fetch_open_prs("owner/repo", 3) == [{"number": 1}, {"number": 2}] assert seen[0]["pageSize"] == 3 assert seen[1]["cursor"] == "cursor" @@ -185,11 +187,38 @@ def fake_graphql(query, **fields): } monkeypatch.setattr(sched, "gh_graphql", fake_graphql) + monkeypatch.setattr(sched, "enrich_rest_mergeable_states", lambda repo, prs: None) assert sched.fetch_open_prs("owner/repo", 120) == [{"number": sched.OPEN_PRS_PAGE_SIZE}] assert seen[0]["pageSize"] == sched.OPEN_PRS_PAGE_SIZE +def test_rest_mergeable_state_helpers(monkeypatch): + calls = [] + + def fake_run(args, stdin=None): + calls.append(args) + return "dirty\n" + + monkeypatch.setattr(sched, "run", fake_run) + + assert sched.fetch_rest_mergeable_state("owner/repo", 7) == "DIRTY" + assert calls == [["gh", "api", "repos/owner/repo/pulls/7", "--jq", ".mergeable_state // \"\""]] + + prs = [{"number": 8}] + monkeypatch.setattr(sched, "fetch_rest_mergeable_state", lambda repo, number: f"{repo}:{number}") + sched.enrich_rest_mergeable_states("owner/repo", prs) + assert prs == [{"number": 8, "restMergeableState": "owner/repo:8"}] + + def raise_lookup_error(repo, number): + raise RuntimeError("transient REST failure") + + prs = [{"number": 9}] + monkeypatch.setattr(sched, "fetch_rest_mergeable_state", raise_lookup_error) + sched.enrich_rest_mergeable_states("owner/repo", prs) + assert prs == [{"number": 9, "restMergeableStateError": "transient REST failure"}] + + def test_context_review_and_check_helpers(): assert sched.context_nodes({}) == [] assert sched.context_nodes(make_pr()) == [] @@ -511,6 +540,35 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): conflicting = inspect(make_pr(mergeStateStatus="CONFLICTING")) assert conflicting.action == "block" assert "merge conflict: CONFLICTING" in conflicting.reason + rest_conflict = inspect( + make_pr( + mergeStateStatus="CLEAN", + restMergeableState="DIRTY", + autoMergeRequest={"enabledAt": "now"}, + ) + ) + assert rest_conflict.action == "disable_auto_merge" + assert "merge conflict: DIRTY" in rest_conflict.reason + unknown_mergeability = inspect(make_pr(mergeStateStatus="CLEAN", restMergeableState="UNKNOWN")) + assert unknown_mergeability.action == "wait" + assert unknown_mergeability.reason == "mergeability is still being calculated" + unknown_auto_merge = inspect( + make_pr( + mergeStateStatus="CLEAN", + restMergeableState="UNKNOWN", + autoMergeRequest={"enabledAt": "now"}, + ) + ) + assert unknown_auto_merge.action == "disable_auto_merge" + assert "mergeability is still being calculated" in unknown_auto_merge.reason + rest_clean = inspect( + make_pr( + mergeStateStatus="BEHIND", + restMergeableState="CLEAN", + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + ) + ) + assert rest_clean.action == "auto_merge" assert inspect(make_pr(reviewThreads={"nodes": [{"isResolved": False}]})).reason == "1 unresolved review thread(s)" unresolved_auto = inspect( make_pr( @@ -567,6 +625,17 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): ) assert inspect(behind_auto_merge_enabled).action == "update_branch" assert called == [("owner/repo", 1, True)] + called.clear() + rest_behind = make_pr( + mergeStateStatus="CLEAN", + restMergeableState="BEHIND", + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + autoMergeRequest={"enabledAt": "now"}, + ) + rest_behind_decision = inspect(rest_behind) + assert rest_behind_decision.action == "update_branch" + assert "github-actions[bot]" in rest_behind_decision.reason + assert called == [("owner/repo", 1, True)] def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch):