Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions scripts/ci/pr_review_fix_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ def process_queue(args: argparse.Namespace) -> int:
max_workers = min(10, len(prs_needing_comments))
with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor:
def fetch_comments(pr_number: int) -> tuple[int, list[dict[str, Any]]]:
"""Fetch one PR's issue comments for parallel queue inspection."""
return pr_number, issue_comments(args.repo, pr_number)

futures = [executor.submit(fetch_comments, int(pr["number"])) for pr in prs_needing_comments]
Expand Down
58 changes: 57 additions & 1 deletion scripts/ci/pr_review_merge_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@
reviewThreads(first: 100) {
nodes { id isResolved isOutdated }
}
files(first: 20) {
nodes { path }
}
reviews(last: 50) {
nodes {
state
Expand Down Expand Up @@ -258,7 +261,7 @@ def decision_guidance(decision: Decision) -> dict[str, Any] | None:
base_remote = f"origin/{base_ref}"
quoted_base_ref = shlex.quote(base_ref)
quoted_base_remote = shlex.quote(base_remote)
return {
guidance: dict[str, Any] = {
"type": "merge_conflict_repair",
"merge_state": state,
"base_ref": base_ref,
Expand Down Expand Up @@ -286,6 +289,10 @@ def decision_guidance(decision: Decision) -> dict[str, Any] | None:
"# rebase path only: git push --force-with-lease",
],
}
changed_files = parse_conflict_changed_files(decision.reason)
if changed_files:
guidance["changed_files_to_inspect"] = changed_files
return guidance
action_required = parse_workflow_action_required_reason(decision.reason)
if action_required:
return {
Expand Down Expand Up @@ -549,6 +556,7 @@ def rest_pr_node(repo: str, pr: dict[str, Any]) -> dict[str, Any]:
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")
files = gh_api_json(f"repos/{repo}/pulls/{number}/files?per_page=20")
rest_merge_state = REST_MERGEABLE_STATE_MAP.get(
str(pr.get("mergeable_state") or "").lower(),
str(pr.get("mergeable_state") or "").upper(),
Expand All @@ -569,6 +577,7 @@ def rest_pr_node(repo: str, pr: dict[str, Any]) -> dict[str, Any]:
"headRepository": {"nameWithOwner": head_repo.get("full_name") or repo},
"autoMergeRequest": pr.get("auto_merge"),
"reviewThreads": {"nodes": []},
"files": {"nodes": [{"path": file.get("filename")} for file in files if file.get("filename")]},
"reviews": {"nodes": [rest_review_node(review) for review in reviews]},
"statusCheckRollup": {
"contexts": {
Expand Down Expand Up @@ -1393,8 +1402,15 @@ def merge_conflict_guidance(pr: dict[str, Any], merge_state: str) -> str:
"""Return actionable conflict repair guidance for a conflicting PR."""
base_ref = pr.get("baseRefName") or "base"
head_ref = pr.get("headRefName") or "head"
changed_files = conflict_changed_files_text(pr)
changed_files_note = (
f"changed files to inspect first: {changed_files}; "
if changed_files
else ""
)
return (
f"merge conflict: {merge_state}; base={base_ref}, head={head_ref}; "
f"{changed_files_note}"
f"run `gh pr checkout {pr.get('number', '<pr>')}`, `git fetch origin {base_ref}`, then "
f"`git merge --no-ff origin/{base_ref}` or `git rebase origin/{base_ref}`; "
"use `git status --short` to find conflicted files, resolve conflict markers in the PR branch, "
Expand All @@ -1404,6 +1420,22 @@ def merge_conflict_guidance(pr: dict[str, Any], merge_state: str) -> str:
)


def changed_file_paths(pr: dict[str, Any], *, limit: int = 10) -> list[str]:
"""Return changed file paths already present in the pull request payload."""
nodes = ((pr.get("files") or {}).get("nodes") or [])[:limit]
return [path for node in nodes if isinstance(path := node.get("path"), str) and path]


def conflict_changed_files_text(pr: dict[str, Any], *, limit: int = 10) -> str:
"""Return compact changed-file guidance for conflict repair text."""
paths = changed_file_paths(pr, limit=limit)
if not paths:
return ""
total = len(((pr.get("files") or {}).get("nodes") or []))
suffix = f" | +{total - len(paths)} more" if total > len(paths) else ""
return " | ".join(paths) + suffix


def auto_merge_wait_reason(merge_state: str) -> str:
"""Explain why an approved PR with auto-merge enabled is still waiting."""
if merge_state == "CLEAN":
Expand Down Expand Up @@ -1872,6 +1904,21 @@ def parse_conflict_reason(reason: str) -> tuple[str, str, str] | None:
return state, base_ref, head_ref


def parse_conflict_changed_files(reason: str) -> list[str]:
"""Extract changed-file conflict hints from scheduler guidance text."""
prefix = "changed files to inspect first: "
for segment in reason.split(";"):
segment = segment.strip()
if not segment.startswith(prefix):
continue
return [
file_path
for file_path in (part.strip() for part in segment[len(prefix) :].split("|"))
if file_path and not file_path.startswith("+")
]
return []


def conflict_repair_summary(decisions: list[Decision]) -> list[str]:
"""Return a GitHub Actions Summary section with concrete conflict repair steps."""
conflicted = [(decision, parse_conflict_reason(decision.reason)) for decision in decisions]
Expand All @@ -1890,6 +1937,7 @@ def conflict_repair_summary(decisions: list[Decision]) -> list[str]:
assert parsed is not None
state, base_ref, head_ref = parsed
base_remote = f"origin/{base_ref}"
changed_files = parse_conflict_changed_files(decision.reason)
lines.extend(
[
"",
Expand All @@ -1910,6 +1958,14 @@ def conflict_repair_summary(decisions: list[Decision]) -> list[str]:
"```",
]
)
if changed_files:
lines.extend(
[
"",
"Changed files to inspect first:",
*(f"- `{path.replace('`', '\\`')}`" for path in changed_files),
]
)
return lines


Expand Down
29 changes: 26 additions & 3 deletions tests/test_pr_review_merge_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def make_pr(**overrides):
]
},
"reviewThreads": {"nodes": []},
"files": {"nodes": []},
"reviews": {"nodes": []},
"statusCheckRollup": {"contexts": {"nodes": []}},
}
Expand Down Expand Up @@ -410,6 +411,9 @@ def test_rest_pr_fallback_shapes_reviews_and_checks(monkeypatch):
}
]
},
"repos/owner/repo/pulls/42/files?per_page=20": [
{"filename": "scripts/ci/pr_review_merge_scheduler.py"},
],
}

