diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 9d55a68d8..1ea0213a6 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -15,6 +15,7 @@ from dataclasses import dataclass from datetime import datetime, timezone from typing import Any +from urllib.parse import quote PULL_REQUEST_FIELDS_FRAGMENT = """\ @@ -409,6 +410,31 @@ def fetch_rest_mergeable_state(repo: str, number: int) -> str: return REST_MERGEABLE_STATE_MAP.get(raw_state.lower(), raw_state.upper()) +def compare_ref_for_pr_head(repo: str, pr: dict[str, Any]) -> str: + """Return the compare-API head ref for a PR branch.""" + head_ref = pr.get("headRefName") or "HEAD" + head_repo = (pr.get("headRepository") or {}).get("nameWithOwner") + if not head_repo or head_repo == repo: + return head_ref + head_owner, _ = split_repo(head_repo) + return f"{head_owner}:{head_ref}" + + +def fetch_compare_branch_freshness(repo: str, pr: dict[str, Any]) -> dict[str, Any]: + """Fetch compare evidence showing whether the PR head lacks base commits.""" + base = quote(pr.get("baseRefName") or "base", safe="") + head = quote(compare_ref_for_pr_head(repo, pr), safe=":") + return json.loads( + run( + [ + "gh", + "api", + f"repos/{repo}/compare/{base}...{head}", + ] + ) + ) + + def enrich_rest_mergeable_states(repo: str, prs: list[dict[str, Any]]) -> None: """Attach REST mergeability evidence to GraphQL pull request payloads.""" def enrich(pr: dict[str, Any]) -> None: @@ -417,6 +443,12 @@ def enrich(pr: dict[str, Any]) -> None: pr["restMergeableState"] = fetch_rest_mergeable_state(repo, int(pr["number"])) except RuntimeError as exc: pr["restMergeableStateError"] = bounded_error_summary(str(exc)) + try: + compare = fetch_compare_branch_freshness(repo, pr) + pr["compareStatus"] = compare.get("status") + pr["compareBehindBy"] = compare.get("behind_by") + except RuntimeError as exc: + pr["compareBranchFreshnessError"] = bounded_error_summary(str(exc)) with concurrent.futures.ThreadPoolExecutor(max_workers=min(10, len(prs) or 1)) as executor: for _ in executor.map(enrich, prs): @@ -434,6 +466,23 @@ def effective_merge_state(pr: dict[str, Any]) -> str: return rest_state or graph_state +def compare_behind_by(pr: dict[str, Any]) -> int: + """Return the compare API's behind_by count as a safe integer.""" + behind_by = pr.get("compareBehindBy") + if isinstance(behind_by, int): + return max(0, behind_by) + if isinstance(behind_by, str) and behind_by.isdigit(): + return int(behind_by) + return 0 + + +def branch_outdated_by_base(pr: dict[str, Any], merge_state: str) -> int: + """Return known count of base commits missing from the PR head.""" + if merge_state == "BEHIND": + return max(1, compare_behind_by(pr)) + return compare_behind_by(pr) + + 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 {} @@ -1028,16 +1077,24 @@ def decide(action: str, reason: str) -> Decision: return decide("block", "current-head OpenCode review requested changes") current_head_approved = has_current_head_approval(pr) - if merge_state == "BEHIND" and current_head_approved: + behind_by = branch_outdated_by_base(pr, merge_state) + if behind_by and current_head_approved: if not update_branches: return decide("wait", "current-head OpenCode review approved; branch update disabled") if not can_update_pr_head(repo, pr): return decide("wait", non_mutable_head_reason(repo, pr)) update_branch(repo, pr, dry_run=dry_run) suffix = "; existing auto-merge request remains queued" if pr.get("autoMergeRequest") else "" + if merge_state == "BEHIND": + freshness_reason = "current-head OpenCode review approved" + else: + freshness_reason = ( + "current-head OpenCode review approved; " + f"base branch is {behind_by} commit(s) ahead even though GitHub mergeability is {merge_state}" + ) return decide( "update_branch", - "current-head OpenCode review approved; branch update requested with workflow GH_TOKEN " + f"{freshness_reason}; branch update requested with workflow GH_TOKEN " f"(github-actions[bot] in GitHub Actions){suffix}", ) diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 80a97d380..4fd1f85bb 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -232,24 +232,94 @@ def fake_run(args, stdin=None): assert sched.fetch_rest_mergeable_state("owner/repo", 7) == "DIRTY" assert calls == [["gh", "api", "repos/owner/repo/pulls/7", "--jq", ".mergeable_state // \"\""]] + calls.clear() + + def fake_compare_run(args, stdin=None): + calls.append(args) + return '{"status":"behind","behind_by":3}' - prs = [{"number": 8}] + monkeypatch.setattr(sched, "run", fake_compare_run) + compare = sched.fetch_compare_branch_freshness( + "owner/repo", + { + "baseRefName": "main", + "headRefName": "feature/update-branch", + "headRepository": {"nameWithOwner": "fork/repo"}, + }, + ) + assert compare == {"status": "behind", "behind_by": 3} + assert calls == [["gh", "api", "repos/owner/repo/compare/main...fork:feature%2Fupdate-branch"]] + calls.clear() + same_repo_compare = sched.fetch_compare_branch_freshness( + "owner/repo", + { + "baseRefName": "main", + "headRefName": "feature/update-branch", + "headRepository": {"nameWithOwner": "owner/repo"}, + }, + ) + assert same_repo_compare == {"status": "behind", "behind_by": 3} + assert calls == [["gh", "api", "repos/owner/repo/compare/main...feature%2Fupdate-branch"]] + + prs = [{"number": 8, "baseRefName": "main", "headRefName": "feature"}] monkeypatch.setattr(sched, "fetch_rest_mergeable_state", lambda repo, number: f"{repo}:{number}") + monkeypatch.setattr( + sched, + "fetch_compare_branch_freshness", + lambda repo, pr: {"status": "behind", "behind_by": 2}, + ) sched.enrich_rest_mergeable_states("owner/repo", prs) - assert prs == [{"number": 8, "restMergeableState": "owner/repo:8"}] + assert prs == [ + { + "number": 8, + "baseRefName": "main", + "headRefName": "feature", + "restMergeableState": "owner/repo:8", + "compareStatus": "behind", + "compareBehindBy": 2, + } + ] def raise_lookup_error(repo, number): raise RuntimeError("transient REST failure") - prs = [{"number": 9}] + prs = [{"number": 9, "baseRefName": "main", "headRefName": "feature"}] 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"}] + assert prs == [ + { + "number": 9, + "baseRefName": "main", + "headRefName": "feature", + "restMergeableStateError": "transient REST failure", + "compareStatus": "behind", + "compareBehindBy": 2, + } + ] + + def raise_compare_error(repo, pr): + raise RuntimeError("transient compare failure") + + prs = [{"number": 10, "baseRefName": "main", "headRefName": "feature"}] + monkeypatch.setattr(sched, "fetch_rest_mergeable_state", lambda repo, number: "CLEAN") + monkeypatch.setattr(sched, "fetch_compare_branch_freshness", raise_compare_error) + sched.enrich_rest_mergeable_states("owner/repo", prs) + assert prs == [ + { + "number": 10, + "baseRefName": "main", + "headRefName": "feature", + "restMergeableState": "CLEAN", + "compareBranchFreshnessError": "transient compare failure", + } + ] def test_context_review_and_check_helpers(): assert sched.context_nodes({}) == [] assert sched.context_nodes(make_pr()) == [] + assert sched.compare_behind_by({"compareBehindBy": "2"}) == 2 + assert sched.compare_behind_by({"compareBehindBy": "unknown"}) == 0 assert sched.is_opencode_context({"__typename": "CheckRun", "name": "opencode-review"}) assert sched.is_opencode_context( { @@ -1089,6 +1159,27 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): assert rest_behind_decision.action == "update_branch" assert "github-actions[bot]" in rest_behind_decision.reason assert called == [("owner/repo", 1, True)] + called.clear() + blocked_failed_behind_auto = make_pr( + mergeStateStatus="BLOCKED", + restMergeableState="BLOCKED", + compareBehindBy=2, + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + autoMergeRequest={"enabledAt": "now"}, + statusCheckRollup={ + "contexts": { + "nodes": [{"__typename": "CheckRun", "name": "strix", "conclusion": "FAILURE"}], + } + }, + ) + disabled.clear() + blocked_failed_behind_decision = inspect(blocked_failed_behind_auto) + assert blocked_failed_behind_decision.action == "update_branch" + assert "base branch is 2 commit(s) ahead" in blocked_failed_behind_decision.reason + assert "GitHub mergeability is BLOCKED" in blocked_failed_behind_decision.reason + assert "existing auto-merge request remains queued" in blocked_failed_behind_decision.reason + assert called == [("owner/repo", 1, True)] + assert disabled == [] def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch):