diff --git a/docs/pr-review-and-merge-procedure.md b/docs/pr-review-and-merge-procedure.md index 8da4703f32..7866f9851b 100644 --- a/docs/pr-review-and-merge-procedure.md +++ b/docs/pr-review-and-merge-procedure.md @@ -74,6 +74,14 @@ changes. OpenCode review evidence must be internally same-head as well as GitHub-attached same-head. If the review body includes `Gate evidence` with `Head SHA: `, that SHA must match the PR current `headRefOid`. +A current-head OpenCode `CHANGES_REQUESTED` review normally blocks the PR. The +only retry exception is the exact automation review stating that approval was +withheld because GitHub Checks failed and listing those failed checks. After +the live rollup is empty, the scheduler may dispatch a fresh same-head +OpenCode review when review dispatch is enabled and no native auto-merge +request is active. It never treats the recovered checks as approval, dismisses +the existing review, or merges until a new exact-head OpenCode approval exists. + ## Do-not-merge and DIRTY / CONFLICTING repair The `update_branch` path is deliberately not used for `DIRTY` or diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index d4fb7f468e..c9804b492e 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -131,6 +131,9 @@ GIT_SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") GITHUB_REPOSITORY_RE = re.compile(r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$") REVIEW_BODY_HEAD_SHA_RE = re.compile(r"Head SHA:\s*`([0-9a-fA-F]{40})`") +CHECK_GATED_OPENCODE_CHANGE_REQUEST_MARKER = ( + "OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed." +) ACTIONS_JOB_DETAILS_URL_RE = re.compile(r"/actions/runs/\d+/job/(\d+)(?:[/?#]|$)") DIRECT_MERGE_AUTO_FALLBACK_MARKERS = ( "base branch policy prohibits the merge", @@ -1277,6 +1280,21 @@ def has_current_head_changes_requested(pr: dict[str, Any]) -> bool: return current_head_review_state(pr, "CHANGES_REQUESTED") +def can_retry_check_gated_opencode_review(pr: dict[str, Any]) -> bool: + """Return whether recovered checks justify replacing a gate-only request.""" + for review in reversed((pr.get("reviews") or {}).get("nodes") or []): + if not is_opencode_review(review) or not review_matches_current_head(review, pr): + continue + body = str(review.get("body") or "") + return ( + (review.get("state") or "").upper() == "CHANGES_REQUESTED" + and CHECK_GATED_OPENCODE_CHANGE_REQUEST_MARKER in body + and "Failed checks:" in body + and not failed_status_checks(pr) + ) + return False + + def stale_opencode_change_request_ids(pr: dict[str, Any]) -> list[int]: """Return dismissible automated change requests tied to previous heads.""" review_ids: list[int] = [] @@ -2390,6 +2408,11 @@ def inspect_pr( # Merge automation stays default-branch-only; rulesets do not gate # feature-branch merges. opencode_state = opencode_progress_state(pr, stale_after_minutes=stale_opencode_minutes) + check_gated_retry = can_retry_check_gated_opencode_review(pr) + if check_gated_retry and pr.get("autoMergeRequest"): + opencode_state = "complete" + elif check_gated_retry and trigger_reviews and opencode_state != "running": + opencode_state = "absent" if opencode_state in {"absent", "stale"} and trigger_reviews and review_dispatch_allowed: wait_reason = repository_dispatch_wait_reason(repo, workflow) if wait_reason: @@ -2525,16 +2548,22 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio return request_branch_update( "current-head OpenCode review requested changes; branch is outdated before re-review" ) - if pr.get("autoMergeRequest"): - return finish( - disable_auto_merge_decision( - repo, - pr, - dry_run=dry_run, - reason="current-head OpenCode review requested changes; address the review before re-enabling auto-merge", + if not ( + can_retry_check_gated_opencode_review(pr) + and trigger_reviews + and review_dispatch_allowed + and not pr.get("autoMergeRequest") + ): + if pr.get("autoMergeRequest"): + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason="current-head OpenCode review requested changes; address the review before re-enabling auto-merge", + ) ) - ) - return decide("block", "current-head OpenCode review requested changes") + return decide("block", "current-head OpenCode review requested changes") current_head_approved = has_current_head_approval(pr) if current_head_approved: diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index f3877a4b5d..6a874ea0be 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1428,6 +1428,148 @@ def test_workflow_run_followup_defers_deterministic_fallback_retry(monkeypatch): assert dispatched == [("owner/repo", "OpenCode Review", head, True)] +def test_retries_check_only_opencode_request_after_failed_checks_recover(monkeypatch): + """A recovered external check gate must receive a fresh model review.""" + review = { + **opencode_review("CHANGES_REQUESTED", "head"), + "body": ( + "OpenCode could not approve from deterministic current-head evidence " + "because GitHub Checks have failed.\n\n" + "Failed checks:\n- strix: FAILURE" + ), + } + pr = make_pr( + reviews={"nodes": [review]}, + statusCheckRollup={ + "contexts": { + "nodes": [opencode_check(status="COMPLETED"), strix_check()] + } + }, + ) + dispatched = [] + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: dispatched.append((repo, workflow)) or "dispatched", + ) + + decision = inspect(pr) + + assert decision.action == "review_dispatch" + assert decision.reason == ( + "current head has completed Strix evidence; same-head OpenCode dispatched" + ) + assert dispatched == [("owner/repo", "OpenCode Review")] + + +def test_retries_check_only_opencode_request_for_stacked_pr(monkeypatch): + """Stacked PRs also receive a fresh review after gate checks recover.""" + review = { + **opencode_review("CHANGES_REQUESTED", "head"), + "body": ( + "OpenCode could not approve from deterministic current-head evidence " + "because GitHub Checks have failed.\n\n" + "Failed checks:\n- strix: FAILURE" + ), + } + pr = make_pr( + baseRefName="feature-base", + reviews={"nodes": [review]}, + statusCheckRollup={ + "contexts": { + "nodes": [opencode_check(status="COMPLETED"), strix_check()] + } + }, + ) + dispatched = [] + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: dispatched.append((repo, workflow)) or "dispatched", + ) + + decision = inspect(pr) + + assert decision.action == "review_dispatch" + assert decision.reason == ( + "stacked PR onto feature-base; OpenCode review dispatched" + ) + assert dispatched == [("owner/repo", "OpenCode Review")] + + +def test_stacked_check_gated_retry_does_not_bypass_auto_merge(monkeypatch): + """An active auto-merge request prevents stacked retry dispatch.""" + review = { + **opencode_review("CHANGES_REQUESTED", "head"), + "body": ( + "OpenCode could not approve from deterministic current-head evidence " + "because GitHub Checks have failed.\n\n" + "Failed checks:\n- strix: FAILURE" + ), + } + pr = make_pr( + baseRefName="feature-base", + autoMergeRequest={"enabledAt": "now"}, + reviews={"nodes": [review]}, + statusCheckRollup={ + "contexts": { + "nodes": [ + opencode_check( + status="IN_PROGRESS", + started_at="2026-06-25T07:00:00Z", + ), + strix_check(), + ] + } + }, + ) + dispatched = [] + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: dispatched.append((repo, workflow)) or "dispatched", + ) + + decision = inspect(pr, stale_opencode_minutes=0) + + assert decision.action == "skip" + assert dispatched == [] + + +def test_check_gated_opencode_retry_stays_blocked_until_checks_recover(): + """A gate-only request cannot bypass a still-failing check.""" + review = { + **opencode_review("CHANGES_REQUESTED", "head"), + "body": ( + "OpenCode could not approve from deterministic current-head evidence " + "because GitHub Checks have failed.\n\n" + "Failed checks:\n- strix: FAILURE" + ), + } + pr = make_pr( + reviews={"nodes": [review]}, + statusCheckRollup={ + "contexts": { + "nodes": [ + opencode_check(status="COMPLETED"), + strix_check(conclusion="FAILURE"), + ] + } + }, + ) + + assert not sched.can_retry_check_gated_opencode_review(pr) + assert not sched.can_retry_check_gated_opencode_review(make_pr()) + assert not sched.can_retry_check_gated_opencode_review( + make_pr(reviews={"nodes": [opencode_review("CHANGES_REQUESTED", "old")]}) + ) + assert inspect(pr).action == "block" + assert inspect(pr, trigger_reviews=False).action == "block" + assert inspect(pr, review_dispatch_allowed=False).action == "block" + auto_merge_pr = {**pr, "autoMergeRequest": {"enabledAt": "now"}} + assert inspect(auto_merge_pr).action == "disable_auto_merge" + + def test_body_head_sha_approval_prevents_same_run_opencode_rerun(monkeypatch): head = "a" * 40 pr = make_pr(