diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 38cd4c6913..87827f5322 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -9,13 +9,31 @@ on: # content and never binds repository secrets. Privileged review execution is # isolated in opencode-review-dispatch.yml on repository_dispatch only. pull_request_target: - types: [opened, synchronize, reopened, ready_for_review, closed] + # `converted_to_draft` is included so a PR going draft mid-poll fires a + # fresh run of this same workflow: the head-scoped concurrency group below + # (`cancel-in-progress: true`) cancels any in-flight non-draft + # "Fail closed without a current-head OpenCode verdict" poll for that + # exact same head. Every non-closed admission path revalidates the live + # PR/head/state before dispatching, exempting, or polling so out-of-order + # draft/ready/closed events cannot publish stale evidence or wait on an + # impossible verdict. + types: [opened, synchronize, reopened, ready_for_review, converted_to_draft, closed] concurrency: + # Scoped by exact head SHA (not just PR number) so a delayed, out-of-order + # run for an older head cannot cancel the authoritative run already active + # for a newer head -- GitHub cancels whichever run is currently active in + # the group when a new one starts, with no notion of "older"/"newer", so + # sharing a group across different heads let a stale event retire the + # current head's still-valid run before its own live-head check could ever + # reject it (Devin Review on `#1568`). Same-head events (draft<->ready + # transitions, a synchronize retry) still share one group, so + # `converted_to_draft` still cancels an active same-head verdict poll. group: >- opencode-review-bootstrap-${{ github.event.pull_request.base.repo.full_name || github.repository }}-${{ - github.event.pull_request.number || github.run_id }} + github.event.pull_request.number || github.run_id }}-${{ + github.event.pull_request.head.sha || github.run_id }} cancel-in-progress: true permissions: @@ -270,11 +288,39 @@ jobs: WORKFLOW_SHA: ${{ github.workflow_sha }} run: | set -euo pipefail + live_pr="$(gh api "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}")" + live_head="$(printf '%s' "$live_pr" | jq -r '.head.sha // empty')" + live_draft="$(printf '%s' "$live_pr" | jq -r 'if (.draft | type) == "boolean" then (.draft | tostring) else empty end')" + live_state="$(printf '%s' "$live_pr" | jq -r 'if (.state | type) == "string" then .state else empty end')" + if [ -z "$live_head" ] || [ -z "$live_draft" ] || [ -z "$live_state" ]; then + echo "::error::Could not validate live pull request state before review dispatch." + exit 1 + fi + if [ "$live_state" != "open" ] && [ "$live_state" != "closed" ]; then + echo "::error::Could not validate live pull request state before review dispatch." + exit 1 + fi + if [ "${live_head,,}" != "${HEAD_SHA,,}" ]; then + echo "::error::Pull request head moved while validating live review state." + exit 1 + fi + if [ "$live_state" = "closed" ]; then + echo "PR is closed on the live exact head; a current-head OpenCode review is not requested." + exit 0 + fi + if [ "$live_draft" = "true" ]; then + echo "PR is still a draft on the live exact head; a current-head OpenCode review is not requested until it is marked ready for review." + exit 0 + fi + if [ "$PR_DRAFT" = "true" ]; then + echo "Event draft snapshot is stale; continuing current-head OpenCode review dispatch for the live ready PR." + fi + effective_pr_draft="$live_draft" helper="$(mktemp)" trap 'rm -f "$helper"' EXIT gh api "repos/ContextualWisdomLab/.github/contents/scripts/ci/opencode_review_receipt_gate.py?ref=${WORKFLOW_SHA}" \ --jq .content | base64 --decode >"$helper" - receipt_state="$(python3 - "$helper" "$TARGET_REPOSITORY" "$PR_NUMBER" "$HEAD_SHA" "$PR_DRAFT" <<'PY' + receipt_state="$(python3 - "$helper" "$TARGET_REPOSITORY" "$PR_NUMBER" "$HEAD_SHA" "$effective_pr_draft" <<'PY' import importlib.machinery import importlib.util import sys @@ -334,6 +380,7 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} PR_ACTION: ${{ github.event.action }} + PR_DRAFT: ${{ github.event.pull_request.draft }} run: | set -euo pipefail if [ "$PR_ACTION" = "closed" ]; then @@ -344,6 +391,33 @@ jobs: echo "::error::Missing PR number or head SHA; cannot verify a current-head OpenCode verdict." exit 1 fi + live_pr="$(gh api "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}")" + live_head="$(printf '%s' "$live_pr" | jq -r '.head.sha // empty')" + live_draft="$(printf '%s' "$live_pr" | jq -r 'if (.draft | type) == "boolean" then (.draft | tostring) else empty end')" + live_state="$(printf '%s' "$live_pr" | jq -r 'if (.state | type) == "string" then .state else empty end')" + if [ -z "$live_head" ] || [ -z "$live_draft" ] || [ -z "$live_state" ]; then + echo "::error::Could not validate live pull request state before verdict admission." + exit 1 + fi + if [ "$live_state" != "open" ] && [ "$live_state" != "closed" ]; then + echo "::error::Could not validate live pull request state before verdict admission." + exit 1 + fi + if [ "${live_head,,}" != "${HEAD_SHA,,}" ]; then + echo "::error::Pull request head moved while validating live verdict state." + exit 1 + fi + if [ "$live_state" = "closed" ]; then + echo "PR is closed on the live exact head; a current-head OpenCode verdict is not required." + exit 0 + fi + if [ "$live_draft" = "true" ]; then + echo "PR is still a draft on the live exact head; a current-head OpenCode verdict is not required until it is marked ready for review." + exit 0 + fi + if [ "$PR_DRAFT" = "true" ]; then + echo "Event draft snapshot is stale; continuing verdict polling for the live ready PR." + fi verdict="" while :; do reviews="$(gh api --paginate "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}/reviews")" @@ -385,3 +459,100 @@ jobs: exit 1 fi echo "Current-head OpenCode verdict: ${verdict}." + + cancel-superseded-opencode-review-runs: + # Scoping the concurrency group above by exact head SHA (so a delayed + # old-head run can no longer cancel the authoritative newer-head run -- + # Devin Review on `#1568`) also means a *legitimate* new commit no + # longer auto-cancels its own PR's now-obsolete previous-head poll: that + # older run's own live-head check only ran once, before it entered its + # unbounded Reviews API wait, and nothing in that wait loop re-validates + # the head. Left alone, it would occupy a runner until GitHub's own + # per-job ceiling. This job retires it directly, mirroring the + # live-head-validated cleanup pattern in strix.yml's own + # `cancel-superseded-pr-runs` job: every cancellation candidate and + # every cancellation itself is re-verified against the live PR head + # immediately beforehand, so a run for this job that is itself somehow + # delayed/stale cannot wrongly cancel a still-authoritative run. + if: github.event_name == 'pull_request_target' && github.event.action == 'synchronize' + runs-on: ubuntu-latest + permissions: + actions: write + contents: read + pull-requests: read + env: + GH_TOKEN: ${{ github.token }} + TARGET_REPOSITORY: ${{ github.event.pull_request.base.repo.full_name || github.repository }} + TARGET_PR_NUMBER: ${{ github.event.pull_request.number }} + TARGET_PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + CURRENT_RUN_ID: ${{ github.run_id }} + steps: + - name: Cancel queued and running OpenCode review runs for a superseded pull request head + shell: bash + run: | + set -euo pipefail + + live_head_matches() { + local live_head + if ! live_head="$(gh api "repos/${TARGET_REPOSITORY}/pulls/${TARGET_PR_NUMBER}" --jq '.head.sha' 2>/tmp/opencode-cleanup-gh-error)"; then + echo "::warning::OpenCode review cleanup could not verify the live pull request head; leaving runs unchanged." + sed 's/^/ /' /tmp/opencode-cleanup-gh-error >&2 || true + return 1 + fi + [ "${live_head,,}" = "${TARGET_PR_HEAD_SHA,,}" ] + } + + cancel_runs() { + local status="$1" + if ! live_head_matches; then + echo "::notice::OpenCode review cleanup target changed before run selection; leaving runs unchanged." + return 0 + fi + local runs_url="repos/${TARGET_REPOSITORY}/actions/runs?status=${status}&per_page=100" + local runs_json + if ! runs_json="$(gh api --paginate "$runs_url" 2>/tmp/opencode-cleanup-gh-error)"; then + echo "::warning::OpenCode review cleanup could not inspect ${TARGET_REPOSITORY}; leaving runs unchanged." + sed 's/^/ /' /tmp/opencode-cleanup-gh-error >&2 || true + return 0 + fi + local run_ids + if ! run_ids="$(jq -r --arg pr "$TARGET_PR_NUMBER" --arg head_sha "$TARGET_PR_HEAD_SHA" \ + --arg repo "$TARGET_REPOSITORY" --arg current "$CURRENT_RUN_ID" ' + .workflow_runs[] + | select((.id | tostring) != $current) + | select(.name == "Required OpenCode Review") + | select(.event == "pull_request_target") + | ((.display_title // "") | startswith("Required OpenCode Review " + $repo + "#" + $pr + "@")) as $title_matches + | ((.pull_requests // []) | any((.number | tostring) == $pr)) as $metadata_matches + | select($title_matches or $metadata_matches) + | ((.display_title // "") | endswith("@" + $head_sha)) as $title_is_current + | ((.pull_requests // []) | any( + ((.number | tostring) == $pr) + and ((.head.sha // "") | ascii_downcase) == ($head_sha | ascii_downcase) + )) as $metadata_is_current + | select(($title_is_current or $metadata_is_current) | not) + | .id + ' <<<"$runs_json")"; then + echo "::warning::OpenCode review cleanup received invalid run data for ${TARGET_REPOSITORY}; leaving runs unchanged." + return 0 + fi + while IFS= read -r run_id; do + [ -n "$run_id" ] || continue + if ! live_head_matches; then + echo "::notice::OpenCode review cleanup target changed before cancellation; leaving runs unchanged." + return 0 + fi + if gh api --method POST "repos/${TARGET_REPOSITORY}/actions/runs/${run_id}/cancel" >/dev/null 2>/tmp/opencode-cleanup-cancel-error || + gh api --method POST "repos/${TARGET_REPOSITORY}/actions/runs/${run_id}/force-cancel" >/dev/null 2>>/tmp/opencode-cleanup-cancel-error; then + echo "Cancelled superseded Required OpenCode Review run ${run_id} in ${TARGET_REPOSITORY} for PR #${TARGET_PR_NUMBER}." + else + echo "::warning::OpenCode review cleanup could not cancel run ${run_id} in ${TARGET_REPOSITORY}; it may have finished or the credential lacks Actions write access." + sed 's/^/ /' /tmp/opencode-cleanup-cancel-error >&2 || true + fi + done <<<"$run_ids" + } + + for active_status in queued in_progress requested waiting pending; do + cancel_runs "$active_status" + done + echo "Superseded OpenCode review run cleanup completed." \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e8633515b..d7c6d40ae7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,34 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- **Fix `opencode-review.yml` admission gaps around stale/out-of-order events (`#1568`).** + Building on the draft-poll exemption's live PR/head validation, Devin Review found two + further defects. (1) The concurrency group was keyed only by repository and PR number, so + a delayed run for an *older* head could cancel the *newer*, authoritative head's still-valid + run before that older run's own live-head check ever had a chance to reject it (GitHub cancels + whichever run is currently active in a group with no notion of "older"/"newer"). Fixed by also + scoping the group by exact head SHA, so different heads no longer share a cancellation domain + while same-head events (a `converted_to_draft`/`ready_for_review` transition, a `synchronize` + retry) still do. (2) A delayed non-closed event ignored a live-closed PR, since `live_pr` only + ever extracted `head` and `draft`. Both admission blocks now also validate live `state` and exit + before any further API call when it is `"closed"`, failing closed on a missing, null, + non-string, or otherwise unrecognized value rather than assuming open. New regressions: a + structural contract test for the head-scoped concurrency group; step-body coverage for a stale + non-closed event against a live-closed PR (both admission steps), live-closed state taking + precedence over a stale live-draft flag, and each invalid `state` shape failing closed. Full + suite: 2294 passed, 1 skipped, 21 subtests; `scripts/ci` coverage and docstrings both 100%. + A third Devin Review round then found that head-scoping the concurrency group above, while + fixing the wrong-direction cancellation, also disabled the legitimate one: a genuine new + commit no longer cancels its own PR's now-obsolete previous-head poll, which would otherwise + occupy a runner until GitHub's own per-job ceiling. Added a `cancel-superseded-opencode-review-runs` + job, scoped to `synchronize` events, mirroring the already-established live-head-validated + cleanup pattern in `strix.yml`'s `cancel-superseded-pr-runs` job: it re-verifies the live head + immediately before both listing candidates and cancelling each one, so a delayed/stale + invocation of this same job cannot itself wrongly cancel a still-authoritative run. New + regressions: the embedded run-selection `jq` filter executed against synthetic run payloads + (superseded-run selection, current-head/self-run/other-PR/other-workflow exclusion, and + `pull_requests[]` metadata matching), plus a structural test for the job's trigger and + permissions. Full suite: 2301 passed, 1 skipped, 21 subtests; coverage and docstrings both 100%. - **Fix a live crash: `noema-review` failed with an unhandled `HTTPError` instead of failing closed.** Live incident on `ContextualWisdomLab/naruon#1486`: `scripts/ci/noema_review_gate.py::call_llm`'s `opener.open(request)` call sat diff --git a/docs/doctoring/opencode-draft-verdict-cycle.md b/docs/doctoring/opencode-draft-verdict-cycle.md new file mode 100644 index 0000000000..2347e65acd --- /dev/null +++ b/docs/doctoring/opencode-draft-verdict-cycle.md @@ -0,0 +1,46 @@ +# OpenCode draft-verdict chicken-and-egg repair + +Date: 2026-09-01 +Repository: `ContextualWisdomLab/.github` +Original owner PR: #1568 +Protected base at reconciliation: `main@b4f7b082536d2be8dceab0a40a484161b50e5acd` + +## Root cause + +The required `opencode-review` workflow polled for an exact-head OpenCode verdict even when a pull request was a draft. The central scheduler intentionally does not dispatch ordinary review work for a draft unless an explicit agent-review path is requested. That created a self-hosting cycle: the required check waited for a verdict that the same governance system intentionally would not produce. + +A second edge existed when a ready PR was converted back to draft while a poll was already running. Without a `converted_to_draft` trigger, no fresh PR-scoped run existed to cancel the stale poll. After adding that trigger, the request-review step also needed its own draft early exit so the replacement run could not fetch Reviews API evidence, exchange an OIDC token, or dispatch scheduler work before the later verdict step noticed draft state. + +## Repair + +- Add `converted_to_draft` to the `pull_request_target` trigger set. +- Both the request-review and required-verdict polling steps first make one unconditional, authoritative `gh api` live PR lookup (added after the initial fix, per Devin Review on this PR: a stale event-payload `PR_DRAFT`/head cannot be trusted on its own) and fail closed on a lookup error or an exact-head mismatch. Only after that live lookup confirms the PR is still draft on the live exact head does each step exit -- before any *further* GitHub API call or token exchange. +- Preserve `ready_for_review` behavior and the separate explicit marker-backed draft-review path. +- Keep `cancel-in-progress: true` concurrency behavior, now scoped by exact head SHA in addition to PR number (see "Head-scoped concurrency" below) so the converted-to-draft event still replaces a stale same-head poll. + +Executable regressions cover the trigger, the request-step and verdict-step live-state-then-exit exemptions, closed-event precedence, moved-head fail-closed behavior, and unchanged non-draft behavior. + +## Head-scoped concurrency and live closed-state validation (second Devin Review round) + +Devin Review found two further defects once the live head/draft lookup above landed: + +1. **Stale runs could cancel the current check.** The concurrency group was keyed only by repository and PR number. GitHub cancels whichever run is currently active in a group when a new one starts -- it has no notion of "older" or "newer" -- so a delayed, out-of-order run for an *older* head (e.g. a `synchronize` webhook delivered late under the org's saturated Actions queue) could cancel the *newer*, authoritative head's still-valid run before that older run's own live-head check ever had a chance to reject it. Fixed by also scoping the group by `github.event.pull_request.head.sha`: different heads no longer share a cancellation domain, while events for the exact same head (a `converted_to_draft`/`ready_for_review` transition, a `synchronize` retry) still do, which is what lets `converted_to_draft` retire an active same-head verdict poll. +2. **A delayed non-closed event ignored a live-closed PR.** `live_pr` only ever extracted `head` and `draft`; a stale `synchronize`/`ready_for_review`/etc. event arriving after the PR was actually closed had no way to notice and could still fetch the receipt-gate helper, exchange an OIDC token, dispatch a scheduler wake, or poll the Reviews API indefinitely. Both admission blocks now also extract and validate live `state`, exiting before any of that when it is `"closed"` -- mirroring the pre-existing `PR_ACTION == "closed"` event-level short-circuit, but driven by live API truth instead of the (possibly stale) event payload. A missing, null, non-string, or otherwise unrecognized `state` value fails closed rather than being treated as open, matching the existing `live_head`/`live_draft` validation style. + +Executable regressions: a structural contract test pins the concurrency group's head-SHA scoping; step-body regressions cover a stale non-closed event against a live-closed PR (for both admission steps), live-closed state taking precedence over a stale live-draft flag, and each invalid `state` shape (missing/null/non-string/unexpected value) failing closed. + +## Superseded-run cleanup for legitimate new commits (third Devin Review round) + +Head-scoping the concurrency group above fixed the wrong-direction cancellation, but Devin Review found it also disabled a *legitimate* one: a genuine new commit (`synchronize`, head A -> B) no longer shares a concurrency group with head A's now-obsolete run, so nothing cancels it anymore. That older run's own live-head check ran once, before it entered the unbounded Reviews API wait loop, which never re-validates the head on later iterations -- left alone, it would occupy a hosted runner polling for a verdict OpenCode will never produce for that head, until GitHub's own per-job ceiling. + +Fixed by adding a dedicated `cancel-superseded-opencode-review-runs` job, scoped to `synchronize` events, mirroring the already-established live-head-validated cleanup pattern in `strix.yml`'s own `cancel-superseded-pr-runs` job (and `noema-review.yml`'s in-job equivalent): it lists this PR's other active `Required OpenCode Review` runs (matched by workflow name/event plus a display-title or `pull_requests[]` PR-number match), excludes the currently-executing run and any run already on the live head, and cancels the rest -- re-verifying the live head immediately before both the listing pass and each individual cancellation, so a delayed/stale invocation of this same cleanup job cannot itself wrongly cancel a still-authoritative run. + +Executable regressions: the embedded run-selection `jq` filter is extracted and executed against synthetic `workflow_runs` payloads (mirroring how `runtime_verdict()` already exercises the required-verdict filter), covering selection of a genuinely superseded older-head run, exclusion of a current-head run, exclusion of the cleanup job's own run, exclusion of a different PR, exclusion of a differently-named/triggered run, and matching via `pull_requests[]` metadata when `display_title` never rendered the head suffix; a structural test pins the job's `synchronize`-only trigger and `actions: write` permission. + +## Reconciliation + +The original branch diverged while unrelated protected-main repairs landed, including the Noema transport repair and the `graphql-core` security update. The branch is reconciled with current protected `main` through a normal two-parent merge commit; no force push or destructive rebase is used. Newer protected-main documentation is retained rather than replaced with stale branch copies. The concurrent review-event scheduler wake regression is retained in a dedicated regression file. + +## Governance boundary + +This repair removes an impossible required-check dependency; it does not weaken exact-head review requirements for non-draft PRs, fabricate review evidence, self-approve, suppress security findings, or change branch-protection thresholds. The separate repository-wide scheduler coverage repair is tracked on #1572. diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh old mode 100644 new mode 100755 index 9b58be0fbe..d5db849145 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -514,7 +514,7 @@ assert_opencode_review_uses_codegraph_and_contextual_orchestrator() { local opencode_config="$REPO_ROOT/opencode.jsonc" assert_file_contains "$bootstrap_file" "pull_request_target:" "opencode required workflow loads its metadata-only bootstrap from the protected base ref" - assert_file_contains "$bootstrap_file" "types: [opened, synchronize, reopened, ready_for_review, closed]" "opencode required workflow reacts to current PR head changes and closed-PR cleanup" + assert_file_contains "$bootstrap_file" "types: [opened, synchronize, reopened, ready_for_review, converted_to_draft, closed]" "opencode required workflow reacts to current PR head changes, mid-poll draft conversion, and closed-PR cleanup" assert_file_contains "$bootstrap_file" "required-workflow-bootstrap:" "opencode required workflow materializes at least one job for pull_request ruleset runs" assert_file_contains "$bootstrap_file" "Required OpenCode workflow materialized without checking out or" "opencode required workflow bootstrap documents its data-only trust boundary" assert_file_contains "$bootstrap_file" "coverage-source-tree:" "opencode required workflow preserves the stable coverage-source-tree branch-protection context" diff --git a/tests/conftest.py b/tests/conftest.py index 983b36d92e..6f0c91d00f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,12 +11,16 @@ @pytest.fixture(autouse=True) def clear_trusted_uv_process_caches() -> Iterator[None]: - """Isolate process-global trusted uv caches even when a test fails early.""" - materializer._install_trusted_uv.cache_clear() - materializer._install_trusted_uv_url_opener.cache_clear() + """Isolate the original process-global trusted uv caches across monkeypatches.""" + install_cache_clear = materializer._install_trusted_uv.cache_clear + opener_cache_clear = materializer._install_trusted_uv_url_opener.cache_clear + install_cache_clear() + opener_cache_clear() yield - materializer._install_trusted_uv.cache_clear() - materializer._install_trusted_uv_url_opener.cache_clear() + install_cache_clear() + opener_cache_clear() + + class FakeHttpResponse: """Expose bounded context-managed reads from one deterministic final URL.""" diff --git a/tests/test_opencode_live_draft_state_regression.py b/tests/test_opencode_live_draft_state_regression.py new file mode 100644 index 0000000000..18fd482b64 --- /dev/null +++ b/tests/test_opencode_live_draft_state_regression.py @@ -0,0 +1,271 @@ +"""Regression coverage for live draft/head validation in required OpenCode review.""" + +from __future__ import annotations + +import base64 +import json +import os +from pathlib import Path +import shutil +import subprocess + +import pytest + +from tests.test_opencode_required_verdict_regression import ( + HEAD, + fail_closed_script, + request_review_script, +) + + +def _write_live_state_gh( + bin_dir: Path, + *, + live_draft: bool, + live_head: str = HEAD, + live_state: str = "open", + later_exit: int = 19, + approved_receipt: bool = False, + live_payload_override: dict[str, object] | None = None, +) -> None: + """Serve live PR state and optionally one approved receipt helper fixture. + + ``live_payload_override`` replaces the whole live-PR JSON body outright, + for exercising a missing/null/non-string/unexpected ``state`` field that + the convenience ``live_draft``/``live_head``/``live_state`` parameters + cannot express. + """ + payload = json.dumps( + live_payload_override + if live_payload_override is not None + else {"draft": live_draft, "head": {"sha": live_head}, "state": live_state} + ) + helper_source = """def fetch_reviews(repository, number): + return [{\"state\": \"APPROVED\"}] + + +def evaluate_receipts(reviews, head_sha, *, is_draft): + if is_draft: + return None, \"draft\" + return {\"state\": \"APPROVED\"}, \"approved\" +""" + helper_b64 = base64.b64encode(helper_source.encode()).decode() + fake_gh = bin_dir / "gh" + fake_gh.write_text( + "#!/usr/bin/env bash\n" + "set -euo pipefail\n" + "if [[ \"$*\" == \"api repos/ContextualWisdomLab/example/pulls/1437\" ]]; then\n" + f" printf '%s' {json.dumps(payload)}\n" + " exit 0\n" + "fi\n" + + ( + "if [[ \"$*\" == api\\ repos/ContextualWisdomLab/.github/contents/scripts/ci/opencode_review_receipt_gate.py?ref=* ]]; then\n" + f" printf '%s' {json.dumps(helper_b64)}\n" + " exit 0\n" + "fi\n" + if approved_receipt + else "" + ) + + f"exit {later_exit}\n", + encoding="utf-8", + ) + fake_gh.chmod(fake_gh.stat().st_mode | 0o111) + + +def _run_step( + tmp_path: Path, + script: str, + *, + live_draft: bool, + live_head: str = HEAD, + live_state: str = "open", + event_draft: bool = True, + action: str = "converted_to_draft", + approved_receipt: bool = False, + live_payload_override: dict[str, object] | None = None, +) -> subprocess.CompletedProcess[str]: + """Execute one production step against independently controlled live state.""" + bash = shutil.which("bash") + jq = shutil.which("jq") + if bash is None or jq is None: + pytest.skip("bash and jq are required to execute the production step body") + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + _write_live_state_gh( + bin_dir, + live_draft=live_draft, + live_head=live_head, + live_state=live_state, + approved_receipt=approved_receipt, + live_payload_override=live_payload_override, + ) + return subprocess.run( + [bash, "-c", script], + env={ + **os.environ, + "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + "GH_TOKEN": "fake-token", + "OIDC_AUDIENCE": "opencode-github-action", + "OPENCODE_API_BASE_URL": "https://api.opencode.ai", + "TARGET_REPOSITORY": "ContextualWisdomLab/example", + "PR_NUMBER": "1437", + "HEAD_SHA": HEAD, + "PR_ACTION": action, + "PR_DRAFT": "true" if event_draft else "false", + "BASE_BRANCH": "main", + "WORKFLOW_SHA": "c" * 40, + }, + text=True, + capture_output=True, + check=False, + ) + + +def test_stale_draft_request_event_does_not_exempt_live_ready_pr( + tmp_path: Path, +) -> None: + """A stale draft request snapshot continues into the ready-PR review path.""" + result = _run_step(tmp_path, request_review_script(), live_draft=False) + + assert result.returncode == 19 + assert "Event draft snapshot is stale" in result.stdout + + +def test_stale_draft_verdict_event_does_not_exempt_live_ready_pr( + tmp_path: Path, +) -> None: + """A stale draft verdict snapshot cannot publish a success for a ready PR.""" + result = _run_step(tmp_path, fail_closed_script(), live_draft=False) + + assert result.returncode == 19 + assert "Event draft snapshot is stale" in result.stdout + + +@pytest.mark.parametrize("script", (request_review_script(), fail_closed_script())) +def test_stale_ready_event_exempts_live_draft_pr( + tmp_path: Path, + script: str, +) -> None: + """A delayed ready event cannot keep dispatching or polling after live draft conversion.""" + result = _run_step( + tmp_path, + script, + live_draft=True, + event_draft=False, + action="ready_for_review", + ) + + assert result.returncode == 0, result.stderr + assert "still a draft on the live exact head" in result.stdout + + +def test_stale_draft_request_reuses_live_ready_approval(tmp_path: Path) -> None: + """Validated live-ready state must be used by the receipt gate, not stale metadata.""" + result = _run_step( + tmp_path, + request_review_script(), + live_draft=False, + event_draft=True, + action="converted_to_draft", + approved_receipt=True, + ) + + assert result.returncode == 0, result.stderr + assert "Current-head substantive OpenCode verdict already exists" in result.stdout + + +@pytest.mark.parametrize("script", (request_review_script(), fail_closed_script())) +def test_draft_exemption_fails_closed_when_live_head_moved( + tmp_path: Path, + script: str, +) -> None: + """The event cannot exempt a different live head even when it is still draft.""" + result = _run_step(tmp_path, script, live_draft=True, live_head="b" * 40) + + assert result.returncode == 1 + assert "head moved while validating live" in result.stdout + + +@pytest.mark.parametrize("script", (request_review_script(), fail_closed_script())) +def test_stale_non_closed_event_exempts_a_live_closed_pr( + tmp_path: Path, + script: str, +) -> None: + """A delayed non-closed event cannot dispatch or poll against a live-closed PR. + + Devin Review on `#1568` found that `live_pr` only ever extracted `head` + and `draft` -- a delayed `synchronize`/`ready_for_review`/etc. event + arriving after the PR was actually closed would ignore that live closed + state entirely and could still fetch the receipt-gate helper, exchange + an OIDC token, dispatch a scheduler wake, or poll the Reviews API + indefinitely. Both admission blocks now also validate live `state` and + exit before any of that when it is `"closed"`, exactly like the + pre-existing `PR_ACTION == "closed"` short-circuit for a genuinely + closed *event*. + """ + result = _run_step( + tmp_path, + script, + live_draft=False, + live_state="closed", + event_draft=False, + action="synchronize", + ) + + assert result.returncode == 0, result.stderr + assert "PR is closed on the live exact head" in result.stdout + + +@pytest.mark.parametrize("script", (request_review_script(), fail_closed_script())) +def test_live_closed_state_takes_precedence_over_live_draft( + tmp_path: Path, + script: str, +) -> None: + """A live-closed PR is reported as closed, not draft, even if also draft.""" + result = _run_step( + tmp_path, + script, + live_draft=True, + live_state="closed", + event_draft=False, + action="synchronize", + ) + + assert result.returncode == 0, result.stderr + assert "PR is closed on the live exact head" in result.stdout + assert "still a draft on the live exact head" not in result.stdout + + +@pytest.mark.parametrize("script", (request_review_script(), fail_closed_script())) +@pytest.mark.parametrize( + "live_payload_override", + ( + {"draft": False, "head": {"sha": HEAD}}, + {"draft": False, "head": {"sha": HEAD}, "state": None}, + {"draft": False, "head": {"sha": HEAD}, "state": 1}, + {"draft": False, "head": {"sha": HEAD}, "state": "merged"}, + ), + ids=("missing", "null", "non-string", "unexpected-value"), +) +def test_live_invalid_state_fails_closed( + tmp_path: Path, + script: str, + live_payload_override: dict[str, object], +) -> None: + """A missing, null, non-string, or unrecognized live `state` fails closed. + + GitHub's own REST API only ever reports `"open"` or `"closed"`; anything + else is treated as untrustworthy live evidence rather than assumed open + (Devin Review on `#1568`). + """ + result = _run_step( + tmp_path, + script, + live_draft=False, + event_draft=False, + action="synchronize", + live_payload_override=live_payload_override, + ) + + assert result.returncode == 1 + assert "Could not validate live pull request state" in result.stdout diff --git a/tests/test_opencode_required_verdict_regression.py b/tests/test_opencode_required_verdict_regression.py index 0e5d30805b..4b098f9296 100644 --- a/tests/test_opencode_required_verdict_regression.py +++ b/tests/test_opencode_required_verdict_regression.py @@ -31,6 +31,15 @@ def request_review_script() -> str: return textwrap.dedent(block) +def fail_closed_script() -> str: + """Extract the production "Fail closed without a current-head OpenCode verdict" run block.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + step = workflow.split( + " - name: Fail closed without a current-head OpenCode verdict\n", 1 + )[1] + return textwrap.dedent(step.split(" run: |\n", 1)[1]) + + def review(*, state: str, commit_id: str = HEAD, body: str = "") -> dict[str, object]: """Build one Reviews API record from the OpenCode GitHub App.""" return { @@ -102,6 +111,129 @@ def test_runtime_required_verdict_rejects_other_actor() -> None: assert runtime_verdict([human]) == "" +def cleanup_candidate_run_ids( + runs: list[dict[str, object]], + *, + pr_number: str = "1437", + head_sha: str = HEAD, + repository: str = "ContextualWisdomLab/example", + current_run_id: str = "999", +) -> list[str]: + """Execute the jq program embedded in the superseded-run cleanup job.""" + jq = shutil.which("jq") + if jq is None: + pytest.skip("jq is required to execute the production cleanup filter") + workflow = WORKFLOW.read_text(encoding="utf-8") + marker = ( + 'jq -r --arg pr "$TARGET_PR_NUMBER" --arg head_sha "$TARGET_PR_HEAD_SHA" \\\n' + ' --arg repo "$TARGET_REPOSITORY" --arg current "$CURRENT_RUN_ID" \'' + ) + start = workflow.index(marker) + len(marker) + end = workflow.index("\n ' <<<\"$runs_json\")", start) + result = subprocess.run( + [ + jq, + "-r", + "--arg", + "pr", + pr_number, + "--arg", + "head_sha", + head_sha, + "--arg", + "repo", + repository, + "--arg", + "current", + current_run_id, + workflow[start:end], + ], + input=json.dumps({"workflow_runs": runs}), + text=True, + capture_output=True, + check=False, + ) + assert result.returncode == 0, result.stderr + return [line for line in result.stdout.splitlines() if line] + + +def _cleanup_run( + *, + run_id: int, + head_sha: str = HEAD, + name: str = "Required OpenCode Review", + event: str = "pull_request_target", + display_title: str | None = None, + pr_number: int = 1437, +) -> dict[str, object]: + """Build one synthetic workflow-run record for the cleanup filter.""" + title = ( + display_title + if display_title is not None + else f"Required OpenCode Review ContextualWisdomLab/example#{pr_number}@{head_sha}" + ) + return { + "id": run_id, + "name": name, + "event": event, + "display_title": title, + "pull_requests": [{"number": pr_number, "head": {"sha": head_sha}}], + } + + +def test_cleanup_selects_a_superseded_older_head_run() -> None: + """An older run for a different, no-longer-live head is selected.""" + stale = _cleanup_run(run_id=1, head_sha="b" * 40) + assert cleanup_candidate_run_ids([stale], current_run_id="999") == ["1"] + + +def test_cleanup_excludes_the_current_live_head_run() -> None: + """A run already on the live exact head is never selected.""" + current_head_run = _cleanup_run(run_id=1, head_sha=HEAD) + assert cleanup_candidate_run_ids([current_head_run], current_run_id="999") == [] + + +def test_cleanup_excludes_the_currently_executing_run_itself() -> None: + """The cleanup job's own run is never a cancellation candidate.""" + self_run = _cleanup_run(run_id=999, head_sha="b" * 40) + assert cleanup_candidate_run_ids([self_run], current_run_id="999") == [] + + +def test_cleanup_excludes_a_different_pull_request() -> None: + """A stale-head run for an unrelated PR is left untouched.""" + other_pr = _cleanup_run(run_id=1, head_sha="b" * 40, pr_number=9999) + assert cleanup_candidate_run_ids([other_pr], current_run_id="999") == [] + + +def test_cleanup_excludes_a_differently_named_or_triggered_run() -> None: + """A same-PR run for another workflow or trigger is left untouched.""" + other_workflow = _cleanup_run(run_id=1, head_sha="b" * 40, name="Strix Security Scan") + other_event = _cleanup_run(run_id=2, head_sha="b" * 40, event="workflow_dispatch") + assert ( + cleanup_candidate_run_ids([other_workflow, other_event], current_run_id="999") + == [] + ) + + +def test_cleanup_matches_by_pull_requests_metadata_when_title_omits_the_suffix() -> None: + """A run whose display_title never rendered the head suffix still resolves.""" + metadata_only = _cleanup_run( + run_id=1, head_sha="b" * 40, display_title="Required OpenCode Review" + ) + assert cleanup_candidate_run_ids([metadata_only], current_run_id="999") == ["1"] + + +def test_cleanup_job_is_scoped_to_synchronize_events_with_actions_write() -> None: + """The cleanup job only fires on synchronize and can cancel runs.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + job = workflow.split(" cancel-superseded-opencode-review-runs:\n", 1)[1] + assert ( + "if: github.event_name == 'pull_request_target' && " + "github.event.action == 'synchronize'" + ) in job + assert "actions: write" in job.split("steps:", 1)[0] + + def test_required_verdict_has_one_executable_owner() -> None: """Tests must execute the workflow gate, not a test-only Python mirror.""" status_source = STATUS_HELPER.read_text(encoding="utf-8") @@ -147,6 +279,256 @@ def test_required_workflow_cannot_succeed_with_an_echo_only_placeholder() -> Non ) +def _write_live_pr_then_refusing_gh(bin_dir: Path) -> None: + """Serve the authoritative live PR lookup, then reject downstream GitHub I/O.""" + fake_gh = bin_dir / "gh" + fake_gh.write_text( + "#!/usr/bin/env bash\n" + "set -euo pipefail\n" + "if [[ \"$*\" == \"api repos/ContextualWisdomLab/example/pulls/1437\" ]]; then\n" + " printf '%s' \"$LIVE_PR_JSON\"\n" + " exit 0\n" + "fi\n" + "echo 'unexpected gh invocation after live-state validation' >&2\n" + "exit 17\n", + encoding="utf-8", + ) + fake_gh.chmod(fake_gh.stat().st_mode | 0o111) + + +def _run_fail_closed_step( + tmp_path: Path, + *, + pr_action: str = "", + pr_draft: str = "false", + pr_number: str = "1437", + head_sha: str = HEAD, +) -> subprocess.CompletedProcess[str]: + """Execute the "Fail closed without a current-head OpenCode verdict" step body. + + A fake ``gh`` that fails loudly is installed on ``PATH`` so a closed or + draft early exit that reaches the Reviews API call at all fails the test + immediately, rather than actually looping (the production step's + ``while :; do ... sleep 30; done`` never naturally terminates on a + non-matching review, so a real ``gh`` fixture serving no match would hang + a test rather than fail it). + """ + bash = shutil.which("bash") + jq = shutil.which("jq") + if bash is None or jq is None: + pytest.skip("bash and jq are required to execute the production step body") + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + _write_live_pr_then_refusing_gh(bin_dir) + return subprocess.run( + [bash, "-c", fail_closed_script()], + env={ + **os.environ, + "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + "GH_TOKEN": "fake-token", + "TARGET_REPOSITORY": "ContextualWisdomLab/example", + "PR_NUMBER": pr_number, + "HEAD_SHA": head_sha, + "PR_ACTION": pr_action, + "PR_DRAFT": pr_draft, + "LIVE_PR_JSON": json.dumps( + { + "draft": pr_draft.lower() == "true", + "head": {"sha": head_sha}, + "state": "open", + } + ), + }, + text=True, + capture_output=True, + check=False, + ) + + +def test_fail_closed_step_exempts_a_draft_pr_before_polling(tmp_path: Path) -> None: + """A draft PR's required check must pass without ever polling Reviews API. + + `#1546` added `PR_DRAFT` to the dispatch step's receipt-gate check + (`evaluate_receipts(..., is_draft=...)`), but that only narrows which + reviews the gate accepts -- it never exempts a draft PR from needing one, + and the scheduler's own draft path + (`scripts/ci/pr_review_merge_scheduler.py`'s `inspect_pr`) skips + dispatching a review for an ordinary draft entirely (no + `@opencode-agent` mention). With no draft exemption here, this step's + `while :; do ... sleep 30; done` loop would poll for a verdict OpenCode + will never post, until the job's own ~360-minute runtime ceiling kills + it -- reproduced against this exact commit before this fix (`#1443` + fixed the same class of bug on a now-superseded design; this restores + the equivalent exemption on the current receipt/scheduler-gated design). + """ + result = _run_fail_closed_step(tmp_path, pr_action="synchronize", pr_draft="true") + assert result.returncode == 0, result.stderr + assert "PR is still a draft on the live exact head; a current-head OpenCode verdict is not required" in result.stdout + + +def _run_request_review_step( + tmp_path: Path, + *, + pr_draft: str = "false", +) -> subprocess.CompletedProcess[str]: + """Execute the "Request current-head OpenCode review execution" step body. + + A fake ``gh`` that fails loudly is installed on ``PATH`` so a draft + early exit that reaches any API call at all -- fetching the receipt-gate + helper source, or the Reviews API it wraps -- fails the test + immediately. + """ + bash = shutil.which("bash") + if bash is None: + pytest.skip("bash is required to execute the production step body") + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + _write_live_pr_then_refusing_gh(bin_dir) + return subprocess.run( + [bash, "-c", request_review_script()], + env={ + **os.environ, + "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + "GH_TOKEN": "fake-token", + "OIDC_AUDIENCE": "opencode-github-action", + "OPENCODE_API_BASE_URL": "https://api.opencode.ai", + "TARGET_REPOSITORY": "ContextualWisdomLab/example", + "PR_NUMBER": "1437", + "HEAD_SHA": HEAD, + "PR_DRAFT": pr_draft, + "BASE_BRANCH": "main", + "WORKFLOW_SHA": "c" * 40, + "LIVE_PR_JSON": json.dumps( + { + "draft": pr_draft.lower() == "true", + "head": {"sha": HEAD}, + "state": "open", + } + ), + }, + text=True, + capture_output=True, + check=False, + ) + + +def test_request_review_step_exempts_a_pr_converted_to_draft_before_any_api_call( + tmp_path: Path, +) -> None: + """A PR converted to draft must not dispatch a new review request either. + + Devin Review on `#1568` found that `converted_to_draft` firing this + workflow only fixed the "Fail closed" step's own poll -- the sibling + "Request current-head OpenCode review execution" step (which runs first) + had no draft exemption at all, so it still fetched the receipt-gate + helper source and queried the Reviews API, and could reach OIDC token + exchange and a `repository_dispatch` scheduler wake, before the "Fail + closed" step's exemption ever ran. This proves the request step now performs only the authoritative live-state lookup, then + exits before helper-source, review, token, or dispatch API calls when + `PR_DRAFT` is `"true"` (the value GitHub sends for `converted_to_draft`), + while `ready_for_review` and explicit draft-review dispatch paths + elsewhere (`pr_review_merge_scheduler.py`'s own draft handling) are + untouched by this step-body change. + """ + result = _run_request_review_step(tmp_path, pr_draft="true") + assert result.returncode == 0, result.stderr + assert "PR is still a draft on the live exact head; a current-head OpenCode review is not requested" in result.stdout + + +def test_request_review_step_still_dispatches_for_a_non_draft_pr( + tmp_path: Path, +) -> None: + """A non-draft PR must still reach the receipt-gate helper fetch.""" + result = _run_request_review_step(tmp_path, pr_draft="false") + assert result.returncode == 17, result.stderr + assert "unexpected gh invocation after live-state validation" in result.stderr + + +def test_fail_closed_step_exempts_a_pr_converted_to_draft_mid_poll( + tmp_path: Path, +) -> None: + """A PR converted to draft while a poll is in flight exits before polling. + + Devin Review on `#1568` found that `converted_to_draft` was missing from + this workflow's `pull_request_target.types`, so converting a PR to draft + while an earlier event's "Fail closed" poll was still running never fired + a fresh run to cancel it via the PR-scoped `cancel-in-progress: true` + concurrency group -- the stale non-draft poll kept waiting for a verdict + the now-draft PR can never receive. Adding `converted_to_draft` to the + trigger set lets a fresh run's draft exemption below take over; this test + proves that exemption exits before ever reaching the Reviews API for the + exact `PR_ACTION=converted_to_draft` value GitHub sends for that event + (`PR_DRAFT` is always `"true"` on that event, mirroring GitHub's own + payload). + """ + result = _run_fail_closed_step( + tmp_path, pr_action="converted_to_draft", pr_draft="true" + ) + assert result.returncode == 0, result.stderr + assert "PR is still a draft on the live exact head; a current-head OpenCode verdict is not required" in result.stdout + + +def test_opencode_review_trigger_reacts_to_mid_poll_draft_conversion() -> None: + """The workflow's own trigger set -- not just the step body -- covers it. + + A step-level test alone cannot prove the draft exemption above is + actually reachable in production: GitHub only re-invokes this workflow + for event types listed in `pull_request_target.types`. This pins that + `converted_to_draft` is present there, so a mid-poll draft conversion + fires a fresh run at all. + """ + workflow = WORKFLOW.read_text(encoding="utf-8") + trigger_block = workflow.split(" pull_request_target:\n", 1)[1].split( + "\n\nconcurrency:", 1 + )[0] + assert "converted_to_draft" in trigger_block + assert ( + "types: [opened, synchronize, reopened, ready_for_review, " + "converted_to_draft, closed]" + ) in trigger_block + assert "cancel-in-progress: true" in workflow + + +def test_opencode_review_concurrency_group_is_scoped_by_exact_head() -> None: + """The bootstrap concurrency group is keyed by head SHA, not just PR number. + + Devin Review on `#1568` found that a delayed, out-of-order run for an + older head could cancel the authoritative run already active for a + newer head: GitHub cancels whichever run is currently active in a + concurrency group when a new one starts, with no notion of "older" or + "newer", so a group shared across different heads let a stale event + retire the current head's still-valid run before its own live-head + check could ever reject it. Scoping the group by exact head SHA + isolates different heads from each other while events for the exact + same head (a `converted_to_draft`/`ready_for_review` transition, a + `synchronize` retry) still share one group and can still cancel each + other, which is what lets `converted_to_draft` retire an active + same-head verdict poll. + """ + workflow = WORKFLOW.read_text(encoding="utf-8") + concurrency_block = workflow.split("\n\nconcurrency:\n", 1)[1].split( + "\n\npermissions:", 1 + )[0] + assert "github.event.pull_request.head.sha || github.run_id" in concurrency_block + assert "github.event.pull_request.number || github.run_id" in concurrency_block + assert "cancel-in-progress: true" in concurrency_block + + +def test_fail_closed_step_closed_still_takes_precedence_over_draft(tmp_path: Path) -> None: + """The pre-existing ``closed`` early exit still runs before the new draft check.""" + result = _run_fail_closed_step(tmp_path, pr_action="closed", pr_draft="true") + assert result.returncode == 0, result.stderr + assert "PR closed; a current-head OpenCode verdict is not required." in result.stdout + assert "PR is a draft" not in result.stdout + + +def test_fail_closed_step_still_polls_for_a_non_draft_pr(tmp_path: Path) -> None: + """A non-draft PR must still reach the Reviews API call (not exempted).""" + result = _run_fail_closed_step(tmp_path, pr_action="synchronize", pr_draft="false") + assert result.returncode == 17, result.stderr + assert "unexpected gh invocation after live-state validation" in result.stderr + + @pytest.mark.parametrize( ("reviews", "dispatches"), ( @@ -168,7 +550,9 @@ def test_scheduler_wake_reuses_trusted_receipt_predicate( fake_gh.write_text( """#!/usr/bin/env bash set -euo pipefail -if [[ "$*" == *"contents/scripts/ci/opencode_review_receipt_gate.py"* ]]; then +if [[ "$*" == "api repos/owner/repo/pulls/7" ]]; then + printf '%s' "$LIVE_PR_JSON" +elif [[ "$*" == *"contents/scripts/ci/opencode_review_receipt_gate.py"* ]]; then python3 -c 'import base64, pathlib, sys; sys.stdout.write(base64.b64encode(pathlib.Path(sys.argv[1]).read_bytes()).decode())' "$REAL_RECEIPT_HELPER" elif [[ "$*" == *"/pulls/7/reviews"* ]]; then printf '[%s]' "$FAKE_REVIEWS" @@ -205,6 +589,9 @@ def test_scheduler_wake_reuses_trusted_receipt_predicate( "BASE_BRANCH": "main", "WORKFLOW_SHA": "c" * 40, "GH_TOKEN": "token", + "LIVE_PR_JSON": json.dumps( + {"draft": False, "head": {"sha": HEAD}, "state": "open"} + ), } result = subprocess.run( ["bash", "-c", request_review_script()], env=env, text=True, capture_output=True diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index f456770580..a5079daa67 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -253,6 +253,16 @@ def test_required_pull_request_workflows_cancel_superseded_runs() -> None: ) elif filename == "opencode-review.yml": assert "opencode-review-bootstrap-" in concurrency_contract + # Unlike the other required pull-request workflows below, this + # group is deliberately also scoped by exact head SHA: a + # delayed, out-of-order run for an older head must not be able + # to cancel the authoritative run already active for a newer + # head (Devin Review on `#1568`). Same-head events still share + # one group and can still cancel each other. + assert ( + "github.event.pull_request.head.sha || github.run_id" + in concurrency_contract + ) elif filename == "noema-review.yml": assert "github.event.workflow_run" not in concurrency_contract assert "noema-review-${{" in concurrency_contract @@ -268,7 +278,7 @@ def test_required_pull_request_workflows_cancel_superseded_runs() -> None: assert ( "github.event_name == 'pull_request_target'" in concurrency_contract ) - if filename != "noema-review.yml": + if filename not in {"noema-review.yml", "opencode-review.yml"}: assert "github.event.pull_request.head.sha" not in concurrency_contract assert "format('pr-{0}-{1}'" not in concurrency_contract @@ -578,7 +588,7 @@ def test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs() - assert "github.event.action != 'closed'" in workflow opencode_bootstrap = workflow_text("opencode-review.yml") - assert "types: [opened, synchronize, reopened, ready_for_review, closed]" in ( + assert "types: [opened, synchronize, reopened, ready_for_review, converted_to_draft, closed]" in ( opencode_bootstrap ) assert "actions/checkout" not in opencode_bootstrap