From 13b1c22314cfa01c2796a271beaa119c0ba7e5e9 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 19:20:38 +0900 Subject: [PATCH 1/6] fix(scheduler): retry gate-only OpenCode reviews --- CHANGELOG.md | 5 ++ docs/pr-review-and-merge-procedure.md | 8 +++ scripts/ci/pr_review_merge_scheduler.py | 42 +++++++++++---- tests/test_pr_review_merge_scheduler.py | 68 +++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9717d09a9..71c04f8a5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- Retry a current-head OpenCode review that requested changes solely because + GitHub Checks had failed once those checks recover. The scheduler requires + the exact check-gate review marker and an empty failed-check rollup before + dispatching a fresh review; ordinary review findings, disabled review + dispatch, and auto-merge requests remain blocked. - Give stacked pull requests a separately bounded organization-sweep OpenCode dispatch budget, so default-branch review traffic cannot leave a stacked PR at `OpenCode review absent` without changing the protected merge 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..ab49783e7d 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] = [] @@ -2525,16 +2543,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..aa302ccc69 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1428,6 +1428,74 @@ 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_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( From 28e89909263d3ef7baf66070be723b6c6e05258c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 19:26:53 +0900 Subject: [PATCH 2/6] test(sidecar): match pinned interpreter invocation --- tests/test_contextual_orchestrator_review_sidecar_contract.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_contextual_orchestrator_review_sidecar_contract.py b/tests/test_contextual_orchestrator_review_sidecar_contract.py index 499c12b466..96c84229eb 100644 --- a/tests/test_contextual_orchestrator_review_sidecar_contract.py +++ b/tests/test_contextual_orchestrator_review_sidecar_contract.py @@ -259,7 +259,7 @@ def test_sidecar_masks_gateway_token_before_startup_can_emit_logs() -> None: mask_index = text.index(mask) for later_operation in ( "git clone", - "python3 -m pip install", + '"$sidecar_python" -m pip install', '"$ORCHESTRATOR_WORK/launch_sidecar.py"', "healthz", ): From adaf7eec125cf9af2559dabebb1a330a844bec53 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 19:31:01 +0900 Subject: [PATCH 3/6] fix(scheduler): retry gate reviews on stacked PRs --- scripts/ci/pr_review_merge_scheduler.py | 6 +++++ tests/test_pr_review_merge_scheduler.py | 35 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index ab49783e7d..b1e5bf758b 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -2408,6 +2408,12 @@ 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) + if ( + can_retry_check_gated_opencode_review(pr) + 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: diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index aa302ccc69..c95428854e 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1462,6 +1462,41 @@ def test_retries_check_only_opencode_request_after_failed_checks_recover(monkeyp 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_check_gated_opencode_retry_stays_blocked_until_checks_recover(): """A gate-only request cannot bypass a still-failing check.""" review = { From b79f92705d0fe98d0679a31139dff58bf280e91b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 19:35:42 +0900 Subject: [PATCH 4/6] fix(scheduler): honor stacked auto-merge guard --- scripts/ci/pr_review_merge_scheduler.py | 1 + tests/test_pr_review_merge_scheduler.py | 33 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index b1e5bf758b..4b6c2c18bc 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -2411,6 +2411,7 @@ def inspect_pr( if ( can_retry_check_gated_opencode_review(pr) and trigger_reviews + and not pr.get("autoMergeRequest") and opencode_state != "running" ): opencode_state = "absent" diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index c95428854e..d849cac386 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1497,6 +1497,39 @@ def test_retries_check_only_opencode_request_for_stacked_pr(monkeypatch): 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="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 == "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 = { From 7e52d2e853d9f0f727fce348f0f848aa1359315a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 19:40:23 +0900 Subject: [PATCH 5/6] fix(scheduler): block stale stacked auto-merge retries --- scripts/ci/pr_review_merge_scheduler.py | 10 ++++------ tests/test_pr_review_merge_scheduler.py | 10 ++++++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 4b6c2c18bc..c9804b492e 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -2408,12 +2408,10 @@ 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) - if ( - can_retry_check_gated_opencode_review(pr) - and trigger_reviews - and not pr.get("autoMergeRequest") - and opencode_state != "running" - ): + 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) diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index d849cac386..6a874ea0be 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1513,7 +1513,13 @@ def test_stacked_check_gated_retry_does_not_bypass_auto_merge(monkeypatch): reviews={"nodes": [review]}, statusCheckRollup={ "contexts": { - "nodes": [opencode_check(status="COMPLETED"), strix_check()] + "nodes": [ + opencode_check( + status="IN_PROGRESS", + started_at="2026-06-25T07:00:00Z", + ), + strix_check(), + ] } }, ) @@ -1524,7 +1530,7 @@ def test_stacked_check_gated_retry_does_not_bypass_auto_merge(monkeypatch): lambda repo, workflow, pr, dry_run: dispatched.append((repo, workflow)) or "dispatched", ) - decision = inspect(pr) + decision = inspect(pr, stale_opencode_minutes=0) assert decision.action == "skip" assert dispatched == [] From 34682598252c444deeb251d08e132af010ff7d26 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 03:53:54 -0700 Subject: [PATCH 6/6] chore(ci): re-evaluate against repaired Strix trusted main