diff --git a/CHANGELOG.md b/CHANGELOG.md index 44145d4ec..5e4214b2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -636,6 +636,11 @@ Semantic Versioning where the repository publishes a release. ### Changed +- Require the PR Review Merge Scheduler to observe both GitHub's aggregate + `APPROVED` decision and the latest effective non-author, non-OpenCode formal + approval bound to the exact live head before direct merge or auto-merge. + A later same-head change request revokes that reviewer's earlier approval, + and existing auto-merge is disarmed when either authorization is absent. - Emit completed repository pull-list requests as they finish in the five-minute agent-mention sweep, while retaining the four-worker ceiling, rotation, and exact-name dispatch ledger, so one slow repository cannot hide ready sibling @@ -654,6 +659,15 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Keep the central required-workflow coverage placeholder from superseding a + failed repository-dispatch coverage run; coverage retry and merge decisions + now use authoritative execution evidence for the central scheduler. +- Re-dispatch an exact-head OpenCode review after its coverage-only blocker is + cleared, selecting the newest coverage rerun by timestamp across workflow + names and ignoring only the superseded `opencode-review` failure and central + required-workflow placeholder. Conflicting heads and failed sibling jobs in an + OpenCode workflow remain fail-closed alongside unresolved threads, Strix, + coverage, and unrelated failed checks. - Web verification now checks services through local readiness addresses only. Start the backend and frontend on this computer and use their local health URLs when running the check. diff --git a/docs/doctoring/scheduler-independent-current-head-approval.md b/docs/doctoring/scheduler-independent-current-head-approval.md new file mode 100644 index 000000000..ab7c5f205 --- /dev/null +++ b/docs/doctoring/scheduler-independent-current-head-approval.md @@ -0,0 +1,60 @@ +# Scheduler independent exact-head approval + +## Decision + +The organization merge scheduler fails closed unless both of these statements +are true: + +1. GitHub reports the pull request's aggregate `reviewDecision` as `APPROVED`. +2. A formal `APPROVED` review from a non-author, non-OpenCode identity is bound + to the exact live head SHA. + +Exact-head OpenCode approval remains a review gate, not independent merge +authority. Missing identities, author self-review, generic GitHub Actions +reviews, comment-only reviews, predecessor-head approvals, and absent aggregate +state never satisfy this control. + +## Root cause and repair + +The prior scheduler could call direct merge or enable native auto-merge after an +exact-head OpenCode approval without first proving repository approval state or +an independent exact-head review. A credential with bypass capability could +therefore turn advisory automation evidence into merge authority. + +The repair reuses the existing scheduler and review/head matcher: + +- the existing GitHub query now includes the pull-request author; +- the REST fallback records the author but remains fail closed because it does + not provide an authoritative aggregate review decision; +- one helper filters exact-head formal approvals by author and automation + identity, considering only each reviewer's latest approval-affecting state so + a later change request or dismissal revokes that reviewer's earlier approval; +- both direct/automatic merge paths share the same authorization reason; and +- an already armed auto-merge request is disabled when authorization is absent. + +Checks, security evidence, unresolved conversations, mergeability, branch +freshness, and expected-head merge protection remain independent blockers. +GitHub's server-side last-pusher, CODEOWNERS, required-review, and ruleset checks +remain authoritative; the scheduler does not infer those identities. + +## Verification and operations + +Regression cases cover missing author/reviewer identity, self-review, generic +Actions review, OpenCode review, non-approval, stale head, a later same-head +change request, missing aggregate approval, disarming existing auto-merge, and +the complete authorized direct merge path. Before lifecycle action, refetch the +live base, head, reviews, threads, checks, and rules. A head change invalidates +all predecessor evidence. + +Rollback means reverting the reviewed scheduler change while keeping scheduler +merge modes disabled until an equivalent fail-closed authorization control is +available. Never roll back by lowering required-review counts or granting a +bypass. + +## References + +GitHub. (n.d.). *About protected branches*. GitHub Docs. Retrieved August 24, +2026, from https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches + +GitHub. (n.d.). *Available rules for rulesets*. GitHub Docs. Retrieved August +24, 2026, from https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets/available-rules-for-rulesets diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 4644d3e78..e91d37b1e 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -15,7 +15,7 @@ import time from collections.abc import Iterator, Sequence from dataclasses import dataclass -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone from typing import Any from urllib.parse import quote @@ -24,6 +24,7 @@ fragment SchedulerPullRequestFields on PullRequest { number title + author { login } isDraft mergeable mergeStateStatus @@ -53,12 +54,13 @@ nodes { path } } reviews(last: 100) { + pageInfo { hasPreviousPage startCursor } nodes { databaseId state body submittedAt - author { login } + author { login __typename } commit { oid } } } @@ -73,6 +75,7 @@ startedAt detailsUrl checkSuite { + createdAt workflowRun { workflow { name } } @@ -111,12 +114,43 @@ } """ + PULL_REQUEST_FIELDS_FRAGMENT +# Follow-up query for one pull request's reviews, walking backward past the +# ``reviews(last: 100)`` window in SchedulerPullRequestFields. GraphQL +# connections keep chronological (oldest-first) node order regardless of +# pagination direction, so ``last: 100, before: $cursor`` returns the up-to-100 +# reviews immediately preceding the cursor, still oldest-first. +PR_REVIEWS_PAGE_QUERY = """\ +query($owner: String!, $name: String!, $number: Int!, $cursor: String!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + reviews(last: 100, before: $cursor) { + pageInfo { hasPreviousPage startCursor } + nodes { + databaseId + state + body + submittedAt + author { login __typename } + commit { oid } + } + } + } + } +} +""" + OPEN_PRS_PAGE_SIZE = 25 +# Defends against a pathological GraphQL pageInfo loop when backfilling a PR's +# full review history; 500 pages * 100 reviews/page is far beyond any +# realistic PR review count, so hitting it indicates a bug upstream rather +# than a PR that legitimately needs more pagination. +MAX_REVIEW_PAGINATION_PAGES = 500 # Must exceed the 45-minute OpenCode job cap plus typical runner-queue wait. # QUEUED counts as running and the age clock starts at check creation, so this # remains deliberately larger than the job cap while recovering genuine zombie # checks in the same operating window instead of leaving them for seven hours. DEFAULT_STALE_OPENCODE_MINUTES = 90 +DEFAULT_COVERAGE_RETRY_FLOOR_MINUTES = 60 DEFAULT_UPDATE_BRANCH_HEAD_POLL_ATTEMPTS = 6 DEFAULT_UPDATE_BRANCH_HEAD_POLL_SECONDS = 5.0 OPENCODE_WORKFLOW_NAMES = { @@ -162,6 +196,11 @@ "deterministic fallback approval", "did not emit a usable current-head control block", ) +COVERAGE_REVIEW_MARKERS = ( + "coverage evidence did not pass", + "coverage-evidence", + "required test/docstring evidence", +) LAST_PUSH_APPROVAL_RESTAMP_MESSAGE = "chore: refresh head for last-push approval" @@ -753,6 +792,69 @@ def gh_graphql(query: str, **fields: str | int) -> dict[str, Any]: time.sleep(delay) +def complete_paginated_pr_reviews( + owner: str, name: str, number: int, reviews: dict[str, Any] +) -> dict[str, Any]: + """Backfill one pull request's review history past the GraphQL 100-node window. + + ``reviews(last: 100)`` in SchedulerPullRequestFields only returns the + newest 100 reviews on a pull request. Once a PR accumulates more than 100 + review events (bot reviewers post multiple reviews per push in this org), + ``pageInfo.hasPreviousPage`` comes back true and earlier reviews -- + including a genuine independent APPROVED review made early in the PR's + life -- are silently missing from ``nodes``. This walks backward with + ``before`` cursors via PR_REVIEWS_PAGE_QUERY, merging each page in front of + the ones already collected so the result stays oldest-first (the order + every ``reversed(...)`` consumer in this module expects), until GitHub + reports no earlier page. A page-fetch failure propagates (fail closed) + rather than returning a partial history. + """ + page_info = reviews.get("pageInfo") or {} + nodes = list(reviews.get("nodes") or []) + pages_fetched = 0 + while page_info.get("hasPreviousPage"): + pages_fetched += 1 + if pages_fetched > MAX_REVIEW_PAGINATION_PAGES: + raise RuntimeError( + f"Pull request {owner}/{name}#{number} review pagination exceeded " + f"{MAX_REVIEW_PAGINATION_PAGES} pages without exhausting hasPreviousPage; " + "refusing to loop indefinitely." + ) + cursor = page_info.get("startCursor") + if not cursor: + raise RuntimeError( + f"Pull request {owner}/{name}#{number} reported hasPreviousPage=true " + "without a startCursor; cannot continue review pagination." + ) + payload = gh_graphql( + PR_REVIEWS_PAGE_QUERY, owner=owner, name=name, number=number, cursor=cursor + ) + pull_request = ((payload.get("data") or {}).get("repository") or {}).get( + "pullRequest" + ) or {} + page = pull_request.get("reviews") or {} + nodes = list(page.get("nodes") or []) + nodes + page_info = page.get("pageInfo") or {} + return {"nodes": nodes} + + +def complete_all_pr_reviews(owner: str, name: str, prs: list[dict[str, Any]]) -> None: + """Backfill full review history in place for every fetched PR node that needs it. + + Only PRs whose initial ``reviews(last: 100)`` window reported + ``hasPreviousPage`` pay the extra round trip; PRs with 100 or fewer + reviews (the overwhelming majority) are untouched. + """ + for pr in prs: + reviews = pr.get("reviews") + if not reviews: + continue + if (reviews.get("pageInfo") or {}).get("hasPreviousPage"): + pr["reviews"] = complete_paginated_pr_reviews( + owner, name, pr.get("number"), reviews + ) + + def github_resource_inaccessible(exc: RuntimeError) -> bool: """Return whether GitHub denied an API read for the current integration token.""" @@ -796,9 +898,43 @@ def rest_review_node(review: dict[str, Any]) -> dict[str, Any]: } -def rest_check_node(check: dict[str, Any]) -> dict[str, Any]: - """Convert a REST check-run payload into the GraphQL status rollup shape.""" +def fetch_all_pr_reviews_rest(repo: str, number: int) -> list[dict[str, Any]]: + """Fetch every REST review for a pull request, paginating past 100. + + A single ``per_page=100`` page silently drops earlier reviews once a PR + accumulates more than 100 review events, the same truncation the GraphQL + ``reviews(last: 100)`` window hits. This walks ``page=1,2,3,...`` -- + mirroring ``fetch_open_prs_rest``'s pagination style -- until a page + shorter than 100 rows confirms the end of the history. A page-fetch + failure propagates (fail closed) rather than returning a partial history. + """ + reviews: list[dict[str, Any]] = [] + page = 1 + while True: + batch = gh_api_json(f"repos/{repo}/pulls/{number}/reviews?per_page=100&page={page}") + if not batch: + break + reviews.extend(batch) + if len(batch) < 100: + break + page += 1 + return reviews + +def rest_check_node( + check: dict[str, Any], suite_created_at_by_id: dict[int, str] | None = None +) -> dict[str, Any]: + """Convert a REST check-run payload into the GraphQL status rollup shape. + + ``suite_created_at_by_id`` maps each check suite's REST ``id`` to its + ``created_at`` timestamp -- the REST check-run payload itself only + carries the check suite's bare ``id`` (see ``rest_pr_node``), so that + lookup is how this function attaches the same ``checkSuite.createdAt`` + signal the GraphQL fragment fetches directly, keeping + ``check_run_recency_key`` behaviorally consistent across both paths. + """ + suite_id = (check.get("check_suite") or {}).get("id") + suite_created_at = (suite_created_at_by_id or {}).get(suite_id) return { "__typename": "CheckRun", "name": check.get("name"), @@ -806,7 +942,7 @@ def rest_check_node(check: dict[str, Any]) -> dict[str, Any]: "conclusion": (check.get("conclusion") or "").upper() if check.get("conclusion") else None, "startedAt": check.get("started_at"), "detailsUrl": check.get("details_url"), - "checkSuite": {"workflowRun": {"workflow": {}}}, + "checkSuite": {"createdAt": suite_created_at, "workflowRun": {"workflow": {}}}, } @@ -827,8 +963,14 @@ def rest_pr_node(repo: str, pr: dict[str, Any]) -> dict[str, Any]: head = pr.get("head") or {} base = pr.get("base") or {} head_repo = head.get("repo") or {} - reviews = gh_api_json(f"repos/{repo}/pulls/{number}/reviews?per_page=100") + reviews = fetch_all_pr_reviews_rest(repo, number) checks = gh_api_json(f"repos/{repo}/commits/{head.get('sha')}/check-runs?per_page=100") + check_suites = gh_api_json(f"repos/{repo}/commits/{head.get('sha')}/check-suites?per_page=100") + suite_created_at_by_id = { + suite["id"]: suite.get("created_at") + for suite in (check_suites.get("check_suites") or []) + if suite.get("id") is not None + } statuses = gh_api_json(f"repos/{repo}/commits/{head.get('sha')}/statuses?per_page=100") files = gh_api_json(f"repos/{repo}/pulls/{number}/files?per_page=20") rest_merge_state = REST_MERGEABLE_STATE_MAP.get( @@ -838,6 +980,7 @@ def rest_pr_node(repo: str, pr: dict[str, Any]) -> dict[str, Any]: return { "number": number, "title": pr.get("title"), + "author": {"login": ((pr.get("user") or {}).get("login"))}, "isDraft": bool(pr.get("draft")), "mergeable": pr.get("mergeable"), "mergeStateStatus": rest_merge_state, @@ -856,7 +999,7 @@ def rest_pr_node(repo: str, pr: dict[str, Any]) -> dict[str, Any]: "statusCheckRollup": { "contexts": { "nodes": [ - rest_check_node(check) + rest_check_node(check, suite_created_at_by_id) for check in (checks.get("check_runs") or []) ] + [ @@ -932,6 +1075,11 @@ def fetch_open_prs(repo: str, max_prs: int) -> list[dict[str, Any]]: break cursor = pr_page["pageInfo"]["endCursor"] + # Bulk-scan results feed merge decisions directly (the scheduler's push- + # triggered and org-queue-sweep runs never re-fetch a single PR before + # calling inspect_pr), so this path needs the same full review history as + # fetch_pr, not just the first/last 100-review window. + complete_all_pr_reviews(owner, name, prs) enrich_rest_mergeable_states(repo, prs) return prs @@ -947,6 +1095,7 @@ def fetch_pr(repo: str, number: int) -> list[dict[str, Any]]: raise pr = payload["data"]["repository"].get("pullRequest") prs = [pr] if pr else [] + complete_all_pr_reviews(owner, name, prs) enrich_rest_mergeable_states(repo, prs) return prs @@ -1056,6 +1205,20 @@ def context_nodes(pr: dict[str, Any]) -> list[dict[str, Any]]: return contexts.get("nodes") or [] +def is_opencode_check_run(node: dict[str, Any]) -> bool: + """Return whether a CheckRun carries the OpenCode workflow identity.""" + if node.get("__typename") != "CheckRun": + return False + workflow = ( + ((node.get("checkSuite") or {}).get("workflowRun") or {}).get("workflow") + or {} + ) + return ( + node.get("name") == "opencode-review" + or workflow.get("name") in OPENCODE_WORKFLOW_NAMES + ) + + def is_opencode_context(node: dict[str, Any]) -> bool: """Return whether a check or status context belongs to OpenCode Review.""" if node.get("__typename") == "CheckRun": @@ -1064,11 +1227,7 @@ def is_opencode_context(node: dict[str, Any]) -> bool: # status. Organization required-workflow CheckRuns are deliberately # non-authoritative placeholders and must not suppress that dispatch. return False - workflow = ( - ((node.get("checkSuite") or {}).get("workflowRun") or {}).get("workflow") - or {} - ) - return node.get("name") == "opencode-review" or workflow.get("name") in OPENCODE_WORKFLOW_NAMES + return is_opencode_check_run(node) return node.get("context") == "opencode-review" @@ -1118,6 +1277,119 @@ def parse_github_datetime(value: str | None) -> datetime | None: return parsed.astimezone(timezone.utc) +def check_run_recency_key( + node: dict[str, Any], started_at: datetime | None, index: int +) -> tuple[int, datetime, int]: + """Return a single comparable recency key for one same-purpose check run. + + Ranking a sequence of same-purpose check runs (either the reruns sharing + one (workflow, name) key in ``latest_check_runs``, or the + coverage-evidence runs ``latest_coverage_evidence_index`` compares across + workflow names) down to the single newest one used to be done by folding + a pairwise "does B supersede A" predicate left-to-right across the + candidates. That is only valid when the predicate is a transitive total + order, and it was not: a queued/null-``startedAt`` candidate could + legitimately supersede an older completed predecessor, but a later, + differently-timestamped completed candidate could then override that + queued winner too -- purely because "a timestamped candidate beats a + null-timestamp current-best" -- even when the later candidate was itself + older than whichever run the queued candidate had already displaced. + + Building one derived key per candidate instead, and comparing those keys + directly, cannot go non-transitive: Python tuple ordering is already a + valid total order, so ``max()``/``sorted()`` over these keys give a + result that does not depend on candidate order. + + A ``startedAt``-only signal has a gap: GitHub reports a check run as + ``completed``/``cancelled`` with ``startedAt: null`` when a queued rerun + is cancelled before it ever starts, so that row carries no timestamp and + is not pending either -- nothing (short of hardcoding the ``cancelled`` + conclusion, which would only patch this one case) distinguishes it from + a run that legitimately never mattered. ``checkSuite.createdAt`` closes + that gap generally instead of special-casing it: GitHub creates the + check suite unconditionally the moment the triggering push, rerun, or + dispatch happens, strictly before any check run inside it can be queued, + start, or be cancelled before starting, and ``CheckSuite.createdAt`` is + non-nullable in GitHub's schema. So it is a recency signal that is + always available, for every check run regardless of how it resolved -- + unlike ``startedAt``, which is genuinely absent for a run that never + started. + + Three tiers, low to high: + + * ``0`` -- no recency signal at all: neither the check run's own check + suite ``createdAt`` nor its ``startedAt`` is available, and it is not + currently pending either. Real GitHub responses always carry + ``checkSuite.createdAt``, so this tier is only reachable for + payloads that omit it (e.g. hand-built fixtures). + * ``1`` -- a real timestamp: the check run's own check suite + ``createdAt`` when present, else its ``startedAt``. Preferring the + check-suite timestamp means two runs are ranked by when each was + actually triggered, not by whether either one got far enough to + start -- a rerun cancelled before starting still ranks correctly + relative to an older, already-completed run. + * ``2`` -- no timestamp of any kind, but actively pending (queued/in + progress/etc, via ``running_check_state``): kept only as the + fallback for payloads without ``checkSuite.createdAt``, where GitHub + only ever creates such a row after any run it might supersede, so it + is presumed newer than every already-resolved run in that same + payload shape, regardless of that run's timestamp. + + Ties within a tier fall back to the later index, matching the order + ``context_nodes`` returns them in. + """ + epoch = datetime.min.replace(tzinfo=timezone.utc) + suite_created_at = parse_github_datetime((node.get("checkSuite") or {}).get("createdAt")) + recency_timestamp = suite_created_at or started_at + if recency_timestamp is not None: + return (1, recency_timestamp, index) + if running_check_state(node) == "running": + return (2, epoch, index) + return (0, epoch, index) + + +def _newest_check_run_per_identity( + indexed_check_runs: Sequence[tuple[int, dict[str, Any]]] +) -> list[tuple[int, dict[str, Any]]]: + """Return the newest CheckRun per (workflow, name) identity, index-tagged. + + Shared core for ``latest_check_runs`` (which keeps only CheckRun nodes) + and ``latest_check_run_attempts`` (which also passes non-CheckRun nodes + through unchanged): both resolve CheckRun reruns sharing one + (workflow, name) identity down to the single newest attempt, and both + must rank candidates with the identical ``check_run_recency_key`` signal + so they cannot silently diverge again the way ``latest_check_run_attempts`` + once did with its own ``startedAt``-only comparison. Each input + ``(index, node)`` pair's original position is preserved in the return + value so callers can restore overall document order after merging back + any non-CheckRun nodes. + """ + latest: dict[tuple[str, str], tuple[tuple[int, datetime, int], int, dict[str, Any]]] = {} + for index, node in indexed_check_runs: + workflow = ( + (((node.get("checkSuite") or {}).get("workflowRun") or {}).get("workflow") or {}).get("name") + or "" + ) + key = (workflow, node.get("name") or "check-run") + started_at = parse_github_datetime(node.get("startedAt")) + recency_key = check_run_recency_key(node, started_at, index) + previous = latest.get(key) + if previous is None or recency_key >= previous[0]: + latest[key] = (recency_key, index, node) + return [(index, node) for _, index, node in latest.values()] + + +def latest_check_runs(pr: dict[str, Any]) -> list[dict[str, Any]]: + """Return the newest check run for each workflow and check-name pair.""" + indexed_check_runs = [ + (index, node) + for index, node in enumerate(context_nodes(pr)) + if node.get("__typename") == "CheckRun" + ] + deduped = _newest_check_run_per_identity(indexed_check_runs) + return [node for _, node in sorted(deduped, key=lambda item: item[0])] + + def review_matches_current_head(review: dict[str, Any], pr: dict[str, Any]) -> bool: """Return whether a review is valid evidence for the current head commit.""" head = pr.get("headRefOid") @@ -1187,42 +1459,31 @@ def latest_check_run_attempts(nodes: list[dict[str, Any]]) -> list[dict[str, Any """Return each CheckRun's most recent attempt per (workflow, name) identity. A rerun leaves every earlier attempt's CheckRun node in the rollup - alongside the latest one, so callers that walk ``context_nodes`` directly - can see a stale failed attempt outlive a later successful retry. This - resolves each CheckRun identity to only its most recently started - attempt (falling back to rollup order when ``startedAt`` is missing), - while passing every non-CheckRun (classic commit-status) node through - unchanged. The result preserves the original relative ordering. + alongside the latest one, so callers that walk ``nodes`` directly can see + a stale failed attempt outlive a later successful retry. This used to + resolve each CheckRun identity with its own inline ``startedAt``-only + comparison, which had the same gap ``check_run_recency_key`` documents + for ``latest_check_runs``: GitHub reports a rerun cancelled before it + ever started as completed with ``startedAt: null``, so that row carried + no timestamp and could never outrank an older, already-completed + attempt -- even though it was the genuinely newer one. This now shares + the exact ``check_run_recency_key`` ranking (via + ``_newest_check_run_per_identity``) that ``latest_check_runs`` uses -- + preferring ``checkSuite.createdAt`` over ``startedAt``, with a + "currently pending" fallback tier -- so the two dedup passes rank + CheckRun reruns identically and cannot silently diverge again. Every + non-CheckRun (classic commit-status) node is passed through unchanged: + classic commit statuses never appear as duplicate reruns in + ``context_nodes``, so no dedup is needed for them. The result preserves + the original relative ordering. """ - latest: dict[tuple[str, str], tuple[datetime | None, int, dict[str, Any]]] = {} - ordered: list[tuple[int, dict[str, Any]]] = [] - for index, node in enumerate(nodes): - if node.get("__typename") != "CheckRun": - ordered.append((index, node)) - continue - workflow = ( - (((node.get("checkSuite") or {}).get("workflowRun") or {}).get("workflow") or {}).get("name") - or "" - ) - key = (workflow, node.get("name") or "check-run") - started_at = parse_github_datetime(node.get("startedAt")) - previous = latest.get(key) - if previous is None: - latest[key] = (started_at, index, node) - continue - previous_started_at, previous_index, _ = previous - if started_at is None and previous_started_at is not None: - continue - if previous_started_at is None and started_at is not None: - latest[key] = (started_at, index, node) - continue - if (started_at or datetime.min.replace(tzinfo=timezone.utc), index) >= ( - previous_started_at or datetime.min.replace(tzinfo=timezone.utc), - previous_index, - ): - latest[key] = (started_at, index, node) - for started_at, index, node in latest.values(): - ordered.append((index, node)) + indexed_check_runs = [ + (index, node) for index, node in enumerate(nodes) if node.get("__typename") == "CheckRun" + ] + ordered: list[tuple[int, dict[str, Any]]] = [ + (index, node) for index, node in enumerate(nodes) if node.get("__typename") != "CheckRun" + ] + ordered.extend(_newest_check_run_per_identity(indexed_check_runs)) ordered.sort(key=lambda item: item[0]) return [node for _, node in ordered] @@ -1337,6 +1598,23 @@ def review_author_login(review: dict[str, Any]) -> str: return ((review.get("author") or {}).get("login") or "").lower() +def is_bot_review_author(review: dict[str, Any]) -> bool: + """Return whether a review's author is a GitHub bot actor. + + GitHub's REST API appends the ``[bot]`` suffix to a bot actor's + ``login`` (e.g. ``dependabot[bot]``), but GitHub's GraphQL API can + return the bare account name for that same actor (e.g. ``dependabot``) + while exposing ``__typename: "Bot"`` on the ``author`` field instead of + the suffix. Checking both keeps bot exclusion correct regardless of + which API surface -- and which suffix convention -- produced the + review node; ``rest_review_node`` never sets ``__typename``, so REST + reviews continue to rely solely on the login suffix. + """ + if review_author_login(review).endswith("[bot]"): + return True + return ((review.get("author") or {}).get("__typename")) == "Bot" + + def is_opencode_review(review: dict[str, Any]) -> bool: """Return whether a review was authored by the OpenCode agent.""" return review_author_login(review) in {"opencode-agent", "opencode-agent[bot]"} @@ -1393,11 +1671,191 @@ def has_current_head_approval(pr: dict[str, Any]) -> bool: return current_head_review_state(pr, "APPROVED") +def has_independent_current_head_approval(pr: dict[str, Any]) -> bool: + """Return whether an eligible reviewer's latest exact-head policy state approves.""" + author = ((pr.get("author") or {}).get("login") or "").lower() + if not author: + return False + seen_reviewers: set[str] = set() + for review in reversed((pr.get("reviews") or {}).get("nodes") or []): + reviewer = review_author_login(review) + state = (review.get("state") or "").upper() + if ( + not reviewer + or reviewer == author + or is_automated_opencode_review(review) + or reviewer == "github-actions" + or is_bot_review_author(review) + or not review_matches_current_head(review, pr) + or state not in {"APPROVED", "CHANGES_REQUESTED", "DISMISSED"} + or reviewer in seen_reviewers + ): + continue + seen_reviewers.add(reviewer) + if state == "APPROVED": + return True + return False + + +def merge_approval_block_reason(pr: dict[str, Any]) -> str | None: + """Return the fail-closed repository and independent approval blocker.""" + review_decision = str(pr.get("reviewDecision") or "").upper() + if review_decision != "APPROVED": + return ( + "current-head OpenCode review approved, but GitHub reviewDecision is " + f"{review_decision or ''}; repository approval policy is unsatisfied" + ) + if not has_independent_current_head_approval(pr): + return ( + "current-head OpenCode review approved, but no independent non-author " + "exact-current-head formal APPROVED review exists" + ) + return None + + def has_current_head_changes_requested(pr: dict[str, Any]) -> bool: """Return whether OpenCode requested changes on the exact current head.""" return current_head_review_state(pr, "CHANGES_REQUESTED") +def latest_current_head_coverage_change_request( + pr: dict[str, Any], +) -> dict[str, Any] | None: + """Return the latest exact-head OpenCode request that only cites coverage.""" + 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 + if (review.get("state") or "").upper() != "CHANGES_REQUESTED": + return None + body = (review.get("body") or "").lower() + return review if all(marker in body for marker in COVERAGE_REVIEW_MARKERS) else None + return None + + +def current_head_coverage_change_request(pr: dict[str, Any]) -> bool: + """Return whether the latest current-head request is only a coverage gate.""" + return latest_current_head_coverage_change_request(pr) is not None + + +def coverage_retry_wait_reason( + pr: dict[str, Any], + *, + repo: str | None = None, + workflow: str | None = None, + now: datetime | None = None, + floor_minutes: int = DEFAULT_COVERAGE_RETRY_FLOOR_MINUTES, +) -> str | None: + """Return a wait reason until one same-head coverage retry interval elapses. + + The latest exact-head review submission or completed dispatch timestamp is the + durable same-head retry marker. Missing or malformed timestamps fail closed so + a repeated coverage-only review cannot create an unbounded dispatch loop. + """ + review = latest_current_head_coverage_change_request(pr) + if review is None: + return None + submitted_at = parse_github_datetime(review.get("submittedAt")) + if submitted_at is None: + return "current-head OpenCode coverage review has no valid submission timestamp; defer same-head re-review" + retry_anchor = submitted_at + if repo and workflow: + try: + dispatch_started_at = latest_opencode_dispatch_started_at( + repo, workflow, pr, since=retry_anchor + ) + except RuntimeError: + return "same-head OpenCode dispatch history is unavailable; defer same-head re-review" + if dispatch_started_at and dispatch_started_at > retry_anchor: + retry_anchor = dispatch_started_at + current_time = now or datetime.now(timezone.utc) + if current_time < retry_anchor + timedelta(minutes=max(0, floor_minutes)): + return "same-head OpenCode coverage retry floor has not elapsed" + return None + + +def is_non_authoritative_coverage_check_run(node: dict[str, Any]) -> bool: + """Return whether central metadata-only coverage evidence is non-authoritative.""" + if not (os.environ.get("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY") or "").strip(): + return False + if (node.get("name") or "").lower() != "coverage-evidence": + return False + workflow = ( + ((node.get("checkSuite") or {}).get("workflowRun") or {}).get("workflow") + or {} + ) + return workflow.get("name") == "Required OpenCode Review" + + +def coverage_evidence_indices(check_runs: Sequence[dict[str, Any]]) -> list[int]: + """Return indexes of coverage-evidence checks in one check-run snapshot.""" + return [ + index + for index, node in enumerate(check_runs) + if (node.get("name") or "").lower() == "coverage-evidence" + and not is_non_authoritative_coverage_check_run(node) + ] + + +def latest_coverage_evidence_index(check_runs: Sequence[dict[str, Any]]) -> int | None: + """Return the newest coverage-evidence index across workflow names. + + Ranks every coverage-evidence candidate with ``check_run_recency_key`` + and picks the single largest key via ``max()``, so a freshly QUEUED + coverage-evidence rerun (``startedAt: null``) in one workflow correctly + outranks an older, already-completed coverage-evidence run in a + *different* workflow instead of losing a naive timestamp comparison + because it has not started yet -- and, unlike folding a pairwise + supersession predicate two at a time, the answer does not depend on how + many other candidates are present or what order they arrive in, because + each candidate's key depends only on its own timestamp/pending status. + """ + coverage_indices = coverage_evidence_indices(check_runs) + if not coverage_indices: + return None + return max( + coverage_indices, + key=lambda item: check_run_recency_key( + check_runs[item], + parse_github_datetime(check_runs[item].get("startedAt")), + item, + ), + ) + + +def coverage_evidence_state(pr: dict[str, Any]) -> str: + """Return missing, running, complete, or failed for the latest coverage gate.""" + check_runs = latest_check_runs(pr) + latest_index = latest_coverage_evidence_index(check_runs) + if latest_index is not None: + node = check_runs[latest_index] + status = (node.get("status") or "").upper() + if status in RUNNING_CHECK_STATES: + return "running" + return "complete" if (node.get("conclusion") or "").upper() == "SUCCESS" else "failed" + for node in reversed(context_nodes(pr)): + if node.get("__typename") == "CheckRun": + continue + name = (node.get("name") or node.get("context") or "").lower() + if name != "coverage-evidence": + continue + status = (node.get("status") or node.get("state") or "").upper() + if status in RUNNING_CHECK_STATES: + return "running" + return "complete" if status == "SUCCESS" else "failed" + return "missing" + + +def superseded_coverage_evidence_indices(check_runs: Sequence[dict[str, Any]]) -> set[int]: + """Return older coverage checks superseded by a newer successful run.""" + authoritative_index = latest_coverage_evidence_index(check_runs) + if authoritative_index is None: + return set() + authoritative = check_runs[authoritative_index] + if (authoritative.get("conclusion") or "").upper() != "SUCCESS": + return set() + return set(coverage_evidence_indices(check_runs)) - {authoritative_index} + + 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 []): @@ -1595,21 +2053,41 @@ def dismiss_stale_opencode_change_requests(repo: str, pr: dict[str, Any], *, dry return len(review_ids) -def failed_status_checks(pr: dict[str, Any]) -> list[str]: - """Return failing check or status context names from the PR rollup.""" +def failed_status_checks( + pr: dict[str, Any], + *, + ignore_opencode: bool = False, +) -> list[str]: + """Return failing check or status context names from the PR rollup. + + ``ignore_opencode`` is reserved for the authenticated coverage-only retry + path: the previous ``opencode-review`` job or status is expected to be + failing there because it published the current-head coverage change request + being retried. Sibling jobs in the same workflow remain authoritative. + """ failed: list[str] = [] - nodes = latest_check_run_attempts(context_nodes(pr)) - status_contexts = [node for node in nodes if node.get("__typename") != "CheckRun"] + check_runs = latest_check_runs(pr) + superseded_coverage_indices = superseded_coverage_evidence_indices(check_runs) + status_contexts = [ + node + for node in context_nodes(pr) + if node.get("__typename") != "CheckRun" + ] + successful_status_contexts = { node.get("context") for node in status_contexts if (node.get("state") or "").upper() == "SUCCESS" } - for node in nodes: - if node.get("__typename") != "CheckRun": + for index, node in enumerate(check_runs): + if is_non_authoritative_coverage_check_run(node): continue conclusion = (node.get("conclusion") or "").upper() if conclusion in FAILED_CHECK_CONCLUSIONS: + if index in superseded_coverage_indices: + continue + if ignore_opencode and node.get("name") == "opencode-review": + continue if is_strix_context(node) and "strix" in successful_status_contexts: continue if is_opencode_context(node) and "opencode-review" in successful_status_contexts: @@ -1618,6 +2096,8 @@ def failed_status_checks(pr: dict[str, Any]) -> list[str]: for node in status_contexts: state = (node.get("state") or "").upper() if state in {"FAILURE", "ERROR"}: + if ignore_opencode and is_opencode_context(node): + continue failed.append(node.get("context") or "status-context") return failed @@ -2034,27 +2514,45 @@ def rerun_actions_job(repo: str, job_id: str, *, dry_run: bool, action: str) -> run_github_actions(["gh", "api", "-X", "POST", f"repos/{repo}/actions/jobs/{job_id}/rerun"]) -def active_workflow_runs(repo: str, statuses: Sequence[str] = ("queued", "in_progress")) -> list[dict[str, Any]]: - """Return active workflow runs for a repository.""" +def active_workflow_runs( + repo: str, + statuses: Sequence[str] = ("queued", "in_progress"), + *, + event: str | None = None, + created: str | None = None, +) -> list[dict[str, Any]]: + """Return workflow runs for a repository, optionally narrowed server-side. + + ``event`` and ``created`` map directly onto GitHub's ``List workflow + runs for a repository`` REST query parameters (``event`` selects the + triggering webhook event, ``created`` accepts a date/range qualifier + such as ``>=2026-08-24T00:00:00Z``). Both are omitted by default so + existing callers keep fetching every run for the given statuses + unfiltered; a caller with a naturally bounded lookup -- one whose + target repository's run history only grows, such as a same-head + dispatch search -- should pass them to avoid paginating history it can + never use. + """ runs: list[dict[str, Any]] = [] for status in statuses: - payload = json.loads( - run_github_actions( - [ - "gh", - "api", - "--method", - "GET", - f"repos/{repo}/actions/runs", - "--paginate", - "--slurp", - "-f", - f"status={status}", - "-F", - "per_page=100", - ] - ) - ) + args = [ + "gh", + "api", + "--method", + "GET", + f"repos/{repo}/actions/runs", + "--paginate", + "--slurp", + "-f", + f"status={status}", + "-F", + "per_page=100", + ] + if event: + args += ["-f", f"event={event}"] + if created: + args += ["-f", f"created={created}"] + payload = json.loads(run_github_actions(args)) pages = payload if isinstance(payload, list) else [payload] for page in pages: runs.extend(page.get("workflow_runs") or []) @@ -2186,6 +2684,62 @@ def active_opencode_run_refs( ) +def latest_opencode_dispatch_started_at( + repo: str, + workflow: str, + pr: dict[str, Any], + *, + since: datetime | None = None, +) -> datetime | None: + """Return the latest completed same-head OpenCode dispatch start time. + + The dispatch repository hosting ``repository_dispatch`` runs only + accumulates completed-run history over time, so this narrows GitHub's + REST query server-side to ``event=repository_dispatch`` plus a + ``created`` lower bound of ``since``, instead of paginating every + completed run ever recorded there and filtering client-side. ``since`` + is safe to pass whenever the caller only cares about a dispatch strictly + newer than a known anchor timestamp -- any run created at or before that + anchor cannot become the returned maximum -- and is left unset (no lower + bound) for callers with no such anchor. + """ + target_repo = validate_github_repository(repo) + dispatch_repo = repository_dispatch_target(target_repo) + head = str(pr.get("headRefOid") or "").lower() + number = int(pr["number"]) + title_prefixes = tuple( + f"{title} {target_repo}#{number}@" + for title in sorted( + {"Required OpenCode Review", *OPENCODE_WORKFLOW_NAMES}, + key=len, + reverse=True, + ) + ) + created = f">={since.strftime('%Y-%m-%dT%H:%M:%SZ')}" if since else None + latest: datetime | None = None + for run_data in active_workflow_runs( + dispatch_repo, ("completed",), event="repository_dispatch", created=created + ): + if run_data.get("event") != "repository_dispatch": + continue + display_title = str(run_data.get("display_title") or "") + prefix = next( + (candidate for candidate in title_prefixes if display_title.startswith(candidate)), + None, + ) + if prefix is None: + continue + dispatched_head = display_title.removeprefix(prefix).lower() + if not GIT_SHA_RE.fullmatch(dispatched_head) or dispatched_head != head: + continue + started_at = parse_github_datetime( + run_data.get("run_started_at") or run_data.get("created_at") + ) + if started_at and (latest is None or started_at > latest): + latest = started_at + return latest + + def active_opencode_run_ids( repo: str, workflow: str, @@ -2468,6 +3022,63 @@ def current_head_can_attempt_merge(pr: dict[str, Any], merge_state: str) -> bool return False +def revalidate_current_head_approval(repo: str, pr: dict[str, Any]) -> str | None: + """Re-check exact-head approval immediately before a merge-authorizing mutation. + + ``inspect_pr`` computes ``current_head_approved``/``approval_reason`` once, early + in the function, from the GraphQL/REST snapshot this scheduler invocation fetched + at the start of its run. Much later in the same invocation it reaches a branch + that calls ``merge_pr``/``enable_auto_merge`` using that stale snapshot. If the + reviewer who approved the exact head SHA dismisses or revokes that review -- or + GitHub otherwise recomputes ``reviewDecision`` -- in the window between the + snapshot and the mutating call, the merge would proceed on authorization that no + longer holds. The ``--match-head-commit`` guard those mutations carry only + protects against the *commit* changing in that window; it does nothing to protect + against the *review state* changing on the identical commit. + + Re-fetch the pull request right before the mutating call and recompute the exact + same independent exact-head approval decision (``has_current_head_approval`` and + ``merge_approval_block_reason``, the same helpers used for the original snapshot) + from the fresh data. Returns ``None`` when the fresh snapshot still authorizes the + merge, or a human-readable reason to block it otherwise. Any failure to re-fetch -- + a transient API error, or the pull request no longer being open or accessible -- + fails closed: it is treated exactly like a freshly observed missing approval so a + merge can never proceed on evidence this scheduler could not actually reconfirm. + """ + number = pr["number"] + try: + refreshed = fetch_pr(repo, number) + except RuntimeError as exc: + return ( + "re-checking current-head approval immediately before merge failed " + f"({exc}); treating the exact-head approval as unconfirmed" + ) + if not refreshed: + return ( + "re-checking current-head approval immediately before merge found PR " + f"#{number} no longer open or accessible; treating the exact-head " + "approval as unconfirmed" + ) + fresh_pr = refreshed[0] + expected_head = pr.get("headRefOid") + fresh_head = fresh_pr.get("headRefOid") + if expected_head and fresh_head and fresh_head != expected_head: + return ( + f"current head changed from {short_sha(expected_head)} to " + f"{short_sha(fresh_head)} immediately before merge; the exact-head " + "approval no longer applies to the current commit" + ) + if not has_current_head_approval(fresh_pr): + return ( + "current-head OpenCode approval was revoked immediately before merge; " + "the merge-authorizing snapshot is no longer current" + ) + reason = merge_approval_block_reason(fresh_pr) + if reason: + return f"{reason} (re-confirmed immediately before merge)" + return None + + def draft_review_request_artifact_name(repo: str, pr_number: int, head_sha: str) -> str: """Return one draft review-only request marker's exact artifact name.""" return f"cwl-draft-review-request-{repo.replace('/', '-')}-{pr_number}-{head_sha}" @@ -2784,6 +3395,26 @@ def decide(action: str, reason: str) -> Decision: """Create a decision after applying shared cleanup notes.""" return finish(Decision(number, action, reason)) + def revalidate_before_merge() -> Decision | None: + """Return a blocking decision if a fresh re-check just revoked approval. + + Call this immediately before every ``merge_pr``/``enable_auto_merge`` + invocation below, after every other authorization check has already passed + against the (possibly stale) snapshot fetched at the top of this scheduler + invocation -- closing the TOCTOU window between that snapshot and the + mutating call. Returns ``None`` when the fresh re-check still authorizes the + merge, so the caller proceeds unchanged. dry-run inspection never mutates + anything, so it skips the extra re-fetch entirely. + """ + if dry_run: + return None + reason = revalidate_current_head_approval(repo, pr) + if not reason: + return None + if pr.get("autoMergeRequest"): + return finish(disable_auto_merge_decision(repo, pr, dry_run=dry_run, reason=reason)) + return decide("wait", reason) + def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decision: """Request update-branch and attach any same-head evidence follow-up.""" if not branch_update_allowed: @@ -2845,24 +3476,116 @@ 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 not ( + coverage_retry_progress = opencode_progress_state( + pr, stale_after_minutes=stale_opencode_minutes + ) + coverage_ready = ( + merge_state not in {"DIRTY", "CONFLICTING"} + and trigger_reviews + and review_dispatch_allowed + and current_head_coverage_change_request(pr) + and coverage_evidence_state(pr) == "complete" + and strix_evidence_state(pr) == "complete" + and not failed_status_checks(pr, ignore_opencode=True) + ) + if coverage_ready: + if coverage_retry_progress == "running": + if pr.get("autoMergeRequest"): + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason=( + "current-head OpenCode coverage evidence is complete; disable " + "auto-merge while same-head re-review is already running" + ), + ) + ) + return decide( + "wait", + "current-head OpenCode coverage evidence is complete; " + "same-head OpenCode re-review is already running", + ) + retry_wait_reason = coverage_retry_wait_reason( + pr, + repo=repo if not dry_run else None, + workflow=workflow if not dry_run else None, + ) + if retry_wait_reason: + if pr.get("autoMergeRequest"): + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason=( + f"{retry_wait_reason}; disable auto-merge until the same-head " + "coverage retry floor elapses" + ), + ) + ) + return decide("wait", retry_wait_reason) + if pr.get("autoMergeRequest"): + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason=( + "current-head OpenCode coverage blocker is cleared; disable auto-merge " + "before same-head re-review" + ), + ) + ) + wait_reason = repository_dispatch_wait_reason(repo, workflow) + if wait_reason: + return decide("wait", wait_reason) + dispatch_result = dispatch_opencode_review(repo, workflow, pr, dry_run=dry_run) + if dispatch_result == "already_running": + return decide( + "wait", + "current-head coverage evidence is complete, but a same-head OpenCode workflow run is already active", + ) + return decide( + "review_dispatch", + "current-head OpenCode coverage blocker is cleared; same-head OpenCode re-dispatched", + ) + # Not a coverage-only gate: a separately eligible check-gated retry (the + # review was blocked only on then-failing GitHub Checks, which have + # since cleared) also earns a fall-through instead of a block, so the + # ordinary Strix/OpenCode dispatch pipeline below can re-review it. + check_gated_retry_ready = ( can_retry_check_gated_opencode_review(pr) and trigger_reviews and review_dispatch_allowed and not pr.get("autoMergeRequest") - ): + ) + if not check_gated_retry_ready: + conflict_suffix = ( + f"; {merge_conflict_guidance(pr, merge_state)}" + if merge_state in {"DIRTY", "CONFLICTING"} + else "" + ) 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", + reason=( + "current-head OpenCode review requested changes; address the review " + f"before re-enabling auto-merge{conflict_suffix}" + ), ) ) - return decide("block", "current-head OpenCode review requested changes") + return decide( + "block", + f"current-head OpenCode review requested changes{conflict_suffix}", + ) current_head_approved = has_current_head_approval(pr) + approval_reason = merge_approval_block_reason(pr) if current_head_approved else None if current_head_approved: stale_review_cleanup_count = dismiss_stale_opencode_change_requests( repo, @@ -2870,6 +3593,18 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio dry_run=dry_run, ) auto_merge_enabled = bool(pr.get("autoMergeRequest")) + if approval_reason and auto_merge_enabled: + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason=( + f"{approval_reason}; obtain fresh independent approval before " + "re-enabling auto-merge" + ), + ) + ) if merge_state in {"DIRTY", "CONFLICTING"}: conflict_reason = merge_conflict_guidance(pr, merge_state) if current_head_approved: @@ -2938,6 +3673,8 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio merge_state == "CLEAN" or merge_mode in {"direct", "direct_or_auto"} ) if current_head_approved and merge_before_update: + if approval_reason: + return decide("wait", approval_reason) if not same_repository_head(repo, pr): return decide("wait", external_head_merge_reason(repo, pr)) if not enable_auto_merge_flag: @@ -2949,6 +3686,9 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio return decide("wait", auto_merge_wait_reason(merge_state, pr)) return decide("wait", "current head is approved; merge mode disabled by scheduler inputs") if merge_mode in {"direct", "direct_or_auto"}: + revalidation = revalidate_before_merge() + if revalidation: + return revalidation try: merge_pr(repo, pr, dry_run=dry_run) except RuntimeError as exc: @@ -2979,30 +3719,48 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio return decide("wait", f"current head is approved; unsupported merge mode: {merge_mode}") if pr.get("autoMergeRequest"): return decide("wait", auto_merge_wait_reason(merge_state, pr)) + revalidation = revalidate_before_merge() + if revalidation: + return revalidation enable_auto_merge(repo, pr, dry_run=dry_run) return decide("auto_merge", "current head is approved; auto-merge enabled") behind_by = branch_outdated_by_base(pr, merge_state) if behind_by and (current_head_approved or auto_merge_enabled): + if not current_head_approved: + # auto_merge_enabled must be True to have reached this branch (the + # outer condition requires current_head_approved or + # auto_merge_enabled). An outdated branch is routine and does not + # by itself justify disarming auto-merge -- but an auto-merge + # request armed with no live current-head approval is exactly the + # stale authorization this scheduler exists to catch, and simply + # requesting a branch update here would leave it queued: once the + # updated head's required checks pass, GitHub's own native + # auto-merge could merge it without this scheduler ever getting a + # chance to require a fresh independent approval on that new + # head. Disarm before requesting the update rather than after. + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason=( + f"branch is {behind_by} commit(s) behind base (GitHub mergeability is " + f"{merge_state}) with no live current-head approval to authorize " + "auto-merge; obtain fresh independent approval before re-enabling auto-merge" + ), + ) + ) if not update_branches: - if current_head_approved: - return decide("wait", "current-head OpenCode review approved; branch update disabled") - return decide("wait", "auto-merge already enabled; branch update disabled") + return decide("wait", "current-head OpenCode review approved; branch update disabled") if not can_update_pr_head(repo, pr): return decide("wait", non_mutable_head_reason(repo, pr)) suffix = "; existing auto-merge request remains queued" if auto_merge_enabled else "" - if current_head_approved and merge_state == "BEHIND": + if merge_state == "BEHIND": freshness_reason = "current-head OpenCode review approved" - elif current_head_approved: - freshness_reason = ( - "current-head OpenCode review approved; " - f"base branch is {behind_by} commit(s) ahead even though GitHub mergeability is {merge_state}" - ) - elif merge_state == "BEHIND": - freshness_reason = "auto-merge already enabled" else: freshness_reason = ( - "auto-merge already enabled; " + "current-head OpenCode review approved; " f"base branch is {behind_by} commit(s) ahead even though GitHub mergeability is {merge_state}" ) return request_branch_update(freshness_reason, suffix=suffix) @@ -3052,6 +3810,35 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio ) ) + if not current_head_approved and auto_merge_enabled: + # Neither behind-by disarm path applies (the branch is not behind + # base) and the last-push-approval restamp does not apply either (it + # requires current_head_approved). Yet auto-merge is still armed with + # no live current-head approval -- whether from a previously valid + # approval a new push has since invalidated, or from auto-merge armed + # before any review ever ran, this scheduler draws no distinction + # between the two (see the behind-by disarm path and the prior + # unconditional catch-all below, neither of which drew one either). + # Disarm immediately here, before any of the wait/dispatch branches + # below (OpenCode running, deterministic-fallback wait, stale-review + # retry, or the ordinary Strix/OpenCode dispatch cascade -- the + # everyday state for a PR between or during reviews) can return + # without having done so. Relying on a catch-all reached only once + # dispatch has nothing left to do would let GitHub's own native + # auto-merge complete the merge first if this scheduler is the only + # thing enforcing the OpenCode-approval requirement. + return finish( + disable_auto_merge_decision( + repo, + pr, + dry_run=dry_run, + reason=( + "current head has no OpenCode approval; wait for fresh same-head " + "approval before re-enabling auto-merge" + ), + ) + ) + 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") @@ -3098,6 +3885,8 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio return decide("wait", "mergeability is still being calculated and no branch freshness evidence is available") if current_head_approved: + if approval_reason: + return decide("wait", approval_reason) if pr.get("autoMergeRequest"): return decide("wait", auto_merge_wait_reason(merge_state, pr)) if not same_repository_head(repo, pr): @@ -3108,6 +3897,9 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio return decide("wait", "current head is approved; merge mode disabled by scheduler inputs") if merge_mode in {"direct", "direct_or_auto"}: if merge_mode == "direct_or_auto": + revalidation = revalidate_before_merge() + if revalidation: + return revalidation try: merge_pr(repo, pr, dry_run=dry_run) except RuntimeError as exc: @@ -3132,6 +3924,9 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio ) if merge_mode != "auto": return decide("wait", f"current head is approved; unsupported merge mode: {merge_mode}") + revalidation = revalidate_before_merge() + if revalidation: + return revalidation enable_auto_merge(repo, pr, dry_run=dry_run) return decide("auto_merge", "current head is approved; auto-merge enabled") @@ -3203,16 +3998,11 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio "current head has completed Strix evidence; same-head OpenCode dispatched", ) - if pr.get("autoMergeRequest"): - return finish( - disable_auto_merge_decision( - repo, - pr, - dry_run=dry_run, - reason="current head has no OpenCode approval; wait for fresh same-head approval before re-enabling auto-merge", - ) - ) - + # No autoMergeRequest re-check is needed here: the hoisted + # `not current_head_approved and auto_merge_enabled` guard above already + # disarmed and returned before any of the wait/dispatch branches between + # it and here could be reached, so auto-merge cannot still be armed by + # this point. return decide("block", "current head has no OpenCode approval") @@ -3702,6 +4492,7 @@ def self_test_scheduler_invariants() -> None: pass sample = { "number": 1, + "author": {"login": "pull-request-author"}, "headRefOid": "abc", "baseRefName": "main", "baseRefOid": "base", @@ -3712,7 +4503,7 @@ def self_test_scheduler_invariants() -> None: "isCrossRepository": False, "maintainerCanModify": False, "headRepository": {"nameWithOwner": "owner/repo"}, - "reviewDecision": "REVIEW_REQUIRED", + "reviewDecision": "APPROVED", "commits": { "nodes": [ { @@ -3733,7 +4524,13 @@ def self_test_scheduler_invariants() -> None: "body": "OpenCode Agent approved this head.", "submittedAt": "2026-06-25T15:42:19Z", "commit": {"oid": "abc"}, - } + }, + { + "state": "APPROVED", + "author": {"login": "independent-reviewer"}, + "submittedAt": "2026-06-25T15:43:19Z", + "commit": {"oid": "abc"}, + }, ] }, "statusCheckRollup": {"contexts": {"nodes": []}}, @@ -3967,6 +4764,13 @@ def self_test_scheduler_invariants() -> None: sample["isCrossRepository"] = False sample["maintainerCanModify"] = False sample["autoMergeRequest"] = {"enabledAt": "2026-01-01T00:02:00Z"} + sample["reviews"]["nodes"].append( + { + "state": "APPROVED", + "author": {"login": "independent-reviewer"}, + "commit": {"oid": "abc"}, + } + ) decision = inspect_pr( "owner/repo", sample, @@ -4071,6 +4875,7 @@ def self_test_scheduler_invariants() -> None: assert "git status --short" in conflict_guidance["commands"] blocked_sample = { "number": 2, + "author": {"login": "pull-request-author"}, "headRefOid": "abc", "baseRefName": "main", "baseRefOid": "base", @@ -4099,14 +4904,20 @@ def self_test_scheduler_invariants() -> None: "reviewThreads": {"nodes": []}, "reviews": { "nodes": [ - { - "state": "APPROVED", - "author": {"login": "opencode-agent"}, + { + "state": "APPROVED", + "author": {"login": "opencode-agent"}, "body": "OpenCode Agent approved this head.", "submittedAt": "2026-06-25T15:42:19Z", - "commit": {"oid": "abc"}, - } - ] + "commit": {"oid": "abc"}, + }, + { + "state": "APPROVED", + "author": {"login": "independent-reviewer"}, + "submittedAt": "2026-06-25T15:43:19Z", + "commit": {"oid": "abc"}, + }, + ] }, "statusCheckRollup": { "contexts": { diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 5727863f8..afc92ebdf 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -47,6 +47,7 @@ def make_pr(**overrides): value = { "number": 1, "title": "Central review", + "author": {"login": "pull-request-author"}, "isDraft": False, "mergeable": "MERGEABLE", "mergeStateStatus": "CLEAN", @@ -85,11 +86,37 @@ def opencode_review( commit="head", login="opencode-agent", submitted_at="2026-06-25T07:01:00Z", + author_typename=None, ): + author = {"login": login} + if author_typename is not None: + author["__typename"] = author_typename return { "state": state, - "author": {"login": login}, + "author": author, + "submittedAt": submitted_at, + "commit": {"oid": commit}, + } + + +def merge_approved_reviews(commit="head"): + """Return exact-head OpenCode and independent formal approvals.""" + return { + "nodes": [ + opencode_review("APPROVED", commit), + opencode_review("APPROVED", commit, login="independent-reviewer"), + ] + } + + +def review_node(state="APPROVED", login="reviewer", commit="head", submitted_at="2026-06-25T07:00:00Z"): + """Return a minimal GraphQL-shaped review node for pagination tests.""" + return { + "databaseId": None, + "state": state, + "body": None, "submittedAt": submitted_at, + "author": {"login": login}, "commit": {"oid": commit}, } @@ -141,7 +168,7 @@ def last_push_restamp_candidate(**overrides): restMergeableState="BLOCKED", reviewDecision="APPROVED", autoMergeRequest={"enabledAt": "now"}, - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviews=merge_approved_reviews(), statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, commits={ "nodes": [ @@ -340,6 +367,420 @@ def fake_graphql(query, **fields): assert seen == [{"owner": "owner", "name": "repo", "number": 42}] +def test_complete_paginated_pr_reviews_merges_pages_oldest_first(monkeypatch): + """Backward pagination must prepend older pages so nodes stay oldest-first.""" + calls = [] + + def fake_graphql(query, **fields): + calls.append(fields) + assert fields == {"owner": "owner", "name": "repo", "number": 7, "cursor": "cursor-1"} + return { + "data": { + "repository": { + "pullRequest": { + "reviews": { + "nodes": [review_node(login="independent-reviewer", submitted_at="t0")], + "pageInfo": {"hasPreviousPage": False, "startCursor": None}, + } + } + } + } + } + + monkeypatch.setattr(sched, "gh_graphql", fake_graphql) + + newer_page = { + "nodes": [review_node(state="COMMENTED", login="bot[bot]", submitted_at="t1")], + "pageInfo": {"hasPreviousPage": True, "startCursor": "cursor-1"}, + } + + merged = sched.complete_paginated_pr_reviews("owner", "repo", 7, newer_page) + + assert [node["author"]["login"] for node in merged["nodes"]] == [ + "independent-reviewer", + "bot[bot]", + ] + assert len(calls) == 1 + + +def test_complete_paginated_pr_reviews_walks_multiple_pages(monkeypatch): + """More than one prior page must all be folded in, oldest page first.""" + pages_by_cursor = { + "cursor-2": { + "nodes": [review_node(login="independent-reviewer", submitted_at="t0")], + "pageInfo": {"hasPreviousPage": False, "startCursor": None}, + }, + "cursor-1": { + "nodes": [review_node(state="COMMENTED", login="bot-a[bot]", submitted_at="t1")], + "pageInfo": {"hasPreviousPage": True, "startCursor": "cursor-2"}, + }, + } + + def fake_graphql(query, **fields): + return { + "data": { + "repository": {"pullRequest": {"reviews": pages_by_cursor[fields["cursor"]]}} + } + } + + monkeypatch.setattr(sched, "gh_graphql", fake_graphql) + + newest_page = { + "nodes": [review_node(state="COMMENTED", login="bot-b[bot]", submitted_at="t2")], + "pageInfo": {"hasPreviousPage": True, "startCursor": "cursor-1"}, + } + + merged = sched.complete_paginated_pr_reviews("owner", "repo", 7, newest_page) + + assert [node["author"]["login"] for node in merged["nodes"]] == [ + "independent-reviewer", + "bot-a[bot]", + "bot-b[bot]", + ] + + +def test_complete_paginated_pr_reviews_raises_without_start_cursor(): + """A truthful hasPreviousPage without a cursor cannot be paginated further.""" + reviews = {"nodes": [], "pageInfo": {"hasPreviousPage": True, "startCursor": None}} + + with pytest.raises(RuntimeError, match="startCursor"): + sched.complete_paginated_pr_reviews("owner", "repo", 7, reviews) + + +def test_complete_paginated_pr_reviews_propagates_page_fetch_failure(monkeypatch): + """A page-fetch failure must fail closed, never fall back to a partial history.""" + + def fail_graphql(*args, **kwargs): + raise RuntimeError("gh: HTTP 502 (exhausted retries)") + + monkeypatch.setattr(sched, "gh_graphql", fail_graphql) + reviews = {"nodes": [], "pageInfo": {"hasPreviousPage": True, "startCursor": "cursor-1"}} + + with pytest.raises(RuntimeError, match="HTTP 502"): + sched.complete_paginated_pr_reviews("owner", "repo", 7, reviews) + + +def test_complete_paginated_pr_reviews_bounds_pathological_loop(monkeypatch): + """A pageInfo that never resolves hasPreviousPage=false must not loop forever.""" + + def always_more(query, **fields): + return { + "data": { + "repository": { + "pullRequest": { + "reviews": { + "nodes": [], + "pageInfo": { + "hasPreviousPage": True, + "startCursor": fields["cursor"] + "x", + }, + } + } + } + } + } + + monkeypatch.setattr(sched, "gh_graphql", always_more) + monkeypatch.setattr(sched, "MAX_REVIEW_PAGINATION_PAGES", 3) + reviews = {"nodes": [], "pageInfo": {"hasPreviousPage": True, "startCursor": "c0"}} + + with pytest.raises(RuntimeError, match="exceeded 3 pages"): + sched.complete_paginated_pr_reviews("owner", "repo", 7, reviews) + + +def test_complete_all_pr_reviews_skips_prs_that_do_not_need_pagination(monkeypatch): + """PRs with no reviews key or a complete first page must not trigger a fetch.""" + calls = [] + monkeypatch.setattr(sched, "gh_graphql", lambda *args, **kwargs: calls.append((args, kwargs))) + + no_reviews_key = {"number": 1} + already_complete = { + "number": 2, + "reviews": {"nodes": [], "pageInfo": {"hasPreviousPage": False, "startCursor": None}}, + } + prs = [no_reviews_key, already_complete] + + sched.complete_all_pr_reviews("owner", "repo", prs) + + assert calls == [] + assert prs == [no_reviews_key, already_complete] + + +def test_complete_all_pr_reviews_backfills_only_truncated_prs(monkeypatch): + """Only the PR flagged hasPreviousPage gets its reviews replaced.""" + + def fake_graphql(query, **fields): + assert fields["number"] == 5 + return { + "data": { + "repository": { + "pullRequest": { + "reviews": { + "nodes": [review_node(login="independent-reviewer")], + "pageInfo": {"hasPreviousPage": False, "startCursor": None}, + } + } + } + } + } + + monkeypatch.setattr(sched, "gh_graphql", fake_graphql) + + complete_pr = { + "number": 4, + "reviews": {"nodes": [review_node(login="already-here")], "pageInfo": {"hasPreviousPage": False}}, + } + truncated_pr = { + "number": 5, + "reviews": { + "nodes": [review_node(state="COMMENTED", login="bot[bot]", submitted_at="t1")], + "pageInfo": {"hasPreviousPage": True, "startCursor": "cursor-1"}, + }, + } + prs = [complete_pr, truncated_pr] + + sched.complete_all_pr_reviews("owner", "repo", prs) + + assert [n["author"]["login"] for n in complete_pr["reviews"]["nodes"]] == ["already-here"] + assert [n["author"]["login"] for n in truncated_pr["reviews"]["nodes"]] == [ + "independent-reviewer", + "bot[bot]", + ] + + +def test_fetch_pr_backfills_reviews_past_graphql_window(monkeypatch): + """fetch_pr (the single-PR path used right before a merge decision) must + paginate past the reviews(last: 100) window before returning.""" + + def fake_graphql(query, **fields): + if "before: $cursor" in query: + assert fields["cursor"] == "cursor-1" + return { + "data": { + "repository": { + "pullRequest": { + "reviews": { + "nodes": [review_node(login="independent-reviewer", submitted_at="t0")], + "pageInfo": {"hasPreviousPage": False, "startCursor": None}, + } + } + } + } + } + return { + "data": { + "repository": { + "pullRequest": { + "number": fields["number"], + "reviews": { + "nodes": [review_node(state="COMMENTED", login="bot[bot]", submitted_at="t1")], + "pageInfo": {"hasPreviousPage": True, "startCursor": "cursor-1"}, + }, + } + } + } + } + + monkeypatch.setattr(sched, "gh_graphql", fake_graphql) + monkeypatch.setattr(sched, "enrich_rest_mergeable_states", lambda repo, prs: None) + + prs = sched.fetch_pr("owner/repo", 7) + + assert [n["author"]["login"] for n in prs[0]["reviews"]["nodes"]] == [ + "independent-reviewer", + "bot[bot]", + ] + + +def test_fetch_open_prs_backfills_reviews_past_graphql_window(monkeypatch): + """fetch_open_prs (the bulk queue-scan path) also feeds merge decisions + directly -- see inspect_pr's callers in main() -- so it must paginate past + the reviews(last: 100) window too, not just the single-PR fetch path.""" + + def fake_graphql(query, **fields): + if "before: $cursor" in query: + return { + "data": { + "repository": { + "pullRequest": { + "reviews": { + "nodes": [review_node(login="independent-reviewer", submitted_at="t0")], + "pageInfo": {"hasPreviousPage": False, "startCursor": None}, + } + } + } + } + } + return { + "data": { + "repository": { + "pullRequests": { + "nodes": [ + { + "number": 7, + "reviews": { + "nodes": [ + review_node(state="COMMENTED", login="bot[bot]", submitted_at="t1") + ], + "pageInfo": {"hasPreviousPage": True, "startCursor": "cursor-1"}, + }, + } + ], + "pageInfo": {"hasNextPage": False, "endCursor": None}, + } + } + } + } + + monkeypatch.setattr(sched, "gh_graphql", fake_graphql) + monkeypatch.setattr(sched, "enrich_rest_mergeable_states", lambda repo, prs: None) + + prs = sched.fetch_open_prs("owner/repo", 5) + + assert [n["author"]["login"] for n in prs[0]["reviews"]["nodes"]] == [ + "independent-reviewer", + "bot[bot]", + ] + + +def test_fetch_all_pr_reviews_rest_paginates_past_100(monkeypatch): + """The REST fallback must walk page=1,2,... until a short page ends it.""" + page1 = [{"id": i} for i in range(100)] + page2 = [{"id": 100}, {"id": 101}] + calls = [] + + def fake_api(path): + calls.append(path) + if path.endswith("page=1"): + return page1 + if path.endswith("page=2"): + return page2 + raise AssertionError(f"unexpected path {path}") + + monkeypatch.setattr(sched, "gh_api_json", fake_api) + + reviews = sched.fetch_all_pr_reviews_rest("owner/repo", 7) + + assert [r["id"] for r in reviews] == list(range(102)) + assert calls == [ + "repos/owner/repo/pulls/7/reviews?per_page=100&page=1", + "repos/owner/repo/pulls/7/reviews?per_page=100&page=2", + ] + + +def test_fetch_all_pr_reviews_rest_stops_at_first_short_page(monkeypatch): + """A first page shorter than per_page must not trigger a second request.""" + calls = [] + + def fake_api(path): + calls.append(path) + return [{"id": 1}] + + monkeypatch.setattr(sched, "gh_api_json", fake_api) + + assert sched.fetch_all_pr_reviews_rest("owner/repo", 7) == [{"id": 1}] + assert calls == ["repos/owner/repo/pulls/7/reviews?per_page=100&page=1"] + + +def test_fetch_all_pr_reviews_rest_stops_on_empty_page_after_full_page(monkeypatch): + """A full 100-row page followed by an empty page must terminate cleanly.""" + calls = [] + + def fake_api(path): + calls.append(path) + if path.endswith("page=1"): + return [{"id": i} for i in range(100)] + return [] + + monkeypatch.setattr(sched, "gh_api_json", fake_api) + + reviews = sched.fetch_all_pr_reviews_rest("owner/repo", 7) + + assert len(reviews) == 100 + assert calls == [ + "repos/owner/repo/pulls/7/reviews?per_page=100&page=1", + "repos/owner/repo/pulls/7/reviews?per_page=100&page=2", + ] + + +def test_fetch_all_pr_reviews_rest_propagates_page_fetch_failure(monkeypatch): + """A later-page REST failure must fail closed rather than return a partial history.""" + + def fake_api(path): + if path.endswith("page=1"): + return [{"id": i} for i in range(100)] + raise RuntimeError("gh: HTTP 502 (exhausted retries)") + + monkeypatch.setattr(sched, "gh_api_json", fake_api) + + with pytest.raises(RuntimeError, match="HTTP 502"): + sched.fetch_all_pr_reviews_rest("owner/repo", 7) + + +def test_fetch_pr_pagination_recovers_independent_approval_past_100_reviews(monkeypatch): + """End-to-end regression for the reported bug: a genuine independent + APPROVED review made early in a PR's life must still satisfy + has_independent_current_head_approval once the PR has accumulated more + than 100 total review events and fetch_pr completes GraphQL's pagination. + """ + genuine_approval = review_node( + state="APPROVED", login="independent-reviewer", submitted_at="t000", commit="head" + ) + noise = [ + review_node( + state="COMMENTED", + login=f"noise-bot-{i}[bot]", + submitted_at=f"t{i + 1:03d}", + commit="head", + ) + for i in range(105) + ] + full_history = [genuine_approval] + noise # oldest-first, 106 reviews total + first_page = full_history[-100:] # reviews(last: 100) -- drops the genuine approval + second_page = full_history[:6] # the remaining 6, including the genuine approval + assert genuine_approval not in first_page + assert genuine_approval in second_page + + def fake_graphql(query, **fields): + if "before: $cursor" in query: + assert fields["cursor"] == "cursor-1" + return { + "data": { + "repository": { + "pullRequest": { + "reviews": { + "nodes": second_page, + "pageInfo": {"hasPreviousPage": False, "startCursor": None}, + } + } + } + } + } + return { + "data": { + "repository": { + "pullRequest": { + "number": 7, + "author": {"login": "pull-request-author"}, + "headRefOid": "head", + "reviews": { + "nodes": first_page, + "pageInfo": {"hasPreviousPage": True, "startCursor": "cursor-1"}, + }, + } + } + } + } + + monkeypatch.setattr(sched, "gh_graphql", fake_graphql) + monkeypatch.setattr(sched, "enrich_rest_mergeable_states", lambda repo, prs: None) + + pr = sched.fetch_pr("owner/repo", 7)[0] + + assert len(pr["reviews"]["nodes"]) == 106 + assert sched.has_independent_current_head_approval(pr) + + def test_gh_graphql_retries_transient_gateway_errors(monkeypatch): calls = [] sleeps = [] @@ -541,7 +982,7 @@ def raise_compare_error(repo, pr): def test_rest_pr_fallback_shapes_reviews_and_checks(monkeypatch): calls = [] payloads = { - "repos/owner/repo/pulls/42/reviews?per_page=100": [ + "repos/owner/repo/pulls/42/reviews?per_page=100&page=1": [ { "state": "APPROVED", "body": "Head SHA: `abc123`", @@ -558,9 +999,15 @@ def test_rest_pr_fallback_shapes_reviews_and_checks(monkeypatch): "conclusion": "success", "started_at": "2026-06-30T00:00:00Z", "details_url": "https://github.com/owner/repo/actions/runs/1/job/2", + "check_suite": {"id": 555}, } ] }, + "repos/owner/repo/commits/abc123/check-suites?per_page=100": { + "check_suites": [ + {"id": 555, "created_at": "2026-06-30T00:00:00Z"}, + ] + }, "repos/owner/repo/commits/abc123/statuses?per_page=100": [ { "context": "strix", @@ -598,8 +1045,9 @@ def fake_api(path): ) assert calls == [ - "repos/owner/repo/pulls/42/reviews?per_page=100", + "repos/owner/repo/pulls/42/reviews?per_page=100&page=1", "repos/owner/repo/commits/abc123/check-runs?per_page=100", + "repos/owner/repo/commits/abc123/check-suites?per_page=100", "repos/owner/repo/commits/abc123/statuses?per_page=100", "repos/owner/repo/pulls/42/files?per_page=20", ] @@ -613,6 +1061,10 @@ def fake_api(path): assert node["reviews"]["nodes"][0]["commit"]["oid"] == "abc123" assert node["statusCheckRollup"]["contexts"]["nodes"][0]["status"] == "COMPLETED" assert node["statusCheckRollup"]["contexts"]["nodes"][0]["conclusion"] == "SUCCESS" + assert ( + node["statusCheckRollup"]["contexts"]["nodes"][0]["checkSuite"]["createdAt"] + == "2026-06-30T00:00:00Z" + ) # A classic commit status (e.g. a same-head manual `workflow_dispatch` # Strix run's evidence) must survive the REST fallback too -- omitting # it here would silently erase that evidence for every caller, since @@ -944,6 +1396,7 @@ def test_context_review_and_check_helpers(monkeypatch): assert sched.context_nodes(make_pr()) == [] assert sched.compare_behind_by({"compareBehindBy": "2"}) == 2 assert sched.compare_behind_by({"compareBehindBy": "unknown"}) == 0 + assert not sched.is_opencode_check_run({"context": "opencode-review"}) assert sched.is_opencode_context({"__typename": "CheckRun", "name": "opencode-review"}) assert sched.is_opencode_context( { @@ -1225,666 +1678,2117 @@ def test_central_progress_ignores_required_workflow_checkrun_placeholder( ) -def test_review_state_and_failed_checks(): - pr = make_pr(reviews={"nodes": [opencode_review("APPROVED", "old"), opencode_review("APPROVED", "head")]}) - assert sched.current_head_review_state(pr, "APPROVED") - assert sched.has_current_head_approval(pr) - assert not sched.has_current_head_changes_requested(pr) - assert not sched.has_current_head_approval( - make_pr(headRefOid="", reviews={"nodes": [opencode_review("APPROVED", "head")]}) +def test_central_coverage_retry_ignores_failed_required_workflow_placeholder( + monkeypatch, +): + """A non-authoritative OpenCode CheckRun cannot self-block its retry.""" + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", ) - exact_head = "a" * 40 - stale_body_head = "b" * 40 - body_sha_mismatch = make_pr( - headRefOid=exact_head, - reviews={ - "nodes": [ - { - **opencode_review("APPROVED", exact_head), - "body": f"## Gate evidence\n\n- Head SHA: `{stale_body_head}`", - } - ] - }, + monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda *_: None) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda *args, **kwargs: "dispatched", ) - assert sched.review_body_head_sha(body_sha_mismatch["reviews"]["nodes"][0]) == stale_body_head - assert not sched.has_current_head_approval(body_sha_mismatch) - body_sha_match = make_pr( - headRefOid=exact_head, + coverage_request = make_pr( reviews={ "nodes": [ { - **opencode_review("APPROVED", exact_head), - "body": f"## Gate evidence\n\n- Head SHA: `{exact_head.upper()}`", + **opencode_review("CHANGES_REQUESTED", "head"), + "body": ( + "OpenCode cannot approve yet because required coverage evidence " + "did not pass. The coverage-evidence gate reported that required " + "test/docstring evidence was not proven." + ), } ] }, - ) - assert sched.has_current_head_approval(body_sha_match) - body_sha_only_match = make_pr( - headRefOid=exact_head, - reviews={ - "nodes": [ - { - **opencode_review("APPROVED", ""), - "body": f"## Gate evidence\n\n- Head SHA: `{exact_head}`", - } - ] + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(), + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": "SUCCESS", + }, + { + **opencode_check(status="COMPLETED"), + "conclusion": "FAILURE", + }, + ] + } }, ) - assert sched.has_current_head_approval(body_sha_only_match) - body_sha_only_mismatch = make_pr( - headRefOid=exact_head, + + decision = inspect(coverage_request) + + assert decision.action == "review_dispatch" + assert decision.reason == ( + "current-head OpenCode coverage blocker is cleared; " + "same-head OpenCode re-dispatched" + ) + + +def test_coverage_retry_disables_auto_merge_before_dispatch(monkeypatch): + """A coverage retry must not leave an unsafe auto-merge request enabled.""" + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", + ) + disabled = [] + dispatched = [] + monkeypatch.setattr( + sched, + "disable_auto_merge", + lambda repo, pr, dry_run: disabled.append((repo, pr["number"], dry_run)), + ) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda *args, **kwargs: dispatched.append((args, kwargs)) or "dispatched", + ) + coverage_request = make_pr( + autoMergeRequest={"enabledAt": "now"}, reviews={ "nodes": [ { - **opencode_review("APPROVED", ""), - "body": f"## Gate evidence\n\n- Head SHA: `{stale_body_head}`", + **opencode_review("CHANGES_REQUESTED", "head"), + "body": ( + "OpenCode cannot approve yet because required coverage evidence " + "did not pass. The coverage-evidence gate reported that required " + "test/docstring evidence was not proven." + ), } ] }, + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(), + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": "SUCCESS", + }, + { + **opencode_check(status="COMPLETED"), + "conclusion": "FAILURE", + }, + ] + } + }, ) - assert not sched.has_current_head_approval(body_sha_only_mismatch) - body_sha_does_not_override_commit = make_pr( - headRefOid=exact_head, + + decision = inspect(coverage_request) + + assert decision.action == "disable_auto_merge" + assert "before same-head re-review" in decision.reason + assert disabled == [("owner/repo", 1, True)] + assert dispatched == [] + + +def test_coverage_retry_waits_for_visible_opencode_run(monkeypatch): + """A visible same-head OpenCode run prevents duplicate coverage dispatch.""" + monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda *_: None) + dispatched = [] + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda *args, **kwargs: dispatched.append((args, kwargs)) or "dispatched", + ) + coverage_request = make_pr( reviews={ "nodes": [ { - **opencode_review("APPROVED", stale_body_head), - "body": f"## Gate evidence\n\n- Head SHA: `{exact_head}`", + **opencode_review("CHANGES_REQUESTED", "head"), + "body": ( + "OpenCode cannot approve yet because required coverage evidence " + "did not pass. The coverage-evidence gate reported that required " + "test/docstring evidence was not proven." + ), } ] }, + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(), + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": "SUCCESS", + }, + opencode_check( + status="IN_PROGRESS", + started_at=datetime.now(timezone.utc).isoformat(), + ), + ] + } + }, ) - assert not sched.has_current_head_approval(body_sha_does_not_override_commit) - deterministic_fallback = make_pr( - headRefOid=exact_head, + + decision = inspect(coverage_request) + + assert decision.action == "wait" + assert decision.reason == ( + "current-head OpenCode coverage evidence is complete; " + "same-head OpenCode re-review is already running" + ) + assert dispatched == [] + + +def test_coverage_retry_disables_auto_merge_for_visible_opencode_run(monkeypatch): + """An active same-head re-review disables any pre-existing auto-merge request.""" + disabled = [] + monkeypatch.setattr( + sched, + "disable_auto_merge", + lambda repo, pr, dry_run: disabled.append((repo, pr["number"], dry_run)), + ) + coverage_request = make_pr( + autoMergeRequest={"enabledAt": "now"}, reviews={ "nodes": [ { - **opencode_review("APPROVED", exact_head), + **opencode_review("CHANGES_REQUESTED", "head"), "body": ( - "OpenCode model attempts did not emit a usable current-head " - "control block, so the approval gate used deterministic " - "current-head evidence instead of model prose." + "coverage evidence did not pass; coverage-evidence reported that " + "required test/docstring evidence was not proven" ), } ] }, + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(), + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": "SUCCESS", + }, + opencode_check( + status="IN_PROGRESS", + started_at=datetime.now(timezone.utc).isoformat(), + ), + ] + } + }, ) - assert sched.is_deterministic_fallback_approval( - deterministic_fallback["reviews"]["nodes"][0] + + decision = inspect(coverage_request) + + assert decision.action == "disable_auto_merge" + assert disabled == [("owner/repo", 1, True)] + + +def test_coverage_retry_waits_for_same_head_retry_floor(monkeypatch): + """A fresh coverage-only review disables auto-merge during its retry floor.""" + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", ) - assert not sched.is_deterministic_fallback_approval( - opencode_review("CHANGES_REQUESTED", exact_head) + monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda *_: None) + disabled = [] + monkeypatch.setattr( + sched, + "disable_auto_merge", + lambda repo, pr, dry_run: disabled.append((repo, pr["number"], dry_run)), ) - assert sched.has_current_head_deterministic_fallback_approval(deterministic_fallback) - assert not sched.has_current_head_approval(deterministic_fallback) - fallback_scan_without_current_opencode = make_pr( + dispatched = [] + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda *args, **kwargs: dispatched.append((args, kwargs)) or "dispatched", + ) + coverage_request = make_pr( + autoMergeRequest={"enabledAt": "now"}, reviews={ "nodes": [ - opencode_review("APPROVED", "old"), - opencode_review("APPROVED", "head", login="human"), + { + **opencode_review( + "CHANGES_REQUESTED", + "head", + submitted_at="2999-01-01T00:00:00Z", + ), + "body": ( + "OpenCode cannot approve yet because required coverage evidence " + "did not pass. The coverage-evidence gate reported that required " + "test/docstring evidence was not proven." + ), + } ] - } + }, + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(), + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": "SUCCESS", + }, + {**opencode_check(status="COMPLETED"), "conclusion": "FAILURE"}, + ] + } + }, ) - assert not sched.has_current_head_deterministic_fallback_approval( - fallback_scan_without_current_opencode + + decision = inspect(coverage_request) + + assert decision.action == "disable_auto_merge" + assert "same-head OpenCode coverage retry floor has not elapsed" in decision.reason + assert "disable auto-merge until the same-head coverage retry floor elapses" in decision.reason + assert disabled == [("owner/repo", 1, True)] + assert dispatched == [] + + +def test_coverage_retry_floor_uses_latest_dispatch_timestamp(monkeypatch): + """A completed dispatch without a review still receives a bounded retry floor.""" + coverage_review = { + **opencode_review( + "CHANGES_REQUESTED", + "head", + submitted_at="2026-08-24T00:00:00Z", + ), + "body": ( + "OpenCode cannot approve yet because required coverage evidence did not pass. " + "The coverage-evidence gate reported that required test/docstring evidence was not proven." + ), + } + coverage_request = make_pr(reviews={"nodes": [coverage_review]}) + monkeypatch.setattr( + sched, + "latest_opencode_dispatch_started_at", + lambda repo, workflow, pr, since=None: datetime(2026, 8, 24, 1, 0, tzinfo=timezone.utc), ) - stale_review = make_pr( + + assert sched.coverage_retry_wait_reason( + coverage_request, + repo="owner/repo", + workflow="OpenCode Review", + now=datetime(2026, 8, 24, 1, 59, tzinfo=timezone.utc), + ) == "same-head OpenCode coverage retry floor has not elapsed" + assert sched.coverage_retry_wait_reason( + coverage_request, + repo="owner/repo", + workflow="OpenCode Review", + now=datetime(2026, 8, 24, 2, 0, tzinfo=timezone.utc), + ) is None + + +def test_coverage_retry_wait_reason_fails_closed_without_coverage_review(): + """A non-coverage change request cannot authorize a retry.""" + assert sched.coverage_retry_wait_reason(make_pr()) is None + + +def test_coverage_retry_wait_reason_fails_closed_when_dispatch_history_is_unavailable( + monkeypatch, +): + """Unavailable dispatch history prevents an unbounded same-head retry.""" + coverage_request = make_pr( reviews={ "nodes": [ - opencode_review( - "APPROVED", - "head", - submitted_at="2026-06-25T06:59:59Z", - ) + { + **opencode_review( + "CHANGES_REQUESTED", + "head", + submitted_at="2026-08-24T00:00:00Z", + ), + "body": ( + "coverage evidence did not pass; coverage-evidence reported that " + "required test/docstring evidence was not proven" + ), + } ] } ) - assert sched.has_current_head_approval(stale_review) - same_timestamp_review = make_pr( - reviews={ - "nodes": [ - opencode_review( - "APPROVED", - "head", - submitted_at="2026-06-25T07:00:00Z", - ) - ] - } + monkeypatch.setattr( + sched, + "latest_opencode_dispatch_started_at", + lambda repo, workflow, pr, since=None: (_ for _ in ()).throw( + RuntimeError("temporary API failure") + ), ) - assert sched.has_current_head_approval(same_timestamp_review) - missing_review_time = make_pr( + + assert sched.coverage_retry_wait_reason( + coverage_request, + repo="owner/repo", + workflow="OpenCode Review", + ) == "same-head OpenCode dispatch history is unavailable; defer same-head re-review" + + +def test_coverage_retry_floor_keeps_newer_review_timestamp(monkeypatch): + """An older dispatch cannot extend or replace the newer review timestamp.""" + coverage_request = make_pr( reviews={ "nodes": [ { - "state": "APPROVED", - "author": {"login": "opencode-agent"}, - "commit": {"oid": "head"}, + **opencode_review( + "CHANGES_REQUESTED", + "head", + submitted_at="2026-08-24T02:00:00Z", + ), + "body": ( + "coverage evidence did not pass; coverage-evidence reported that " + "required test/docstring evidence was not proven" + ), } ] } ) - assert sched.has_current_head_approval(missing_review_time) - human_review_only = make_pr( - reviews={"nodes": [opencode_review("APPROVED", "head", login="human")]} + monkeypatch.setattr( + sched, + "latest_opencode_dispatch_started_at", + lambda repo, workflow, pr, since=None: datetime(2026, 8, 24, 1, 0, tzinfo=timezone.utc), ) - assert not sched.has_current_head_approval(human_review_only) - superseded = make_pr( + + assert sched.coverage_retry_wait_reason( + coverage_request, + repo="owner/repo", + workflow="OpenCode Review", + now=datetime(2026, 8, 24, 2, 30, tzinfo=timezone.utc), + ) == "same-head OpenCode coverage retry floor has not elapsed" + + +def test_coverage_retry_without_timestamp_fails_closed(monkeypatch): + """A coverage-only review without a timestamp cannot authorize redispatch.""" + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", + ) + monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda *_: None) + coverage_review = opencode_review("CHANGES_REQUESTED", "head") + coverage_review.pop("submittedAt") + coverage_request = make_pr( reviews={ "nodes": [ - opencode_review("CHANGES_REQUESTED", "head"), - opencode_review("APPROVED", "head"), + { + **coverage_review, + "body": ( + "OpenCode cannot approve yet because required coverage evidence " + "did not pass. The coverage-evidence gate reported that required " + "test/docstring evidence was not proven." + ), + } ] - } + }, + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(), + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": "SUCCESS", + }, + {**opencode_check(status="COMPLETED"), "conclusion": "FAILURE"}, + ] + } + }, ) - assert sched.has_current_head_approval(superseded) - assert not sched.has_current_head_changes_requested(superseded) - stale_gate_reviews = make_pr( + decision = inspect(coverage_request) + + assert decision.action == "wait" + assert "no valid submission timestamp" in decision.reason + + +def test_coverage_retry_keeps_failed_opencode_workflow_siblings_fail_closed( + monkeypatch, +): + """Only the superseded review job is ignored during coverage retry.""" + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", + ) + monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda *_: None) + dispatched = [] + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda *args, **kwargs: dispatched.append((args, kwargs)) or "dispatched", + ) + coverage_request = make_pr( reviews={ "nodes": [ { - **opencode_review("CHANGES_REQUESTED", "old", login="github-actions[bot]"), - "databaseId": 101, - "body": "OpenCode cannot approve this previous head.", - }, + **opencode_review("CHANGES_REQUESTED", "head"), + "body": ( + "OpenCode cannot approve yet because required coverage evidence " + "did not pass. The coverage-evidence gate reported that required " + "test/docstring evidence was not proven." + ), + } + ] + }, + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(), + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": "SUCCESS", + }, + { + **opencode_check(status="COMPLETED"), + "conclusion": "FAILURE", + }, + { + "__typename": "CheckRun", + "name": "coverage-source-tree", + "status": "COMPLETED", + "conclusion": "FAILURE", + "checkSuite": { + "workflowRun": { + "workflow": {"name": "Required OpenCode Review"} + } + }, + }, + ] + } + }, + ) + + decision = inspect(coverage_request) + + assert sched.failed_status_checks(coverage_request, ignore_opencode=True) == [ + "coverage-source-tree" + ] + assert decision.action == "block" + assert decision.reason == "current-head OpenCode review requested changes" + assert dispatched == [] + + +def test_conflicting_coverage_retry_blocks_with_conflict_guidance(monkeypatch): + """Coverage-only retries never dispatch while the exact head conflicts.""" + monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda *_: None) + dispatched = [] + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda *args, **kwargs: dispatched.append((args, kwargs)) or "dispatched", + ) + coverage_request = make_pr( + mergeStateStatus="CONFLICTING", + restMergeableState="CONFLICTING", + reviews={ + "nodes": [ { - **opencode_review("CHANGES_REQUESTED", "older"), - "databaseId": 102, - }, + **opencode_review("CHANGES_REQUESTED", "head"), + "body": ( + "OpenCode cannot approve yet because required coverage evidence " + "did not pass. The coverage-evidence gate reported that required " + "test/docstring evidence was not proven." + ), + } + ] + }, + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(), + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": "SUCCESS", + }, + { + "__typename": "CheckRun", + "name": "opencode-review", + "status": "COMPLETED", + "conclusion": "FAILURE", + }, + ] + } + }, + ) + + decision = inspect(coverage_request) + + assert decision.action == "block" + assert "merge conflict: CONFLICTING" in decision.reason + assert dispatched == [] + + +def test_review_state_and_failed_checks(): + pr = make_pr(reviews={"nodes": [opencode_review("APPROVED", "old"), opencode_review("APPROVED", "head")]}) + assert sched.current_head_review_state(pr, "APPROVED") + assert sched.has_current_head_approval(pr) + assert not sched.has_current_head_changes_requested(pr) + assert not sched.has_current_head_approval( + make_pr(headRefOid="", reviews={"nodes": [opencode_review("APPROVED", "head")]}) + ) + exact_head = "a" * 40 + stale_body_head = "b" * 40 + body_sha_mismatch = make_pr( + headRefOid=exact_head, + reviews={ + "nodes": [ { - **opencode_review("CHANGES_REQUESTED", "old", login="github-actions[bot]"), - "databaseId": 103, - "body": "An unrelated automation requested changes.", - }, + **opencode_review("APPROVED", exact_head), + "body": f"## Gate evidence\n\n- Head SHA: `{stale_body_head}`", + } + ] + }, + ) + assert sched.review_body_head_sha(body_sha_mismatch["reviews"]["nodes"][0]) == stale_body_head + assert not sched.has_current_head_approval(body_sha_mismatch) + body_sha_match = make_pr( + headRefOid=exact_head, + reviews={ + "nodes": [ { - **opencode_review("CHANGES_REQUESTED", "old", login="human"), - "databaseId": 104, - "body": "OpenCode was mentioned by a human reviewer.", - }, + **opencode_review("APPROVED", exact_head), + "body": f"## Gate evidence\n\n- Head SHA: `{exact_head.upper()}`", + } + ] + }, + ) + assert sched.has_current_head_approval(body_sha_match) + body_sha_only_match = make_pr( + headRefOid=exact_head, + reviews={ + "nodes": [ { - **opencode_review("CHANGES_REQUESTED", "head"), - "databaseId": 105, - }, - opencode_review("CHANGES_REQUESTED", "old"), + **opencode_review("APPROVED", ""), + "body": f"## Gate evidence\n\n- Head SHA: `{exact_head}`", + } + ] + }, + ) + assert sched.has_current_head_approval(body_sha_only_match) + body_sha_only_mismatch = make_pr( + headRefOid=exact_head, + reviews={ + "nodes": [ + { + **opencode_review("APPROVED", ""), + "body": f"## Gate evidence\n\n- Head SHA: `{stale_body_head}`", + } + ] + }, + ) + assert not sched.has_current_head_approval(body_sha_only_mismatch) + body_sha_does_not_override_commit = make_pr( + headRefOid=exact_head, + reviews={ + "nodes": [ + { + **opencode_review("APPROVED", stale_body_head), + "body": f"## Gate evidence\n\n- Head SHA: `{exact_head}`", + } + ] + }, + ) + assert not sched.has_current_head_approval(body_sha_does_not_override_commit) + deterministic_fallback = make_pr( + headRefOid=exact_head, + reviews={ + "nodes": [ + { + **opencode_review("APPROVED", exact_head), + "body": ( + "OpenCode model attempts did not emit a usable current-head " + "control block, so the approval gate used deterministic " + "current-head evidence instead of model prose." + ), + } + ] + }, + ) + assert sched.is_deterministic_fallback_approval( + deterministic_fallback["reviews"]["nodes"][0] + ) + assert not sched.is_deterministic_fallback_approval( + opencode_review("CHANGES_REQUESTED", exact_head) + ) + assert sched.has_current_head_deterministic_fallback_approval(deterministic_fallback) + assert not sched.has_current_head_approval(deterministic_fallback) + fallback_scan_without_current_opencode = make_pr( + reviews={ + "nodes": [ + opencode_review("APPROVED", "old"), + opencode_review("APPROVED", "head", login="human"), + ] + } + ) + assert not sched.has_current_head_deterministic_fallback_approval( + fallback_scan_without_current_opencode + ) + stale_review = make_pr( + reviews={ + "nodes": [ + opencode_review( + "APPROVED", + "head", + submitted_at="2026-06-25T06:59:59Z", + ) + ] + } + ) + assert sched.has_current_head_approval(stale_review) + same_timestamp_review = make_pr( + reviews={ + "nodes": [ + opencode_review( + "APPROVED", + "head", + submitted_at="2026-06-25T07:00:00Z", + ) ] } ) - assert sched.stale_opencode_change_request_ids(stale_gate_reviews) == [101, 102] + assert sched.has_current_head_approval(same_timestamp_review) + missing_review_time = make_pr( + reviews={ + "nodes": [ + { + "state": "APPROVED", + "author": {"login": "opencode-agent"}, + "commit": {"oid": "head"}, + } + ] + } + ) + assert sched.has_current_head_approval(missing_review_time) + human_review_only = make_pr( + reviews={"nodes": [opencode_review("APPROVED", "head", login="human")]} + ) + assert not sched.has_current_head_approval(human_review_only) + superseded = make_pr( + reviews={ + "nodes": [ + opencode_review("CHANGES_REQUESTED", "head"), + opencode_review("APPROVED", "head"), + ] + } + ) + assert sched.has_current_head_approval(superseded) + assert not sched.has_current_head_changes_requested(superseded) + + coverage_request = make_pr( + reviews={ + "nodes": [ + { + **opencode_review("CHANGES_REQUESTED", "head"), + "body": ( + "OpenCode cannot approve yet because required coverage evidence did not pass. " + "The coverage-evidence gate reported that required test/docstring evidence was not proven." + ), + } + ] + }, + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(), + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": "SUCCESS", + }, + ] + } + }, + ) + assert sched.current_head_coverage_change_request(coverage_request) + assert sched.coverage_evidence_state(coverage_request) == "complete" + assert sched.coverage_evidence_state( + make_pr( + statusCheckRollup={ + "contexts": {"nodes": [{"name": "coverage-evidence", "state": "PENDING"}]} + } + ) + ) == "running" + assert sched.coverage_evidence_state( + make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "IN_PROGRESS", + } + ] + } + } + ) + ) == "running" + assert sched.coverage_evidence_state( + make_pr(statusCheckRollup={"contexts": {"nodes": [strix_check()]}}) + ) == "missing" + assert sched.coverage_evidence_state( + make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + {"name": "coverage-evidence", "state": "SUCCESS"}, + {"name": "lint", "state": "SUCCESS"}, + ] + } + } + ) + ) == "complete" + assert sched.coverage_evidence_state( + make_pr( + statusCheckRollup={ + "contexts": {"nodes": [{"name": "coverage-evidence", "state": "FAILURE"}]} + } + ) + ) == "failed" + assert sched.coverage_evidence_state(make_pr()) == "missing" + human_coverage_request = make_pr( + reviews={ + "nodes": [ + { + **opencode_review("CHANGES_REQUESTED", "head", login="human"), + "body": "coverage evidence did not pass; coverage-evidence; required test/docstring evidence", + } + ] + } + ) + assert not sched.current_head_coverage_change_request(human_coverage_request) + assert not sched.current_head_coverage_change_request( + make_pr(reviews={"nodes": [opencode_review("APPROVED", "head")]}) + ) + ordinary_request = make_pr( + reviews={ + "nodes": [ + {**opencode_review("CHANGES_REQUESTED", "head"), "body": "Fix the estimator."} + ] + } + ) + assert not sched.current_head_coverage_change_request(ordinary_request) + + stale_gate_reviews = make_pr( + reviews={ + "nodes": [ + { + **opencode_review("CHANGES_REQUESTED", "old", login="github-actions[bot]"), + "databaseId": 101, + "body": "OpenCode cannot approve this previous head.", + }, + { + **opencode_review("CHANGES_REQUESTED", "older"), + "databaseId": 102, + }, + { + **opencode_review("CHANGES_REQUESTED", "old", login="github-actions[bot]"), + "databaseId": 103, + "body": "An unrelated automation requested changes.", + }, + { + **opencode_review("CHANGES_REQUESTED", "old", login="human"), + "databaseId": 104, + "body": "OpenCode was mentioned by a human reviewer.", + }, + { + **opencode_review("CHANGES_REQUESTED", "head"), + "databaseId": 105, + }, + opencode_review("CHANGES_REQUESTED", "old"), + ] + } + ) + assert sched.stale_opencode_change_request_ids(stale_gate_reviews) == [101, 102] + + exact_head_approval = { + **opencode_review("APPROVED", exact_head), + "databaseId": 302, + "body": f"## Gate evidence\n\n- Head SHA: `{exact_head}`", + } + stale_approval_history = make_pr( + headRefOid=exact_head, + reviews={ + "nodes": [ + { + **opencode_review("APPROVED", exact_head), + "databaseId": 300, + "body": f"## Gate evidence\n\n- Head SHA: `{stale_body_head}`", + }, + { + **opencode_review("APPROVED", exact_head), + "databaseId": 301, + "body": f"## Gate evidence\n\n- Head SHA: `{stale_body_head}`", + }, + exact_head_approval, + { + **opencode_review("APPROVED", exact_head, login="github-actions[bot]"), + "databaseId": 303, + "body": f"OpenCode gate.\n\n- Head SHA: `{stale_body_head}`", + }, + { + **opencode_review("APPROVED", exact_head, login="human"), + "databaseId": 304, + "body": f"OpenCode mentioned.\n\n- Head SHA: `{stale_body_head}`", + }, + ] + }, + ) + assert sched.stale_opencode_approval_ids(stale_approval_history) == [303] + stale_approval_history["reviews"]["nodes"].remove(exact_head_approval) + assert sched.stale_opencode_approval_ids(stale_approval_history) == [301, 303] + + failed = make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + {"__typename": "CheckRun", "name": "strix", "conclusion": "FAILURE"}, + {"context": "lint", "state": "ERROR"}, + {"context": "ok", "state": "SUCCESS"}, + ] + } + } + ) + assert sched.failed_status_checks(failed) == ["strix", "lint"] + assert sched.failed_status_checks( + make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + { + "__typename": "StatusContext", + "context": "opencode-review", + "state": "FAILURE", + } + ] + } + } + ), + ignore_opencode=True, + ) == [] + action_required = make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + {"__typename": "CheckRun", "name": "opencode-review", "conclusion": "ACTION_REQUIRED"}, + {"context": "lint", "state": "SUCCESS"}, + ] + } + } + ) + assert sched.failed_status_checks(action_required) == [] + assert sched.action_required_checks(action_required) == ["opencode-review"] + assert sched.workflow_action_required_reason(["a", "b", "c", "d", "e", "f"]).startswith( + "workflow action required: a, b, c, d, e, +1 more" + ) + manual_strix_supersedes_pr_target_failure = make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + {"__typename": "CheckRun", "name": "strix", "conclusion": "FAILURE"}, + {"context": "strix", "state": "SUCCESS"}, + {"context": "lint", "state": "ERROR"}, + ] + } + } + ) + assert sched.failed_status_checks(manual_strix_supersedes_pr_target_failure) == ["lint"] + opencode_pr_target_failure_without_status = make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + {"__typename": "CheckRun", "name": "opencode-review", "conclusion": "FAILURE"}, + ] + } + } + ) + assert sched.failed_status_checks(opencode_pr_target_failure_without_status) == ["opencode-review"] + manual_opencode_supersedes_pr_target_failure = make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + {"__typename": "CheckRun", "name": "opencode-review", "conclusion": "FAILURE"}, + {"context": "opencode-review", "state": "SUCCESS"}, + {"context": "lint", "state": "ERROR"}, + ] + } + } + ) + assert sched.failed_status_checks(manual_opencode_supersedes_pr_target_failure) == ["lint"] + + +def test_scheduler_query_requests_pull_request_author(): + """Fetch the authoritative author identity used by the independent-review gate.""" + assert "\n author { login }\n" in sched.PULL_REQUEST_FIELDS_FRAGMENT + + +def test_scheduler_review_queries_request_bot_typename(): + """Request `__typename` on review authors so GraphQL bot actors are detectable. + + GitHub's GraphQL API can omit the `[bot]` suffix from a bot actor's + `login` (unlike REST, which reliably appends it), but exposes + `__typename: "Bot"` instead. Both review-fetching queries must select + it so `is_bot_review_author` can fall back to it. + """ + assert "author { login __typename }" in sched.PULL_REQUEST_FIELDS_FRAGMENT + assert "author { login __typename }" in sched.PR_REVIEWS_PAGE_QUERY + + +@pytest.mark.parametrize( + ("author", "reviewer", "state", "commit"), + ( + ("", "independent-reviewer", "APPROVED", "head"), + ("pull-request-author", "", "APPROVED", "head"), + ("pull-request-author", "pull-request-author", "APPROVED", "head"), + ("pull-request-author", "opencode-agent", "APPROVED", "head"), + ("pull-request-author", "github-actions[bot]", "APPROVED", "head"), + ("pull-request-author", "noema-review[bot]", "APPROVED", "head"), + ("pull-request-author", "independent-reviewer", "COMMENTED", "head"), + ("pull-request-author", "independent-reviewer", "APPROVED", "old"), + ), +) +def test_independent_approval_fails_closed_for_invalid_evidence( + author, + reviewer, + state, + commit, +): + """Reject missing, self, automation, non-approval, and stale-head evidence.""" + pr = make_pr( + author={"login": author}, + reviewDecision="APPROVED", + reviews={ + "nodes": [ + opencode_review("APPROVED", "head"), + opencode_review(state, commit, login=reviewer), + ] + }, + ) + + assert not sched.has_independent_current_head_approval(pr) + assert "independent" in sched.merge_approval_block_reason(pr).lower() + + +def test_independent_approval_excludes_graphql_bot_actor_missing_suffix(): + """Exclude a GraphQL bot actor even when its `login` lacks the `[bot]` suffix. + + GitHub's GraphQL API can return a bot actor's raw account name (e.g. + `noema-review` instead of REST's `noema-review[bot]`) while still + identifying it as a bot via `__typename: "Bot"`. A login-suffix-only + check would let this slip through as an "independent" human approval -- + an authorization bypass in the exact separation-of-duties gate this + scheduler enforces. + """ + pr = make_pr( + reviewDecision="APPROVED", + reviews={ + "nodes": [ + opencode_review("APPROVED", "head"), + opencode_review( + "APPROVED", + "head", + login="noema-review", + author_typename="Bot", + ), + ] + }, + ) + + assert sched.is_bot_review_author(pr["reviews"]["nodes"][1]) + assert not sched.has_independent_current_head_approval(pr) + assert "independent" in sched.merge_approval_block_reason(pr).lower() + + +def test_is_bot_review_author_covers_suffix_and_typename_and_neither(): + """Detect a bot via either the REST `[bot]` login suffix or GraphQL `__typename`.""" + assert sched.is_bot_review_author(opencode_review(login="dependabot[bot]")) + assert sched.is_bot_review_author( + opencode_review(login="dependabot", author_typename="Bot") + ) + assert not sched.is_bot_review_author( + opencode_review(login="independent-reviewer", author_typename="User") + ) + + +def test_independent_exact_head_approval_allows_direct_merge(): + """Preserve direct merge only when GitHub and independent evidence both pass.""" + pr = make_pr( + reviewDecision="APPROVED", + reviews={ + "nodes": [ + opencode_review("APPROVED", "head"), + opencode_review("APPROVED", "head", login="independent-reviewer"), + ] + }, + ) + + assert sched.has_independent_current_head_approval(pr) + assert sched.merge_approval_block_reason(pr) is None + assert inspect(pr, merge_mode="direct").action == "merge" + + +def test_independent_approval_uses_reviewers_latest_policy_state(): + """A later same-head change request revokes that reviewer's approval.""" + pr = make_pr( + reviewDecision="CHANGES_REQUESTED", + reviews={ + "nodes": [ + opencode_review("APPROVED", "head"), + opencode_review( + "APPROVED", + "head", + login="independent-reviewer", + submitted_at="2026-06-25T07:01:00Z", + ), + opencode_review( + "CHANGES_REQUESTED", + "head", + login="independent-reviewer", + submitted_at="2026-06-25T07:02:00Z", + ), + ] + }, + ) + + assert not sched.has_independent_current_head_approval(pr) + + +@pytest.mark.parametrize("review_decision", ("", "REVIEW_REQUIRED")) +def test_repository_review_policy_blocks_merge_and_disarms_auto_merge(review_decision): + """Never leave merge authority armed while GitHub review policy is unsatisfied.""" + reviews = { + "nodes": [ + opencode_review("APPROVED", "head"), + opencode_review("APPROVED", "head", login="independent-reviewer"), + ] + } + blocked = make_pr(reviewDecision=review_decision, reviews=reviews) + blocked_mergeability = make_pr( + mergeStateStatus="BLOCKED", + reviewDecision=review_decision, + reviews=reviews, + ) + armed = make_pr( + reviewDecision=review_decision, + reviews=reviews, + autoMergeRequest={"enabledAt": "now"}, + ) + + decision = inspect(blocked, merge_mode="direct") + disarm = inspect(armed, merge_mode="direct") + + assert decision.action == "wait" + assert "reviewDecision" in decision.reason + assert inspect(blocked_mergeability, merge_mode="direct").action == "wait" + assert disarm.action == "disable_auto_merge" + assert "reviewDecision" in disarm.reason + + +def test_missing_independent_approval_blocks_and_disarms_auto_merge(): + """Do not let aggregate GitHub state substitute for exact independent evidence.""" + reviews = {"nodes": [opencode_review("APPROVED", "head")]} + blocked = make_pr(reviewDecision="APPROVED", reviews=reviews) + armed = make_pr( + reviewDecision="APPROVED", + reviews=reviews, + autoMergeRequest={"enabledAt": "now"}, + ) + + assert inspect(blocked, merge_mode="direct").action == "wait" + assert inspect(armed, merge_mode="direct").action == "disable_auto_merge" + + +def test_outdated_unapproved_branch_disarms_stale_auto_merge_instead_of_updating(): + """A stale auto-merge request must not survive an unapproved branch update. + + ``autoMergeRequest`` can still be armed from before a new, as-yet-unreviewed + push landed (GitHub does not always dismiss stale approvals on push). If the + branch is also behind base, silently requesting a branch update while + leaving that auto-merge request queued would let GitHub's own native + auto-merge complete the merge once the updated head's required checks pass + -- without this scheduler ever getting a chance to require a fresh + independent approval on the new head. The scheduler must disarm auto-merge + instead of preserving it through the branch-update wait path. + """ + outdated_unapproved_armed = make_pr( + mergeStateStatus="BEHIND", + autoMergeRequest={"enabledAt": "now"}, + reviews={"nodes": []}, + ) + + decision = inspect(outdated_unapproved_armed) + + assert decision.action == "disable_auto_merge" + assert "no live current-head approval" in decision.reason + assert "branch is 1 commit(s) behind base" in decision.reason + # The prior behavior's literal wording documented the bug: it kept the + # stale auto-merge request queued instead of disarming it. + assert "remains queued" not in decision.reason + + +def test_clean_unapproved_armed_pr_disarms_auto_merge_before_review_dispatch(): + """An unapproved CLEAN PR with stale auto-merge armed must disarm, not dispatch-and-leave-armed. + + ``behind_by`` is falsy here (the branch is not behind base), so neither + behind-by disarm path applies. With no current-head approval and no + completed Strix evidence, the ordinary review-dispatch cascade would + normally return a plain ``security_dispatch``/``wait`` decision -- and + every branch in that cascade previously returned without ever checking + ``autoMergeRequest``. That left a stale auto-merge request (e.g. armed by + an approval a later push has since invalidated, or armed by a human + before any review ran) queued through the PR's ordinary, everyday + review-in-progress state. If GitHub's own required checks are not + themselves gated on this scheduler's OpenCode approval, GitHub's native + auto-merge could complete the merge without this scheduler ever getting a + chance to require a fresh independent approval -- the exact bypass this + scheduler exists to prevent. + """ + clean_unapproved_armed = make_pr( + mergeStateStatus="CLEAN", + autoMergeRequest={"enabledAt": "now"}, + reviews={"nodes": []}, + ) + + decision = inspect(clean_unapproved_armed) + + assert decision.action == "disable_auto_merge" + assert "current head has no OpenCode approval" in decision.reason + assert "wait for fresh same-head approval" in decision.reason + + +def test_workflow_run_followup_defers_deterministic_fallback_retry(monkeypatch): + head = "a" * 40 + fallback_review = { + **opencode_review("APPROVED", head), + "body": ( + "OpenCode model providers were unavailable, so deterministic current-head evidence " + f"was used.\n\n- Head SHA: `{head}`" + ), + } + pr = make_pr( + headRefOid=head, + reviews={"nodes": [fallback_review]}, + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(), + opencode_check(status="COMPLETED"), + ] + } + }, + ) + dispatched = [] + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, current_pr, dry_run: dispatched.append( + (repo, workflow, current_pr["headRefOid"], dry_run) + ), + ) + + monkeypatch.setenv("GITHUB_EVENT_NAME", "workflow_run") + followup = inspect(pr) + + assert followup.action == "wait" + assert "next scheduler heartbeat" in followup.reason + assert dispatched == [] + + monkeypatch.setenv("GITHUB_EVENT_NAME", "schedule") + heartbeat = inspect(pr) + + assert heartbeat.action == "review_dispatch" + 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( + headRefOid=head, + reviews={ + "nodes": [ + { + **opencode_review("APPROVED", ""), + "body": f"## Gate evidence\n\n- Head SHA: `{head}`", + }, + opencode_review("APPROVED", head, login="independent-reviewer"), + ] + }, + reviewDecision="APPROVED", + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(), + opencode_check(status="COMPLETED"), + ] + } + }, + ) + dispatched = [] + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, current_pr, dry_run: dispatched.append( + (repo, workflow, current_pr["headRefOid"], dry_run) + ), + ) - exact_head_approval = { - **opencode_review("APPROVED", exact_head), - "databaseId": 302, - "body": f"## Gate evidence\n\n- Head SHA: `{exact_head}`", - } - stale_approval_history = make_pr( - headRefOid=exact_head, + decision = inspect(pr) + + assert decision.action == "auto_merge" + assert "current head is approved" in decision.reason + assert dispatched == [] + + +def test_deterministic_fallback_detection_ignores_unrelated_reviews(): + head = "a" * 40 + pr = make_pr( + headRefOid=head, reviews={ "nodes": [ + opencode_review("APPROVED", head), + opencode_review("APPROVED", "b" * 40), { - **opencode_review("APPROVED", exact_head), - "databaseId": 300, - "body": f"## Gate evidence\n\n- Head SHA: `{stale_body_head}`", - }, - { - **opencode_review("APPROVED", exact_head), - "databaseId": 301, - "body": f"## Gate evidence\n\n- Head SHA: `{stale_body_head}`", - }, - exact_head_approval, - { - **opencode_review("APPROVED", exact_head, login="github-actions[bot]"), - "databaseId": 303, - "body": f"OpenCode gate.\n\n- Head SHA: `{stale_body_head}`", - }, - { - **opencode_review("APPROVED", exact_head, login="human"), - "databaseId": 304, - "body": f"OpenCode mentioned.\n\n- Head SHA: `{stale_body_head}`", + "state": "APPROVED", + "author": {"login": "human-reviewer"}, + "commit": {"oid": head}, }, ] }, ) - assert sched.stale_opencode_approval_ids(stale_approval_history) == [303] - stale_approval_history["reviews"]["nodes"].remove(exact_head_approval) - assert sched.stale_opencode_approval_ids(stale_approval_history) == [301, 303] - failed = make_pr( - statusCheckRollup={ - "contexts": { - "nodes": [ - {"__typename": "CheckRun", "name": "strix", "conclusion": "FAILURE"}, - {"context": "lint", "state": "ERROR"}, - {"context": "ok", "state": "SUCCESS"}, - ] - } - } + assert not sched.has_current_head_deterministic_fallback_approval(pr) + assert not sched.has_current_head_deterministic_fallback_approval( + make_pr(reviews={"nodes": []}) ) - assert sched.failed_status_checks(failed) == ["strix", "lint"] - action_required = make_pr( - statusCheckRollup={ - "contexts": { - "nodes": [ - {"__typename": "CheckRun", "name": "opencode-review", "conclusion": "ACTION_REQUIRED"}, - {"context": "lint", "state": "SUCCESS"}, + + +def test_current_head_approval_cleans_previous_head_change_gate_before_merge(): + pr = make_pr( + reviews={ + "nodes": [ + { + **opencode_review("CHANGES_REQUESTED", "old"), + "databaseId": 301, + }, + opencode_review("APPROVED", "head"), + opencode_review("APPROVED", "head", login="independent-reviewer"), ] - } - } - ) - assert sched.failed_status_checks(action_required) == [] - assert sched.action_required_checks(action_required) == ["opencode-review"] - assert sched.workflow_action_required_reason(["a", "b", "c", "d", "e", "f"]).startswith( - "workflow action required: a, b, c, d, e, +1 more" + }, + reviewDecision="APPROVED", + ) + + decision = inspect(pr) + + assert decision.action == "auto_merge" + assert decision.notes == ( + "Would dismiss 1 previous-head automated OpenCode change-request review(s); " + "exact-current-head approval supersedes those stale gates.", ) - manual_strix_supersedes_pr_target_failure = make_pr( + + +def test_failed_status_checks_uses_latest_check_run_for_same_workflow_name(): + pr = make_pr( statusCheckRollup={ "contexts": { "nodes": [ - {"__typename": "CheckRun", "name": "strix", "conclusion": "FAILURE"}, - {"context": "strix", "state": "SUCCESS"}, - {"context": "lint", "state": "ERROR"}, + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "conclusion": "CANCELLED", + "startedAt": "2026-07-10T09:00:00Z", + "checkSuite": { + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + } + }, + }, + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "conclusion": "SUCCESS", + "startedAt": "2026-07-10T09:30:00Z", + "checkSuite": { + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + } + }, + }, + { + "__typename": "CheckRun", + "name": "lint", + "conclusion": "FAILURE", + "startedAt": "2026-07-10T09:31:00Z", + "checkSuite": {"workflowRun": {"workflow": {"name": "CI"}}}, + }, ] } } ) - assert sched.failed_status_checks(manual_strix_supersedes_pr_target_failure) == ["lint"] - opencode_pr_target_failure_without_status = make_pr( + + assert sched.failed_status_checks(pr) == ["lint"] + + +def test_failed_status_checks_prefers_timestamped_duplicate_check_runs(): + timestamped_then_missing = make_pr( statusCheckRollup={ "contexts": { "nodes": [ - {"__typename": "CheckRun", "name": "opencode-review", "conclusion": "FAILURE"}, + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "conclusion": "SUCCESS", + "startedAt": "2026-07-10T09:30:00Z", + "checkSuite": { + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + } + }, + }, + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "conclusion": "CANCELLED", + "checkSuite": { + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + } + }, + }, ] } } ) - assert sched.failed_status_checks(opencode_pr_target_failure_without_status) == ["opencode-review"] - manual_opencode_supersedes_pr_target_failure = make_pr( + assert sched.failed_status_checks(timestamped_then_missing) == [] + + missing_then_timestamped = make_pr( statusCheckRollup={ "contexts": { "nodes": [ - {"__typename": "CheckRun", "name": "opencode-review", "conclusion": "FAILURE"}, - {"context": "opencode-review", "state": "SUCCESS"}, - {"context": "lint", "state": "ERROR"}, + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "conclusion": "CANCELLED", + "checkSuite": { + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + } + }, + }, + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "conclusion": "SUCCESS", + "startedAt": "2026-07-10T09:30:00Z", + "checkSuite": { + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + } + }, + }, ] } } ) - assert sched.failed_status_checks(manual_opencode_supersedes_pr_target_failure) == ["lint"] + assert sched.failed_status_checks(missing_then_timestamped) == [] -def test_workflow_run_followup_defers_deterministic_fallback_retry(monkeypatch): - head = "a" * 40 - fallback_review = { - **opencode_review("APPROVED", head), - "body": ( - "OpenCode model providers were unavailable, so deterministic current-head evidence " - f"was used.\n\n- Head SHA: `{head}`" - ), - } +def test_failed_status_checks_treats_cancelled_before_start_rerun_as_authoritative(): + """A newer rerun cancelled before starting outranks an older stale success. + + Regression test for the BUG a GitHub bot reviewer ("Devin") found on this + PR: ``check_run_recency_key`` gave a completed-with-no-``startedAt`` row + (GitHub's shape for "cancelled before it ever started") the lowest tier, + unconditionally below any row with a real ``startedAt`` -- so an older, + already-successful run could outrank a newer rerun that got cancelled + before starting, purely because the older run happened to have a + timestamp and the newer one did not, regardless of which one actually + happened more recently. This must fail against the pre-fix code and pass + once ``check_run_recency_key`` prefers ``checkSuite.createdAt`` -- set + unconditionally the moment the triggering push/rerun/dispatch creates the + check suite, strictly before any of its check runs can start -- over a + bare ``startedAt``. Both runs here carry a ``checkSuite.createdAt``, as + real GitHub responses always do; the newer one must win even though it + never started. + """ pr = make_pr( - headRefOid=head, - reviews={"nodes": [fallback_review]}, statusCheckRollup={ "contexts": { "nodes": [ - strix_check(), - opencode_check(status="COMPLETED"), + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "status": "COMPLETED", + "conclusion": "SUCCESS", + "startedAt": "2026-08-24T01:00:00Z", + "checkSuite": { + "createdAt": "2026-08-24T00:59:00Z", + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + }, + }, + }, + { + "__typename": "CheckRun", + "name": "scan-pr-queue", + "status": "COMPLETED", + "conclusion": "CANCELLED", + "startedAt": None, + "checkSuite": { + "createdAt": "2026-08-24T02:00:00Z", + "workflowRun": { + "workflow": {"name": "Required PR Review Merge Scheduler"} + }, + }, + }, ] - } - }, - ) - dispatched = [] - monkeypatch.setattr( - sched, - "dispatch_opencode_review", - lambda repo, workflow, current_pr, dry_run: dispatched.append( - (repo, workflow, current_pr["headRefOid"], dry_run) - ), - ) - - monkeypatch.setenv("GITHUB_EVENT_NAME", "workflow_run") - followup = inspect(pr) - - assert followup.action == "wait" - assert "next scheduler heartbeat" in followup.reason - assert dispatched == [] - - monkeypatch.setenv("GITHUB_EVENT_NAME", "schedule") - heartbeat = inspect(pr) - - assert heartbeat.action == "review_dispatch" - 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" - ), - } + check_runs = sched.latest_check_runs(pr) + assert len(check_runs) == 1 + assert check_runs[0]["conclusion"] == "CANCELLED" + assert sched.failed_status_checks(pr) == ["scan-pr-queue"] + + +def test_strix_evidence_state_treats_cancelled_before_start_rerun_as_authoritative(): + """A newer Strix rerun cancelled before starting outranks an older stale success. + + Sibling regression to + ``test_failed_status_checks_treats_cancelled_before_start_rerun_as_authoritative``, + but through ``latest_check_run_attempts``/``strix_evidence_state`` rather + than ``latest_check_runs``/``failed_status_checks``. Before + ``latest_check_run_attempts`` was refactored to share + ``check_run_recency_key`` with ``latest_check_runs``, it ranked same-identity + CheckRun reruns with its own inline ``startedAt``-only comparison: a + completed-with-no-``startedAt`` row (GitHub's shape for "cancelled before + it ever started") could never outrank an older row that does carry a + ``startedAt``, purely because the older row has a timestamp and the newer + one does not -- the exact bug class ``check_run_recency_key`` fixed for + ``latest_check_runs``. Both runs here carry a ``checkSuite.createdAt``, as + real GitHub responses always do; the newer cancellation must win, and + ``strix_evidence_state`` must report the genuinely current "failed" + (a cancellation is a terminal non-success), not the stale "complete" a + lingering older success would otherwise leave in place. + """ pr = make_pr( - reviews={"nodes": [review]}, statusCheckRollup={ "contexts": { - "nodes": [opencode_check(status="COMPLETED"), strix_check()] + "nodes": [ + { + "__typename": "CheckRun", + "name": "strix", + "status": "COMPLETED", + "conclusion": "SUCCESS", + "startedAt": "2026-08-24T01:00:00Z", + "checkSuite": { + "createdAt": "2026-08-24T00:59:00Z", + "workflowRun": {"workflow": {"name": "Strix Security Scan"}}, + }, + }, + { + "__typename": "CheckRun", + "name": "strix", + "status": "COMPLETED", + "conclusion": "CANCELLED", + "startedAt": None, + "checkSuite": { + "createdAt": "2026-08-24T02:00:00Z", + "workflowRun": {"workflow": {"name": "Strix Security Scan"}}, + }, + }, + ] } - }, - ) - dispatched = [] - monkeypatch.setattr( - sched, - "dispatch_opencode_review", - lambda repo, workflow, pr, dry_run: dispatched.append((repo, workflow)) or "dispatched", + } ) - decision = inspect(pr) + attempts = sched.latest_check_run_attempts(sched.context_nodes(pr)) + assert len(attempts) == 1 + assert attempts[0]["conclusion"] == "CANCELLED" + assert sched.strix_evidence_state(pr) == "failed" - 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_coverage_evidence_state_prefers_newest_rerun(): + """An older failed rerun cannot hide newer successful coverage evidence.""" + + def coverage_check(started_at: str, conclusion: str) -> dict: + return { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": conclusion, + "startedAt": started_at, + "checkSuite": {"workflowRun": {"workflow": {"name": "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()] + "nodes": [ + coverage_check("2026-08-24T02:00:00Z", "SUCCESS"), + coverage_check("2026-08-24T01:00:00Z", "FAILURE"), + ] } - }, - ) - dispatched = [] - monkeypatch.setattr( - sched, - "dispatch_opencode_review", - lambda repo, workflow, pr, dry_run: dispatched.append((repo, workflow)) or "dispatched", + } ) - decision = inspect(pr) + assert sched.coverage_evidence_state(pr) == "complete" - assert decision.action == "review_dispatch" - assert decision.reason == ( - "stacked PR onto feature-base; OpenCode review dispatched" - ) - assert dispatched == [("owner/repo", "OpenCode Review")] +def test_coverage_evidence_state_prefers_queued_rerun_over_stale_completed_run(): + """A freshly queued rerun with no startedAt outranks an older completed run. -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" - ), - } + GitHub's ``CheckRun.startedAt`` is legitimately null while a check is + still ``QUEUED``. A newly dispatched coverage-evidence rerun (appearing + later in the rollup than the run it replaces) must not lose to that + older, already-completed run just because it has not started yet -- + otherwise the scheduler could authorize a redispatch decision based on a + stale conclusion while the real, currently-relevant rerun is still + pending. + """ 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(), + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": "SUCCESS", + "startedAt": "2026-08-24T01:00:00Z", + "checkSuite": {"workflowRun": {"workflow": {"name": "OpenCode Review"}}}, + }, + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "QUEUED", + "conclusion": None, + "startedAt": None, + "checkSuite": {"workflowRun": {"workflow": {"name": "OpenCode Review"}}}, + }, ] } - }, - ) - 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) + check_runs = sched.latest_check_runs(pr) + assert len(check_runs) == 1 + assert check_runs[0]["status"] == "QUEUED" + assert sched.coverage_evidence_state(pr) == "running" - assert decision.action == "skip" - assert dispatched == [] +def test_coverage_evidence_state_prefers_newest_run_across_workflows(): + """Coverage evidence uses time, not rollup order, across workflow names.""" + + def coverage_check(workflow: str, started_at: str, conclusion: str) -> dict: + return { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": conclusion, + "startedAt": started_at, + "checkSuite": {"workflowRun": {"workflow": {"name": workflow}}}, + } -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"), + coverage_check( + "OpenCode Review Dispatch", + "2026-08-24T02:00:00Z", + "SUCCESS", + ), + coverage_check( + "Required OpenCode Review", + "2026-08-24T01:00:00Z", + "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" + assert sched.coverage_evidence_state(pr) == "complete" -def test_body_head_sha_approval_prevents_same_run_opencode_rerun(monkeypatch): - head = "a" * 40 +def test_coverage_evidence_state_prefers_queued_rerun_over_stale_completed_run_across_workflows(): + """A freshly queued rerun in a different workflow outranks an older completed run. + + Sibling of test_coverage_evidence_state_prefers_queued_rerun_over_stale_completed_run: + that test covers the same-(workflow, name) case inside latest_check_runs; this one + covers latest_coverage_evidence_index's own cross-workflow-name selection, which has + the same null-``startedAt``-while-``QUEUED`` pitfall. An older, already-completed + "Required OpenCode Review" coverage-evidence run must not beat a newer, still-QUEUED + "OpenCode Review Dispatch" coverage-evidence run just because GitHub has not yet + populated the queued run's ``startedAt`` -- otherwise the scheduler could report + stale "complete" coverage state while the real, currently-relevant rerun is pending. + """ pr = make_pr( - headRefOid=head, - reviews={ - "nodes": [ - { - **opencode_review("APPROVED", ""), - "body": f"## Gate evidence\n\n- Head SHA: `{head}`", - } - ] - }, statusCheckRollup={ "contexts": { "nodes": [ - strix_check(), - opencode_check(status="COMPLETED"), + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": "SUCCESS", + "startedAt": "2026-08-24T01:00:00Z", + "checkSuite": { + "workflowRun": {"workflow": {"name": "Required OpenCode Review"}} + }, + }, + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "QUEUED", + "conclusion": None, + "startedAt": None, + "checkSuite": { + "workflowRun": {"workflow": {"name": "OpenCode Review Dispatch"}} + }, + }, ] } - }, + } ) - dispatched = [] - monkeypatch.setattr( - sched, - "dispatch_opencode_review", - lambda repo, workflow, current_pr, dry_run: dispatched.append( - (repo, workflow, current_pr["headRefOid"], dry_run) + + check_runs = sched.latest_check_runs(pr) + assert len(check_runs) == 2 + latest_index = sched.latest_coverage_evidence_index(check_runs) + assert check_runs[latest_index]["status"] == "QUEUED" + assert sched.coverage_evidence_state(pr) == "running" + + +def _coverage_check( + workflow: str, + status: str, + conclusion: str | None, + started_at: str | None, + suite_created_at: str | None = None, +) -> dict: + """Build a minimal coverage-evidence CheckRun node for fold-order tests.""" + return { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": status, + "conclusion": conclusion, + "startedAt": started_at, + "checkSuite": { + "createdAt": suite_created_at, + "workflowRun": {"workflow": {"name": workflow}}, + }, + } + + +def test_latest_coverage_evidence_index_stays_transitive_across_three_candidates(): + """A left-to-right pairwise fold over 3+ candidates must not go non-transitive. + + Regression test for the exact scenario a GitHub bot reviewer ("Devin") + identified on this PR: folding ``check_run_supersedes`` two at a time + across MORE than two coverage-evidence candidates is only valid if the + relation it applies is a transitive total order. It was not: a + queued/null-``startedAt`` candidate legitimately supersedes an older + completed predecessor in isolation, but a THIRD, differently-timestamped + completed candidate arriving later in fold order could silently override + that queued winner just because "a timestamped candidate beats a + null-timestamp current-best" -- even though that third candidate is + itself OLDER than the timestamped run the queued candidate had already + displaced. + + Candidate order: completed@02:00, queued(startedAt=None), completed@01:00, + each in a different workflow so all three survive into + ``latest_coverage_evidence_index`` unchanged. A currently-pending rerun + is presumed newer than any already-resolved run (the same rule the + queued-vs-single-completed-run tests above already lock in), so the + queued candidate must win outright -- and, either way, the stale 01:00 + completed run (older than the 02:00 run it never legitimately beat) must + never be the answer. + """ + check_runs = [ + _coverage_check("Workflow A", "COMPLETED", "SUCCESS", "2026-08-24T02:00:00Z"), + _coverage_check("Workflow B", "QUEUED", None, None), + _coverage_check("Workflow C", "COMPLETED", "FAILURE", "2026-08-24T01:00:00Z"), + ] + + latest_index = sched.latest_coverage_evidence_index(check_runs) + + assert latest_index != 2, "the stale 01:00 completed run must never win the fold" + assert check_runs[latest_index]["status"] == "QUEUED" + + +def test_latest_coverage_evidence_index_prefers_queued_when_it_appears_first(): + """A queued rerun ranks correctly regardless of its position in the fold. + + Sibling of the reordering check above: places the queued/null-``startedAt`` + candidate FIRST instead of last, so a left-to-right fold cannot lean on + "the queued run happened to already be the running best" to get the + right answer by accident. + """ + check_runs = [ + _coverage_check("OpenCode Review Dispatch", "QUEUED", None, None), + _coverage_check("Required OpenCode Review", "COMPLETED", "SUCCESS", "2026-08-24T02:00:00Z"), + ] + + latest_index = sched.latest_coverage_evidence_index(check_runs) + + assert check_runs[latest_index]["status"] == "QUEUED" + + +def test_latest_coverage_evidence_index_ranks_cancelled_before_start_rerun_above_stale_success(): + """Cross-workflow sibling of the same-name cancelled-before-start regression. + + Same BUG as ``test_failed_status_checks_treats_cancelled_before_start_rerun_as_authoritative``, + exercised through ``latest_coverage_evidence_index``'s cross-workflow-name + comparison instead of ``latest_check_runs``' same-(workflow, name) one. An + older "Required OpenCode Review" success must not beat a newer "OpenCode + Review Dispatch" rerun that was cancelled before it ever started, once + both carry a ``checkSuite.createdAt``. + """ + check_runs = [ + _coverage_check( + "Required OpenCode Review", + "COMPLETED", + "SUCCESS", + "2026-08-24T01:00:00Z", + suite_created_at="2026-08-24T00:59:00Z", ), - ) + _coverage_check( + "OpenCode Review Dispatch", + "COMPLETED", + "CANCELLED", + None, + suite_created_at="2026-08-24T02:00:00Z", + ), + ] - decision = inspect(pr) + latest_index = sched.latest_coverage_evidence_index(check_runs) - assert decision.action == "auto_merge" - assert "current head is approved" in decision.reason - assert dispatched == [] + assert check_runs[latest_index]["conclusion"] == "CANCELLED" -def test_deterministic_fallback_detection_ignores_unrelated_reviews(): - head = "a" * 40 - pr = make_pr( - headRefOid=head, - reviews={ - "nodes": [ - opencode_review("APPROVED", head), - opencode_review("APPROVED", "b" * 40), - { - "state": "APPROVED", - "author": {"login": "human-reviewer"}, - "commit": {"oid": head}, - }, - ] - }, - ) +def test_latest_coverage_evidence_index_prefers_newer_completed_run_over_older_queued_run(): + """A genuinely newer completed rerun still outranks an older queued run. - assert not sched.has_current_head_deterministic_fallback_approval(pr) - assert not sched.has_current_head_deterministic_fallback_approval( - make_pr(reviews={"nodes": []}) - ) + Devin's fix framing's second scenario: with ``checkSuite.createdAt`` + available for both candidates (as real GitHub responses always provide), + the fold must rank by that real creation time rather than blindly + presuming "queued always means newest" -- otherwise an older queued run + could wrongly out-rank a newer, already-completed result. + """ + check_runs = [ + _coverage_check( + "OpenCode Review Dispatch", + "QUEUED", + None, + None, + suite_created_at="2026-08-24T01:00:00Z", + ), + _coverage_check( + "Required OpenCode Review", + "COMPLETED", + "SUCCESS", + "2026-08-24T02:05:00Z", + suite_created_at="2026-08-24T02:00:00Z", + ), + ] + latest_index = sched.latest_coverage_evidence_index(check_runs) -def test_current_head_approval_cleans_previous_head_change_gate_before_merge(): - pr = make_pr( - reviews={ - "nodes": [ - { - **opencode_review("CHANGES_REQUESTED", "old"), - "databaseId": 301, - }, - opencode_review("APPROVED", "head"), - ] + assert check_runs[latest_index]["conclusion"] == "SUCCESS" + + +def test_central_coverage_placeholder_cannot_mask_dispatch_failure(monkeypatch): + """Central metadata-only coverage success cannot hide failed dispatch evidence.""" + monkeypatch.setenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", "ContextualWisdomLab/.github") + + def coverage_check(workflow: str, started_at: str, conclusion: str) -> dict: + return { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": conclusion, + "startedAt": started_at, + "checkSuite": {"workflowRun": {"workflow": {"name": workflow}}}, + } + + pr = make_pr( + statusCheckRollup={ + "contexts": { + "nodes": [ + coverage_check( + "Required OpenCode Review", + "2026-08-24T03:00:00Z", + "SUCCESS", + ), + coverage_check( + "OpenCode Review Dispatch", + "2026-08-24T02:00:00Z", + "FAILURE", + ), + ] + } } ) - decision = inspect(pr) + assert sched.coverage_evidence_state(pr) == "failed" + assert sched.failed_status_checks(pr) == ["coverage-evidence"] - assert decision.action == "auto_merge" - assert decision.notes == ( - "Would dismiss 1 previous-head automated OpenCode change-request review(s); " - "exact-current-head approval supersedes those stale gates.", - ) +def test_central_coverage_placeholder_failure_cannot_block_dispatch_success(monkeypatch): + """Central metadata-only coverage failure cannot block successful dispatch evidence.""" + monkeypatch.setenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", "ContextualWisdomLab/.github") + + def coverage_check(workflow: str, started_at: str, conclusion: str) -> dict: + return { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": conclusion, + "startedAt": started_at, + "checkSuite": {"workflowRun": {"workflow": {"name": workflow}}}, + } -def test_failed_status_checks_uses_latest_check_run_for_same_workflow_name(): pr = make_pr( statusCheckRollup={ "contexts": { "nodes": [ - { - "__typename": "CheckRun", - "name": "scan-pr-queue", - "conclusion": "CANCELLED", - "startedAt": "2026-07-10T09:00:00Z", - "checkSuite": { - "workflowRun": { - "workflow": {"name": "Required PR Review Merge Scheduler"} - } - }, - }, - { - "__typename": "CheckRun", - "name": "scan-pr-queue", - "conclusion": "SUCCESS", - "startedAt": "2026-07-10T09:30:00Z", - "checkSuite": { - "workflowRun": { - "workflow": {"name": "Required PR Review Merge Scheduler"} - } - }, - }, - { - "__typename": "CheckRun", - "name": "lint", - "conclusion": "FAILURE", - "startedAt": "2026-07-10T09:31:00Z", - "checkSuite": {"workflowRun": {"workflow": {"name": "CI"}}}, - }, + coverage_check( + "Required OpenCode Review", + "2026-08-24T03:00:00Z", + "FAILURE", + ), + coverage_check( + "OpenCode Review Dispatch", + "2026-08-24T02:00:00Z", + "SUCCESS", + ), ] } } ) - assert sched.failed_status_checks(pr) == ["lint"] + assert sched.coverage_evidence_state(pr) == "complete" + assert sched.failed_status_checks(pr) == [] -def test_failed_status_checks_prefers_timestamped_duplicate_check_runs(): - timestamped_then_missing = make_pr( +def test_coverage_retry_ignores_superseded_failure_across_workflows(): + """A newer successful workflow supersedes an older coverage failure.""" + + def coverage_check(workflow: str, started_at: str, conclusion: str) -> dict: + return { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": conclusion, + "startedAt": started_at, + "checkSuite": {"workflowRun": {"workflow": {"name": workflow}}}, + } + + pr = make_pr( statusCheckRollup={ "contexts": { "nodes": [ - { - "__typename": "CheckRun", - "name": "scan-pr-queue", - "conclusion": "SUCCESS", - "startedAt": "2026-07-10T09:30:00Z", - "checkSuite": { - "workflowRun": { - "workflow": {"name": "Required PR Review Merge Scheduler"} - } - }, - }, - { - "__typename": "CheckRun", - "name": "scan-pr-queue", - "conclusion": "CANCELLED", - "checkSuite": { - "workflowRun": { - "workflow": {"name": "Required PR Review Merge Scheduler"} - } - }, - }, + coverage_check( + "Required OpenCode Review", + "2026-08-24T01:00:00Z", + "FAILURE", + ), + coverage_check( + "OpenCode Review Dispatch", + "2026-08-24T02:00:00Z", + "SUCCESS", + ), ] } } ) - assert sched.failed_status_checks(timestamped_then_missing) == [] - missing_then_timestamped = make_pr( + assert sched.failed_status_checks(pr) == [] + assert sched.failed_status_checks(pr, ignore_opencode=True) == [] + + +def test_coverage_retry_keeps_newest_failed_run_authoritative_across_workflows(): + """An unsuccessful newest run remains a blocking coverage failure.""" + + def coverage_check(workflow: str, started_at: str, conclusion: str) -> dict: + return { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": conclusion, + "startedAt": started_at, + "checkSuite": {"workflowRun": {"workflow": {"name": workflow}}}, + } + + pr = make_pr( statusCheckRollup={ "contexts": { "nodes": [ - { - "__typename": "CheckRun", - "name": "scan-pr-queue", - "conclusion": "CANCELLED", - "checkSuite": { - "workflowRun": { - "workflow": {"name": "Required PR Review Merge Scheduler"} - } - }, - }, - { - "__typename": "CheckRun", - "name": "scan-pr-queue", - "conclusion": "SUCCESS", - "startedAt": "2026-07-10T09:30:00Z", - "checkSuite": { - "workflowRun": { - "workflow": {"name": "Required PR Review Merge Scheduler"} - } - }, - }, + coverage_check( + "Required OpenCode Review", + "2026-08-24T01:00:00Z", + "SUCCESS", + ), + coverage_check( + "OpenCode Review Dispatch", + "2026-08-24T02:00:00Z", + "FAILURE", + ), ] } } ) - assert sched.failed_status_checks(missing_then_timestamped) == [] + + assert sched.failed_status_checks(pr, ignore_opencode=True) == ["coverage-evidence"] def test_run_command_failure_scrubs_secrets(monkeypatch): @@ -2615,6 +4519,175 @@ def fake_run(args, stdin=None): assert not any(call[:3] == ["gh", "workflow", "run"] for call in calls) +def test_latest_opencode_dispatch_started_at_matches_exact_completed_run(monkeypatch): + """Completed same-head repository dispatch runs provide a retry timestamp.""" + head_sha = "a" * 40 + monkeypatch.setattr( + sched, + "active_workflow_runs", + lambda repo, statuses, event=None, created=None: [ + {"event": "push", "display_title": "irrelevant"}, + { + "event": "repository_dispatch", + "display_title": "Different workflow owner/repo#1@" + head_sha, + "created_at": "2026-08-24T00:30:00Z", + }, + { + "event": "repository_dispatch", + "display_title": f"Required OpenCode Review owner/repo#1@{head_sha}", + "run_started_at": "2026-08-24T01:00:00Z", + }, + { + "event": "repository_dispatch", + "display_title": f"Required OpenCode Review owner/repo#1@{head_sha}", + "created_at": "2026-08-24T02:00:00Z", + }, + { + "event": "repository_dispatch", + "display_title": f"Required OpenCode Review owner/repo#1@{head_sha}", + "created_at": "2026-08-24T01:30:00Z", + }, + { + "event": "repository_dispatch", + "display_title": f"Required OpenCode Review owner/repo#1@{head_sha}", + }, + { + "event": "repository_dispatch", + "display_title": "Required OpenCode Review owner/repo#1@not-a-sha", + "created_at": "2026-08-24T03:00:00Z", + }, + ], + ) + + assert sched.latest_opencode_dispatch_started_at( + "owner/repo", + "OpenCode Review", + make_pr(headRefOid=head_sha), + ) == datetime(2026, 8, 24, 2, 0, tzinfo=timezone.utc) + + +def test_latest_opencode_dispatch_started_at_returns_none_without_exact_run(monkeypatch): + """Unrelated completed runs cannot become a same-head retry marker.""" + monkeypatch.setattr( + sched, + "active_workflow_runs", + lambda repo, statuses, event=None, created=None: [ + { + "event": "repository_dispatch", + "display_title": "Required OpenCode Review owner/repo#1@" + "b" * 40, + "created_at": "2026-08-24T02:00:00Z", + } + ], + ) + + assert sched.latest_opencode_dispatch_started_at( + "owner/repo", + "OpenCode Review", + make_pr(headRefOid="a" * 40), + ) is None + + +def test_latest_opencode_dispatch_started_at_passes_since_as_created_lower_bound( + monkeypatch, +): + """Forward ``since`` into the underlying ``active_workflow_runs`` REST filters.""" + captured = {} + + def fake_active_workflow_runs(repo, statuses, event=None, created=None): + captured["repo"] = repo + captured["statuses"] = statuses + captured["event"] = event + captured["created"] = created + return [] + + monkeypatch.setattr(sched, "active_workflow_runs", fake_active_workflow_runs) + + sched.latest_opencode_dispatch_started_at( + "owner/repo", + "OpenCode Review", + make_pr(headRefOid="a" * 40), + since=datetime(2026, 8, 24, 0, 0, tzinfo=timezone.utc), + ) + + assert captured["statuses"] == ("completed",) + assert captured["event"] == "repository_dispatch" + assert captured["created"] == ">=2026-08-24T00:00:00Z" + + +def test_latest_opencode_dispatch_started_at_without_since_leaves_created_unbounded( + monkeypatch, +): + """No ``since`` anchor means no ``created`` lower bound is requested.""" + captured = {} + + def fake_active_workflow_runs(repo, statuses, event=None, created=None): + captured["event"] = event + captured["created"] = created + return [] + + monkeypatch.setattr(sched, "active_workflow_runs", fake_active_workflow_runs) + + sched.latest_opencode_dispatch_started_at( + "owner/repo", + "OpenCode Review", + make_pr(headRefOid="a" * 40), + ) + + assert captured["event"] == "repository_dispatch" + assert captured["created"] is None + + +def test_active_workflow_runs_narrows_query_with_event_and_created(monkeypatch): + """Request server-side ``event``/``created`` filters instead of full history. + + ``active_workflow_runs(dispatch_repo, ("completed",))`` with no filter + fetches the dispatch repository's *entire* completed-run history, since + matching against a target PR happens client-side after the fetch. The + central dispatch repository's completed-run count only grows, so a + same-head dispatch-history lookup must narrow the REST query itself. + """ + calls = [] + + def fake_run(args, stdin=None): + del stdin + calls.append(args) + return json.dumps([{"workflow_runs": []}]) + + monkeypatch.setattr(sched, "run_github_actions", fake_run) + + sched.active_workflow_runs( + "owner/repo", + ("completed",), + event="repository_dispatch", + created=">=2026-08-24T00:00:00Z", + ) + + assert len(calls) == 1 + args = calls[0] + assert "event=repository_dispatch" in args + assert "created=>=2026-08-24T00:00:00Z" in args + assert args.count("-f") == 3 # status, event, created + + +def test_active_workflow_runs_omits_filters_by_default(monkeypatch): + """Existing unfiltered callers keep requesting no ``event``/``created`` params.""" + calls = [] + + def fake_run(args, stdin=None): + del stdin + calls.append(args) + return json.dumps([{"workflow_runs": []}]) + + monkeypatch.setattr(sched, "run_github_actions", fake_run) + + sched.active_workflow_runs("owner/repo", ("queued",)) + + assert len(calls) == 1 + args = calls[0] + assert not any(str(arg).startswith("event=") for arg in args) + assert not any(str(arg).startswith("created=") for arg in args) + + def test_dispatch_strix_cancels_stale_central_run_and_keeps_current(monkeypatch, capsys): calls = [] head_sha = "a" * 40 @@ -3413,13 +5486,20 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): autoMergeRequest={"enabledAt": "now"}, ) ) + # An unapproved PR disarms as soon as "no current-head approval" is + # established, regardless of mergeability -- the hoisted guard in + # inspect_pr() fires before the merge_state == "UNKNOWN" branch's own + # (approved-only, see test_approved_unknown_mergeability_disarms_auto_merge) + # check is ever reached, so the reason names the real blocker (missing + # approval) rather than the unresolved mergeability calculation. assert unknown_auto_merge.action == "disable_auto_merge" - assert "mergeability is still being calculated" in unknown_auto_merge.reason + assert "current head has no OpenCode approval" in unknown_auto_merge.reason rest_clean = inspect( make_pr( mergeStateStatus="BEHIND", restMergeableState="CLEAN", - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ) ) assert rest_clean.action == "auto_merge" @@ -3427,7 +5507,8 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): outdated_only = inspect( make_pr( reviewThreads={"nodes": [{"id": "outdated-thread", "isResolved": False, "isOutdated": True}]}, - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ) ) assert outdated_only.action == "auto_merge" @@ -3496,10 +5577,101 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): ) ) assert conflict_with_stale_review.action == "block" - assert conflict_with_stale_review.reason == ( - "current-head OpenCode review requested changes" + assert conflict_with_stale_review.reason.startswith( + "current-head OpenCode review requested changes; merge conflict:" + ) + assert merge_state in conflict_with_stale_review.reason + assert update_calls == [] + coverage_request = make_pr( + reviews={ + "nodes": [ + { + **opencode_review("CHANGES_REQUESTED", "head"), + "body": ( + "OpenCode cannot approve yet because required coverage evidence did not pass. " + "The coverage-evidence gate reported that required test/docstring evidence was not proven." + ), + } + ] + }, + statusCheckRollup={ + "contexts": { + "nodes": [ + strix_check(), + { + "__typename": "CheckRun", + "name": "coverage-evidence", + "status": "COMPLETED", + "conclusion": "SUCCESS", + }, + { + "__typename": "CheckRun", + "name": "opencode-review", + "status": "COMPLETED", + "conclusion": "FAILURE", + "checkSuite": { + "workflowRun": {"workflow": {"name": "OpenCode Review"}} + }, + }, + ] + } + }, + ) + dispatched = [] + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: dispatched.append( + (repo, workflow, pr["headRefOid"], dry_run) + ) + or "dispatched", + ) + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda repo, workflow: "review dispatch waits", + ) + coverage_wait = inspect(coverage_request) + assert coverage_wait.action == "wait" + assert coverage_wait.reason == "review dispatch waits" + monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda repo, workflow: None) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: "already_running", + ) + coverage_active = inspect(coverage_request) + assert coverage_active.action == "wait" + assert coverage_active.reason == ( + "current-head coverage evidence is complete, but a same-head OpenCode workflow run is already active" + ) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: dispatched.append( + (repo, workflow, pr["headRefOid"], dry_run) ) - assert update_calls == [] + or "dispatched", + ) + coverage_decision = inspect(coverage_request) + assert coverage_decision.action == "review_dispatch" + assert coverage_decision.reason == ( + "current-head OpenCode coverage blocker is cleared; same-head OpenCode re-dispatched" + ) + assert dispatched == [("owner/repo", "OpenCode Review", "head", True)] + + coverage_request["statusCheckRollup"]["contexts"]["nodes"].append( + { + "__typename": "CheckRun", + "name": "Security Scan", + "status": "COMPLETED", + "conclusion": "FAILURE", + } + ) + unrelated_failure = inspect(coverage_request) + assert unrelated_failure.action == "block" + assert unrelated_failure.reason == "current-head OpenCode review requested changes" + action_required_pr = make_pr( statusCheckRollup={ "contexts": { @@ -3528,7 +5700,8 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): assert "workflow action required: opencode-review" in action_required_auto.reason same_head_auto = make_pr( autoMergeRequest={"enabledAt": "now"}, - reviews={"nodes": [opencode_review("APPROVED", "head", submitted_at="2026-06-25T06:59:59Z")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ) disabled = [] monkeypatch.setattr(sched, "disable_auto_merge", lambda repo, pr, dry_run: disabled.append((repo, pr["number"], dry_run))) @@ -3545,11 +5718,9 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): reviews={"nodes": [opencode_review("APPROVED", "head")]}, ) blocked_auto_decision = inspect(blocked_auto) - assert blocked_auto_decision.action == "wait" - assert "GitHub mergeability is BLOCKED" in blocked_auto_decision.reason + assert blocked_auto_decision.action == "disable_auto_merge" assert "GitHub reviewDecision is REVIEW_REQUIRED" in blocked_auto_decision.reason - assert "required approving review" in blocked_auto_decision.reason - assert "rerun the scheduler" in blocked_auto_decision.reason + assert disabled == [("owner/repo", 1, True)] assert sched.latest_commit_headline(make_pr(commits={"nodes": []})) == "" restamp_candidate = last_push_restamp_candidate() @@ -3580,6 +5751,13 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): current_head_approved=True, auto_merge_enabled=True, ) + assert not sched.should_restamp_for_last_push_approval( + "owner/repo", + last_push_restamp_candidate(reviewDecision="REVIEW_REQUIRED"), + "BLOCKED", + current_head_approved=True, + auto_merge_enabled=True, + ) disabled_restamp = inspect(restamp_candidate, update_branches=False) assert disabled_restamp.action == "wait" @@ -3726,7 +5904,8 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): called.clear() behind_auto_merge_enabled = make_pr( mergeStateStatus="BEHIND", - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), autoMergeRequest={"enabledAt": "now"}, ) disabled.clear() @@ -3739,7 +5918,8 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): rest_behind = make_pr( mergeStateStatus="CLEAN", restMergeableState="BEHIND", - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), autoMergeRequest={"enabledAt": "now"}, ) rest_behind_decision = inspect(rest_behind) @@ -3757,7 +5937,8 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): mergeStateStatus="BLOCKED", restMergeableState="BLOCKED", compareBehindBy=2, - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), autoMergeRequest={"enabledAt": "now"}, statusCheckRollup={ "contexts": { @@ -3783,13 +5964,16 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): } }, ) + disabled.clear() blocked_without_opencode_decision = inspect(blocked_failed_behind_auto_without_opencode_approval) - assert blocked_without_opencode_decision.action == "update_branch" - assert "auto-merge already enabled" in blocked_without_opencode_decision.reason - assert "base branch is 2 commit(s) ahead" in blocked_without_opencode_decision.reason - assert "existing auto-merge request remains queued" in blocked_without_opencode_decision.reason - assert called == [("owner/repo", 1, True)] + assert blocked_without_opencode_decision.action == "disable_auto_merge" + assert "branch is 2 commit(s) behind base" in blocked_without_opencode_decision.reason + assert "GitHub mergeability is BLOCKED" in blocked_without_opencode_decision.reason + assert "no live current-head approval" in blocked_without_opencode_decision.reason + assert called == [] + assert disabled == [("owner/repo", 1, True)] called.clear() + disabled.clear() blocked_compare_behind_auto = make_pr( mergeStateStatus="BLOCKED", restMergeableState="BLOCKED", @@ -3805,12 +5989,14 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): }, ) blocked_compare_behind_decision = inspect(blocked_compare_behind_auto) - assert blocked_compare_behind_decision.action == "update_branch" - assert "auto-merge already enabled" in blocked_compare_behind_decision.reason - assert "base branch is 1 commit(s) ahead" in blocked_compare_behind_decision.reason - assert "existing auto-merge request remains queued" in blocked_compare_behind_decision.reason - assert called == [("owner/repo", 1, True)] + assert blocked_compare_behind_decision.action == "disable_auto_merge" + assert "branch is 1 commit(s) behind base" in blocked_compare_behind_decision.reason + assert "GitHub mergeability is BLOCKED" in blocked_compare_behind_decision.reason + assert "no live current-head approval" in blocked_compare_behind_decision.reason + assert called == [] + assert disabled == [("owner/repo", 1, True)] called.clear() + disabled.clear() diverged_failed_auto = make_pr( mergeStateStatus="BLOCKED", restMergeableState="BLOCKED", @@ -3828,12 +6014,14 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): }, ) diverged_failed_decision = inspect(diverged_failed_auto) - assert diverged_failed_decision.action == "update_branch" - assert "auto-merge already enabled" in diverged_failed_decision.reason - assert "base branch is 184 commit(s) ahead" in diverged_failed_decision.reason - assert "existing auto-merge request remains queued" in diverged_failed_decision.reason - assert called == [("owner/repo", 1, True)] + assert diverged_failed_decision.action == "disable_auto_merge" + assert "branch is 184 commit(s) behind base" in diverged_failed_decision.reason + assert "GitHub mergeability is BLOCKED" in diverged_failed_decision.reason + assert "no live current-head approval" in diverged_failed_decision.reason + assert called == [] + assert disabled == [("owner/repo", 1, True)] called.clear() + disabled.clear() unknown_compare_behind_auto = make_pr( mergeStateStatus="UNKNOWN", restMergeableState="UNKNOWN", @@ -3849,29 +6037,65 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): }, ) unknown_compare_behind_decision = inspect(unknown_compare_behind_auto) - assert unknown_compare_behind_decision.action == "update_branch" - assert "auto-merge already enabled" in unknown_compare_behind_decision.reason - assert "base branch is 1 commit(s) ahead" in unknown_compare_behind_decision.reason + assert unknown_compare_behind_decision.action == "disable_auto_merge" + assert "branch is 1 commit(s) behind base" in unknown_compare_behind_decision.reason assert "GitHub mergeability is UNKNOWN" in unknown_compare_behind_decision.reason - assert "existing auto-merge request remains queued" in unknown_compare_behind_decision.reason - assert called == [("owner/repo", 1, True)] + assert "no live current-head approval" in unknown_compare_behind_decision.reason + assert called == [] + assert disabled == [("owner/repo", 1, True)] called.clear() disabled.clear() - assert ( - inspect(blocked_failed_behind_auto_without_opencode_approval, update_branches=False).reason - == "auto-merge already enabled; branch update disabled" - ) + # An unapproved, outdated PR with auto-merge already armed is disarmed + # regardless of the update_branches flag -- disarming a stale + # authorization is a safety action, not a branch mutation, so it is not + # gated behind the same feature flag that controls branch updates. + update_branches_disabled_decision = inspect( + blocked_failed_behind_auto_without_opencode_approval, update_branches=False + ) + assert update_branches_disabled_decision.action == "disable_auto_merge" + assert "branch is 2 commit(s) behind base" in update_branches_disabled_decision.reason + assert "no live current-head approval" in update_branches_disabled_decision.reason assert called == [] - assert disabled == [] + assert disabled == [("owner/repo", 1, True)] + disabled.clear() behind_auto_without_opencode_approval = make_pr( mergeStateStatus="BEHIND", autoMergeRequest={"enabledAt": "now"}, ) behind_without_opencode_decision = inspect(behind_auto_without_opencode_approval) - assert behind_without_opencode_decision.action == "update_branch" - assert behind_without_opencode_decision.reason.startswith("auto-merge already enabled; branch update requested") - assert "existing auto-merge request remains queued" in behind_without_opencode_decision.reason - assert called == [("owner/repo", 1, True)] + assert behind_without_opencode_decision.action == "disable_auto_merge" + assert "branch is 1 commit(s) behind base" in behind_without_opencode_decision.reason + assert "GitHub mergeability is BEHIND" in behind_without_opencode_decision.reason + assert "no live current-head approval" in behind_without_opencode_decision.reason + assert called == [] + assert disabled == [("owner/repo", 1, True)] + + +def test_approved_unknown_mergeability_disarms_auto_merge_pending_evaluation(): + """An approved PR whose mergeability is still UNKNOWN keeps its own disarm reason. + + Unlike the unapproved case (see the ``unknown_auto_merge`` assertion in + ``test_inspect_pr_blocks_and_waits_for_policy_states``), the hoisted + "no current-head approval" guard in ``inspect_pr`` does not apply here -- + ``current_head_approved`` is True, so this PR still reaches the + ``merge_state == "UNKNOWN"`` branch's own, more specific check: with + ``merge_mode="auto"`` and mergeability not yet CLEAN, the scheduler cannot + yet attempt the merge, so an auto-merge request armed on this still-being- + evaluated head is disarmed pending a resolved mergeability calculation, + not for any approval defect. + """ + approved_unknown_armed = make_pr( + mergeStateStatus="CLEAN", + restMergeableState="UNKNOWN", + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), + autoMergeRequest={"enabledAt": "now"}, + ) + + decision = inspect(approved_unknown_armed, merge_mode="auto") + + assert decision.action == "disable_auto_merge" + assert "mergeability is still being calculated" in decision.reason def test_draft_pr_still_skipped_by_default_and_without_trigger_reviews(monkeypatch): @@ -4331,7 +6555,8 @@ def test_inspect_pr_blocks_auto_merge_for_approved_conflicts(monkeypatch): approved_conflict = make_pr( mergeStateStatus="DIRTY", - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ) decision = inspect(approved_conflict) assert decision.action == "block" @@ -4342,10 +6567,11 @@ def test_inspect_pr_blocks_auto_merge_for_approved_conflicts(monkeypatch): assert disables == [] already_queued = inspect( - make_pr( - mergeStateStatus="CONFLICTING", - reviews={"nodes": [opencode_review("APPROVED", "head")]}, - autoMergeRequest={"enabledAt": "now"}, + make_pr( + mergeStateStatus="CONFLICTING", + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), + autoMergeRequest={"enabledAt": "now"}, ) ) assert already_queued.action == "disable_auto_merge" @@ -4450,7 +6676,8 @@ def test_inspect_pr_dispatches_strix_after_update_branch_observes_new_head(monke dispatched = [] old_head_pr = make_pr( mergeStateStatus="BEHIND", - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), autoMergeRequest={"enabledAt": "now"}, ) new_head_pr = make_pr(headRefOid="new-head", reviews={"nodes": []}) @@ -4812,17 +7039,19 @@ def test_update_branch_summary_includes_followup_notes(): def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): - approved = make_pr(reviews={"nodes": [opencode_review("APPROVED", "head")]}) + approved = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) failed = make_pr( - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), statusCheckRollup={"contexts": {"nodes": [{"__typename": "CheckRun", "name": "strix", "conclusion": "FAILURE"}]}}, ) assert inspect(failed).reason == "failed check(s): strix" - assert inspect(make_pr(reviews={"nodes": [opencode_review("APPROVED", "head")]}, autoMergeRequest={"enabledAt": "now"})).reason == ( + assert inspect(make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews(), autoMergeRequest={"enabledAt": "now"})).reason == ( "current head is approved; auto-merge already enabled" ) approved_with_auto_merge = make_pr( - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), autoMergeRequest={"enabledAt": "now"}, ) assert inspect(approved_with_auto_merge, enable_auto_merge_flag=False).reason == ( @@ -4842,7 +7071,8 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): ) blocked_approved = make_pr( mergeStateStatus="BLOCKED", - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ) assert inspect(blocked_approved, enable_auto_merge_flag=False).reason == ( "current head is approved; auto-merge disabled by scheduler inputs" @@ -4856,7 +7086,8 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): blocked_unmergeable = make_pr( mergeable="UNKNOWN", mergeStateStatus="BLOCKED", - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ) assert inspect(blocked_unmergeable, enable_auto_merge_flag=False).reason == ( "current head is approved; auto-merge disabled by scheduler inputs" @@ -4879,7 +7110,8 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): mergeStateStatus="BLOCKED", isCrossRepository=True, headRepository={"nameWithOwner": "fork/repo"}, - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ), merge_mode="direct_or_auto", ) @@ -4895,7 +7127,8 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): blocked_direct = inspect( make_pr( mergeStateStatus="BLOCKED", - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ), merge_mode="direct", ) @@ -4916,7 +7149,8 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): already_auto_direct_or_auto = inspect( make_pr( autoMergeRequest={"enabledAt": "now"}, - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ), merge_mode="direct_or_auto", ) @@ -4932,7 +7166,8 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): make_pr( mergeStateStatus="CLEAN", compareBehindBy=20, - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ), merge_mode="direct_or_auto", ) @@ -4949,7 +7184,8 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): make_pr( mergeStateStatus="BLOCKED", compareBehindBy=20, - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ), merge_mode="direct_or_auto", ) @@ -4970,7 +7206,8 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): blocked_direct_or_auto = inspect( make_pr( mergeStateStatus="BLOCKED", - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ), merge_mode="direct_or_auto", ) @@ -4989,15 +7226,14 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): make_pr( mergeStateStatus="BLOCKED", autoMergeRequest={"enabledAt": "now"}, - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ), merge_mode="direct_or_auto", ) assert blocked_already_auto.action == "wait" assert "auto-merge is already enabled" in blocked_already_auto.reason assert "GitHub mergeability is BLOCKED" in blocked_already_auto.reason - assert "GitHub reviewDecision is REVIEW_REQUIRED" in blocked_already_auto.reason - assert "required approving review" in blocked_already_auto.reason assert direct_merges == [ ("owner/repo", 1, True), ("owner/repo", 1, True), @@ -5017,7 +7253,8 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): make_pr( isCrossRepository=True, headRepository={"nameWithOwner": "fork/repo"}, - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ), merge_mode="direct_or_auto", ) @@ -5029,7 +7266,8 @@ def test_inspect_pr_handles_approved_reviews_and_dispatch(monkeypatch): mergeStateStatus="BLOCKED", isCrossRepository=True, headRepository={"nameWithOwner": "fork/repo"}, - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ), merge_mode="direct_or_auto", ) @@ -5156,7 +7394,7 @@ def test_inspect_pr_waits_when_same_head_dispatch_is_already_running(monkeypatch def test_direct_or_auto_falls_back_to_auto_merge_when_branch_policy_blocks_direct_merge(monkeypatch): - approved = make_pr(reviews={"nodes": [opencode_review("APPROVED", "head")]}) + approved = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) auto_merges = [] def policy_blocked_merge(repo, pr, dry_run): @@ -5184,7 +7422,8 @@ def policy_blocked_merge(repo, pr, dry_run): already_queued = inspect( make_pr( autoMergeRequest={"enabledAt": "now"}, - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ), merge_mode="direct_or_auto", ) @@ -5195,7 +7434,8 @@ def policy_blocked_merge(repo, pr, dry_run): blocked = make_pr( mergeStateStatus="BLOCKED", - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ) blocked_decision = inspect(blocked, merge_mode="direct_or_auto") @@ -5220,7 +7460,8 @@ def non_policy_merge_failure(repo, pr, dry_run): def test_direct_or_auto_attempts_direct_merge_when_mergeability_is_blocked(monkeypatch): approved = make_pr( mergeStateStatus="BLOCKED", - reviews={"nodes": [opencode_review("APPROVED", "head")]}, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), ) direct_merges = [] @@ -5259,6 +7500,8 @@ def test_main_limits_review_dispatches_and_branch_updates(monkeypatch, capsys): mergeStateStatus="BLOCKED", restMergeableState="BLOCKED", compareBehindBy=2, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), autoMergeRequest={"enabledAt": "now"}, ), make_pr( @@ -5266,6 +7509,8 @@ def test_main_limits_review_dispatches_and_branch_updates(monkeypatch, capsys): mergeStateStatus="BLOCKED", restMergeableState="BLOCKED", compareBehindBy=3, + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), autoMergeRequest={"enabledAt": "now"}, ), ] @@ -5801,3 +8046,370 @@ def test_run_masks_secrets_in_args(): err_msg = str(exc_info.value) assert token not in err_msg assert "***" in err_msg + + +# --- TOCTOU close: re-validate current-head approval immediately before merge --- +# +# `current_head_approved`/`approval_reason` are computed once, early in +# `inspect_pr`, from the snapshot this scheduler invocation fetched at the +# start of its run. `revalidate_current_head_approval` re-fetches the PR and +# recomputes the same decision immediately before each merge_pr/enable_auto_merge +# call site, so a same-head approval revoked in that window cannot still +# authorize a merge. The `--match-head-commit` guard on those mutations only +# protects against the commit changing, not the review state changing on the +# identical commit. + + +def test_revalidate_current_head_approval_confirms_still_valid_approval(monkeypatch): + """A fresh re-fetch that still shows exact-head approval returns no block.""" + approved = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + monkeypatch.setattr(sched, "fetch_pr", lambda repo, number: [approved]) + + assert sched.revalidate_current_head_approval("owner/repo", approved) is None + + +def test_revalidate_current_head_approval_catches_revoked_review_decision(monkeypatch): + """GitHub reviewDecision falling out of APPROVED between snapshot and merge blocks.""" + snapshot = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + revoked_fresh = make_pr(reviewDecision="REVIEW_REQUIRED", reviews=merge_approved_reviews()) + monkeypatch.setattr(sched, "fetch_pr", lambda repo, number: [revoked_fresh]) + + reason = sched.revalidate_current_head_approval("owner/repo", snapshot) + + assert reason is not None + assert "reviewDecision is REVIEW_REQUIRED" in reason + assert "re-confirmed immediately before merge" in reason + + +def test_revalidate_current_head_approval_catches_dismissed_independent_review(monkeypatch): + """An independent approver's exact-head review moving to DISMISSED blocks, even + when GitHub's cached reviewDecision has not yet caught up.""" + snapshot = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + revoked_fresh = make_pr( + reviewDecision="APPROVED", + reviews={ + "nodes": [ + opencode_review("APPROVED", "head"), + opencode_review("APPROVED", "head", login="independent-reviewer", submitted_at="2026-06-25T07:01:00Z"), + opencode_review("DISMISSED", "head", login="independent-reviewer", submitted_at="2026-06-25T07:02:00Z"), + ] + }, + ) + monkeypatch.setattr(sched, "fetch_pr", lambda repo, number: [revoked_fresh]) + + reason = sched.revalidate_current_head_approval("owner/repo", snapshot) + + assert reason is not None + assert "no independent non-author exact-current-head formal APPROVED review exists" in reason + assert "re-confirmed immediately before merge" in reason + + +def test_revalidate_current_head_approval_catches_revoked_opencode_approval(monkeypatch): + """OpenCode's own exact-head approval being superseded blocks before merge.""" + snapshot = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + revoked_fresh = make_pr( + reviewDecision="APPROVED", + reviews={ + "nodes": [ + opencode_review("APPROVED", "head", submitted_at="2026-06-25T07:01:00Z"), + opencode_review("CHANGES_REQUESTED", "head", submitted_at="2026-06-25T07:02:00Z"), + opencode_review("APPROVED", "head", login="independent-reviewer"), + ] + }, + ) + monkeypatch.setattr(sched, "fetch_pr", lambda repo, number: [revoked_fresh]) + + reason = sched.revalidate_current_head_approval("owner/repo", snapshot) + + assert reason == ( + "current-head OpenCode approval was revoked immediately before merge; " + "the merge-authorizing snapshot is no longer current" + ) + + +def test_revalidate_current_head_approval_catches_head_moving_before_merge(monkeypatch): + """A head that moved between the snapshot and the re-check is never treated as + still-approved evidence, even before GitHub's own --match-head-commit guard runs.""" + snapshot = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + moved_fresh = make_pr( + headRefOid="new-head", + reviewDecision="APPROVED", + reviews=merge_approved_reviews(commit="new-head"), + ) + monkeypatch.setattr(sched, "fetch_pr", lambda repo, number: [moved_fresh]) + + reason = sched.revalidate_current_head_approval("owner/repo", snapshot) + + assert reason is not None + assert "current head changed" in reason + + +def test_revalidate_current_head_approval_fails_closed_on_refetch_error(monkeypatch): + """A re-fetch failure must never be treated as a still-valid approval.""" + snapshot = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + + def raise_refetch(repo, number): + raise RuntimeError("gh api graphql: 502 Bad Gateway") + + monkeypatch.setattr(sched, "fetch_pr", raise_refetch) + + reason = sched.revalidate_current_head_approval("owner/repo", snapshot) + + assert reason is not None + assert "re-checking current-head approval immediately before merge failed" in reason + assert "502 Bad Gateway" in reason + + +def test_revalidate_current_head_approval_fails_closed_when_pr_disappears(monkeypatch): + """A PR no longer returned by a re-fetch (closed/inaccessible) fails closed.""" + snapshot = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + monkeypatch.setattr(sched, "fetch_pr", lambda repo, number: []) + + reason = sched.revalidate_current_head_approval("owner/repo", snapshot) + + assert reason is not None + assert "no longer open or accessible" in reason + + +def test_inspect_pr_direct_merge_blocked_when_approval_revoked_before_merge(monkeypatch): + """Reproduces the confirmed TOCTOU finding: a same-head approval revoked after + approval_reason is computed must not still authorize a direct merge.""" + approved = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + revoked_fresh = make_pr(reviewDecision="REVIEW_REQUIRED", reviews=merge_approved_reviews()) + + fetch_calls = [] + merge_calls = [] + monkeypatch.setattr(sched, "cancel_stale_pr_runs", lambda repo, pr, dry_run: []) + monkeypatch.setattr( + sched, + "fetch_pr", + lambda repo, number: fetch_calls.append((repo, number)) or [revoked_fresh], + ) + monkeypatch.setattr( + sched, "merge_pr", lambda repo, pr, dry_run: merge_calls.append((repo, pr["number"], dry_run)) + ) + + decision = inspect(approved, merge_mode="direct", dry_run=False) + + assert fetch_calls == [("owner/repo", 1)] + assert merge_calls == [] + assert decision.action == "wait" + assert "reviewDecision is REVIEW_REQUIRED" in decision.reason + assert "re-confirmed immediately before merge" in decision.reason + + +def test_inspect_pr_direct_or_auto_merge_blocked_when_approval_revoked_before_merge(monkeypatch): + """The direct_or_auto merge path re-validates before attempting merge_pr too.""" + approved = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + revoked_fresh = make_pr(reviewDecision="CHANGES_REQUESTED", reviews=merge_approved_reviews()) + + fetch_calls = [] + merge_calls = [] + monkeypatch.setattr(sched, "cancel_stale_pr_runs", lambda repo, pr, dry_run: []) + monkeypatch.setattr( + sched, + "fetch_pr", + lambda repo, number: fetch_calls.append((repo, number)) or [revoked_fresh], + ) + monkeypatch.setattr( + sched, "merge_pr", lambda repo, pr, dry_run: merge_calls.append((repo, pr["number"], dry_run)) + ) + + decision = inspect(approved, merge_mode="direct_or_auto", dry_run=False) + + assert fetch_calls == [("owner/repo", 1)] + assert merge_calls == [] + assert decision.action == "wait" + assert "reviewDecision is CHANGES_REQUESTED" in decision.reason + + +def test_inspect_pr_auto_merge_blocked_when_approval_revoked_before_enable(monkeypatch): + """The plain auto merge-mode path re-validates before enable_auto_merge too.""" + approved = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + revoked_fresh = make_pr(reviewDecision="REVIEW_REQUIRED", reviews=merge_approved_reviews()) + + fetch_calls = [] + auto_merge_calls = [] + monkeypatch.setattr(sched, "cancel_stale_pr_runs", lambda repo, pr, dry_run: []) + monkeypatch.setattr( + sched, + "fetch_pr", + lambda repo, number: fetch_calls.append((repo, number)) or [revoked_fresh], + ) + monkeypatch.setattr( + sched, + "enable_auto_merge", + lambda repo, pr, dry_run: auto_merge_calls.append((repo, pr["number"], dry_run)), + ) + + decision = inspect(approved, merge_mode="auto", dry_run=False) + + assert fetch_calls == [("owner/repo", 1)] + assert auto_merge_calls == [] + assert decision.action == "wait" + assert "reviewDecision is REVIEW_REQUIRED" in decision.reason + + +def test_inspect_pr_disables_queued_auto_merge_when_approval_revoked_before_merge(monkeypatch): + """A queued auto-merge request must be disarmed, not left queued, when the fresh + re-check reveals the authorizing approval no longer holds.""" + approved_with_auto_merge = make_pr( + reviewDecision="APPROVED", + reviews=merge_approved_reviews(), + autoMergeRequest={"enabledAt": "now"}, + ) + revoked_fresh = make_pr( + reviewDecision="REVIEW_REQUIRED", + reviews=merge_approved_reviews(), + autoMergeRequest={"enabledAt": "now"}, + ) + + fetch_calls = [] + merge_calls = [] + disabled = [] + monkeypatch.setattr(sched, "cancel_stale_pr_runs", lambda repo, pr, dry_run: []) + monkeypatch.setattr( + sched, + "fetch_pr", + lambda repo, number: fetch_calls.append((repo, number)) or [revoked_fresh], + ) + monkeypatch.setattr( + sched, "merge_pr", lambda repo, pr, dry_run: merge_calls.append((repo, pr["number"], dry_run)) + ) + monkeypatch.setattr( + sched, + "disable_auto_merge", + lambda repo, pr, dry_run: disabled.append((repo, pr["number"], dry_run)), + ) + + decision = inspect(approved_with_auto_merge, merge_mode="direct_or_auto", dry_run=False) + + assert fetch_calls == [("owner/repo", 1)] + assert merge_calls == [] + assert disabled == [("owner/repo", 1, False)] + assert decision.action == "disable_auto_merge" + assert "reviewDecision is REVIEW_REQUIRED" in decision.reason + assert "re-confirmed immediately before merge" in decision.reason + + +def test_inspect_pr_blocked_direct_or_auto_merge_blocked_when_approval_revoked_before_merge(monkeypatch): + """The BLOCKED-mergeability direct_or_auto branch (reached when merge_state is not + CLEAN, so outside the merge_before_update gate) also re-validates before merge_pr.""" + approved = make_pr(mergeStateStatus="BLOCKED", reviewDecision="APPROVED", reviews=merge_approved_reviews()) + revoked_fresh = make_pr(mergeStateStatus="BLOCKED", reviewDecision="REVIEW_REQUIRED", reviews=merge_approved_reviews()) + + fetch_calls = [] + merge_calls = [] + monkeypatch.setattr(sched, "cancel_stale_pr_runs", lambda repo, pr, dry_run: []) + monkeypatch.setattr( + sched, + "fetch_pr", + lambda repo, number: fetch_calls.append((repo, number)) or [revoked_fresh], + ) + monkeypatch.setattr( + sched, "merge_pr", lambda repo, pr, dry_run: merge_calls.append((repo, pr["number"], dry_run)) + ) + + decision = inspect(approved, merge_mode="direct_or_auto", dry_run=False) + + assert fetch_calls == [("owner/repo", 1)] + assert merge_calls == [] + assert decision.action == "wait" + assert "reviewDecision is REVIEW_REQUIRED" in decision.reason + + +def test_inspect_pr_blocked_auto_merge_blocked_when_approval_revoked_before_enable(monkeypatch): + """The BLOCKED-mergeability plain auto branch (outside the merge_before_update + gate) also re-validates before enable_auto_merge.""" + approved = make_pr(mergeStateStatus="BLOCKED", reviewDecision="APPROVED", reviews=merge_approved_reviews()) + revoked_fresh = make_pr(mergeStateStatus="BLOCKED", reviewDecision="REVIEW_REQUIRED", reviews=merge_approved_reviews()) + + fetch_calls = [] + auto_merge_calls = [] + monkeypatch.setattr(sched, "cancel_stale_pr_runs", lambda repo, pr, dry_run: []) + monkeypatch.setattr( + sched, + "fetch_pr", + lambda repo, number: fetch_calls.append((repo, number)) or [revoked_fresh], + ) + monkeypatch.setattr( + sched, + "enable_auto_merge", + lambda repo, pr, dry_run: auto_merge_calls.append((repo, pr["number"], dry_run)), + ) + + decision = inspect(approved, merge_mode="auto", dry_run=False) + + assert fetch_calls == [("owner/repo", 1)] + assert auto_merge_calls == [] + assert decision.action == "wait" + assert "reviewDecision is REVIEW_REQUIRED" in decision.reason + + +def test_inspect_pr_direct_merge_proceeds_when_revalidation_confirms_approval(monkeypatch): + """Approval still valid at the immediate re-check: the merge proceeds normally.""" + approved = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + still_approved_fresh = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + + fetch_calls = [] + merge_calls = [] + monkeypatch.setattr(sched, "cancel_stale_pr_runs", lambda repo, pr, dry_run: []) + monkeypatch.setattr( + sched, + "fetch_pr", + lambda repo, number: fetch_calls.append((repo, number)) or [still_approved_fresh], + ) + monkeypatch.setattr( + sched, "merge_pr", lambda repo, pr, dry_run: merge_calls.append((repo, pr["number"], dry_run)) + ) + + decision = inspect(approved, merge_mode="direct", dry_run=False) + + assert fetch_calls == [("owner/repo", 1)] + assert merge_calls == [("owner/repo", 1, False)] + assert decision.action == "merge" + + +def test_inspect_pr_fails_closed_when_revalidation_refetch_errors(monkeypatch): + """A re-check API failure right before merge must never let the merge proceed.""" + approved = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + + fetch_calls = [] + merge_calls = [] + + def raise_refetch(repo, number): + fetch_calls.append((repo, number)) + raise RuntimeError("gh api graphql: 502 Bad Gateway") + + monkeypatch.setattr(sched, "cancel_stale_pr_runs", lambda repo, pr, dry_run: []) + monkeypatch.setattr(sched, "fetch_pr", raise_refetch) + monkeypatch.setattr( + sched, "merge_pr", lambda repo, pr, dry_run: merge_calls.append((repo, pr["number"], dry_run)) + ) + + decision = inspect(approved, merge_mode="direct", dry_run=False) + + assert fetch_calls == [("owner/repo", 1)] + assert merge_calls == [] + assert decision.action == "wait" + assert "re-checking current-head approval immediately before merge failed" in decision.reason + + +def test_inspect_pr_dry_run_skips_merge_revalidation_refetch(monkeypatch): + """Dry-run inspection never mutates anything, so it must not pay for the extra + re-fetch either -- and every existing dry-run merge/auto_merge assertion in this + file relies on that (no fetch_pr mock is installed for those).""" + approved = make_pr(reviewDecision="APPROVED", reviews=merge_approved_reviews()) + fetch_calls = [] + monkeypatch.setattr( + sched, + "fetch_pr", + lambda repo, number: fetch_calls.append((repo, number)) or [approved], + ) + + direct_decision = inspect(approved, merge_mode="direct") + auto_decision = inspect(approved, merge_mode="auto") + + assert direct_decision.action == "merge" + assert auto_decision.action == "auto_merge" + assert fetch_calls == []