def fake_api(path):
Expand Down Expand Up @@ -439,10 +443,12 @@ def fake_api(path):
assert calls == [
"repos/owner/repo/pulls/42/reviews?per_page=100",
"repos/owner/repo/commits/abc123/check-runs?per_page=100",
"repos/owner/repo/pulls/42/files?per_page=20",
]
assert node["number"] == 42
assert node["mergeStateStatus"] == "CLEAN"
assert node["restMergeableState"] == "CLEAN"
assert node["files"]["nodes"] == [{"path": "scripts/ci/pr_review_merge_scheduler.py"}]
assert node["headRepository"] == {"nameWithOwner": "owner/repo"}
assert not node["isCrossRepository"]
assert node["reviews"]["nodes"][0]["author"]["login"] == "opencode-agent[bot]"
Expand Down Expand Up @@ -1236,7 +1242,11 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys)
summary_path = tmp_path / "summary.md"
monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary_path))
conflict_reason = sched.merge_conflict_guidance(
make_pr(number=7, headRefName="feature|x"),
make_pr(
number=7,
headRefName="feature|x",
files={"nodes": [{"path": "scripts/ci/pr_review_merge_scheduler.py"}, {"path": "tests/test_pr_review_merge_scheduler.py"}]},
),
"DIRTY",
)
decisions = [
Expand Down Expand Up @@ -1293,6 +1303,10 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys)
assert payload["decisions"][0]["guidance"]["merge_state"] == "DIRTY"
assert payload["decisions"][0]["guidance"]["base_ref"] == "main"
assert payload["decisions"][0]["guidance"]["head_ref"] == "feature|x"
assert payload["decisions"][0]["guidance"]["changed_files_to_inspect"] == [
"scripts/ci/pr_review_merge_scheduler.py",
"tests/test_pr_review_merge_scheduler.py",
]
assert "update-branch cannot choose" in payload["decisions"][0]["guidance"]["automation_limit"]
assert "gh pr checkout 7" in payload["decisions"][0]["guidance"]["commands"]
assert "git merge --no-ff origin/main" in payload["decisions"][0]["guidance"]["commands"]
Expand All @@ -1311,7 +1325,7 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys)
assert payload["decisions"][5]["guidance"]["head_repository"] == "fork/repo"
summary = summary_path.read_text(encoding="utf-8")
assert "## PR review merge scheduler" in summary
assert "| #7 | block | merge conflict: DIRTY; base=main, head=feature\\|x; run" in summary
assert "| #7 | block | merge conflict: DIRTY; base=main, head=feature\\|x; changed files to inspect first:" in summary
assert "do not retry update-branch until the conflict is repaired" in summary
assert "### Outdated review threads" in summary
assert "Would resolve 1 outdated review thread(s)" in summary
Expand All @@ -1329,6 +1343,9 @@ def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys)
assert "gh pr checkout 7" in summary
assert "git fetch origin main" in summary
assert "git merge --no-ff origin/main" in summary
assert "Changed files to inspect first:" in summary
assert "- `scripts/ci/pr_review_merge_scheduler.py`" in summary
assert "- `tests/test_pr_review_merge_scheduler.py`" in summary
assert "git push --force-with-lease" in summary
assert "### Branch update requests" in summary
assert "Requested `update-branch` for PR #8 with `workflow GITHUB_TOKEN`" in summary
Expand Down Expand Up @@ -1417,9 +1434,15 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch):
assert inspect(make_pr(baseRefName="develop")).reason == "base branch is develop; expected main"
external_head = inspect(make_pr(headRepository={"nameWithOwner": "fork/repo"}, isCrossRepository=True))
assert external_head.action == "security_dispatch"
conflict = inspect(make_pr(mergeStateStatus="DIRTY"))
conflict = inspect(
make_pr(
mergeStateStatus="DIRTY",
files={"nodes": [{"path": "scripts/ci/pr_review_merge_scheduler.py"}]},
)
)
assert conflict.action == "block"
assert "merge conflict: DIRTY" in conflict.reason
assert "changed files to inspect first: scripts/ci/pr_review_merge_scheduler.py" in conflict.reason
assert "base=main, head=feature" in conflict.reason
assert "gh pr checkout 1" in conflict.reason
assert "git fetch origin main" in conflict.reason
Expand Down
Loading