diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index cbc8d21439..239a7bd4d3 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -12,26 +12,18 @@ on: types: [opencode-review] concurrency: - # Workflow-level admission, for the same reason strix.yml, noema-review.yml and - # opencode-review.yml carry theirs at this level: a job-level group is never - # evaluated while the whole run waits behind the organization job ceiling, so - # superseded dispatches for one pull request coalesce only after each of them - # has already been allocated a runner. Measured on 2026-09-06: of the five - # dispatch runs that passed `validate-pr-metadata`, four were rejected hours - # later by `opencode-review`'s privileged metadata check because the head had - # moved while they queued (runs 34002473295, 34010256951, 34015973300, - # 34016922761) -- each after `coverage-source-tree` and `coverage-evidence` - # had run. Cancelling the superseded run at creation returns that slot instead - # of spending it to discover the review's subject no longer exists. - # - # The key is the target pull request, matching the job-level group below and - # codeql-scan-dispatch.yml's workflow-level group; `github.run_id` keeps runs - # without a payload in their own groups rather than colliding. + # Workflow-level admission serializes dispatches for one exact pull-request + # head before a runner is allocated. GitHub's default single-pending policy + # replaces an existing pending run, so queue every same-head event instead. + # A newer head uses a different workflow group and can reach the downstream + # PR-scoped cancel-in-progress boundary to retire stale semantic work. + # `github.run_id` keeps malformed payloads in separate groups. group: >- opencode-review-dispatch-${{ github.event.client_payload.target_repository || github.repository }}-${{ - github.event.client_payload.pr_number || github.run_id }} - cancel-in-progress: true + github.event.client_payload.pr_number || github.run_id }}-${{ + github.event.client_payload.pr_head_sha || github.run_id }} + queue: max permissions: contents: read @@ -69,6 +61,7 @@ jobs: head_ref: ${{ steps.validate.outputs.head_ref }} head_sha: ${{ steps.validate.outputs.head_sha }} is_private: ${{ steps.validate.outputs.is_private }} + needs_review: ${{ steps.existing_receipt.outputs.needs_review }} steps: - name: Exchange OpenCode app token for target repository metadata reads id: metadata_read_app_token @@ -256,12 +249,62 @@ jobs: } >>"$GITHUB_OUTPUT" printf 'Validated current live metadata for %s#%s: base=%s/%s head=%s/%s.\n' "$TARGET_REPOSITORY" "$PR_NUMBER" "$live_base_ref" "$live_base_sha" "$live_head_ref" "$live_head_sha" + - name: Retire serialized same-head duplicate with formal receipt + id: existing_receipt + env: + CENTRAL_GH_TOKEN: ${{ github.token }} + GH_TOKEN: ${{ steps.metadata_read_app_token.outputs.token || secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }} + TARGET_REPOSITORY: ${{ steps.validate.outputs.target_repository }} + PR_NUMBER: ${{ steps.validate.outputs.pr_number }} + HEAD_SHA: ${{ steps.validate.outputs.head_sha }} + run: | + set -euo pipefail + helper="$(mktemp)" + trap 'rm -f "$helper"' EXIT + GH_TOKEN="$CENTRAL_GH_TOKEN" gh api \ + "repos/ContextualWisdomLab/.github/contents/scripts/ci/opencode_review_receipt_gate.py?ref=${GITHUB_SHA}" \ + --jq .content | base64 --decode >"$helper" + receipt_state="$( + python3 - "$helper" "$TARGET_REPOSITORY" "$PR_NUMBER" "$HEAD_SHA" <<'PY' + import importlib.machinery + import importlib.util + import sys + + helper_path, repository, number, head_sha = sys.argv[1:] + loader = importlib.machinery.SourceFileLoader( + "trusted_opencode_receipt_gate", helper_path + ) + spec = importlib.util.spec_from_loader(loader.name, loader) + if spec is None or spec.loader is None: + raise RuntimeError("trusted OpenCode receipt helper could not be loaded") + gate = importlib.util.module_from_spec(spec) + spec.loader.exec_module(gate) + reviews = gate.fetch_reviews(repository, int(number)) + receipt, _reason = gate.evaluate_receipts(reviews, head_sha, is_draft=False) + print("present" if receipt is not None else "missing") + PY + )" + case "$receipt_state" in + present) + echo "needs_review=false" >>"$GITHUB_OUTPUT" + echo "Serialized same-head duplicate retired: a formal exact-head receipt already exists." + ;; + missing) + echo "needs_review=true" >>"$GITHUB_OUTPUT" + ;; + *) + echo "::error::Trusted OpenCode receipt helper returned an invalid state." + exit 1 + ;; + esac + - name: Exchange OpenCode app token for target repository coverage reads id: coverage_read_app_token if: >- github.event_name == 'repository_dispatch' && steps.validate.outputs.target_repository != '' && steps.validate.outputs.target_repository != github.repository + && steps.existing_receipt.outputs.needs_review == 'true' env: OIDC_AUDIENCE: opencode-github-action OPENCODE_API_BASE_URL: https://api.opencode.ai @@ -327,6 +370,7 @@ jobs: } >>"$GITHUB_OUTPUT" - name: Materialize pull request merge tree for coverage measurement + if: steps.existing_receipt.outputs.needs_review == 'true' env: GH_TOKEN: ${{ steps.coverage_read_app_token.outputs.token || secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }} TARGET_REPOSITORY: ${{ steps.validate.outputs.target_repository }} @@ -381,6 +425,7 @@ jobs: tar -cf "$COVERAGE_SOURCE_ARCHIVE" -C "$COVERAGE_SOURCE_WORKDIR" . - name: Upload materialized pull request merge tree + if: steps.existing_receipt.outputs.needs_review == 'true' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: opencode-coverage-source @@ -393,6 +438,7 @@ jobs: needs: [validate-pr-metadata] if: >- needs.validate-pr-metadata.result == 'success' + && needs.validate-pr-metadata.outputs.needs_review == 'true' && github.event_name == 'repository_dispatch' runs-on: ubuntu-24.04 timeout-minutes: 300 @@ -2332,6 +2378,7 @@ jobs: if: >- always() && needs.validate-pr-metadata.result == 'success' + && needs.validate-pr-metadata.outputs.needs_review == 'true' && needs.coverage-evidence.result != 'cancelled' && github.event_name == 'repository_dispatch' concurrency: diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index ec94e6d24e..2f3bef3fa6 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -497,16 +497,44 @@ jobs: exit 1 fi echo "::add-mask::$app_token" - jq -cn \ - --arg target_repository "$TARGET_REPOSITORY" \ - --arg pr_number "$PR_NUMBER" \ - --arg pr_base_ref "$BASE_BRANCH" \ - --arg pr_base_sha "$BASE_SHA" \ - --arg pr_head_ref "$HEAD_REF" \ - --arg pr_head_sha "$HEAD_SHA" \ - --arg required_run_id "$GITHUB_RUN_ID" \ - '{event_type:"opencode-review",client_payload:{target_repository:$target_repository,pr_number:$pr_number,pr_base_ref:$pr_base_ref,pr_base_sha:$pr_base_sha,pr_head_ref:$pr_head_ref,pr_head_sha:$pr_head_sha,required_run_id:$required_run_id}}' | - GH_TOKEN="$app_token" gh api -X POST repos/ContextualWisdomLab/.github/dispatches --input - + # Same-head in-flight dedupe (pg-erd-cloud#1183 / appguardrail#1247): + # a second repository_dispatch for an unchanged head cancels the + # already-queued/running central review via dispatch concurrency and + # restarts the multi-hour queue→9s-fail handshake. Skip the POST when + # an exact-head OpenCode Review Dispatch is already queued or running; + # still fall through to fail-closed without a formal verdict. + inflight_helper="$(mktemp)" + trap 'rm -f "$helper" "$inflight_helper"' EXIT + gh api "repos/ContextualWisdomLab/.github/contents/scripts/ci/opencode_inflight_dispatch_gate.py?ref=${WORKFLOW_SHA}" \ + --jq .content | base64 --decode >"$inflight_helper" + inflight_state="$( + GH_TOKEN="$app_token" python3 "$inflight_helper" \ + --target-repository "$TARGET_REPOSITORY" \ + --pr-number "$PR_NUMBER" \ + --head-sha "$HEAD_SHA" + )" + if [ "$inflight_state" = "present" ]; then + echo "Exact-head OpenCode Review Dispatch already queued or running; duplicate repository_dispatch skipped." + else + if [ "$inflight_state" = "stale" ]; then + echo "Prior-head OpenCode dispatch observed; the new head will reach PR-scoped stale-work retirement." + elif [ "$inflight_state" = "missing" ]; then + : + else + echo "::error::OpenCode in-flight dispatch gate returned an invalid state." + exit 1 + fi + jq -cn \ + --arg target_repository "$TARGET_REPOSITORY" \ + --arg pr_number "$PR_NUMBER" \ + --arg pr_base_ref "$BASE_BRANCH" \ + --arg pr_base_sha "$BASE_SHA" \ + --arg pr_head_ref "$HEAD_REF" \ + --arg pr_head_sha "$HEAD_SHA" \ + --arg required_run_id "$GITHUB_RUN_ID" \ + '{event_type:"opencode-review",client_payload:{target_repository:$target_repository,pr_number:$pr_number,pr_base_ref:$pr_base_ref,pr_base_sha:$pr_base_sha,pr_head_ref:$pr_head_ref,pr_head_sha:$pr_head_sha,required_run_id:$required_run_id}}' | + GH_TOKEN="$app_token" gh api -X POST repos/ContextualWisdomLab/.github/dispatches --input - + fi - name: Fail closed without a current-head OpenCode verdict env: diff --git a/docs/doctoring/opencode-inflight-dispatch-dedupe-20260919.md b/docs/doctoring/opencode-inflight-dispatch-dedupe-20260919.md new file mode 100644 index 0000000000..bed26e0353 --- /dev/null +++ b/docs/doctoring/opencode-inflight-dispatch-dedupe-20260919.md @@ -0,0 +1,218 @@ +# Doctoring: OpenCode same-head in-flight dispatch dedupe (2026-09-19) + +- **Date:** 2026-09-19 +- **Incident:** ContextualWisdomLab/pg-erd-cloud Required OpenCode Review run + `35412595263` job `105868602312` (PR #1183 head `9a759df24e8b714348ac2d240491f74ee3fc852d`, + workflow pin `64aa08d7…`). Job created `08:30:15Z`, started `12:41:37Z`, completed + `12:41:46Z` → **4h11m22s queue / ~9s execution**. Steps: review dispatch succeeded + `12:41:39–43`, then fail-closed `12:41:45` with “No APPROVED or CHANGES_REQUESTED from + opencode-agent on the current head… will rerun this failed job after publishing an + authenticated exact-head verdict.” Same handshake as appguardrail#1247. +- **Shared owner:** `ContextualWisdomLab/.github` (`opencode-review.yml` + + `opencode-review-dispatch.yml`). Callers are org ruleset `18156473` targets (all default + branches except documented exclusions) — do not patch leaf repos. + +## What is *not* broken + +- Fail-closed without a formal exact-head verdict (intentional; must not become success). +- Wake via `required_run_id` + `path=.github/workflows/opencode-review.yml` + + `event=pull_request_target` + exact `head_sha` (live incident run matches; not an + org-ruleset path mismatch). +- Releasing the runner instead of polling the multi-hour model + (`docs/pr-review-and-merge-procedure.md`). + +## What *is* broken under saturation + +1. **Duplicate same-head `repository_dispatch`.** Every new required admission after a + completed fail-closed handshake posts another dispatch when the receipt is still + missing. Dispatch concurrency is `opencode-review-dispatch-${repo}-${pr}` with + `cancel-in-progress: true`, so the new event **cancels** the prior queued/running + review for that PR — including a multi-hour review that had not yet woken the + required check. +2. **Queue amplification.** Each cancelled review + each required job that waits hours + only to spend 9s on dispatch+fail burns org runner admission without advancing a + verdict (measurement pattern in `docs/doctoring/actions-capacity-root-cause-20260917.md`). + +The scheduler already skips with `already_running` via `active_opencode_run_refs`; the +**required** entrypoint did not. + +## Minimal fix + +- Add `scripts/ci/opencode_inflight_dispatch_gate.py` — exact-equality match central + `repository_dispatch` runs whose `display_title` is + `OpenCode Review Dispatch {repo}#{pr}@{40-hex head}`. +- Inspect every GitHub-defined nonterminal workflow status: `queued`, `in_progress`, + `requested`, `waiting`, and `pending`; paginate every result page. The REST + contract caps `per_page` at 100, so first-page admission is not complete evidence. +- In `opencode-review.yml` “Request current-head OpenCode review execution”, after OIDC + app-token exchange and before `dispatches` POST: if gate returns `present`, skip the + POST; always continue to the fail-closed verdict step (no success without receipt). +- Tests: `tests/test_opencode_inflight_dispatch_gate.py` plus updates to + `tests/test_opencode_required_verdict_regression.py`. + +## Forbidden + +- Marking the required check success without an `opencode-agent` / + `opencode-agent[bot]` / formal receipt-gate APPROVED or CHANGES_REQUESTED on the + exact head. +- Weakening wake identity checks or dropping `required_run_id` binding. +- Per-leaf “just re-run the job” as the repair. + +## Before / after (measurement intent) + +| Metric | Before (incident) | After (expected) | +|--------|-------------------|------------------| +| Same-head duplicate dispatch while prior queued/running | Allowed (cancels prior) | Skipped (`present`) | +| Required job still fail-closed without verdict | Yes | Yes (unchanged) | +| Wake after formal receipt | `rerun-failed-jobs` on `required_run_id` | Unchanged | +| Queue waste class | Hours → 9s fail → re-dispatch cancel loop | Hours → 9s fail **without** cancelling in-flight review | + +## Related + +- appguardrail#1247 tracker: `TRACK-appguardrail-1247.md` +- Capacity root cause: `docs/doctoring/actions-capacity-root-cause-20260917.md` +- CodeQL wake-once parallel (not this PR): `.github#2051` + + +## Exact-identity follow-up + +The initial implementation admitted four false boundaries: + +- repository components containing repeated dots or ending in a dot; +- a run title with arbitrary bytes after the 40-hex head; +- only `queued` and `in_progress`, omitting GitHub's active `requested`, + `waiting`, and `pending` states; +- only the first 100 runs during the saturation incident class this gate exists to fix. + +RED commit `5d1ab119d4c0140f68dfef555d2818ac96e0616c` records all four +contracts. GREEN commit `e8b6570b8c9c5930178717c326fc20583e37dcab` +uses canonical repository components, exact title equality, GitHub's complete +nonterminal status set, and `gh api --paginate --slurp`. + +Exact remote verification compiled source and tests (**2/2**) and passed +**10/10** focused contract assertions. Full pytest is not claimed because the +isolated verifier does not provide pytest; fresh hosted exact-head Checks remain +mandatory. + +## Authoritative source + +GitHub. (2026). *REST API endpoints for workflow runs: List workflow runs for a +repository*. Retrieved September 19, 2026, from +https://docs.github.com/en/rest/actions/workflow-runs?apiVersion=2022-11-28#list-workflow-runs-for-a-repository + +## Serialized-owner follow-up + +Review of exact `9535e7a38f3df8ce5a2d438b14484a39c2e4b6e9` found that +list-before-POST was only a best-effort observation: two required admissions +could both observe `missing` before either dispatch became visible, and the +second event would cancel the first under unconditional PR-scoped +`cancel-in-progress: true`. It also listed every central +`repository_dispatch` workflow, so an unrelated workflow with the same +presentation title could suppress the real review. + +RED `b6c5a19243a1d89a1afbaf8aff14fb5eceebd4e0` adds the concurrent-missing, +canonical-workflow identity, stale-head supersession, and duplicate-receipt +retirement contracts. The causal repair: + +- lists runs only from the canonical + `opencode-review-dispatch.yml` workflow endpoint; +- distinguishes exact-head `present`, prior-head `stale`, and `missing`; +- carries `cancel_in_progress=false` for same-head/missing admissions, so the + PR-scoped workflow concurrency serializes racers without cancelling the active + owner; +- carries `true` only when a canonical prior-head run was observed, preserving + current-head supersession; +- reuses the trusted formal receipt predicate at the start of the serialized + workflow and retires a queued duplicate before coverage/model review. If the + first owner failed without a receipt, the queued run remains eligible to + recover. + +The formal exact-head receipt remains the only success authority. In-flight or +serialized state never produces approval. Exact +`ee56022c102df620b3467e8e69f59bba5168c4bf` verification: Python compile +**2/2**, focused policy assertions **18/18**, extracted Bash syntax **2/2**, and +YAML parse **2/2**. The isolated runtime has no pytest package, so full pytest is +not claimed; hosted exact-head Checks and independent current-head review remain +mandatory. + +## Pending-run replacement follow-up (2026-09-20) + +Review of exact `017b563c2effd66caec8e223372d41d11cbfb7a9` found that +conditional `cancel-in-progress: false` did not preserve a second pending run. +GitHub's default concurrency policy permits at most one running and one pending +run in a group; a newly queued run cancels the existing pending run. Therefore +the earlier same-head serialization claim was incomplete even though running +owners were no longer cancelled. + +RED `3f1360e4236220c1ea7e56b6854a346484497129` records the +pending-owner preservation contract. The minimal repair is: + +- GREEN `dfff8b386bc8fd40a9bca153be85cf0ad09c7581` sets the + PR-scoped central workflow to the platform-native `queue: max` policy; +- GREEN `24b24525a4ac7eaf0b9667c34e215a1d692d997b` removes the + dynamic cancellation field from the caller and dispatch payload; +- regression alignment `a33c0b4f0ddc0b2ba297cf3ce8cd3d775f55df87` + preserves both pending and running owners. + +A stale event is still rejected when its first central job obtains live +pull-request metadata, before the formal-receipt check, coverage, or model +execution. That rejection alone is not timely stale-work retirement: if different +heads share the workflow-level queue group, a new head cannot reach the existing +PR-scoped `opencode-review-target` cancellation boundary until the stale workflow +has already released the group. Same-head queued duplicates still retire on an +exact-head formal receipt. In-flight state remains diagnostic only and never +becomes review success. + +## Cross-head admission follow-up (2026-09-20) + +Exact `f3f6cc28f2a39e11d1a1e17036c639a13c4920a3` grouped the entire +workflow by target repository and pull request only. `queue: max` correctly +preserved same-head pending owners, but it also serialized different heads. +Under a long semantic review, a synchronize event therefore could not reach the +downstream PR-scoped `cancel-in-progress: true` group that retires stale model +work. + +RED `1cf2d425342d73c851370dc433b6cab76037dc1d` requires the +workflow-level group to bind the exact `pr_head_sha`: same-head events render +the same key, different heads render different keys, and the downstream review +job remains PR-scoped with cancellation enabled. GREEN +`81f15bfffb088708eac808c4d0df80f69febdfb4` appends the payload head +SHA, using `github.run_id` only for malformed events without one. This preserves +same-head `queue: max` serialization while allowing a new head to validate and +reach the existing stale-review retirement boundary. + +Exact `a33c0b4f0ddc0b2ba297cf3ce8cd3d775f55df87` verification passed +**14/14** focused policy assertions, YAML parse **2/2**, Python test compile +**2/2**, and extracted caller Bash syntax **1/1**. Full pytest is not claimed +because the isolated verifier does not provide pytest; hosted exact-head Checks +and an independent current-head review remain mandatory. + +GitHub. (2026). *Control the concurrency of workflows and jobs*. Retrieved +September 20, 2026, from +https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency + +## Source-contract wording follow-up (2026-09-20) + +Review of exact `9da1d3f5a043edfd2b2cc860e1983f5a6875fc4d` found that the +runtime and workflow tests described the layered concurrency contract, while +the public helper docstrings still described the retired PR-only cancellation +group and exposed only `present` / `missing`. That stale text hid both the +same-head `queue: max` owner and the emitted `stale` state from callers reading +`argparse` help or source documentation. + +A regression now binds the module docstring to the repository/PR/head queue +owner plus the downstream repository/PR cancellation boundary and binds the +CLI docstring to all three emitted states: `present`, `stale`, and `missing`. +The source documentation is aligned without changing dispatch behavior. + +## Malformed-page fail-closed follow-up (2026-09-20) + +Review of the same lineage found one remaining fail-open parse boundary. A +successful GitHub workflow-runs response is required to carry a +`workflow_runs` array, but a missing or non-list field was previously skipped. +That collapsed malformed evidence into an empty result and let +`evaluate_inflight()` return `missing`, which authorizes another dispatch. + +The parser now raises `InFlightDispatchError` for every malformed page. The +regression covers both a missing field and explicit `null`; valid arrays still +filter non-object members without treating them as runs. diff --git a/scripts/ci/opencode_inflight_dispatch_gate.py b/scripts/ci/opencode_inflight_dispatch_gate.py new file mode 100644 index 0000000000..b7be47149c --- /dev/null +++ b/scripts/ci/opencode_inflight_dispatch_gate.py @@ -0,0 +1,303 @@ +#!/usr/bin/env python3 +"""Detect an in-flight same-head OpenCode Review Dispatch before re-dispatching. + +The required OpenCode entrypoint (`opencode-review.yml`) historically posted a +fresh ``repository_dispatch`` whenever a formal receipt was missing, then +fail-closed until wake. Under org queue saturation that handshake is correct +(no success without verdict), but duplicate same-head dispatches amplified the +queue and could replace pending owners. The central workflow now serializes +same-head owners with ``queue: max`` on a repository/PR/head group, while its +downstream repository/PR review group retains ``cancel-in-progress: true`` so a +new head can retire stale semantic work (pg-erd-cloud#1183 run 35412595263 / +appguardrail#1247). + +This helper mirrors the scheduler's ``already_running`` title match against +central ``ContextualWisdomLab/.github`` ``repository_dispatch`` runs so the +required path skips a second dispatch when one exact-head review is already +queued or running. It never treats in-flight work as a green verdict. +""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import subprocess +import sys +from collections.abc import Mapping, Sequence +from typing import Any + +SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") +REPO_RE = re.compile( + r"^[A-Za-z0-9_][A-Za-z0-9_.-]*/(?:\.github|[A-Za-z0-9_][A-Za-z0-9_.-]*)$" +) +PR_NUMBER_RE = re.compile(r"^[1-9][0-9]*$") +CENTRAL_DISPATCH_REPO = "ContextualWisdomLab/.github" +OPENCODE_DISPATCH_TITLE = "OpenCode Review Dispatch" +OPENCODE_DISPATCH_WORKFLOW = "opencode-review-dispatch.yml" +# GitHub REST's complete nonterminal workflow-run status set. Listing every +# member prevents a duplicate dispatch from cancelling a run that is admitted +# but has not yet reached the queued or in-progress states. +DEFAULT_STATUSES: tuple[str, ...] = ( + "queued", + "in_progress", + "requested", + "waiting", + "pending", +) +# GitHub REST list-runs ``status`` parameter vocabulary (status or conclusion +# values). Every workflow_runs[].status we accept must be in this set; unknown +# or missing status fails closed so evaluate_inflight cannot treat the row as +# quietly non-inflight and return ``missing``. +KNOWN_WORKFLOW_RUN_STATUSES: frozenset[str] = frozenset( + { + *DEFAULT_STATUSES, + "completed", + "action_required", + "cancelled", + "failure", + "neutral", + "skipped", + "stale", + "success", + "timed_out", + } +) + + +class InFlightDispatchError(ValueError): + """Raised when in-flight dispatch inputs are malformed.""" + + +def dispatch_run_title(target_repository: str, pr_number: int, head_sha: str) -> str: + """Return the exact ``run-name`` / display_title prefix for one head.""" + return f"{OPENCODE_DISPATCH_TITLE} {target_repository}#{pr_number}@{head_sha}" + + +def validate_inputs( + target_repository: str, pr_number: str, head_sha: str +) -> tuple[str, int, str]: + """Fail closed on non-canonical repository, PR number, or head SHA.""" + repo = target_repository.strip() + number = pr_number.strip() + sha = head_sha.strip() + components = repo.split("/") + if not REPO_RE.fullmatch(repo) or any( + ".." in component or component.endswith(".") for component in components + ): + raise InFlightDispatchError(f"invalid target repository: {target_repository!r}") + if not PR_NUMBER_RE.fullmatch(number): + raise InFlightDispatchError(f"invalid pull request number: {pr_number!r}") + if not SHA_RE.fullmatch(sha): + raise InFlightDispatchError(f"invalid head SHA: {head_sha!r}") + return repo, int(number), sha.lower() + + +def _gh_api_json(args: Sequence[str], *, token: str) -> Any: + """Run ``gh api`` and parse JSON, surfacing stderr on failure.""" + env = os.environ.copy() + env["GH_TOKEN"] = token + completed = subprocess.run( + ["gh", "api", *args], + check=False, + capture_output=True, + text=True, + env=env, + ) + if completed.returncode != 0: + detail = (completed.stderr or completed.stdout or "").strip() + raise InFlightDispatchError( + f"gh api {' '.join(args)} failed ({completed.returncode}): {detail[:500]}" + ) + try: + return json.loads(completed.stdout or "null") + except json.JSONDecodeError as exc: + raise InFlightDispatchError(f"gh api returned non-JSON: {exc}") from exc + + +def _require_workflow_run_mapping(run: Any) -> Mapping[str, Any]: + """Fail closed when a workflow_runs entry cannot identify an in-flight owner.""" + if not isinstance(run, Mapping): + raise InFlightDispatchError( + "actions/runs workflow_runs entry was not an object" + ) + if run.get("id") is None: + raise InFlightDispatchError( + "actions/runs workflow_runs entry is missing a run id" + ) + title = str(run.get("display_title") or run.get("name") or "").strip() + if not title: + raise InFlightDispatchError( + "actions/runs workflow_runs entry is missing display_title/name" + ) + status = run.get("status") + if not isinstance(status, str) or not status.strip(): + raise InFlightDispatchError( + "actions/runs workflow_runs entry is missing a string status" + ) + if status.casefold() not in {s.casefold() for s in KNOWN_WORKFLOW_RUN_STATUSES}: + raise InFlightDispatchError( + f"actions/runs workflow_runs entry has unknown status: {status!r}" + ) + return run + + +def list_repository_dispatch_runs( + *, + token: str, + status: str | None = None, + per_page: int = 100, +) -> list[Mapping[str, Any]]: + """Return canonical OpenCode repository_dispatch runs. + + When ``status`` is omitted, list once without a status filter so a run that + transitions between GitHub status buckets cannot vanish between per-status + queries (false ``missing`` → duplicate dispatch). + """ + query = ( + f"repos/{CENTRAL_DISPATCH_REPO}/actions/workflows/" + f"{OPENCODE_DISPATCH_WORKFLOW}/runs" + f"?event=repository_dispatch&per_page={per_page}" + ) + if status is not None: + query = f"{query}&status={status}" + payload = _gh_api_json( + [ + "--paginate", + "--slurp", + query, + ], + token=token, + ) + pages = [payload] if isinstance(payload, Mapping) else payload + if not isinstance(pages, list) or not pages or any( + not isinstance(page, Mapping) for page in pages + ): + raise InFlightDispatchError("actions/runs payload was not an object or page list") + result: list[Mapping[str, Any]] = [] + for page in pages: + runs = page.get("workflow_runs") + if not isinstance(runs, list): + raise InFlightDispatchError( + "actions/runs page did not contain a workflow_runs list" + ) + result.extend(_require_workflow_run_mapping(run) for run in runs) + return result + + +def matching_inflight_runs( + runs: Sequence[Mapping[str, Any]], + *, + target_repository: str, + pr_number: int, + head_sha: str, +) -> list[Mapping[str, Any]]: + """Select runs whose display title exactly identifies the target head.""" + expected = dispatch_run_title(target_repository, pr_number, head_sha).casefold() + matched: list[Mapping[str, Any]] = [] + for run in runs: + title = str(run.get("display_title") or run.get("name") or "").strip() + if title.casefold() == expected: + matched.append(run) + return matched + + +def matching_target_runs( + runs: Sequence[Mapping[str, Any]], + *, + target_repository: str, + pr_number: int, +) -> list[Mapping[str, Any]]: + """Select canonical-workflow runs for any head of one pull request.""" + prefix = ( + f"{OPENCODE_DISPATCH_TITLE} {target_repository}#{pr_number}@".casefold() + ) + matched: list[Mapping[str, Any]] = [] + for run in runs: + title = str(run.get("display_title") or "").strip().casefold() + if title.startswith(prefix) and SHA_RE.fullmatch(title[len(prefix) :]): + matched.append(run) + return matched + + +def evaluate_inflight( + *, + target_repository: str, + pr_number: str, + head_sha: str, + token: str, + statuses: Sequence[str] = DEFAULT_STATUSES, +) -> tuple[str, list[str]]: + """Return ``present``/``missing`` and matching central run ids.""" + repo, number, sha = validate_inputs(target_repository, pr_number, head_sha) + allowed = {status.casefold() for status in statuses} + # One unfiltered list, then client-side status filter — avoids a race where a + # run leaves status A after that query and arrives in status B before B's query. + runs = [ + run + for run in list_repository_dispatch_runs(token=token) + if str(run.get("status") or "").casefold() in allowed + ] + exact = matching_inflight_runs( + runs, + target_repository=repo, + pr_number=number, + head_sha=sha, + ) + target = matching_target_runs( + runs, + target_repository=repo, + pr_number=number, + ) + exact_ids = [str(run["id"]) for run in exact] + target_ids = [str(run["id"]) for run in target] + if exact_ids: + return "present", exact_ids + if target_ids: + return "stale", target_ids + return "missing", [] + + +def main(argv: Sequence[str] | None = None) -> int: + """CLI: print ``present``, ``stale``, or ``missing`` dispatch state.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--target-repository", required=True) + parser.add_argument("--pr-number", required=True) + parser.add_argument("--head-sha", required=True) + parser.add_argument( + "--token-env", + default="GH_TOKEN", + help="Environment variable holding the GitHub token (default: GH_TOKEN)", + ) + args = parser.parse_args(list(argv) if argv is not None else None) + token = os.environ.get(args.token_env) or os.environ.get("GITHUB_TOKEN") or "" + if not token.strip(): + print( + f"::error::OpenCode in-flight dispatch gate requires {args.token_env}.", + file=sys.stderr, + ) + return 2 + try: + state, run_ids = evaluate_inflight( + target_repository=args.target_repository, + pr_number=args.pr_number, + head_sha=args.head_sha, + token=token, + ) + except InFlightDispatchError as exc: + print(f"::error::{exc}", file=sys.stderr) + return 2 + if run_ids: + subject = "Same-head" if state == "present" else "Prior-head" + print( + f"{subject} OpenCode Review Dispatch already in flight: " + + ", ".join(run_ids), + file=sys.stderr, + ) + print(state) + return 0 + + +if __name__ == "__main__": # pragma: no cover - exercised through ``main`` tests + raise SystemExit(main()) diff --git a/tests/test_opencode_inflight_dispatch_gate.py b/tests/test_opencode_inflight_dispatch_gate.py new file mode 100644 index 0000000000..77d25e5851 --- /dev/null +++ b/tests/test_opencode_inflight_dispatch_gate.py @@ -0,0 +1,615 @@ +"""Tests for same-head OpenCode Review Dispatch in-flight dedupe.""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import pytest + +from scripts.ci import opencode_inflight_dispatch_gate as gate + +ROOT = Path(__file__).resolve().parents[1] +WORKFLOW = ROOT / ".github" / "workflows" / "opencode-review.yml" +DISPATCH_WORKFLOW = ROOT / ".github" / "workflows" / "opencode-review-dispatch.yml" + +TARGET = "ContextualWisdomLab/pg-erd-cloud" +PR = 1183 +HEAD = "9a759df24e8b714348ac2d240491f74ee3fc852d" + + +def test_public_docs_describe_current_layered_concurrency_contract() -> None: + """Public docs must describe queue ownership and every emitted CLI state.""" + module_contract = gate.__doc__ or "" + main_contract = gate.main.__doc__ or "" + + assert "queue: max" in module_contract + assert "repository/PR/head group" in module_contract + assert "downstream repository/PR review group" in module_contract + assert "cancel-in-progress: true" in module_contract + assert "opencode-review-dispatch-${repo}-${pr}" not in module_contract + assert "present" in main_contract + assert "stale" in main_contract + assert "missing" in main_contract + + +def test_dispatch_run_title_matches_workflow_run_name() -> None: + """Title must match opencode-review-dispatch.yml run-name exactly.""" + assert ( + gate.dispatch_run_title(TARGET, PR, HEAD) + == f"OpenCode Review Dispatch {TARGET}#{PR}@{HEAD}" + ) + + +def test_validate_inputs_rejects_non_canonical_values() -> None: + """Malformed repo / PR / SHA fail closed before any Actions listing.""" + with pytest.raises(gate.InFlightDispatchError): + gate.validate_inputs("../evil", "1183", HEAD) + with pytest.raises(gate.InFlightDispatchError): + gate.validate_inputs(TARGET, "0", HEAD) + with pytest.raises(gate.InFlightDispatchError): + gate.validate_inputs(TARGET, "1183", "9a759df") + for repository in ( + "Contextual..WisdomLab/repository", + "ContextualWisdomLab./repository", + "ContextualWisdomLab/repo..name", + "ContextualWisdomLab/repository.", + ): + with pytest.raises(gate.InFlightDispatchError): + gate.validate_inputs(repository, "1183", HEAD) + + +def test_matching_inflight_runs_requires_exact_head() -> None: + """Older-head and other-PR titles must not suppress a current-head dispatch.""" + runs: list[dict[str, Any]] = [ + { + "id": 1, + "display_title": gate.dispatch_run_title(TARGET, PR, HEAD), + }, + { + "id": 2, + "display_title": gate.dispatch_run_title( + TARGET, PR, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + ), + }, + { + "id": 3, + "display_title": gate.dispatch_run_title( + "ContextualWisdomLab/appguardrail", PR, HEAD + ), + }, + { + "id": 4, + "display_title": gate.dispatch_run_title(TARGET, PR, HEAD) + "-other", + }, + ] + matched = gate.matching_inflight_runs( + runs, target_repository=TARGET, pr_number=PR, head_sha=HEAD + ) + assert [run["id"] for run in matched] == [1] + + +def test_default_statuses_cover_every_nonterminal_actions_state() -> None: + """Every GitHub-defined active workflow status must block duplicate dispatch.""" + assert set(gate.DEFAULT_STATUSES) == { + "queued", + "in_progress", + "requested", + "waiting", + "pending", + } + assert set(gate.DEFAULT_STATUSES) <= gate.KNOWN_WORKFLOW_RUN_STATUSES + assert "completed" in gate.KNOWN_WORKFLOW_RUN_STATUSES + + +def test_evaluate_inflight_reports_present_when_queued( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Queued exact-head central runs are present; caller must skip re-dispatch.""" + + def fake_list(*, token: str, status: str | None = None, per_page: int = 100) -> list[dict[str, Any]]: + assert token == "tok" + assert status is None + return [ + { + "id": 35412595263, + "status": "queued", + "display_title": gate.dispatch_run_title(TARGET, PR, HEAD), + } + ] + + monkeypatch.setattr(gate, "list_repository_dispatch_runs", fake_list) + state, run_ids = gate.evaluate_inflight( + target_repository=TARGET, + pr_number=str(PR), + head_sha=HEAD, + token="tok", + ) + assert state == "present" + assert run_ids == ["35412595263"] + + +def test_evaluate_inflight_uses_one_unfiltered_list_call( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """evaluate_inflight must not issue per-status list queries (race window).""" + + calls: list[str | None] = [] + + def fake_list(*, token: str, status: str | None = None, per_page: int = 100) -> list[dict[str, Any]]: + assert token == "tok" + calls.append(status) + return [ + { + "id": 99, + "status": "queued", + "display_title": gate.dispatch_run_title(TARGET, PR, HEAD), + } + ] + + monkeypatch.setattr(gate, "list_repository_dispatch_runs", fake_list) + state, run_ids = gate.evaluate_inflight( + target_repository=TARGET, + pr_number=str(PR), + head_sha=HEAD, + token="tok", + ) + assert calls == [None] + assert state == "present" + assert run_ids == ["99"] + + +def test_evaluate_inflight_fail_closed_when_status_field_absent( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """id+title without status must not become false missing / re-dispatch.""" + + monkeypatch.setattr( + gate, + "_gh_api_json", + lambda *_a, **_k: { + "workflow_runs": [ + { + "id": 123, + "display_title": gate.dispatch_run_title(TARGET, PR, HEAD), + } + ] + }, + ) + with pytest.raises(gate.InFlightDispatchError, match="string status"): + gate.evaluate_inflight( + target_repository=TARGET, + pr_number=str(PR), + head_sha=HEAD, + token="tok", + ) + + +def test_evaluate_inflight_keeps_each_default_status_and_drops_completed( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """requested/waiting/pending count as present; completed does not.""" + + title = gate.dispatch_run_title(TARGET, PR, HEAD) + + def fake_list(*, token: str, status: str | None = None, per_page: int = 100): + assert status is None + return [ + {"id": 1, "status": "requested", "display_title": title}, + {"id": 2, "status": "waiting", "display_title": title}, + {"id": 3, "status": "pending", "display_title": title}, + {"id": 4, "status": "completed", "display_title": title}, + ] + + monkeypatch.setattr(gate, "list_repository_dispatch_runs", fake_list) + state, run_ids = gate.evaluate_inflight( + target_repository=TARGET, + pr_number=str(PR), + head_sha=HEAD, + token="tok", + ) + assert state == "present" + assert run_ids == ["1", "2", "3"] + + +def test_evaluate_inflight_reports_missing_when_empty( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """No matching runs means the required path may post one dispatch.""" + + monkeypatch.setattr( + gate, + "list_repository_dispatch_runs", + lambda **_kwargs: [], + ) + state, run_ids = gate.evaluate_inflight( + target_repository=TARGET, + pr_number=str(PR), + head_sha=HEAD, + token="tok", + ) + assert state == "missing" + assert run_ids == [] + + +def test_required_workflow_skips_duplicate_dispatch_when_inflight() -> None: + """Required OpenCode entrypoint must consult the in-flight gate before POST.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + assert "scripts/ci/opencode_inflight_dispatch_gate.py" in workflow + assert "Exact-head OpenCode Review Dispatch already queued or running" in workflow + assert "duplicate repository_dispatch skipped" in workflow + request = workflow.split("Request current-head OpenCode review execution\n", 1)[1] + request = request.split("\n - name: Fail closed without a current-head OpenCode verdict\n", 1)[0] + assert "opencode_inflight_dispatch_gate.py" in request + assert request.index("opencode_inflight_dispatch_gate.py") < request.index( + "repos/ContextualWisdomLab/.github/dispatches" + ) + assert '[ "$inflight_state" = "present" ]' in request + assert '[ "$inflight_state" = "missing" ]' in request + # Fail-closed verdict step remains after the dispatch step. + assert "will rerun this failed job" in workflow + assert "No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head" in workflow + + +def test_main_prints_state( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """CLI prints only the machine state on stdout for the workflow branch.""" + + monkeypatch.setenv("GH_TOKEN", "tok") + monkeypatch.setattr( + gate, + "evaluate_inflight", + lambda **_kwargs: ("missing", []), + ) + assert gate.main( + [ + "--target-repository", + TARGET, + "--pr-number", + str(PR), + "--head-sha", + HEAD, + ] + ) == 0 + assert capsys.readouterr().out.strip() == "missing" + + +def test_main_prints_present_run_ids_on_stderr( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """Present state lists matching run ids on stderr for operators.""" + + monkeypatch.setenv("GH_TOKEN", "tok") + monkeypatch.setattr( + gate, + "evaluate_inflight", + lambda **_kwargs: ("present", ["99"]), + ) + assert ( + gate.main( + [ + "--target-repository", + TARGET, + "--pr-number", + str(PR), + "--head-sha", + HEAD, + ] + ) + == 0 + ) + captured = capsys.readouterr() + assert captured.out.strip() == "present" + assert "99" in captured.err + + +def test_main_requires_token(monkeypatch: pytest.MonkeyPatch) -> None: + """Missing token fails closed before listing Actions runs.""" + + monkeypatch.delenv("GH_TOKEN", raising=False) + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + assert ( + gate.main( + [ + "--target-repository", + TARGET, + "--pr-number", + str(PR), + "--head-sha", + HEAD, + ] + ) + == 2 + ) + + +def test_main_maps_inflight_errors( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """InFlightDispatchError becomes exit 2 with an error annotation.""" + + monkeypatch.setenv("GH_TOKEN", "tok") + + def boom(**_kwargs: object) -> tuple[str, list[str]]: + raise gate.InFlightDispatchError("nope") + + monkeypatch.setattr(gate, "evaluate_inflight", boom) + assert ( + gate.main( + [ + "--target-repository", + TARGET, + "--pr-number", + str(PR), + "--head-sha", + HEAD, + ] + ) + == 2 + ) + assert "nope" in capsys.readouterr().err + + +def test_gh_api_json_rejects_nonzero_and_non_json( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Transport and parse failures stay fail-closed.""" + + class _Done: + def __init__(self, code: int, stdout: str = "", stderr: str = "") -> None: + self.returncode = code + self.stdout = stdout + self.stderr = stderr + + monkeypatch.setattr( + gate.subprocess, + "run", + lambda *_a, **_k: _Done(1, stderr="boom"), + ) + with pytest.raises(gate.InFlightDispatchError, match="failed"): + gate._gh_api_json(["repos/x/y"], token="t") + + monkeypatch.setattr( + gate.subprocess, + "run", + lambda *_a, **_k: _Done(0, stdout="not-json"), + ) + with pytest.raises(gate.InFlightDispatchError, match="non-JSON"): + gate._gh_api_json(["repos/x/y"], token="t") + + +def test_list_repository_dispatch_runs_paginates_all_pages( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Queue saturation must not hide an exact-head run after page one.""" + + calls: list[list[str]] = [] + + def fake_api(args: list[str], *, token: str) -> list[dict[str, object]]: + assert token == "t" + calls.append(args) + return [ + {"workflow_runs": [{"id": 1, "status": "queued", "display_title": "OpenCode Review Dispatch owner/repo#1@" + ("a"*40)}]}, + {"workflow_runs": [{"id": 101, "status": "queued", "name": "OpenCode Review Dispatch owner/repo#1@" + ("b"*40)}]}, + ] + + monkeypatch.setattr(gate, "_gh_api_json", fake_api) + runs = gate.list_repository_dispatch_runs(token="t", status="queued") + assert [run["id"] for run in runs] == [1, 101] + assert "--paginate" in calls[0] + assert "--slurp" in calls[0] + + +def test_list_repository_dispatch_runs_rejects_malformed_entries( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Malformed workflow_runs entries fail closed instead of vanishing.""" + + title = gate.dispatch_run_title(TARGET, PR, HEAD) + monkeypatch.setattr( + gate, + "_gh_api_json", + lambda *_a, **_k: {"workflow_runs": ["skip"]}, + ) + with pytest.raises(gate.InFlightDispatchError, match="not an object"): + gate.list_repository_dispatch_runs(token="t") + + monkeypatch.setattr( + gate, + "_gh_api_json", + lambda *_a, **_k: {"workflow_runs": [{"display_title": title}]}, + ) + with pytest.raises(gate.InFlightDispatchError, match="missing a run id"): + gate.list_repository_dispatch_runs(token="t") + + monkeypatch.setattr( + gate, + "_gh_api_json", + lambda *_a, **_k: {"workflow_runs": [{"id": 1}]}, + ) + with pytest.raises(gate.InFlightDispatchError, match="display_title/name"): + gate.list_repository_dispatch_runs(token="t") + + title = gate.dispatch_run_title(TARGET, PR, HEAD) + monkeypatch.setattr( + gate, + "_gh_api_json", + lambda *_a, **_k: {"workflow_runs": [{"id": 1, "display_title": title}]}, + ) + with pytest.raises(gate.InFlightDispatchError, match="string status"): + gate.list_repository_dispatch_runs(token="t") + + monkeypatch.setattr( + gate, + "_gh_api_json", + lambda *_a, **_k: { + "workflow_runs": [{"id": 1, "display_title": title, "status": 1}] + }, + ) + with pytest.raises(gate.InFlightDispatchError, match="string status"): + gate.list_repository_dispatch_runs(token="t") + + monkeypatch.setattr( + gate, + "_gh_api_json", + lambda *_a, **_k: { + "workflow_runs": [ + {"id": 1, "display_title": title, "status": "not-a-github-status"} + ] + }, + ) + with pytest.raises(gate.InFlightDispatchError, match="unknown status"): + gate.list_repository_dispatch_runs(token="t") + + monkeypatch.setattr(gate, "_gh_api_json", lambda *_a, **_k: []) + with pytest.raises(gate.InFlightDispatchError, match="not an object"): + gate.list_repository_dispatch_runs(token="t") + + for malformed_page in ({"workflow_runs": None}, {}): + monkeypatch.setattr( + gate, + "_gh_api_json", + lambda *_a, _page=malformed_page, **_k: _page, + ) + with pytest.raises( + gate.InFlightDispatchError, + match="workflow_runs list", + ): + gate.list_repository_dispatch_runs(token="t") + + +def test_list_repository_dispatch_runs_rejects_missing_id_before_evaluate( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Id-less entries must not reach evaluate_inflight as a false missing.""" + + monkeypatch.setattr( + gate, + "_gh_api_json", + lambda *_a, **_k: { + "workflow_runs": [ + {"display_title": gate.dispatch_run_title(TARGET, PR, HEAD)}, + ] + }, + ) + with pytest.raises(gate.InFlightDispatchError, match="missing a run id"): + gate.list_repository_dispatch_runs(token="t") + + +def test_run_listing_is_bound_to_canonical_dispatch_workflow( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Unrelated repository_dispatch workflows must not suppress OpenCode.""" + calls: list[list[str]] = [] + + def fake_api(args: list[str], *, token: str) -> dict[str, list[object]]: + assert token == "t" + calls.append(args) + return {"workflow_runs": []} + + monkeypatch.setattr(gate, "_gh_api_json", fake_api) + assert gate.list_repository_dispatch_runs(token="t") == [] + endpoint = next(arg for arg in calls[0] if arg.startswith("repos/")) + assert ( + endpoint + == "repos/ContextualWisdomLab/.github/actions/workflows/" + "opencode-review-dispatch.yml/runs" + "?event=repository_dispatch&per_page=100" + ) + + +def test_evaluate_inflight_distinguishes_stale_head_for_supersession( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An older canonical head authorizes cancellation; same-head absence does not.""" + old_head = "b" * 40 + monkeypatch.setattr( + gate, + "list_repository_dispatch_runs", + lambda **kwargs: [ + { + "id": 77, + "status": "queued", + "display_title": gate.dispatch_run_title(TARGET, PR, old_head), + } + ], + ) + state, run_ids = gate.evaluate_inflight( + target_repository=TARGET, + pr_number=str(PR), + head_sha=HEAD, + token="tok", + ) + assert state == "stale" + assert run_ids == ["77"] + + +def test_dispatch_concurrency_preserves_same_head_and_admits_new_head() -> None: + """Same-head racers queue together while a newer head reaches retirement.""" + required = WORKFLOW.read_text(encoding="utf-8") + dispatched = DISPATCH_WORKFLOW.read_text(encoding="utf-8") + concurrency = dispatched.split("\nconcurrency:\n", 1)[1].split( + "\npermissions:", 1 + )[0] + assert "queue: max" in concurrency + assert "cancel-in-progress:" not in concurrency + assert ( + "github.event.client_payload.pr_head_sha || github.run_id" + in concurrency + ) + + def workflow_group(head_sha: str) -> str: + """Render the repository, pull request, and exact-head group contract.""" + + return f"opencode-review-dispatch-owner/repo-7-{head_sha}" + + assert workflow_group("a" * 40) == workflow_group("a" * 40) + assert workflow_group("a" * 40) != workflow_group("b" * 40) + + review_job = dispatched.split("\n opencode-review-target:\n", 1)[1] + review_concurrency = review_job.split("\n concurrency:\n", 1)[1].split( + "\n permissions:", 1 + )[0] + assert "needs.validate-pr-metadata.outputs.target_repository" in review_concurrency + assert "needs.validate-pr-metadata.outputs.pr_number || github.run_id" in review_concurrency + assert "needs.validate-pr-metadata.outputs.head_sha" not in review_concurrency + assert "cancel-in-progress: true" in review_concurrency + + request = required.split( + " - name: Request current-head OpenCode review execution\n", 1 + )[1].split( + "\n - name: Fail closed without a current-head OpenCode verdict\n", 1 + )[0] + assert '[ "$inflight_state" = "stale" ]' in request + assert '[ "$inflight_state" = "missing" ]' in request + assert "--argjson cancel_in_progress" not in request + assert "cancel_in_progress:" not in request + + validation = dispatched.index( + "repository_dispatch metadata does not match the live pull request" + ) + receipt = dispatched.index( + "Retire serialized same-head duplicate with formal receipt" + ) + coverage = dispatched.index("\n coverage-evidence:") + assert validation < receipt < coverage + + +def test_serialized_duplicate_retires_on_formal_receipt_before_review() -> None: + """A queued same-head duplicate must not become a second review owner.""" + dispatched = DISPATCH_WORKFLOW.read_text(encoding="utf-8") + validate_job = dispatched.split(" validate-pr-metadata:\n", 1)[1].split( + "\n coverage-evidence:\n", 1 + )[0] + coverage_job = dispatched.split(" coverage-evidence:\n", 1)[1].split( + "\n opencode-review-target:\n", 1 + )[0] + review_job = dispatched.split(" opencode-review-target:\n", 1)[1] + assert "needs_review:" in validate_job + assert "Retire serialized same-head duplicate with formal receipt" in validate_job + assert "opencode_review_receipt_gate.py" in validate_job + assert "needs_review=true" in validate_job + assert "needs_review=false" in validate_job + assert "needs.validate-pr-metadata.outputs.needs_review == 'true'" in coverage_job + assert "needs.validate-pr-metadata.outputs.needs_review == 'true'" in review_job diff --git a/tests/test_opencode_required_verdict_regression.py b/tests/test_opencode_required_verdict_regression.py index c764ad0ad2..5b08c790e0 100644 --- a/tests/test_opencode_required_verdict_regression.py +++ b/tests/test_opencode_required_verdict_regression.py @@ -22,6 +22,7 @@ DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") STATUS_HELPER = Path("scripts/ci/opencode_dispatch_status.py") RECEIPT_HELPER = Path("scripts/ci/opencode_review_receipt_gate.py") +INFLIGHT_HELPER = Path("scripts/ci/opencode_inflight_dispatch_gate.py") def request_review_script() -> str: @@ -82,15 +83,20 @@ def test_stale_opencode_event_never_reaches_review_concurrency(tmp_path: Path) - assert "retired a stale event" in result.stdout -def test_opencode_dispatch_uses_the_same_target_repo_pr_group() -> None: - """PR and repository_dispatch review jobs compute the same group text.""" +def test_opencode_dispatch_groups_exact_heads_before_pr_scoped_review() -> None: + """Exact heads queue independently before the review job retires stale work.""" required = WORKFLOW.read_text(encoding="utf-8") dispatched = DISPATCH_WORKFLOW.read_text(encoding="utf-8") assert "opencode-review-${{" in required assert "opencode-review-${{" in dispatched assert "needs.validate-pr-metadata.outputs.target_repository" in dispatched assert "needs.validate-pr-metadata.outputs.pr_number || github.run_id" in dispatched - assert workflow_level_cancels_in_progress(dispatched) + concurrency = dispatched.split("\nconcurrency:\n", 1)[1].split( + "\npermissions:", 1 + )[0] + assert "github.event.client_payload.pr_head_sha || github.run_id" in concurrency + assert "queue: max" in concurrency + assert "cancel-in-progress:" not in concurrency assert dispatched.index("validate-pr-metadata:") < dispatched.index(" concurrency:") @@ -315,12 +321,17 @@ def test_required_workflow_cannot_succeed_with_an_echo_only_placeholder() -> Non " - name: Request current-head OpenCode review execution", 1 )[1].split(" - name: Fail closed", 1)[0] assert "scripts/ci/opencode_review_receipt_gate.py" in dispatch_step + assert "scripts/ci/opencode_inflight_dispatch_gate.py" in dispatch_step assert "github.workflow_sha" in dispatch_step assert "evaluate_receipts" in dispatch_step assert dispatch_step.index("evaluate_receipts") < dispatch_step.index( "exchange_github_app_token" ) + assert dispatch_step.index("opencode_inflight_dispatch_gate.py") < dispatch_step.index( + "repos/ContextualWisdomLab/.github/dispatches" + ) assert "Current-head substantive OpenCode verdict already exists; scheduler wake skipped." in dispatch_step + assert "Exact-head OpenCode Review Dispatch already queued or running" in dispatch_step assert "while :; do" not in target_job assert "poll_interval_seconds" not in target_job assert "180 minutes of polling" not in target_job @@ -693,6 +704,10 @@ def test_scheduler_wake_reuses_trusted_receipt_predicate( 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 [[ "$*" == *"contents/scripts/ci/opencode_inflight_dispatch_gate.py"* ]]; then + python3 -c 'import base64, pathlib, sys; sys.stdout.write(base64.b64encode(pathlib.Path(sys.argv[1]).read_bytes()).decode())' "$REAL_INFLIGHT_HELPER" +elif [[ "$*" == *"actions/workflows/opencode-review-dispatch.yml/runs"* ]]; then + printf '{"workflow_runs":[]}' elif [[ "$*" == *"/pulls/7/reviews"* ]]; then printf '[%s]' "$FAKE_REVIEWS" elif [[ "$*" == *"repos/ContextualWisdomLab/.github/dispatches"* ]]; then @@ -715,6 +730,7 @@ def test_scheduler_wake_reuses_trusted_receipt_predicate( **os.environ, "PATH": f"{fake_bin}{os.pathsep}{os.environ['PATH']}", "REAL_RECEIPT_HELPER": str(RECEIPT_HELPER.resolve()), + "REAL_INFLIGHT_HELPER": str(INFLIGHT_HELPER.resolve()), "FAKE_REVIEWS": json.dumps(reviews), "DISPATCH_CALLS": str(calls), "ACTIONS_ID_TOKEN_REQUEST_TOKEN": "request", @@ -743,6 +759,74 @@ def test_scheduler_wake_reuses_trusted_receipt_predicate( assert actual == dispatches +def test_scheduler_wake_skips_dispatch_when_same_head_already_inflight( + tmp_path: Path, +) -> None: + """Exact-head queued central dispatch must not be cancelled by a duplicate POST.""" + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + calls = tmp_path / "dispatches" + fake_gh = fake_bin / "gh" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +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 [[ "$*" == *"contents/scripts/ci/opencode_inflight_dispatch_gate.py"* ]]; then + python3 -c 'import base64, pathlib, sys; sys.stdout.write(base64.b64encode(pathlib.Path(sys.argv[1]).read_bytes()).decode())' "$REAL_INFLIGHT_HELPER" +elif [[ "$*" == *"actions/workflows/opencode-review-dispatch.yml/runs"* ]]; then + printf '{"workflow_runs":[{"id":99,"display_title":"OpenCode Review Dispatch owner/repo#7@%s","status":"queued"}]}' "$HEAD_SHA" +elif [[ "$*" == *"/pulls/7/reviews"* ]]; then + printf '[]' +elif [[ "$*" == *"repos/ContextualWisdomLab/.github/dispatches"* ]]; then + printf 'dispatch\n' >>"$DISPATCH_CALLS" +fi +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + fake_curl = fake_bin / "curl" + fake_curl.write_text( + """#!/usr/bin/env bash +[[ "$*" == *"exchange_github_app_token"* ]] && printf '{"token":"app"}' || printf '{"value":"oidc"}' +""", + encoding="utf-8", + ) + fake_curl.chmod(0o755) + env = { + **os.environ, + "PATH": f"{fake_bin}{os.pathsep}{os.environ['PATH']}", + "REAL_RECEIPT_HELPER": str(RECEIPT_HELPER.resolve()), + "REAL_INFLIGHT_HELPER": str(INFLIGHT_HELPER.resolve()), + "DISPATCH_CALLS": str(calls), + "ACTIONS_ID_TOKEN_REQUEST_TOKEN": "request", + "ACTIONS_ID_TOKEN_REQUEST_URL": "https://token.example", + "OIDC_AUDIENCE": "opencode-github-action", + "OPENCODE_API_BASE_URL": "https://api.opencode.ai", + "TARGET_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "HEAD_SHA": HEAD, + "PR_DRAFT": "false", + "BASE_BRANCH": "main", + "BASE_SHA": "b" * 40, + "HEAD_REF": "feature-branch", + "WORKFLOW_SHA": "c" * 40, + "GH_TOKEN": "token", + "GITHUB_RUN_ID": "123456789", + "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 + ) + assert result.returncode == 0, result.stderr + result.stdout + assert "duplicate repository_dispatch skipped" in result.stdout + assert not calls.exists() or calls.read_text(encoding="utf-8").count("dispatch") == 0 + + def test_formal_receipt_wake_reruns_the_immediately_failed_required_job() -> None: """The dispatch receipt wakes the exact failed run without runner polling.""" required = WORKFLOW.read_text(encoding="utf-8") diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 8b7c55a4ef..fd0a610773 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -17,7 +17,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "cbc8d214394c4b7acbe82ce7fba11fd073b91c98" +REVIEW_DISPATCH_BLOB_SHA = "239a7bd4d3aba91507db5c3b67d27215cf458d80" def _workflow_text(path: Path) -> str: diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 87277d45f5..95f4b14279 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -331,22 +331,13 @@ def test_privileged_review_retries_use_default_branch_repository_dispatch() -> N assert '"gh",\n "workflow",\n "run"' not in autofix_scheduler -def test_privileged_review_dispatch_coalesces_superseded_runs_before_admission() -> None: - """A superseded dispatch must be cancelled while queued, not after it takes a runner. - - ``opencode-review-dispatch.yml`` carried its concurrency group only on the - long ``opencode-review-target`` job. A job-level group is not evaluated - while the whole run waits behind the organization job ceiling, so two - dispatches for one pull request each waited hours and each was allocated a - runner before the older one could be discarded. Measured on 2026-09-06: - four of the five dispatch runs that passed ``validate-pr-metadata`` were - then rejected by the privileged metadata check because the head had moved - while they queued, every one of them after ``coverage-source-tree`` and - ``coverage-evidence`` had already run. - - The workflow-level group is keyed by the dispatched pull request, matching - ``codeql-scan-dispatch.yml``'s workflow-level group and the job-level group - this workflow keeps for the review job itself. +def test_privileged_review_dispatch_separates_heads_before_admission() -> None: + """Same-head events queue together while a new head reaches stale-work retirement. + + Workflow-level admission is exact-head-scoped and uses `queue: max`, so + same-head racers cannot replace a pending owner. Different heads use distinct + workflow groups and can reach the downstream PR-scoped review concurrency, + whose `cancel-in-progress: true` retires stale semantic work. """ workflow = workflow_text("opencode-review-dispatch.yml") header = workflow.split("permissions:", 1)[0] @@ -360,9 +351,15 @@ def test_privileged_review_dispatch_coalesces_superseded_runs_before_admission() in group_value ) assert "github.event.client_payload.pr_number || github.run_id" in group_value - assert workflow_level_cancels_in_progress(workflow) - assert "github.event.client_payload.pr_head_sha" not in concurrency_contract - assert re.search(r"(?m)^ concurrency:", workflow) + assert "github.event.client_payload.pr_head_sha || github.run_id" in group_value + assert "queue: max" in concurrency_contract + assert not workflow_level_cancels_in_progress(workflow) + review_job = workflow.split("\n opencode-review-target:\n", 1)[1] + review_concurrency = review_job.split("\n concurrency:\n", 1)[1].split( + "\n runs-on:", 1 + )[0] + assert "needs.validate-pr-metadata.outputs.head_sha" not in review_concurrency + assert "cancel-in-progress: true" in review_concurrency @pytest.mark.parametrize(