diff --git a/scripts/ci/pr_review_fix_scheduler.py b/scripts/ci/pr_review_fix_scheduler.py index 80c564f34..c7948887d 100755 --- a/scripts/ci/pr_review_fix_scheduler.py +++ b/scripts/ci/pr_review_fix_scheduler.py @@ -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] diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index fb90b0fb7..f708fb98d 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -47,6 +47,9 @@ reviewThreads(first: 100) { nodes { id isResolved isOutdated } } + files(first: 20) { + nodes { path } + } reviews(last: 50) { nodes { state @@ -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, @@ -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 { @@ -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(), @@ -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": { @@ -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', '')}`, `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, " @@ -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": @@ -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] @@ -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( [ "", @@ -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 diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 2fa216a81..601e23c3f 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -36,6 +36,7 @@ def make_pr(**overrides): ] }, "reviewThreads": {"nodes": []}, + "files": {"nodes": []}, "reviews": {"nodes": []}, "statusCheckRollup": {"contexts": {"nodes": []}}, } @@ -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): @@ -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]" @@ -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 = [ @@ -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"] @@ -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 @@ -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 @@ -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