From 72d8cb7f9190cb53a82260b161c790d4c1286888 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 30 Jun 2026 12:23:37 +0900 Subject: [PATCH 1/2] Add REST fallback for scheduler PR reads --- scripts/ci/pr_review_merge_scheduler.py | 125 +++++++++++++++++++++++- 1 file changed, 123 insertions(+), 2 deletions(-) diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 3bcd9861b..169cb7467 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -448,6 +448,117 @@ def gh_graphql(query: str, **fields: str | int) -> dict[str, Any]: time.sleep(delay) +def github_resource_inaccessible(exc: RuntimeError) -> bool: + """Return whether GitHub denied an API read for the current integration token.""" + + return "Resource not accessible by integration" in str(exc) + + +def gh_api_json(path: str) -> Any: + """Run a GitHub REST API request through gh and decode the JSON response.""" + + return json.loads(run(["gh", "api", path])) + + +def rest_review_node(review: dict[str, Any]) -> dict[str, Any]: + """Convert a REST review payload into the GraphQL shape used by the scheduler.""" + + commit_id = review.get("commit_id") + return { + "state": review.get("state"), + "body": review.get("body"), + "submittedAt": review.get("submitted_at"), + "author": {"login": ((review.get("user") or {}).get("login"))}, + "commit": {"oid": commit_id} if commit_id else None, + } + + +def rest_check_node(check: dict[str, Any]) -> dict[str, Any]: + """Convert a REST check-run payload into the GraphQL status rollup shape.""" + + return { + "__typename": "CheckRun", + "name": check.get("name"), + "status": (check.get("status") or "").upper(), + "conclusion": (check.get("conclusion") or "").upper() if check.get("conclusion") else None, + "startedAt": check.get("started_at"), + "detailsUrl": check.get("details_url"), + "checkSuite": {"workflowRun": {"workflow": {}}}, + } + + +def rest_pr_node(repo: str, pr: dict[str, Any]) -> dict[str, Any]: + """Convert a REST pull request payload into the GraphQL shape used by the scheduler.""" + + number = int(pr["number"]) + head = pr.get("head") or {} + base = pr.get("base") or {} + head_repo = head.get("repo") or {} + reviews = gh_api_json(f"repos/{repo}/pulls/{number}/reviews?per_page=100") + checks = gh_api_json(f"repos/{repo}/commits/{head.get('sha')}/check-runs?per_page=100") + rest_merge_state = REST_MERGEABLE_STATE_MAP.get( + str(pr.get("mergeable_state") or "").lower(), + str(pr.get("mergeable_state") or "").upper(), + ) + return { + "number": number, + "title": pr.get("title"), + "isDraft": bool(pr.get("draft")), + "mergeable": pr.get("mergeable"), + "mergeStateStatus": rest_merge_state, + "reviewDecision": "REVIEW_REQUIRED", + "baseRefName": base.get("ref"), + "baseRefOid": base.get("sha"), + "headRefName": head.get("ref"), + "headRefOid": head.get("sha"), + "isCrossRepository": (head_repo.get("full_name") or repo).lower() != repo.lower(), + "maintainerCanModify": bool(pr.get("maintainer_can_modify")), + "headRepository": {"nameWithOwner": head_repo.get("full_name") or repo}, + "autoMergeRequest": pr.get("auto_merge"), + "reviewThreads": {"nodes": []}, + "reviews": {"nodes": [rest_review_node(review) for review in reviews]}, + "statusCheckRollup": { + "contexts": { + "nodes": [ + rest_check_node(check) + for check in (checks.get("check_runs") or []) + ] + } + }, + "restMergeableState": rest_merge_state, + } + + +def fetch_open_prs_rest(repo: str, max_prs: int, base_branch: str | None = None) -> list[dict[str, Any]]: + """Fetch open pull requests through REST when GraphQL is unavailable.""" + + prs: list[dict[str, Any]] = [] + page = 1 + while len(prs) < max_prs: + page_size = min(100, max_prs - len(prs)) + path = ( + f"repos/{repo}/pulls?state=open&sort=created&direction=asc" + f"&per_page={page_size}&page={page}" + ) + if base_branch: + path += f"&base={quote(base_branch, safe='')}" + payload = gh_api_json(path) + if not payload: + break + prs.extend(rest_pr_node(repo, pr) for pr in payload) + if len(payload) < page_size: + break + page += 1 + return prs[:max_prs] + + +def fetch_pr_rest(repo: str, number: int) -> list[dict[str, Any]]: + """Fetch one pull request through REST when GraphQL is unavailable.""" + + pr = gh_api_json(f"repos/{repo}/pulls/{number}") + return [rest_pr_node(repo, pr)] if pr else [] + + def fetch_open_prs(repo: str, max_prs: int) -> list[dict[str, Any]]: """Fetch open pull requests from GitHub, paginating up to max_prs.""" owner, name = split_repo(repo) @@ -463,7 +574,12 @@ def fetch_open_prs(repo: str, max_prs: int) -> list[dict[str, Any]]: } if cursor: fields["cursor"] = cursor - payload = gh_graphql(OPEN_PRS_QUERY, **fields) + try: + payload = gh_graphql(OPEN_PRS_QUERY, **fields) + except RuntimeError as exc: + if github_resource_inaccessible(exc): + return fetch_open_prs_rest(repo, max_prs) + raise pr_page = payload["data"]["repository"]["pullRequests"] prs.extend(pr_page.get("nodes") or []) if not pr_page["pageInfo"]["hasNextPage"]: @@ -477,7 +593,12 @@ def fetch_open_prs(repo: str, max_prs: int) -> list[dict[str, Any]]: def fetch_pr(repo: str, number: int) -> list[dict[str, Any]]: """Fetch one pull request by number using the same evidence shape as the queue scan.""" owner, name = split_repo(repo) - payload = gh_graphql(PR_BY_NUMBER_QUERY, owner=owner, name=name, number=number) + try: + payload = gh_graphql(PR_BY_NUMBER_QUERY, owner=owner, name=name, number=number) + except RuntimeError as exc: + if github_resource_inaccessible(exc): + return fetch_pr_rest(repo, number) + raise pr = payload["data"]["repository"].get("pullRequest") prs = [pr] if pr else [] enrich_rest_mergeable_states(repo, prs) From 7f145a3ffbaf9da2dd5a28259c4183746cffb87b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 30 Jun 2026 12:30:35 +0900 Subject: [PATCH 2/2] Cover scheduler REST fallback --- tests/test_pr_review_merge_scheduler.py | 167 ++++++++++++++++++++++++ 1 file changed, 167 insertions(+) diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 0a582e13a..ad884cff6 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -361,6 +361,173 @@ def raise_compare_error(repo, pr): ] +def test_rest_pr_fallback_shapes_reviews_and_checks(monkeypatch): + calls = [] + payloads = { + "repos/owner/repo/pulls/42/reviews?per_page=100": [ + { + "state": "APPROVED", + "body": "Head SHA: `abc123`", + "submitted_at": "2026-06-30T00:00:00Z", + "commit_id": "abc123", + "user": {"login": "opencode-agent[bot]"}, + } + ], + "repos/owner/repo/commits/abc123/check-runs?per_page=100": { + "check_runs": [ + { + "name": "opencode-review", + "status": "completed", + "conclusion": "success", + "started_at": "2026-06-30T00:00:00Z", + "details_url": "https://github.com/owner/repo/actions/runs/1/job/2", + } + ] + }, + } + + def fake_api(path): + calls.append(path) + return payloads[path] + + monkeypatch.setattr(sched, "gh_api_json", fake_api) + node = sched.rest_pr_node( + "owner/repo", + { + "number": 42, + "title": "Fallback", + "draft": False, + "mergeable": True, + "mergeable_state": "clean", + "base": {"ref": "main", "sha": "base123"}, + "head": { + "ref": "feature", + "sha": "abc123", + "repo": {"full_name": "owner/repo"}, + }, + "maintainer_can_modify": True, + "auto_merge": {"enabled_at": "2026-06-30T00:01:00Z"}, + }, + ) + + assert calls == [ + "repos/owner/repo/pulls/42/reviews?per_page=100", + "repos/owner/repo/commits/abc123/check-runs?per_page=100", + ] + assert node["number"] == 42 + assert node["mergeStateStatus"] == "CLEAN" + assert node["restMergeableState"] == "CLEAN" + assert node["headRepository"] == {"nameWithOwner": "owner/repo"} + assert not node["isCrossRepository"] + assert node["reviews"]["nodes"][0]["author"]["login"] == "opencode-agent[bot]" + assert node["reviews"]["nodes"][0]["commit"]["oid"] == "abc123" + assert node["statusCheckRollup"]["contexts"]["nodes"][0]["status"] == "COMPLETED" + assert node["statusCheckRollup"]["contexts"]["nodes"][0]["conclusion"] == "SUCCESS" + + +def test_fetch_pr_falls_back_to_rest_when_graphql_denied(monkeypatch): + def deny_graphql(*args, **kwargs): + raise RuntimeError("gh: Resource not accessible by integration") + + monkeypatch.setattr(sched, "gh_graphql", deny_graphql) + monkeypatch.setattr(sched, "fetch_pr_rest", lambda repo, number: [{"number": number, "repo": repo}]) + + assert sched.github_resource_inaccessible(RuntimeError("Resource not accessible by integration")) + assert sched.fetch_pr("owner/repo", 77) == [{"number": 77, "repo": "owner/repo"}] + + +def test_rest_api_wrapper_and_fetch_pr_rest(monkeypatch): + run_calls = [] + + def fake_run(args): + run_calls.append(args) + return json.dumps({"number": 42}) + + monkeypatch.setattr(sched, "run", fake_run) + assert sched.gh_api_json("repos/owner/repo/pulls/42") == {"number": 42} + assert run_calls == [["gh", "api", "repos/owner/repo/pulls/42"]] + + api_calls = [] + + def fake_api(path): + api_calls.append(path) + if path == "repos/owner/repo/pulls/42": + return {"number": 42} + return {} + + monkeypatch.setattr(sched, "gh_api_json", fake_api) + monkeypatch.setattr(sched, "rest_pr_node", lambda repo, pr: {"repo": repo, "number": pr["number"]}) + assert sched.fetch_pr_rest("owner/repo", 42) == [{"repo": "owner/repo", "number": 42}] + assert sched.fetch_pr_rest("owner/repo", 99) == [] + assert api_calls == ["repos/owner/repo/pulls/42", "repos/owner/repo/pulls/99"] + + +def test_fetch_open_prs_rest_paginates_and_fetch_open_prs_falls_back(monkeypatch): + paths = [] + pages = { + "repos/owner/repo/pulls?state=open&sort=created&direction=asc&per_page=3&page=1": [ + {"number": 1}, + {"number": 2}, + ] + } + + def fake_api(path): + paths.append(path) + return pages[path] + + monkeypatch.setattr(sched, "gh_api_json", fake_api) + monkeypatch.setattr(sched, "rest_pr_node", lambda repo, pr: {"number": pr["number"], "repo": repo}) + + assert sched.fetch_open_prs_rest("owner/repo", 3) == [ + {"number": 1, "repo": "owner/repo"}, + {"number": 2, "repo": "owner/repo"}, + ] + assert paths == [ + "repos/owner/repo/pulls?state=open&sort=created&direction=asc&per_page=3&page=1", + ] + + def deny_graphql(*args, **kwargs): + raise RuntimeError("gh: Resource not accessible by integration") + + monkeypatch.setattr(sched, "gh_graphql", deny_graphql) + monkeypatch.setattr(sched, "fetch_open_prs_rest", lambda repo, max_prs: [{"repo": repo, "max": max_prs}]) + assert sched.fetch_open_prs("owner/repo", 5) == [{"repo": "owner/repo", "max": 5}] + + +def test_fetch_open_prs_rest_base_branch_empty_and_next_page(monkeypatch): + paths = [] + pages = { + "repos/owner/repo/pulls?state=open&sort=created&direction=asc&per_page=100&page=1&base=release%2Fv1": [ + {"number": number} for number in range(1, 101) + ], + "repos/owner/repo/pulls?state=open&sort=created&direction=asc&per_page=1&page=2&base=release%2Fv1": [], + } + + def fake_api(path): + paths.append(path) + return pages[path] + + monkeypatch.setattr(sched, "gh_api_json", fake_api) + monkeypatch.setattr(sched, "rest_pr_node", lambda repo, pr: {"number": pr["number"]}) + + assert sched.fetch_open_prs_rest("owner/repo", 101, base_branch="release/v1") == [ + {"number": number} for number in range(1, 101) + ] + assert paths == list(pages) + + +def test_graphql_read_errors_only_fall_back_for_integration_denials(monkeypatch): + def fail_graphql(*args, **kwargs): + raise RuntimeError("gh: timeout") + + monkeypatch.setattr(sched, "gh_graphql", fail_graphql) + + with pytest.raises(RuntimeError, match="timeout"): + sched.fetch_open_prs("owner/repo", 1) + with pytest.raises(RuntimeError, match="timeout"): + sched.fetch_pr("owner/repo", 1) + + def test_context_review_and_check_helpers(): assert sched.context_nodes({}) == [] assert sched.context_nodes(make_pr()) == []