diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 3b61acb7b..58987b650 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -1427,6 +1427,28 @@ def decide(action: str, reason: str) -> Decision: """Create a decision after applying shared cleanup notes.""" return finish(Decision(number, action, reason)) + def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decision: + """Request update-branch and attach any same-head evidence follow-up.""" + update_branch(repo, pr, dry_run=dry_run) + followup_note = post_update_branch_followup( + repo, + pr, + dry_run=dry_run, + trigger_reviews=trigger_reviews, + review_dispatch_allowed=review_dispatch_allowed, + workflow=workflow, + security_workflow=security_workflow, + stale_opencode_minutes=stale_opencode_minutes, + ) + decision = Decision( + number, + "update_branch", + f"{freshness_reason}; branch update requested with {mutation_token_label()} " + f"inside GitHub Actions as {mutation_actor_label()}{suffix}", + (followup_note,) if followup_note else (), + ) + return finish(decision) + merge_state = effective_merge_state(pr) unresolved = unresolved_thread_count(pr) if unresolved: @@ -1569,7 +1591,6 @@ def decide(action: str, reason: str) -> Decision: return decide("wait", "auto-merge already enabled; 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 auto_merge_enabled else "" if current_head_approved and merge_state == "BEHIND": freshness_reason = "current-head OpenCode review approved" @@ -1585,24 +1606,31 @@ def decide(action: str, reason: str) -> Decision: "auto-merge already enabled; " f"base branch is {behind_by} commit(s) ahead even though GitHub mergeability is {merge_state}" ) - followup_note = post_update_branch_followup( - repo, - pr, - dry_run=dry_run, - trigger_reviews=trigger_reviews, - review_dispatch_allowed=review_dispatch_allowed, - workflow=workflow, - security_workflow=security_workflow, - stale_opencode_minutes=stale_opencode_minutes, - ) - decision = Decision( - number, - "update_branch", - f"{freshness_reason}; branch update requested with {mutation_token_label()} " - f"inside GitHub Actions as {mutation_actor_label()}{suffix}", - (followup_note,) if followup_note else (), - ) - return finish(decision) + return request_branch_update(freshness_reason, suffix=suffix) + + opencode_state = opencode_progress_state(pr, stale_after_minutes=stale_opencode_minutes) + if opencode_state == "running": + return decide("wait", "OpenCode review is already in progress") + + if behind_by and trigger_reviews: + if not update_branches: + return decide("wait", "current head has no OpenCode approval; branch update disabled before review dispatch") + if not can_update_pr_head(repo, pr): + head_repo = (pr.get("headRepository") or {}).get("nameWithOwner") or "" + return decide( + "wait", + f"current head has no OpenCode approval; branch is outdated before review dispatch, " + f"but head repo {head_repo} is not writable by the scheduler credential", + ) + if merge_state == "BEHIND": + freshness_reason = "current head has no OpenCode approval; branch is outdated before review dispatch" + else: + freshness_reason = ( + "current head has no OpenCode approval; " + f"base branch is {behind_by} commit(s) ahead before review dispatch even though " + f"GitHub mergeability is {merge_state}" + ) + return request_branch_update(freshness_reason) if merge_state == "UNKNOWN": if pr.get("autoMergeRequest"): @@ -1641,9 +1669,6 @@ def decide(action: str, reason: str) -> Decision: enable_auto_merge(repo, pr, dry_run=dry_run) return decide("auto_merge", "current head is approved; auto-merge enabled") - opencode_state = opencode_progress_state(pr, stale_after_minutes=stale_opencode_minutes) - if opencode_state == "running": - return decide("wait", "OpenCode review is already in progress") if opencode_state == "stale" and not trigger_reviews: return decide( "wait", @@ -2267,7 +2292,8 @@ def self_test() -> None: security_workflow="Strix Security Scan", base_branch="main", ) - assert decision.action == "security_dispatch" + assert decision.action == "update_branch" + assert "branch is outdated before review dispatch" in decision.reason sample["statusCheckRollup"]["contexts"]["nodes"] = [ { "__typename": "CheckRun", @@ -2288,7 +2314,8 @@ def self_test() -> None: security_workflow="Strix Security Scan", base_branch="main", ) - assert decision.action == "review_dispatch" + assert decision.action == "update_branch" + assert "branch is outdated before review dispatch" in decision.reason sample["reviews"]["nodes"][0]["commit"]["oid"] = "abc" decision = inspect_pr( "owner/repo", diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 5af005fe0..9e7e0b8ff 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1446,8 +1446,10 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): dispatched = [] monkeypatch.setattr(sched, "dispatch_strix_evidence", lambda repo, workflow, pr, dry_run: dispatched.append(workflow)) monkeypatch.setattr(sched, "dispatch_opencode_review", lambda repo, workflow, pr, dry_run: dispatched.append(workflow)) - assert inspect(stale_behind).action == "security_dispatch" - assert dispatched == ["Strix Security Scan"] + stale_behind_decision = inspect(stale_behind) + assert stale_behind_decision.action == "update_branch" + assert "branch is outdated before review dispatch" in stale_behind_decision.reason + assert dispatched == [] behind = make_pr(mergeStateStatus="BEHIND", reviews={"nodes": [opencode_review("APPROVED", "head")]}) assert inspect(behind, update_branches=False).reason == "current-head OpenCode review approved; branch update disabled" @@ -1870,6 +1872,74 @@ def test_inspect_pr_notes_when_update_branch_head_is_not_observed(monkeypatch): ) +def test_inspect_pr_updates_outdated_branch_before_review_dispatch(monkeypatch): + updated = [] + dispatched = [] + old_head_pr = make_pr( + mergeStateStatus="BEHIND", + compareBehindBy=111, + statusCheckRollup={"contexts": {"nodes": [strix_check(), opencode_check(status="COMPLETED")]}}, + ) + new_head_pr = make_pr( + headRefOid="new-head", + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ) + + monkeypatch.setattr(sched, "update_branch", lambda repo, pr, dry_run: updated.append((repo, pr["headRefOid"], dry_run))) + monkeypatch.setattr(sched, "wait_for_updated_branch_head", lambda repo, pr: new_head_pr) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: dispatched.append((repo, workflow, pr["headRefOid"], dry_run)), + ) + + decision = inspect(old_head_pr, dry_run=False) + + assert decision.action == "update_branch" + assert decision.reason.startswith( + "current head has no OpenCode approval; branch is outdated before review dispatch" + ) + assert updated == [("owner/repo", "head", False)] + assert dispatched == [("owner/repo", "OpenCode Review", "new-head", False)] + assert decision.notes == ( + "updated head new-head observed after update-branch; same-head Strix evidence is complete, so OpenCode review was dispatched", + ) + + +def test_inspect_pr_update_before_review_dispatch_boundaries(): + behind_pr = make_pr(mergeStateStatus="BEHIND", compareBehindBy=3) + + disabled = inspect(behind_pr, update_branches=False) + assert disabled.action == "wait" + assert disabled.reason == "current head has no OpenCode approval; branch update disabled before review dispatch" + + external = inspect( + make_pr( + mergeStateStatus="BEHIND", + compareBehindBy=3, + isCrossRepository=True, + maintainerCanModify=False, + headRepository={"nameWithOwner": "fork/repo"}, + ) + ) + assert external.action == "wait" + assert external.reason == ( + "current head has no OpenCode approval; branch is outdated before review dispatch, " + "but head repo fork/repo is not writable by the scheduler credential" + ) + + blocked_but_behind = inspect( + make_pr( + mergeStateStatus="BLOCKED", + restMergeableState="BLOCKED", + compareBehindBy=7, + ) + ) + assert blocked_but_behind.action == "update_branch" + assert "base branch is 7 commit(s) ahead before review dispatch" in blocked_but_behind.reason + assert "GitHub mergeability is BLOCKED" in blocked_but_behind.reason + + def test_post_update_branch_followup_covers_dispatch_boundaries(monkeypatch): original = make_pr(headRefOid="old-head") opencode_dispatched = []