diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 09bbf8181a..ac80537dac 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -11,34 +11,432 @@ on: repository_dispatch: 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. - 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 - permissions: contents: read jobs: + admit-exact-head-dispatch: + name: admit-exact-head-dispatch + if: github.event_name == 'repository_dispatch' + runs-on: + group: CWL central OpenCode + labels: [self-hosted, linux, x64] + timeout-minutes: 10 + permissions: + actions: read + contents: write + pull-requests: read + id-token: write + outputs: + admitted: ${{ steps.single_flight.outputs.admitted }} + steps: + - name: Authorize repository dispatch envelope + env: + EVENT_NAME: ${{ github.event_name }} + DISPATCH_ACTOR: ${{ github.triggering_actor }} + DISPATCH_SENDER: ${{ github.event.sender.login || '' }} + ALLOWED_DISPATCH_ACTOR: ${{ vars.OPENCODE_REPOSITORY_DISPATCH_ACTOR }} + ALLOWED_DISPATCH_TARGETS: ${{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }} + TARGET_REPOSITORY: ${{ github.event.client_payload.target_repository }} + PR_NUMBER: ${{ github.event.client_payload.pr_number }} + SUPPLIED_BASE_REF: ${{ github.event.client_payload.pr_base_ref || '' }} + SUPPLIED_BASE_SHA: ${{ github.event.client_payload.pr_base_sha || '' }} + SUPPLIED_HEAD_REF: ${{ github.event.client_payload.pr_head_ref || '' }} + SUPPLIED_HEAD_SHA: ${{ github.event.client_payload.pr_head_sha || '' }} + DRAFT_REVIEW_ONLY: ${{ toJSON(github.event.client_payload.draft_review_only) }} + run: | + set -euo pipefail + [ "$EVENT_NAME" = "repository_dispatch" ] || { + echo "::error::Central OpenCode admission accepts repository_dispatch only." + exit 1 + } + + actor_allowed=0 + IFS=',' read -r -a allowed_dispatch_actors <<<"$ALLOWED_DISPATCH_ACTOR" + for allowed_actor in "${allowed_dispatch_actors[@]}"; do + allowed_actor="${allowed_actor//[[:space:]]/}" + if [ -n "$allowed_actor" ] && + [ "$DISPATCH_ACTOR" = "$allowed_actor" ] && + [ "$DISPATCH_SENDER" = "$allowed_actor" ]; then + actor_allowed=1 + break + fi + done + if [ "$actor_allowed" -ne 1 ]; then + printf '::error::repository_dispatch authorization rejected actor=%s sender=%s because both must match one configured scheduler identity.\n' "${DISPATCH_ACTOR:-}" "${DISPATCH_SENDER:-}" + exit 1 + fi + + target_allowed=0 + IFS=',' read -r -a allowed_dispatch_targets <<<"$ALLOWED_DISPATCH_TARGETS" + for allowed_target in "${allowed_dispatch_targets[@]}"; do + allowed_target="${allowed_target//[[:space:]]/}" + if [ -n "$allowed_target" ] && [ "$TARGET_REPOSITORY" = "$allowed_target" ]; then + target_allowed=1 + break + fi + done + if [ "$target_allowed" -ne 1 ]; then + printf '::error::repository_dispatch authorization rejected target=%s because it is absent from the configured exact repository allowlist.\n' "${TARGET_REPOSITORY:-}" + exit 1 + fi + + if ! [[ "$TARGET_REPOSITORY" =~ ^ContextualWisdomLab/[A-Za-z0-9_.-]+$ ]] || + ! [[ "$PR_NUMBER" =~ ^[1-9][0-9]*$ ]] || + [ -z "$SUPPLIED_BASE_REF" ] || + [ -z "$SUPPLIED_HEAD_REF" ] || + ! [[ "$SUPPLIED_BASE_SHA" =~ ^[0-9a-fA-F]{40}$ ]] || + ! [[ "$SUPPLIED_HEAD_SHA" =~ ^[0-9a-fA-F]{40}$ ]]; then + printf '::error::repository_dispatch admission rejected malformed PR identity metadata. target=%s pr=%s\n' "${TARGET_REPOSITORY:-}" "${PR_NUMBER:-}" + exit 1 + fi + if ! [[ "$DRAFT_REVIEW_ONLY" =~ ^(true|false)$ ]]; then + echo "::error::repository_dispatch admission rejected malformed draft-review authority." + exit 1 + fi + printf 'Authorized exact repository_dispatch envelope for %s#%s.\n' "$TARGET_REPOSITORY" "$PR_NUMBER" + + - name: Exchange OpenCode app token for target repository metadata reads + id: metadata_read_app_token + if: >- + github.event_name == 'repository_dispatch' + && github.event.client_payload.target_repository != '' + && github.event.client_payload.target_repository != github.repository + env: + OIDC_AUDIENCE: opencode-github-action + OPENCODE_API_BASE_URL: https://api.opencode.ai + run: | + set -euo pipefail + + mark_unavailable() { + echo "available=false" >>"$GITHUB_OUTPUT" + } + + if [ -z "${ACTIONS_ID_TOKEN_REQUEST_TOKEN:-}" ] || + [ -z "${ACTIONS_ID_TOKEN_REQUEST_URL:-}" ]; then + echo "OpenCode app token exchange unavailable: OIDC request environment is missing." + mark_unavailable + exit 0 + fi + + request_url="${ACTIONS_ID_TOKEN_REQUEST_URL}" + separator="&" + case "$request_url" in + *\?*) ;; + *) separator="?" ;; + esac + + if ! oidc_response="$( + curl -fsS \ + -H "Authorization: Bearer ${ACTIONS_ID_TOKEN_REQUEST_TOKEN}" \ + "${request_url}${separator}audience=${OIDC_AUDIENCE}" + )"; then + echo "OpenCode app token exchange unavailable: OIDC token request did not complete." + mark_unavailable + exit 0 + fi + + oidc_token="$(jq -r '.value // empty' <<<"$oidc_response")" + if [ -z "$oidc_token" ]; then + echo "OpenCode app token exchange unavailable: OIDC token response was empty." + mark_unavailable + exit 0 + fi + + if ! token_response="$( + curl -fsS \ + -X POST \ + -H "Authorization: Bearer ${oidc_token}" \ + "${OPENCODE_API_BASE_URL}/exchange_github_app_token" + )"; then + echo "OpenCode app token exchange unavailable: app token request did not complete." + mark_unavailable + exit 0 + fi + + app_token="$(jq -r '.token // empty' <<<"$token_response")" + if [ -z "$app_token" ]; then + echo "OpenCode app token exchange unavailable: app token response was empty." + mark_unavailable + exit 0 + fi + + echo "::add-mask::$app_token" + { + echo "available=true" + echo "token=$app_token" + } >>"$GITHUB_OUTPUT" + + + + - name: Admit one exact-head central dispatch + id: single_flight + env: + GH_TOKEN: ${{ github.token }} + TARGET_READ_TOKEN: ${{ steps.metadata_read_app_token.outputs.token || secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }} + TARGET_REPOSITORY: ${{ github.event.client_payload.target_repository }} + PR_NUMBER: ${{ github.event.client_payload.pr_number }} + HEAD_SHA: ${{ github.event.client_payload.pr_head_sha }} + SUPPLIED_BASE_REF: ${{ github.event.client_payload.pr_base_ref }} + SUPPLIED_BASE_SHA: ${{ github.event.client_payload.pr_base_sha }} + SUPPLIED_HEAD_REF: ${{ github.event.client_payload.pr_head_ref }} + SUPPLIED_HEAD_SHA: ${{ github.event.client_payload.pr_head_sha }} + DRAFT_REVIEW_ONLY: ${{ toJSON(github.event.client_payload.draft_review_only) }} + run: | + set -euo pipefail + exact_title="OpenCode Review Dispatch ${TARGET_REPOSITORY}#${PR_NUMBER}@${HEAD_SHA}" + lease_branch="opencode-dispatch-leases" + lease_key="$(printf '%s#%s' "$TARGET_REPOSITORY" "$PR_NUMBER" | sha256sum | cut -d' ' -f1)" + lease_path="opencode-dispatch-leases/${lease_key}.json" + lease_ref_api="repos/ContextualWisdomLab/.github/git/ref/heads/${lease_branch}" + receipt_helper="$(mktemp)" + trap 'rm -f "$receipt_helper"' EXIT + + authorized_draft_marker_matches() { + local marker_name marker_inventory + [ "${DRAFT_REVIEW_ONLY:-false}" = "true" ] || return 1 + marker_name="cwl-draft-review-request-${TARGET_REPOSITORY//\//-}-${PR_NUMBER}-${HEAD_SHA}" + marker_inventory="$(gh api \ + "repos/${GITHUB_REPOSITORY}/actions/artifacts?name=${marker_name}&per_page=100")" || return 1 + jq -e --arg marker_name "$marker_name" ' + (.total_count | type) == "number" + and (.total_count | floor) == .total_count + and .total_count >= 0 + and (.artifacts | type) == "array" + and .total_count == (.artifacts | length) + and all(.artifacts[]; + type == "object" + and (.id | type) == "number" + and (.id | floor) == .id + and .id >= 1 + and (.name | type) == "string" + and .name == $marker_name + and (.expired | type) == "boolean" + ) + and any(.artifacts[]; .name == $marker_name and .expired == false) + ' <<<"$marker_inventory" >/dev/null + } + + live_authority_matches() { + local live_pr live_base_repo live_base_ref live_base_sha + local live_head_repo live_head_ref live_head_sha live_draft live_state + live_pr="$(GH_TOKEN="$TARGET_READ_TOKEN" gh api \ + "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}")" || return 1 + live_base_repo="$(jq -r '.base.repo.full_name // empty' <<<"$live_pr")" + live_base_ref="$(jq -r '.base.ref // empty' <<<"$live_pr")" + live_base_sha="$(jq -r '.base.sha // empty' <<<"$live_pr")" + live_head_repo="$(jq -r '.head.repo.full_name // empty' <<<"$live_pr")" + live_head_ref="$(jq -r '.head.ref // empty' <<<"$live_pr")" + live_head_sha="$(jq -r '.head.sha // empty' <<<"$live_pr")" + live_draft="$(jq -r \ + 'if (.draft | type) == "boolean" then (.draft | tostring) else empty end' \ + <<<"$live_pr")" + live_state="$(jq -r \ + 'if (.state | type) == "string" then .state else empty end' \ + <<<"$live_pr")" + [ "$live_state" = "open" ] && + [ "$live_base_repo" = "$TARGET_REPOSITORY" ] && + [[ "$live_head_repo" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]] && + [ "$live_base_ref" = "$SUPPLIED_BASE_REF" ] && + [ "$live_base_sha" = "$SUPPLIED_BASE_SHA" ] && + [ "$live_head_ref" = "$SUPPLIED_HEAD_REF" ] && + [ "${live_head_sha,,}" = "${SUPPLIED_HEAD_SHA,,}" ] && + [ "${live_head_sha,,}" = "${HEAD_SHA,,}" ] && + { + { [ "$live_draft" = "false" ] && [ "${DRAFT_REVIEW_ONLY:-false}" = "false" ]; } || + { [ "$live_draft" = "true" ] && authorized_draft_marker_matches; } + } + } + if ! live_authority_matches; then + echo "::error::Pull request authority changed before atomic OpenCode admission." + exit 1 + fi + + definite_receipt_state() { + if [ ! -s "$receipt_helper" ] && ! gh api \ + "repos/ContextualWisdomLab/.github/contents/scripts/ci/opencode_review_receipt_gate.py?ref=${GITHUB_SHA}" \ + --jq .content | base64 --decode >"$receipt_helper"; then + echo "::error::Could not load the trusted OpenCode receipt helper." >&2 + return 1 + fi + GH_TOKEN="$TARGET_READ_TOKEN" python3 -c 'import runpy,sys; gate=runpy.run_path(sys.argv[1]); reviews=gate["fetch_reviews"](sys.argv[2],int(sys.argv[3])); receipt,_reason=gate["evaluate_receipts"](reviews,sys.argv[4],is_draft=sys.argv[5] == "true"); print("present" if receipt is not None else "missing")' \ + "$receipt_helper" "$TARGET_REPOSITORY" "$PR_NUMBER" "$HEAD_SHA" "${DRAFT_REVIEW_ONLY:-false}" + } + + complete_receipt_admission() { + if ! live_authority_matches; then + echo "::error::Pull request authority changed before receiver receipt admission." + exit 1 + fi + if ! receiver_receipt_state="$(definite_receipt_state)"; then + echo "::error::Could not revalidate the formal exact-head receipt after lease acquisition." + exit 1 + fi + case "$receiver_receipt_state" in + present) + echo "admitted=false" >>"$GITHUB_OUTPUT" + echo "A formal exact-head OpenCode receipt already exists; duplicate receiver work skipped." + ;; + missing) + echo "admitted=true" >>"$GITHUB_OUTPUT" + ;; + *) + echo "::error::Trusted OpenCode receipt helper returned an invalid state." + exit 1 + ;; + esac + } + + validate_lease_ref() { + jq -e ' + .object.type == "commit" + and (.object.sha | type) == "string" + and (.object.sha | test("^[0-9a-f]{40}$")) + ' >/dev/null + } + if ! lease_ref="$(gh api "$lease_ref_api" 2>/dev/null)"; then + if ! jq -cn --arg ref "refs/heads/${lease_branch}" --arg sha "$GITHUB_SHA" \ + '{ref:$ref,sha:$sha}' | + gh api --method POST \ + repos/ContextualWisdomLab/.github/git/refs --input - >/dev/null; then + lease_ref="$(gh api "$lease_ref_api")" || { + echo "::error::Could not initialize or read the central OpenCode lease branch." + exit 1 + } + printf '%s' "$lease_ref" | validate_lease_ref || { + echo "::error::Central OpenCode lease branch identity was malformed." + exit 1 + } + fi + else + printf '%s' "$lease_ref" | validate_lease_ref || { + echo "::error::Central OpenCode lease branch identity was malformed." + exit 1 + } + fi + + lease_content="$(jq -cn \ + --argjson owner_run_id "$GITHUB_RUN_ID" \ + --arg exact_title "$exact_title" \ + --arg head_sha "$HEAD_SHA" \ + '{owner_run_id:$owner_run_id,exact_title:$exact_title,head_sha:$head_sha}')" + encoded_lease="$(printf '%s' "$lease_content" | base64 | tr -d '\n')" + for lease_attempt in 1 2 3 4 5; do + current_blob_sha="" + if current_file="$(gh api \ + "repos/ContextualWisdomLab/.github/contents/${lease_path}?ref=${lease_branch}" \ + 2>/dev/null)"; then + if ! current_blob_sha="$(jq -er ' + select(.encoding == "base64") + | .sha + | select(type == "string" and test("^[0-9a-f]{40}$")) + ' <<<"$current_file")"; then + echo "::error::Central OpenCode lease metadata was malformed." + exit 1 + fi + if ! current_content="$(jq -er '.content' <<<"$current_file" | base64 --decode)"; then + echo "::error::Central OpenCode lease content was not valid base64." + exit 1 + fi + if ! owner_run_id="$(jq -er ' + .owner_run_id | select(type == "number" and . > 0) + ' <<<"$current_content")" || + ! owner_title="$(jq -er \ + --arg prefix "OpenCode Review Dispatch ${TARGET_REPOSITORY}#${PR_NUMBER}@" ' + .exact_title + | select( + type == "string" + and ((ascii_downcase) | startswith($prefix | ascii_downcase)) + ) + ' <<<"$current_content")" || + ! owner_head="$(jq -er \ + '.head_sha | select(type == "string" and test("^[0-9a-fA-F]{40}$"))' \ + <<<"$current_content")"; then + echo "::error::Central OpenCode lease identity was malformed." + exit 1 + fi + expected_owner_title="OpenCode Review Dispatch ${TARGET_REPOSITORY}#${PR_NUMBER}@${owner_head}" + if [ "${owner_title,,}" != "${expected_owner_title,,}" ]; then + echo "::error::Central OpenCode lease title and head identity disagreed." + exit 1 + fi + if [ "$owner_run_id" = "$GITHUB_RUN_ID" ] && + [ "${owner_head,,}" = "${HEAD_SHA,,}" ]; then + complete_receipt_admission + [ "$receiver_receipt_state" = "missing" ] || exit 0 + echo "Central exact-head rerun ${GITHUB_RUN_ID} retained its canonical lease." + exit 0 + fi + if ! owner_run="$(gh api \ + "repos/ContextualWisdomLab/.github/actions/runs/${owner_run_id}")"; then + echo "::error::Could not validate the central OpenCode lease owner." + exit 1 + fi + if ! owner_status="$(jq -er \ + --argjson owner "$owner_run_id" --arg title "$owner_title" ' + select( + .id == $owner + and .path == ".github/workflows/opencode-review-dispatch.yml" + and .event == "repository_dispatch" + and (.display_title | ascii_downcase) == ($title | ascii_downcase) + and (.status | type) == "string" + ) + | .status + ' <<<"$owner_run")"; then + echo "::error::Central OpenCode lease owner identity was malformed." + exit 1 + fi + case "$owner_status" in + requested|waiting|pending|queued|in_progress|completed) ;; + *) + echo "::error::Central OpenCode lease owner returned invalid status ${owner_status:-}." + exit 1 + ;; + esac + if [ "${owner_head,,}" = "${HEAD_SHA,,}" ] && + [ "$owner_status" != "completed" ]; then + echo "admitted=false" >>"$GITHUB_OUTPUT" + echo "Central exact-head dispatch ${GITHUB_RUN_ID} deferred to active lease owner ${owner_run_id}." + exit 0 + fi + fi + + lease_request="$(jq -cn \ + --arg message "Acquire OpenCode exact-head lease for run ${GITHUB_RUN_ID}" \ + --arg content "$encoded_lease" \ + --arg branch "$lease_branch" \ + --arg sha "$current_blob_sha" ' + {message:$message,content:$content,branch:$branch} + + if $sha == "" then {} else {sha:$sha} end + ')" + if ! live_authority_matches; then + echo "::error::Pull request authority changed before the atomic OpenCode lease mutation." + exit 1 + fi + if lease_result="$(printf '%s' "$lease_request" | gh api --method PUT \ + "repos/ContextualWisdomLab/.github/contents/${lease_path}" --input - \ + 2>/dev/null)"; then + if ! lease_blob_sha="$(jq -er ' + .content.sha + | select(type == "string" and test("^[0-9a-f]{40}$")) + ' <<<"$lease_result")"; then + echo "::error::Central OpenCode lease acquisition returned malformed metadata." + exit 1 + fi + echo "Central exact-head dispatch ${GITHUB_RUN_ID} acquired the atomic lease." + complete_receipt_admission + [ "$receiver_receipt_state" = "missing" ] || exit 0 + exit 0 + fi + echo "Central exact-head lease changed during attempt ${lease_attempt}; retrying from the authoritative branch." + done + echo "::error::Could not acquire the atomic central OpenCode lease after bounded retries." + exit 1 + + validate-pr-metadata: name: validate-pr-metadata + needs: [admit-exact-head-dispatch] # Folded together with the former coverage-source-tree job (2026-09-17): # both jobs only ever exchanged the OpenCode app token for READ-scoped # data (target-repository metadata, then the PR merge tree) and neither @@ -52,7 +450,10 @@ jobs: # for the measurement (run 34931908846: 21 minutes of job execution # inside a 13h57m run, ~97.5% of which was queue wait between exactly # these job boundaries). - if: github.event_name == 'repository_dispatch' + if: >- + needs.admit-exact-head-dispatch.result == 'success' + && needs.admit-exact-head-dispatch.outputs.admitted == 'true' + && github.event_name == 'repository_dispatch' runs-on: group: CWL central OpenCode labels: [self-hosted, linux, x64] @@ -70,6 +471,7 @@ jobs: base_sha: ${{ steps.validate.outputs.base_sha }} head_ref: ${{ steps.validate.outputs.head_ref }} head_sha: ${{ steps.validate.outputs.head_sha }} + is_draft: ${{ steps.validate.outputs.is_draft }} is_private: ${{ steps.validate.outputs.is_private }} steps: - name: Exchange OpenCode app token for target repository metadata reads @@ -160,6 +562,7 @@ jobs: SUPPLIED_BASE_SHA: ${{ github.event.client_payload.pr_base_sha || '' }} SUPPLIED_HEAD_REF: ${{ github.event.client_payload.pr_head_ref || '' }} SUPPLIED_HEAD_SHA: ${{ github.event.client_payload.pr_head_sha || '' }} + DRAFT_REVIEW_ONLY: ${{ toJSON(github.event.client_payload.draft_review_only) }} run: | set -euo pipefail if [ "$EVENT_NAME" = "repository_dispatch" ]; then @@ -215,6 +618,9 @@ jobs: live_base_sha="$(jq -r '.base.sha // empty' <<<"$pull_request_json")" live_head_ref="$(jq -r '.head.ref // empty' <<<"$pull_request_json")" live_head_sha="$(jq -r '.head.sha // empty' <<<"$pull_request_json")" + live_draft="$(jq -r \ + 'if (.draft | type) == "boolean" then (.draft | tostring) else empty end' \ + <<<"$pull_request_json")" live_state="$(jq -r '.state // empty' <<<"$pull_request_json")" live_visibility="$(jq -r '.base.repo.visibility // empty | ascii_downcase' <<<"$pull_request_json")" case "$live_visibility" in @@ -228,6 +634,7 @@ jobs: ! [[ "$live_head_repository" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]] || ! [[ "$live_base_sha" =~ ^[0-9a-fA-F]{40}$ ]] || ! [[ "$live_head_sha" =~ ^[0-9a-fA-F]{40}$ ]] || + ! [[ "$live_draft" =~ ^(true|false)$ ]] || ! [[ "$live_is_private" =~ ^(true|false)$ ]] || [ -z "$live_base_ref" ] || [ -z "$live_head_ref" ]; then @@ -241,6 +648,7 @@ jobs: [ "$SUPPLIED_BASE_SHA" = "$live_base_sha" ] || mismatches+=("base_sha") [ "$SUPPLIED_HEAD_REF" = "$live_head_ref" ] || mismatches+=("head_ref") [ "$SUPPLIED_HEAD_SHA" = "$live_head_sha" ] || mismatches+=("head_sha") + [ "$DRAFT_REVIEW_ONLY" = "$live_draft" ] || mismatches+=("draft_review_only") if [ "${#mismatches[@]}" -gt 0 ]; then printf '::error::repository_dispatch metadata does not match the live pull request: %s. supplied_base=%s/%s live_base=%s/%s supplied_head=%s/%s live_head=%s/%s\n' "$(IFS=,; printf '%s' "${mismatches[*]}")" "${SUPPLIED_BASE_REF:-}" "${SUPPLIED_BASE_SHA:-}" "$live_base_ref" "$live_base_sha" "${SUPPLIED_HEAD_REF:-}" "${SUPPLIED_HEAD_SHA:-}" "$live_head_ref" "$live_head_sha" exit 1 @@ -254,6 +662,7 @@ jobs: printf 'base_sha=%s\n' "$live_base_sha" printf 'head_ref=%s\n' "$live_head_ref" printf 'head_sha=%s\n' "$live_head_sha" + printf 'is_draft=%s\n' "$live_draft" printf 'is_private=%s\n' "$live_is_private" } >>"$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" @@ -392,8 +801,11 @@ jobs: coverage-evidence: name: coverage-evidence - needs: [validate-pr-metadata] + needs: [admit-exact-head-dispatch, validate-pr-metadata] if: >- + needs.admit-exact-head-dispatch.result == 'success' + && needs.admit-exact-head-dispatch.outputs.admitted == 'true' + && needs.validate-pr-metadata.result == 'success' && github.event_name == 'repository_dispatch' runs-on: @@ -2517,18 +2929,14 @@ jobs: opencode-review-target: name: opencode-review - needs: [validate-pr-metadata, coverage-evidence] + needs: [admit-exact-head-dispatch, validate-pr-metadata, coverage-evidence] if: >- always() + && needs.admit-exact-head-dispatch.result == 'success' + && needs.admit-exact-head-dispatch.outputs.admitted == 'true' && needs.validate-pr-metadata.result == 'success' && needs.coverage-evidence.result != 'cancelled' && github.event_name == 'repository_dispatch' - concurrency: - group: >- - opencode-review-${{ - needs.validate-pr-metadata.outputs.target_repository }}-${{ - needs.validate-pr-metadata.outputs.pr_number || github.run_id }} - cancel-in-progress: true runs-on: group: CWL central OpenCode labels: [self-hosted, linux, x64] @@ -4519,6 +4927,7 @@ jobs: && needs.coverage-evidence.result == 'success' && steps.opencode_review_model_pool.outputs.review_status == 'success' && steps.central_review_process_fallback_scope.outputs.eligible == 'true' + && needs.validate-pr-metadata.outputs.is_draft == 'false' continue-on-error: true # Keep the normal peer-check hold short, but leave bounded room for # dynamic image/package-build extensions and review publication overhead. @@ -4858,6 +5267,7 @@ jobs: NO_COLOR: "1" PR_NUMBER: ${{ needs.validate-pr-metadata.outputs.pr_number }} HEAD_SHA: ${{ needs.validate-pr-metadata.outputs.head_sha }} + PR_DRAFT: ${{ needs.validate-pr-metadata.outputs.is_draft }} RUN_ID: ${{ github.run_id }} RUN_ATTEMPT: ${{ github.run_attempt }} OPENCODE_MODEL_POOL_OUTCOME: ${{ steps.opencode_review_model_pool.outputs.review_status }} @@ -4886,6 +5296,10 @@ jobs: OPENCODE_EXPORT_TIMEOUT_SECONDS: "60" run: | set -euo pipefail + if ! [[ "$PR_DRAFT" =~ ^(true|false)$ ]]; then + echo "::error::OpenCode publication rejected malformed live Draft metadata." + exit 1 + fi quoted_coverage="${COVERAGE_EVIDENCE_RESULT:-}" COVERAGE_EVIDENCE_RESULT="$(python3 scripts/ci/opencode_coverage_identity.py \ --repo "$GH_REPOSITORY" \ @@ -5176,6 +5590,11 @@ jobs: local gh_error_file local review_payload_file local review_response_file + if [ "$event" = "APPROVE" ] && [ "$PR_DRAFT" = "true" ]; then + event="COMMENT" + body="${body//- Result: APPROVE/- Result: DRAFT_REVIEW_COMPLETE}" + body="${body}"$'\n\n'"Draft review-only request completed without publishing merge approval authority." + fi if [ -z "${review_write_token:-}" ]; then printf '::error::OPENCODE_REVIEW_IDENTITY_UNAVAILABLE: refusing to publish %s with a GitHub Actions or PAT identity for head %s.\n' "$event" "$HEAD_SHA" return 1 @@ -7773,6 +8192,7 @@ jobs: if: >- always() && needs.validate-pr-metadata.result == 'success' + && needs.validate-pr-metadata.outputs.is_draft == 'false' && needs.validate-pr-metadata.outputs.target_repository != '' && needs.validate-pr-metadata.outputs.pr_number != '' && needs.validate-pr-metadata.outputs.head_sha != '' @@ -7783,29 +8203,24 @@ jobs: PR_HEAD_SHA: ${{ needs.validate-pr-metadata.outputs.head_sha }} run: | set -euo pipefail - draft_args=() - if [ "$(gh api "repos/${GH_REPOSITORY}/pulls/${PR_NUMBER}" --jq '.draft')" = "true" ]; then - draft_args=(--draft) - fi python3 scripts/ci/opencode_review_receipt_gate.py \ --repo "$GH_REPOSITORY" \ --pr-number "$PR_NUMBER" \ - --head-sha "$PR_HEAD_SHA" \ - "${draft_args[@]}" + --head-sha "$PR_HEAD_SHA" - - name: Wake exact-head required OpenCode workflow + - name: Wake every failed exact-head Required OpenCode workflow if: >- always() && github.event_name == 'repository_dispatch' && steps.formal_review_receipt.outcome == 'success' + && needs.validate-pr-metadata.outputs.is_draft == 'false' && needs.validate-pr-metadata.outputs.target_repository != '' && needs.validate-pr-metadata.outputs.head_sha != '' - && github.event.client_payload.required_run_id != '' env: GH_TOKEN: ${{ needs.validate-pr-metadata.outputs.target_repository == github.repository && github.token || secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN }} GH_REPOSITORY: ${{ needs.validate-pr-metadata.outputs.target_repository }} + PR_NUMBER: ${{ needs.validate-pr-metadata.outputs.pr_number }} PR_HEAD_SHA: ${{ needs.validate-pr-metadata.outputs.head_sha }} - REQUIRED_RUN_ID: ${{ github.event.client_payload.required_run_id }} WAKE_TOKEN_SOURCE: ${{ needs.validate-pr-metadata.outputs.target_repository == github.repository && 'github-token' || secrets.PR_REVIEW_MERGE_TOKEN != '' && 'PR_REVIEW_MERGE_TOKEN' || secrets.OPENCODE_APPROVE_TOKEN != '' && 'OPENCODE_APPROVE_TOKEN' || 'unavailable' }} run: | set -euo pipefail @@ -7813,45 +8228,159 @@ jobs: echo "::error::Actions-capable wake credential is unavailable. Native runs use github.token; sibling runs require PR_REVIEW_MERGE_TOKEN or OPENCODE_APPROVE_TOKEN." exit 1 fi - [[ "$REQUIRED_RUN_ID" =~ ^[1-9][0-9]*$ ]] || { - echo "::error::Required OpenCode run id is missing or non-canonical." + live_authority_matches() { + local live_pr live_state live_draft live_head live_created_at + live_pr="$(gh api "repos/${GH_REPOSITORY}/pulls/${PR_NUMBER}")" || return 1 + live_state="$(jq -r '.state // empty' <<<"$live_pr")" + live_draft="$(jq -r 'if (.draft | type) == "boolean" then (.draft | tostring) else empty end' <<<"$live_pr")" + live_head="$(jq -r '.head.sha // empty' <<<"$live_pr")" + live_created_at="$(jq -r '.created_at // empty' <<<"$live_pr")" + [ "$live_state" = "open" ] && [ "$live_draft" = "false" ] && + [ "${live_head,,}" = "${PR_HEAD_SHA,,}" ] && + [[ "$live_created_at" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || + return 1 + LIVE_PR_CREATED_AT="$live_created_at" + } + if ! live_authority_matches; then + echo "::error::Pull request authority changed before exact-head Required OpenCode wake inventory." exit 1 + fi + inventory_start="$LIVE_PR_CREATED_AT" + collect_required_run_pages() { + local range_start range_end range_total range_probe + local start_epoch end_epoch midpoint_epoch midpoint_time right_start + local probe_url page_url page_output raw_count expected_total=0 + local range_index=0 + local -a range_starts=("$inventory_start") + local -a range_ends=("$(date -u +%Y-%m-%dT%H:%M:%SZ)") + local inventory_file + inventory_file="$(mktemp)" + while [ "$range_index" -lt "${#range_starts[@]}" ]; do + range_start="${range_starts[$range_index]}" + range_end="${range_ends[$range_index]}" + range_index=$((range_index + 1)) + probe_url="repos/${GH_REPOSITORY}/actions/runs?event=pull_request_target&created=${range_start}..${range_end}&per_page=1" + if ! range_probe="$(gh api "$probe_url")" || + ! range_total="$(jq -er ' + .total_count + | if type == "number" and floor == . and . >= 0 then tostring + elif . == "2,500+" then "overflow" + else error("invalid total_count") + end + ' <<<"$range_probe")"; then + echo "::error::Could not validate a bounded Required OpenCode run interval." >&2 + rm -f "$inventory_file" + return 1 + fi + if [ "$range_total" = "overflow" ] || [ "$range_total" -gt 1000 ]; then + start_epoch="$(date -u -d "$range_start" +%s)" || return 1 + end_epoch="$(date -u -d "$range_end" +%s)" || return 1 + if [ "$start_epoch" -ge "$end_epoch" ]; then + echo "::error::More than 1,000 pull_request_target runs share one second; exact inventory cannot be proven." >&2 + rm -f "$inventory_file" + return 1 + fi + midpoint_epoch=$(((start_epoch + end_epoch) / 2)) + midpoint_time="$(date -u -d "@${midpoint_epoch}" +%Y-%m-%dT%H:%M:%SZ)" + right_start="$(date -u -d "@$((midpoint_epoch + 1))" +%Y-%m-%dT%H:%M:%SZ)" + range_starts+=("$range_start" "$right_start") + range_ends+=("$midpoint_time" "$range_end") + continue + fi + page_url="repos/${GH_REPOSITORY}/actions/runs?event=pull_request_target&created=${range_start}..${range_end}&per_page=100" + if ! page_output="$(gh api --paginate "$page_url")"; then + echo "::error::Could not list a bounded Required OpenCode run interval." >&2 + rm -f "$inventory_file" + return 1 + fi + printf '%s\n' "$page_output" >>"$inventory_file" + expected_total=$((expected_total + range_total)) + done + if ! raw_count="$(jq -e -s '[.[] | .workflow_runs[]] | length' "$inventory_file")" || + [ "$raw_count" -ne "$expected_total" ]; then + printf '::error::Required OpenCode inventory was truncated: expected=%s collected=%s.\n' "$expected_total" "${raw_count:-invalid}" >&2 + rm -f "$inventory_file" + return 1 + fi + cat "$inventory_file" + rm -f "$inventory_file" } - # The immutable run id is scoped to GH_REPOSITORY. Revalidate its - # event, central workflow path, and live PR head before rerunning it; - # rendered titles and workflow_url differ between native and - # organization-required workflow contexts. for attempt in $(seq 1 12); do - run="$(gh api "repos/${GH_REPOSITORY}/actions/runs/${REQUIRED_RUN_ID}")" - required_run="$(printf '%s\n' "$run" | jq -r --arg head "$PR_HEAD_SHA" --argjson run_id "$REQUIRED_RUN_ID" ' - select(.id == $run_id) - | select(.event == "pull_request_target") - | select(.path == ".github/workflows/opencode-review.yml") - | select(.head_sha == $head) - | [(.id // ""), (.status // ""), (.conclusion // "")] - | @tsv - ')" - IFS=$'\t' read -r required_run_id required_status required_conclusion <<<"$required_run" - if [ "$required_status" = "completed" ] && [ "$required_conclusion" = "failure" ]; then - gh api -X POST "repos/${GH_REPOSITORY}/actions/runs/${required_run_id}/rerun-failed-jobs" >/dev/null - echo "Re-ran failed jobs for exact-head Required OpenCode Review run ${required_run_id}." + if ! run_pages="$(collect_required_run_pages)"; then + echo "::error::Could not list exact-head Required OpenCode workflow runs." + exit 1 + fi + if ! run_inventory="$(jq -c -s --arg head "$PR_HEAD_SHA" --argjson pr "$PR_NUMBER" ' + if all(.[]; (.workflow_runs | type) == "array") + and all( + .[] | .workflow_runs[]; + (.id | type) == "number" + and (.event | type) == "string" + and (.path | type) == "string" + and (.head_sha | type) == "string" + and (.status | type) == "string" + and ((.conclusion == null) or (.conclusion | type) == "string") + and (.pull_requests | type) == "array" + and all( + .pull_requests[]; + (.number | type) == "number" + and (.head.sha | type) == "string" + ) + ) + then + [ + .[] | .workflow_runs[] + | select( + .event == "pull_request_target" + and .path == ".github/workflows/opencode-review.yml" + and any( + .pull_requests[]; + .number == $pr + and (.head.sha | ascii_downcase) == ($head | ascii_downcase) + ) + ) + ] | unique_by(.id) + else + error("invalid required-run inventory") + end + ' <<<"$run_pages")"; then + echo "::error::Exact-head Required OpenCode workflow inventory was malformed." + exit 1 + fi + failed_ids="$(jq -r ' + .[] + | select(.status == "completed" and .conclusion == "failure") + | .id + ' <<<"$run_inventory")" + if [ -n "$failed_ids" ]; then + while IFS= read -r failed_id; do + [ -n "$failed_id" ] || continue + if ! live_authority_matches; then + echo "::error::Pull request authority changed before Required OpenCode rerun ${failed_id}." + exit 1 + fi + gh api -X POST \ + "repos/${GH_REPOSITORY}/actions/runs/${failed_id}/rerun-failed-jobs" >/dev/null + echo "Re-ran failed jobs for exact-head Required OpenCode Review run ${failed_id}." + done <<<"$failed_ids" exit 0 fi - if [ "$required_status" = "completed" ] && [ "$required_conclusion" = "success" ]; then - echo "Exact-head Required OpenCode Review run ${required_run_id} already succeeded." + if jq -e 'any(.[]; .status == "completed" and .conclusion == "success")' <<<"$run_inventory" >/dev/null; then + echo "An exact-head Required OpenCode Review run already succeeded." exit 0 fi if [ "$attempt" -lt 12 ]; then sleep 5 fi done - echo "::error::Formal OpenCode receipt exists, but the exact-head required workflow did not reach a rerunnable failed state." + echo "::error::Formal OpenCode receipt exists, but no exact-head required workflow reached a rerunnable failed or successful state." exit 1 - name: Publish repository_dispatch OpenCode status if: >- always() && github.event_name == 'repository_dispatch' + && needs.validate-pr-metadata.outputs.is_draft == 'false' && needs.validate-pr-metadata.outputs.target_repository != '' && needs.validate-pr-metadata.outputs.head_sha != '' env: @@ -7916,6 +8445,7 @@ jobs: if: >- always() && github.event_name == 'repository_dispatch' + && needs.validate-pr-metadata.outputs.is_draft == 'false' && needs.validate-pr-metadata.outputs.target_repository != '' && needs.validate-pr-metadata.outputs.pr_number != '' && needs.validate-pr-metadata.outputs.head_sha != '' @@ -7944,6 +8474,9 @@ jobs: --interval-seconds 10 - name: Run merge scheduler after approval + if: >- + always() + && needs.validate-pr-metadata.outputs.is_draft == 'false' continue-on-error: true env: GH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || steps.opencode_app_token.outputs.token || github.token }} diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index bbe692b417..4918b5c641 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -501,6 +501,132 @@ jobs: exit 1 fi echo "::add-mask::$app_token" + + # GitHub native concurrency replaces an older pending group member + # even when cancel-in-progress is false, so the receiver deliberately + # has no lossy concurrency group. Avoid duplicate work here instead: + # inventory every active admission state twice so a transition cannot + # disappear between status queries, and collect older-head central + # runs for exact-identity retirement before posting the current head. + active_dispatch_title="OpenCode Review Dispatch ${TARGET_REPOSITORY}#${PR_NUMBER}@${HEAD_SHA}" + active_dispatch_prefix="OpenCode Review Dispatch ${TARGET_REPOSITORY}#${PR_NUMBER}@" + stale_dispatches_file="$(mktemp)" + same_head_found=false + trap 'rm -f "$helper" "$stale_dispatches_file"' EXIT + for inventory_pass in 1 2; do + for active_status in requested waiting pending queued in_progress; do + runs_url="repos/ContextualWisdomLab/.github/actions/workflows/opencode-review-dispatch.yml/runs?event=repository_dispatch&status=${active_status}&per_page=100" + if ! active_runs="$(GH_TOKEN="$app_token" gh api --paginate "$runs_url")"; then + echo "::error::Could not inspect active OpenCode dispatches; refusing a duplicate scheduler wake." + exit 1 + fi + if ! same_head_active="$(jq -r -s --arg title "$active_dispatch_title" --arg status "$active_status" ' + if all(.[]; (.workflow_runs | type) == "array") + and all( + .[] | .workflow_runs[]; + (.id | type) == "number" + and .path == ".github/workflows/opencode-review-dispatch.yml" + and .event == "repository_dispatch" + and .status == $status + and (.display_title | type) == "string" + ) + then + any( + .[] | .workflow_runs[]; + (.display_title | ascii_downcase) == ($title | ascii_downcase) + ) + else + error("invalid workflow-runs response") + end + ' <<<"$active_runs")"; then + echo "::error::Active OpenCode dispatch data was invalid; refusing a duplicate scheduler wake." + exit 1 + fi + case "$same_head_active" in + true) same_head_found=true ;; + false) ;; + *) + echo "::error::Active OpenCode dispatch guard returned an invalid state." + exit 1 + ;; + esac + jq -r -s --arg prefix "$active_dispatch_prefix" --arg title "$active_dispatch_title" ' + .[] | .workflow_runs[] + | select((.display_title | ascii_downcase) | startswith($prefix | ascii_downcase)) + | select((.display_title | ascii_downcase) != ($title | ascii_downcase)) + | .id + ' <<<"$active_runs" >>"$stale_dispatches_file" + done + done + + live_authority_matches() { + local live_pr live_head live_draft live_state + live_pr="$(gh api "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}")" || return 1 + 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')" + [ "$live_state" = "open" ] && [ "$live_draft" = "false" ] && + [ "${live_head,,}" = "${HEAD_SHA,,}" ] + } + if ! live_authority_matches; then + echo "Pull request authority changed before OpenCode dispatch; scheduler wake skipped." + exit 0 + fi + + while IFS= read -r stale_run_id; do + [ -n "$stale_run_id" ] || continue + if ! live_authority_matches; then + echo "Pull request authority changed before stale OpenCode retirement; scheduler wake skipped." + exit 0 + fi + if ! GH_TOKEN="$app_token" gh api --method POST \ + "repos/ContextualWisdomLab/.github/actions/runs/${stale_run_id}/cancel" >/dev/null; then + echo "::error::Could not cancel superseded central OpenCode dispatch ${stale_run_id}; refusing to add current-head work behind it." + exit 1 + fi + cancellation_verified=false + for cancellation_attempt in 1 2 3 4 5 6; do + if ! stale_state="$(GH_TOKEN="$app_token" gh api \ + "repos/ContextualWisdomLab/.github/actions/runs/${stale_run_id}" \ + --jq '[.status // "", .conclusion // ""] | @tsv')"; then + echo "::error::Could not verify superseded central OpenCode dispatch ${stale_run_id} cancellation." + exit 1 + fi + IFS=$'\t' read -r stale_status stale_conclusion <<<"$stale_state" + if [ "$stale_status" = "completed" ] && + [ "$stale_conclusion" = "cancelled" ]; then + cancellation_verified=true + break + fi + case "$stale_status" in + requested|waiting|pending|queued|in_progress) + ;; + completed) + echo "::error::Superseded central OpenCode dispatch ${stale_run_id} completed with non-cancelled conclusion ${stale_conclusion:-}." + exit 1 + ;; + *) + echo "::error::Superseded central OpenCode dispatch ${stale_run_id} returned invalid status ${stale_status:-}." + exit 1 + ;; + esac + done + if [ "$cancellation_verified" != "true" ]; then + echo "::error::Superseded central OpenCode dispatch ${stale_run_id} did not reach completed/cancelled after accepted cancellation." + exit 1 + fi + echo "Verified cancelled superseded central OpenCode dispatch ${stale_run_id} after live exact-head revalidation." + done < <(sort -nu "$stale_dispatches_file") + + if [ "$same_head_found" = "true" ]; then + echo "An exact-head OpenCode dispatch is already active; scheduler wake skipped after older-head retirement." + exit 0 + fi + + if ! live_authority_matches; then + echo "Pull request authority changed before OpenCode dispatch; scheduler wake skipped." + exit 0 + fi jq -cn \ --arg target_repository "$TARGET_REPOSITORY" \ --arg pr_number "$PR_NUMBER" \ @@ -509,7 +635,8 @@ jobs: --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}}' | + --argjson draft_review_only false \ + '{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,draft_review_only:$draft_review_only,required_run_id:$required_run_id}}' | GH_TOKEN="$app_token" gh api -X POST repos/ContextualWisdomLab/.github/dispatches --input - - name: Fail closed without a current-head OpenCode verdict diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index 7777f02bed..b568bfa4c6 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -87,8 +87,18 @@ concurrency: github.event_name == 'repository_dispatch' && github.event.client_payload.target_repository != '' && github.event.client_payload.pr_number != '' && format('target-{0}-pr-{1}', github.event.client_payload.target_repository, github.event.client_payload.pr_number) || github.event_name == 'repository_dispatch' && github.event.client_payload.pr_number != '' && format('pr-{0}', github.event.client_payload.pr_number) || github.event_name == 'repository_dispatch' && format('repo-dispatch-{0}', github.repository) || - github.ref }} - cancel-in-progress: ${{ github.event_name == 'pull_request_target' || github.event_name == 'pull_request_review' || github.event_name == 'repository_dispatch' }} + github.ref }}-${{ + github.event_name == 'pull_request_target' && github.event.action == 'closed' && format('head-{0}-closed', github.event.pull_request.head.sha) || + github.event_name == 'pull_request_target' && format('head-{0}', github.event.pull_request.head.sha) || + github.event_name == 'pull_request_review' && format('head-{0}', github.event.pull_request.head.sha) || + github.event_name == 'repository_dispatch' && github.event.client_payload.pr_head_sha != '' && format('head-{0}', github.event.client_payload.pr_head_sha) || + 'no-head' }} + # Preserve same-head admissions up to GitHub's documented pending limit + # without single-pending replacement; dispatch ordering remains + # platform-controlled. A new head receives a distinct group; the metadata-only + # cleanup job below retires only revalidated predecessor work and proves + # terminal cancellation. + queue: max # Scorecard Token-Permissions (alert #9): declare a least-privilege default at # the workflow level. The scan-pr-queue job that actually needs write access @@ -98,6 +108,167 @@ permissions: contents: read jobs: + cancel-superseded-pr-runs: + if: >- + github.event_name == 'pull_request_target' && + ( + github.event.action == 'synchronize' || + github.event.action == 'closed' + ) + runs-on: ubuntu-24.04 + timeout-minutes: 5 + permissions: + actions: write + contents: read + pull-requests: read + env: + GH_TOKEN: ${{ github.token }} + TARGET_REPOSITORY: ${{ github.repository }} + TARGET_PR_NUMBER: ${{ github.event.pull_request.number }} + TARGET_PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + TARGET_ACTION: ${{ github.event.action }} + steps: + - name: Cancel revalidated predecessor scheduler runs + shell: bash + run: | + set -euo pipefail + + if ! [[ "$TARGET_REPOSITORY" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]] || + ! [[ "$TARGET_PR_NUMBER" =~ ^[1-9][0-9]*$ ]] || + ! [[ "$TARGET_PR_HEAD_SHA" =~ ^[0-9a-fA-F]{40}$ ]]; then + echo "::error::Superseded-run cleanup rejected malformed target identity." + exit 1 + fi + + live_target_matches() { + local live_pull live_repository live_number live_state live_head_sha + live_pull="$(gh api "repos/${TARGET_REPOSITORY}/pulls/${TARGET_PR_NUMBER}")" + live_repository="$(jq -r '.base.repo.full_name // empty' <<<"$live_pull")" + live_number="$(jq -r '.number // 0' <<<"$live_pull")" + live_state="$(jq -r '.state // empty' <<<"$live_pull")" + live_head_sha="$(jq -r '.head.sha // empty' <<<"$live_pull")" + [ "$live_repository" = "$TARGET_REPOSITORY" ] && + [ "$live_number" = "$TARGET_PR_NUMBER" ] && + [ "${live_head_sha,,}" = "${TARGET_PR_HEAD_SHA,,}" ] && { + [ "$TARGET_ACTION:$live_state" = "synchronize:open" ] || + [ "$TARGET_ACTION:$live_state" = "closed:closed" ] + } + } + + if ! live_target_matches; then + echo "Superseded-run cleanup skipped because the event no longer matches the live pull request." + exit 0 + fi + + inventory_dir="$(mktemp -d)" + trap 'rm -rf "$inventory_dir"' EXIT + : >"$inventory_dir/active-arrays.jsonl" + for inventory_pass in 1 2; do + for active_status in requested waiting pending queued in_progress; do + inventory_path="$inventory_dir/${inventory_pass}-${active_status}.json" + gh api --paginate --slurp \ + "repos/${TARGET_REPOSITORY}/actions/workflows/pr-review-merge-scheduler.yml/runs?status=${active_status}&per_page=100" \ + >"$inventory_path" + if ! inventory_counts="$(jq -er ' + if type == "array" and length > 0 + and all(.[]; (.total_count | type) == "number") + and all(.[]; (.workflow_runs | type) == "array") + then + [([.[] | .workflow_runs[]] | length), (map(.total_count) | max)] + | @tsv + else + error("invalid workflow-run inventory") + end + ' "$inventory_path")"; then + echo "::error::Scheduler workflow-run inventory was invalid." + exit 1 + fi + IFS=$'\t' read -r collected_count expected_count <<<"$inventory_counts" + if [ "$collected_count" -ne "$expected_count" ]; then + echo "::error::Scheduler workflow-run inventory was incomplete for ${active_status}: expected=${expected_count} collected=${collected_count}." + exit 1 + fi + jq -c '[.[] | .workflow_runs[]]' \ + "$inventory_path" \ + >>"$inventory_dir/active-arrays.jsonl" + done + done + workflow_runs="$(jq -sc 'add // [] | unique_by(.id)' "$inventory_dir/active-arrays.jsonl")" + mapfile -t superseded_run_ids < <( + jq -r \ + --argjson pr_number "$TARGET_PR_NUMBER" \ + --argjson current_run_id "$GITHUB_RUN_ID" \ + --arg target_head "${TARGET_PR_HEAD_SHA,,}" \ + --arg target_action "$TARGET_ACTION" \ + ' + .[] + | select(.id != $current_run_id) + | select( + .status == "requested" or + .status == "waiting" or + .status == "pending" or + .status == "queued" or + .status == "in_progress" + ) + | select( + any( + .pull_requests[]?; + (.head.sha // "" | ascii_downcase) as $run_pr_head + | .number == $pr_number + | select( + $target_action == "closed" or + ( + ($run_pr_head | test("^[0-9a-f]{40}$")) and + $run_pr_head != $target_head + ) + ) + ) + ) + | .id + ' <<<"$workflow_runs" + ) + + for run_id in "${superseded_run_ids[@]}"; do + if ! live_target_matches; then + echo "Superseded-run cleanup stopped because the target changed before cancellation." + exit 0 + fi + if gh api --method POST \ + "repos/${TARGET_REPOSITORY}/actions/runs/${run_id}/force-cancel" \ + >/dev/null 2>&1; then + cancellation_verified=false + for attempt in 1 2 3 4 5 6; do + IFS=$'\t' read -r run_status run_conclusion < <( + gh api "repos/${TARGET_REPOSITORY}/actions/runs/${run_id}" \ + --jq '[.status // "", .conclusion // ""] | @tsv' + ) + if [ "$run_status" = "completed" ] && + [ "$run_conclusion" = "cancelled" ]; then + cancellation_verified=true + break + fi + if [ "$attempt" -lt 6 ]; then + sleep 1 + fi + done + if [ "$cancellation_verified" != "true" ]; then + echo "::error::Scheduler run $run_id did not reach completed/cancelled after accepted force-cancel." + exit 1 + fi + echo "Verified cancelled scheduler run $run_id." + continue + fi + IFS=$'\t' read -r run_status run_conclusion < <( + gh api "repos/${TARGET_REPOSITORY}/actions/runs/${run_id}" \ + --jq '[.status // "", .conclusion // ""] | @tsv' + ) + if [ "$run_status" = "completed" ]; then + continue + fi + echo "::error::Could not force-cancel nonterminal scheduler run $run_id." + exit 1 + done + scan-pr-queue: # repository_dispatch review runs do not reliably carry pull_requests metadata. # Without this guard, one completed central review can wake a repo-wide scan. @@ -219,6 +390,7 @@ jobs: GH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || steps.scheduler_app_token.outputs.token || github.token }} TARGET_REPOSITORY_INPUT: ${{ github.event.client_payload.target_repository || '' }} TARGET_PR_NUMBER: ${{ github.event.client_payload.pr_number || '' }} + TARGET_HEAD_SHA_INPUT: ${{ github.event.client_payload.pr_head_sha || '' }} TARGET_BASE_BRANCH_INPUT: ${{ github.event.client_payload.base_branch || '' }} ALLOWED_TARGET_REPOSITORIES: ${{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }} run: | @@ -238,8 +410,9 @@ jobs: exit 1 fi if ! [[ "$TARGET_REPOSITORY_INPUT" =~ ^ContextualWisdomLab/[A-Za-z0-9_.-]+$ ]] || - ! [[ "$TARGET_PR_NUMBER" =~ ^[1-9][0-9]*$ ]]; then - printf '::error::Targeted scheduler dispatch rejected an invalid repository or pull request number. target=%s pr=%s\n' "${TARGET_REPOSITORY_INPUT:-}" "${TARGET_PR_NUMBER:-}" + ! [[ "$TARGET_PR_NUMBER" =~ ^[1-9][0-9]*$ ]] || + ! [[ "$TARGET_HEAD_SHA_INPUT" =~ ^[0-9a-fA-F]{40}$ ]]; then + printf '::error::Targeted scheduler dispatch rejected invalid repository, pull request, or head identity. target=%s pr=%s head=%s\n' "${TARGET_REPOSITORY_INPUT:-}" "${TARGET_PR_NUMBER:-}" "${TARGET_HEAD_SHA_INPUT:-}" exit 1 fi @@ -281,6 +454,10 @@ jobs: printf '::error::Targeted scheduler dispatch base branch does not match the live PR. supplied=%s live=%s\n' "$TARGET_BASE_BRANCH_INPUT" "$live_base_branch" exit 1 fi + if [ "${TARGET_HEAD_SHA_INPUT,,}" != "${live_head_sha,,}" ]; then + printf '::error::Targeted scheduler dispatch head does not match the live PR. supplied=%s live=%s\n' "$TARGET_HEAD_SHA_INPUT" "$live_head_sha" + exit 1 + fi { printf 'repository=%s\n' "$TARGET_REPOSITORY_INPUT" diff --git a/.github/workflows/trusted-uv-materializer-quality-ci.yml b/.github/workflows/trusted-uv-materializer-quality-ci.yml index ea78c30cc7..d38dcd855f 100644 --- a/.github/workflows/trusted-uv-materializer-quality-ci.yml +++ b/.github/workflows/trusted-uv-materializer-quality-ci.yml @@ -155,7 +155,7 @@ jobs: python -m coverage report - name: Enforce complete production docstrings - run: python -m interrogate --fail-under 100 scripts/ci/materialize_base_python_requirements.py + run: python -m interrogate --fail-under 100 scripts/ci - name: Compile production and quality contracts run: | diff --git a/AGENTS.md b/AGENTS.md index 0972af51c5..5087855f7b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,18 +46,23 @@ The materialization contract is also covered by [`docs/doctoring/exact-artifact- and use `github-robot-review-gate` plus `babysit-pr` when diagnosing or monitoring a protected PR. If a named skill is unavailable, preserve its fail-closed trust boundary and exact-current-head evidence rules manually. -- PR-triggered workflow concurrency must be trigger-aware. Group by workflow, - target repository, and pull request number with `cancel-in-progress: true`; - do not include the head SHA, because that prevents a new head from cancelling - its predecessor. Non-PR triggers need an explicit collision-safe fallback. +- PR-triggered workflow concurrency must be trigger-aware. For replaceable + current-state checks, group by workflow, target repository, and pull request + number with `cancel-in-progress: true`. When every same-head admission carries + distinct work that must survive, include the exact head SHA and use bounded + `queue: max`; retire predecessor heads only through a metadata-only cleanup + that inventories every PR-associated trigger, revalidates live authority, + and proves terminal cancellation. Non-PR triggers need an explicit + collision-safe fallback. - Put concurrency at workflow scope when queued jobs must be coalesced before a runner is admitted. Job-level concurrency cannot relieve a saturated runner queue because it is evaluated only after job admission. - Keep cleanup repository-local and event-driven. Do not restore an - organization-wide queue sweep, polling `sleep`, or another scheduled scan to - compensate for incorrect concurrency. Cancel only runs proven to belong to a - superseded head of the same PR, then verify each accepted cancellation - reaches `completed/cancelled`. + organization-wide queue sweep, long-lived polling wait, or another scheduled + scan to compensate for incorrect concurrency. A metadata-only cleanup may use + a few bounded status reads to prove an accepted cancellation reached + `completed/cancelled`; it must fail closed rather than hold a model or review + runner. Cancel only runs proven to belong to a superseded head of the same PR. - Classify a run's PR head by event-specific evidence before cancellation. `pull_request` may use the run's top-level `head_sha`, but `pull_request_target` records the trusted base there; use its PR association diff --git a/CHANGELOG.d/20261001-opencode-same-head-dispatch-idempotency.md b/CHANGELOG.d/20261001-opencode-same-head-dispatch-idempotency.md new file mode 100644 index 0000000000..8609c73085 --- /dev/null +++ b/CHANGELOG.d/20261001-opencode-same-head-dispatch-idempotency.md @@ -0,0 +1,16 @@ +### Fixed + +- Preserve any active exact-head OpenCode dispatch across all GitHub admission + states when the required-check wake path is retried, fail closed on ambiguous + inventory, avoid GitHub's lossy pending concurrency replacement, retire only + live-head-verified older central runs with completion evidence, and revalidate + pull-request authority immediately before a new dispatch. After a receiver + acquires the exact-PR lease, revalidate live authority and the formal + exact-head review receipt again so a completed prior receiver cannot trigger + duplicate coverage or model execution. + Preserve exact-head merge-scheduler admissions within GitHub's documented + `queue: max` bound; a dedicated metadata-only cleanup inventories every + active state twice across all PR-associated scheduler triggers, + rejects incomplete GitHub search results, retires only + live-head-revalidated predecessor runs, and proves each accepted + cancellation reaches `completed/cancelled`. diff --git a/CHANGELOG.md b/CHANGELOG.md index b8e8dfc577..92110230a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,16 +1,67 @@ +### Authorized Draft reviews reach the exact-head receiver + +- Carry an explicit `draft_review_only` boolean from the merge scheduler to the + central OpenCode receiver. The receiver accepts a live Draft only while an + exact repository/PR/head durable request artifact is present, non-expired, + structurally complete, and independently re-fetched with `actions: read`; + malformed payloads, missing markers, and every base/head/state mismatch fail + before the Contents lease can be mutated. Ready pull requests reject Draft + authority. A clean Draft publishes an exact-head formal comment with + `DRAFT_REVIEW_COMPLETE`, never an approval or a merge-authorizing receipt; + the scheduler recognizes only that source-backed comment as completion. + The Ready-only Required producer now sends an explicit boolean `false`, and + a Draft completion lease is retired when the same head becomes Ready so the + mode-independent admission identity cannot suppress the required approval + review. + Review-only runs never publish Ready status, dispatch Noema, invoke the merge + scheduler, or wake merge-required OpenCode jobs. This reconnects the + already-authorized agent-mention/scheduler path without weakening ordinary + ready-for-review admission. + +### OpenCode preserves exact-head queue position + +- Remove the central receiver's lossy native concurrency group after live + evidence showed a later same-head wake cancelling the queued authoritative + run. The required wake now inventories all five active states twice, retires + only identity-validated older-head runs after live authority checks, and + fails closed until every accepted cancellation is proven + `completed/cancelled`. The merge scheduler now preserves exact-head + admissions within GitHub's documented `queue: max` bound and retires + predecessor heads from every PR-associated scheduler trigger only through a + metadata-only, + live-head-revalidated cleanup job. The receiver atomically compare-and-swaps one + repository/PR lease file on a dedicated central branch before source + materialization, coverage, or model execution, closing the cross-producer + check-then-POST race without lossy native concurrency. A dedicated minimal + admission job owns the central `contents: write` grant, rejects unauthorized + or malformed envelopes before OIDC exchange, and revalidates the complete + live state/draft/base/head identity immediately before each compare-and-swap; + metadata and source jobs remain read-only. Self-reruns retain their lease and + different-head takeovers validate the recorded owner. After formal receipt, + the publisher inventories repository-wide runs, binds the intended PR head + through `pull_requests[]` rather than the trusted-base run-level SHA, + recursively partitions the PR-lifetime `created` range below GitHub's + 1,000-result filtered-search ceiling, rejects `total_count`/collection + mismatches, revalidates live authority before each POST, and reruns every + matching failed Required OpenCode job. A losing duplicate therefore needs + neither a callback payload nor a polling runner. + ### Maturin download failures close every transport response - Refactor the bounded Maturin asset downloader so successful and rejected responses share one unconditional close path while `HTTPError` keeps its own explicit close path. A new regression exercises a non-200 response and an - opener-raised HTTP error. This removes an impossible optional-response branch + opener-raised HTTP error through a close-observing body without replacing the + error's real `close()` method. This removes an impossible optional-response branch without changing hosts, redirects, byte limits, hashes, or fail-closed error mapping; the focused suite is 17 passed with 100% statement and branch coverage. The trusted full-suite workflow now also tracks the verifier source and its focused test, so a future lifecycle change cannot omit the repository coverage gate that detected this regression. The pull-request trigger admits stacked canonical-owner bases as well as `main`; the protected-branch push - trigger remains restricted to `main`. + trigger remains restricted to `main`. Document the archive extractor and CLI + entry point, and expand the trusted docstring gate from one materializer file + to all `scripts/ci` production modules. ### Shared Strix lock advances beyond the PyJWT recursion DoS diff --git a/docs/doctoring/maturin-download-response-lifecycle-20261001.md b/docs/doctoring/maturin-download-response-lifecycle-20261001.md index b6765daae8..ec73c8b89a 100644 --- a/docs/doctoring/maturin-download-response-lifecycle-20261001.md +++ b/docs/doctoring/maturin-download-response-lifecycle-20261001.md @@ -27,18 +27,30 @@ still produced no gate because the workflow admitted only PRs whose base was The downloader now closes every returned response in one unconditional nested `finally` block. An opener-raised `HTTPError` remains independently closed by its handler. Tests assert closure for both an HTTP 503 response and an HTTP 502 -exception. A workflow contract now requires both verifier paths in pull-request +exception. The error fixture preserves the real `HTTPError.close()` and observes +its body closing; replacing the method with a spy had suppressed the actual +close and leaked pytest's temporary capture object until a later test, where +warnings-fatal execution correctly rejected the `ResourceWarning`. A workflow +contract now requires both verifier paths in pull-request and protected-branch triggers, and a separate contract admits stacked PR bases while the push trigger remains restricted to protected `main`. The repair does not change admitted hosts, the one-hop redirect contract, credentials, request timeout, byte bounds, digests, or error mapping. +Exact-tree verification then found that the verifier's archive extractor and +CLI entry point lacked docstrings even though the PR claimed complete +production docstring coverage. A RED regression now binds both symbols, the +two trust-boundary docstrings close the omission, and Trusted uv measures all +of `scripts/ci` instead of only the materializer module. + ## Evidence and remaining gates - Hosted RED: 5,314 passed, 5 skipped, 40 subtests; 18,775 statements with 5 missing, 7,652 branches with 2 partial; total 99%. -- Local owner GREEN: 17 focused tests; 107/107 statements and 34/34 branches; - `git diff --check` clean. +- Local integrated GREEN: 18 lifecycle/prescreen tests; complete warnings-fatal + suite 5,364 passed, 5 skipped, 40 subtests; 18,767/18,767 statements and + 7,648/7,648 branches; verifier 107/107 statements and 34/34 branches; + production docstrings 1,456/1,456; `git diff --check` clean. - Required before acceptance: complete exact-head hosted suite, security and CodeQL verdicts, qualifying independent approval, ordinary owner integration, then ordinary merge-forward into #1653 and fresh consumer Checks. diff --git a/docs/doctoring/opencode-authorized-draft-review-admission.md b/docs/doctoring/opencode-authorized-draft-review-admission.md new file mode 100644 index 0000000000..247cf8eb79 --- /dev/null +++ b/docs/doctoring/opencode-authorized-draft-review-admission.md @@ -0,0 +1,57 @@ +# Authorized Draft review admission + +## Incident + +At `.github#2546@db81de7a09268eb0abfc5b5c675ddd9a0a38a3e2`, the explicit Draft-review +path had two individually valid halves that did not compose. The +agent-mention workflow wrote a one-day artifact named +`cwl-draft-review-request---`, and the merge scheduler +used that exact marker to authorize review-only dispatch. The central receiver +then required `live_draft=false` unconditionally before acquiring its lease. +The authorized request therefore could not reach semantic review. + +This was a control-plane contract defect, not a reason to make the pull request +Ready or to weaken ordinary merge admission. + +## Invariants + +- The scheduler sends `draft_review_only` as a JSON boolean derived from live + PR metadata; the receiver rejects any non-boolean envelope before external + token exchange. +- Ready work is admitted only with `draft_review_only=false`. +- Draft work is admitted only with `draft_review_only=true` and a freshly + fetched, exact repository/PR/head, non-expired central artifact. +- Artifact pagination or schema ambiguity, missing/expired markers, and any + live state/base/head mismatch fail before Contents lease mutation. +- Every pre-mutation authority recheck also re-fetches the marker. +- A clean Draft emits an exact-head formal `COMMENTED` review with the explicit + `DRAFT_REVIEW_COMPLETE` result; it never emits `APPROVED` authority. +- Only that exact-head result plus its review-only explanation satisfies the + Draft scheduler, so stale and generic comments cannot suppress a new request. +- Draft review-only work does not create a merge receipt, publish Ready status, + dispatch Noema, invoke the merge scheduler, or wake a merge-required OpenCode + workflow. +- A Draft-to-Ready transition invalidates Draft authority before lease access; + a validated Draft that changes later still publishes only a non-authorizing + comment. +- The Ready-only Required producer always sends typed `draft_review_only=false`; + omitted and malformed types fail closed at the receiver. +- Because the durable admission identity intentionally remains + repository/PR/head/component, reconciliation retires an exact-head Draft + completion lease when that pull request becomes Ready without an approving + verdict. The same head can then acquire a fresh Ready review lease. + +## Executable acceptance + +The regression suite executes the extracted production shell. It proves that +an exact authorized Draft reaches lease validation, malformed/mismatched/ +expired artifacts stop before lease access, ordinary Drafts remain rejected, +the dispatch payload distinguishes Ready and Draft work, and the Required +workflow wake is structurally disabled for a validated Draft. It also proves +that the Draft path cannot publish approval authority or start any Ready-only +follow-up, while its exact-head formal completion comment prevents an unbounded +same-head redispatch loop. A transition regression proves Draft completion, +same-head Ready retirement, and fresh Ready redispatch. The complete affected scheduler and receiver suite +must pass both locally and on the exact published head. Hosted security, +provenance, and independent semantic review remain required before ordinary +protected integration. diff --git a/docs/doctoring/opencode-same-head-dispatch-idempotency.md b/docs/doctoring/opencode-same-head-dispatch-idempotency.md new file mode 100644 index 0000000000..af75351b3c --- /dev/null +++ b/docs/doctoring/opencode-same-head-dispatch-idempotency.md @@ -0,0 +1,154 @@ +# OpenCode exact-head dispatch idempotency + +## Incident + +On 2026-09-30, `ContextualWisdomLab/.github#2545` remained on exact head +`9a4af5e438283a31dc05814d6bc2818caee782a3` while the required OpenCode wake +path requested review execution more than once. Central dispatch run +[`36776536447`](https://github.com/ContextualWisdomLab/.github/actions/runs/36776536447) +was already queued for that exact repository, PR, and head. A later same-head +request created run +[`36778773766`](https://github.com/ContextualWisdomLab/.github/actions/runs/36778773766), +and the receiver's PR-keyed `cancel-in-progress: true` concurrency retired the +older run. No PR head change or substantive review result justified losing its +queue position. + +The required workflow correctly checked for an existing current-head formal +review, but, when that receipt was absent, posted `repository_dispatch` +unconditionally. The merge scheduler already deduplicated active same-head +OpenCode runs; this direct required-check path bypassed that guard. + +## Repair contract + +Before posting a new dispatch, the trusted required-check path now: + +1. validates that the target PR is live, open, ready, and still on the event + head; +2. evaluates the existing formal-review receipt predicate; +3. exchanges the existing repository-scoped OpenCode App token; +4. inventories `requested`, `waiting`, `pending`, `queued`, and `in_progress` + runs of the canonical `opencode-review-dispatch.yml` receiver twice, so a + state transition during the inventory remains observable; +5. validates each returned run's identity fields and matches its protected + exact `repository#PR@head` title plus workflow path and trigger; +6. records an exact-head execution only after every accepted older-head + cancellation reaches terminal `completed/cancelled`; and +7. re-fetches the PR immediately before dispatch, retiring the request if + state, draft status, or head authority changed. + +Run-list failures and malformed run records fail closed. The receiver has no +native concurrency group: GitHub replaces an older pending group member even +when `cancel-in-progress` is false, so native concurrency cannot preserve every +distinct callback payload. Producers deduplicate exact-head work from trusted +inventory. Before a current-head POST, the required workflow revalidates live +repository/PR/head authority and cancels only canonical older-head central +runs. A refused cancellation, failed status lookup, invalid state, terminal +non-cancelled conclusion, or accepted cancellation that remains active after +the bounded status reads fails closed without a new dispatch. Those reads do +not sleep inside the required job: the nonblocking capacity contract releases +the runner and lets a later trusted scheduler admission retry instead of +holding scarce capacity while GitHub converges. A final live-authority read +guards the POST. + +The merge scheduler separately binds native concurrency to the exact PR head +and uses GitHub's bounded `queue: max` FIFO instead of lossy single-pending +replacement. `synchronize` and `closed` events run a metadata-only cleanup job +with `actions: write`; it inventories every active status, revalidates the live +PR/head immediately before each mutation, cancels only predecessor-head work +(or all final-head work on close), and accepts completion only after GitHub +reports `completed/cancelled`. Two complete status passes prevent a state +transition from escaping between filtered queries; every response reconciles +the collected row count with `total_count` and fails closed if GitHub's +filtered-search ceiling truncates the inventory. The inventory spans every +event for this workflow and then binds candidates through PR association and +head SHA; filtering the API to `pull_request_target` would strand predecessor +`pull_request_review` runs in their old exact-head group. + +Producer observation and POST are not atomic, so the receiver first authorizes +the exact actor/sender pair, repository allowlist membership, and complete +payload shape before any OIDC exchange or central mutation. It then acquires a +central repository-owned lease after full live state/draft/base/head metadata +validation and before source +materialization, coverage, or model execution. The lease uses one file per +repository/PR on the dedicated `opencode-dispatch-leases` branch. GitHub's +Contents API compares the observed blob SHA during update: simultaneous cache +misses or terminal-owner takeovers can commit only one owner, while a loser +reloads the authoritative file and defers to the active exact-head run. A +different-head run may replace an owner only after a fresh live PR/head check +immediately before the compare-and-swap. Every recorded owner is validated +against its canonical workflow, event, title, and head; a rerun with the same +GitHub run ID retains its own lease. The lease file remains as auditable +bounded state and is updated, not multiplied, on later heads. + +Lease ownership alone is not a durable completion receipt. Immediately after +acquiring or retaining the exact-PR lease, the receiver revalidates live +repository/PR/head authority and evaluates the formal exact-head review receipt +from the trusted default-branch helper. An existing receipt returns +`admitted=false` before source materialization, coverage, or model execution; +an unavailable or malformed receipt lookup fails closed. + +The dedicated admission job alone holds `contents: write` for that lease +branch; later metadata validation and source materialization return to +`contents: read`. +The required target-repository job retains read-only Actions and contents +authority; central cancellation uses the existing repository-scoped App token. +After a formal receipt, the privileged publisher uses the repository-wide run +inventory and matches the intended PR number plus +`pull_requests[].head.sha`. It deliberately does not treat the run-level +`head_sha` as the PR head because `pull_request_target` executes trusted +default-branch workflow code. GitHub caps every filtered workflow-run search at +1,000 results, so the publisher recursively bisects the PR-lifetime `created` +range until each interval is within the API bound, then requires the sum of +every interval's `total_count` to equal the collected rows. An interval with +more than 1,000 runs in a single second fails closed. The publisher revalidates +live open/non-draft head authority immediately before every mutation and reruns +each matching failed Required OpenCode job. Therefore a duplicate +producer that lost the lease does not need its own callback payload or a +polling runner. Human review events do not enter or cancel the required +workflow. The repair does not weaken the required verdict, accept predecessor +evidence, or broaden model/provider permissions. + +## Executable evidence + +`tests/test_opencode_required_verdict_regression.py` executes the production +shell step. Its RED fixture proved that a queued exact-head run still produced +a second dispatch. The GREEN cases cover all five active admission states, +transition-safe two-pass inventory, malformed and unavailable inventory +fail-closure, current-head preservation, older-head non-suppression, +no-active-run dispatch, existing formal receipts, and a head movement between +initial validation and the mutation boundary. Queue-contract tests prohibit +lossy native receiver concurrency; execution fixtures prove exact older-head +cancellation, deduplication, asynchronous cancellation continuation, and +fail-closure when a cancellation is refused, remains active, returns an invalid +state, or terminates with a non-cancelled conclusion. Scheduler cleanup +fixtures also prove concurrent-head-movement preservation, transition-safe +two-pass discovery, review-event predecessor discovery, +inventory-completeness rejection, and bounded terminal cancellation +verification. +Receiver fixtures execute absent, active-owner, terminal-owner, self-rerun, +different-head takeover, and branch-initialization-race lease paths. +A two-process fixture starts simultaneous cache misses against an atomic fake +Contents API and proves exactly one `admitted=true` result. The independent +negative fixtures prove that unauthorized envelopes perform no OIDC or GitHub +API call and that closed, draft, or mismatched base/head metadata performs no +Contents mutation. Wake fixtures use a trusted-base run-level SHA distinct from +the PR head and cover multiple failures, another PR sharing the same commit, +authority movement between mutations, partial rerun failure, recursive +partitioning above 1,000 results, and detected pagination truncation. The +independent byte-for-byte reviewer pin was regenerated from the repaired +receiver as exact Git blob +`5c87c74863ef6872c1ef7136d5b330071920c09e`. + +Local exact-tree verification on the stacked successor base +`86ddef63ed306d4c7d56d051d9a72570e7d358a5`: + +- required-workflow, nonblocking capacity, queue, receiver, and integrity-pin + contracts after independent-review repair: `233 passed`; +- complete Python 3.14 warnings-fatal suite: `5362 passed, 5 skipped, 40 + subtests passed`, with + owned production `18767/18767` statements and `7648/7648` branches, + Docstring `100%`, and zero warnings. + +Protected merge still requires hosted exact-head security and quality Checks, +zero unresolved review threads, qualifying independent approval, and ordinary +branch protection. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index b4882cee5a..24dc6a4e09 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -1,10 +1,22 @@ # Product and Technical Gap Baseline +## 2026-10-01 authorized Draft review admission + +| Gap | Exact evidence | Action | Status | +|---|---|---|---| +| The agent-mention workflow and scheduler created an exact repository/PR/head durable Draft-review request, but the central receiver unconditionally required `live_draft=false`; every explicitly authorized Draft review therefore stopped before lease admission and could never produce the semantic review it requested. Once admitted, the clean-verdict publisher also attempted `APPROVED` before its downstream Draft receipt guard, which could grant merge authority to review-only work. Finally, a completed Draft lease shared the Ready identity and could suppress same-head Ready review forever | `.github#2546@db81de7a09268eb0abfc5b5c675ddd9a0a38a3e2`; `.github/workflows/agent-mention-opencode-dispatch.yml` marker `cwl-draft-review-request---`; `.github/workflows/opencode-review-dispatch.yml` former `live_authority_matches` and unconditional clean-verdict approval; mode-independent admission identity in `scripts/ci/review_admission_controller.py`; local RED contracts in `tests/test_opencode_required_verdict_regression.py` and `tests/test_pr_review_merge_scheduler.py` | Carry a typed review-only boolean in every producer payload, reject omitted/malformed authority before token exchange, and admit a live Draft only after re-fetching a non-expired exact marker from the central Actions artifact API. Revalidate state/base/head plus marker before every lease mutation. Convert a clean Draft verdict to an exact-head `DRAFT_REVIEW_COMPLETE` formal comment before publication; accept only that source-backed comment as Draft scheduler completion, retire that lease if the same head becomes Ready without approval, and structurally skip formal merge receipt, Ready status, Noema, merge-scheduler, and Required-workflow wake follow-ups. See [RCA and executable acceptance](doctoring/opencode-authorized-draft-review-admission.md) | **Proposed / local RED→GREEN complete; exact-head hosted Checks and independent semantic review required** | + +## 2026-10-01 OpenCode same-head dispatch idempotency + +| Gap | Exact evidence | Action | Status | +|---|---|---|---| +| A repeated required-check wake for unchanged `.github#2545@9a4af5e438283a31dc05814d6bc2818caee782a3` cancelled queued central dispatch `36776536447` when same-head run `36778773766` was created, resetting queue position without new source authority | [Run 36776536447](https://github.com/ContextualWisdomLab/.github/actions/runs/36776536447); [run 36778773766](https://github.com/ContextualWisdomLab/.github/actions/runs/36778773766); protected `main@37b10243cec3d160ecc9c1be75c71428b160a703` | At the central `.github` owner, inventory all five active admission states twice, validate run identity and fail closed on ambiguity, remove lossy native receiver concurrency, retire only canonical older-head central runs after live repository/PR/head validation, and require every accepted cancellation to reach `completed/cancelled` before current-head dispatch. Preserve merge-scheduler exact-head admissions with bounded `queue: max`; on `synchronize` or `closed`, a metadata-only job inventories every active state twice across all PR-associated scheduler triggers, rejects invalid or `total_count`-incomplete results, revalidates live PR/head authority before each mutation, retires only predecessor work, and proves terminal cancellation. Because producer inventory and POST are not atomic, authorize actor/sender/target and payload shape before OIDC, then acquire one repository/PR lease through the Contents API's blob-SHA compare-and-swap only after full live state/draft/base/head validation; active same-head losers terminate cheaply, self-reruns and terminal owners recover, and a freshly revalidated live head may replace a fully validated older-head owner. After lease acquisition or retention, revalidate live repository/PR/head authority and the trusted formal exact-head receipt; an existing receipt must return `admitted=false` before source materialization, coverage, or model execution, and an unavailable or malformed lookup fails closed. Isolate central `contents: write` in a dedicated lease job and keep later jobs read-only. After formal receipt, use repository-wide run inventory, bind PR number and `pull_requests[].head.sha` rather than the trusted-base run-level SHA, recursively partition the PR-lifetime `created` range below GitHub's 1,000-result ceiling, verify `total_count` completeness, and revalidate before each failed-job rerun so duplicate callbacks are unnecessary. See [RCA and executable acceptance](doctoring/opencode-same-head-dispatch-idempotency.md) | **Proposed / local RED→GREEN complete; exact-head hosted Checks and independent approval required** | + ## 2026-10-01 Maturin response-lifecycle coverage closure | Gap | Exact evidence | Action | Status | |---|---|---|---| -| `.github#1653@5cd141ec2c33b631d164af936cd1c9de70e4c9a4` passed all 5,314 tests but failed the complete branch gate because the canonical Maturin downloader left five error-path statements and two branches unexecuted; the owner workflow omitted both verifier paths and stacked PR bases | Trusted uv Materializer run `36811202519`, job `110206427182`; `verify_release_maturin_tool_assets.py` 95%, missing lines 104 and 106-112 plus branch 114→116; no owner run at #2530 predecessor `8cf2ea5f73976d47b2267fb52ac28284323404b7` because its base was #2531 rather than `main` | Repair canonical successor `.github#2530`: exercise non-200 and opener-raised `HTTPError` closure, replace the impossible nullable-response finalizer with one unconditional response-owned close scope, add source/test and stacked-PR trigger contracts to the complete gate while retaining protected-main push scope, preserve all network and fail-closed boundaries, then ordinary-merge the accepted owner head into #1653 | **Proposed / hosted RED reproduced; focused verifier coverage GREEN locally; path and stacked-admission contracts RED→GREEN; exact-head hosted full-suite, security, CodeQL, and independent approval required** | +| `.github#1653@5cd141ec2c33b631d164af936cd1c9de70e4c9a4` passed all 5,314 tests but failed the complete branch gate because the canonical Maturin downloader left five error-path statements and two branches unexecuted; the owner workflow omitted both verifier paths and stacked PR bases | Trusted uv Materializer run `36811202519`, job `110206427182`; `verify_release_maturin_tool_assets.py` 95%, missing lines 104 and 106-112 plus branch 114→116; no owner run at #2530 predecessor `8cf2ea5f73976d47b2267fb52ac28284323404b7` because its base was #2531 rather than `main` | Repair canonical successor `.github#2530`: exercise non-200 and opener-raised `HTTPError` closure, replace the impossible nullable-response finalizer with one unconditional response-owned close scope, document the archive/CLI trust boundaries, expand the trusted docstring gate to all `scripts/ci`, add source/test and stacked-PR trigger contracts to the complete gate while retaining protected-main push scope, preserve all network and fail-closed boundaries, then ordinary-merge the accepted owner head into #1653 | **Proposed / hosted RED reproduced; focused verifier coverage and 100% production docstrings GREEN locally; path and stacked-admission contracts RED→GREEN; exact-head hosted full-suite, security, CodeQL, and independent approval required** | ## 2026-10-01 bounded Maturin release downloader SAST closure diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index 5a86bd24c8..09a248c3fb 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -70,6 +70,34 @@ def admit(self, component: str, repository: str, pr: dict[str, Any]) -> bool: def lease(state): """Apply this request to `state` and record any lease it wins.""" + prior_record = state.records.get(request.identity) + if ( + component == "opencode" + and not bool(pr.get("isDraft")) + and prior_record is not None + and has_current_head_draft_review_completion(pr) + and not has_current_head_approval(pr) + and not has_current_head_changes_requested(pr) + and ( + prior_record.status == "complete" + or ( + prior_record.status == "dispatched" + and opencode_progress_state( + pr, stale_after_minutes=DEFAULT_STALE_OPENCODE_MINUTES + ) + != "running" + ) + ) + ): + # The durable identity intentionally omits Draft/Ready mode. + # Retire only a terminal prior lease at the moment a Ready + # dispatch asks for admission; an in-flight Ready lease keeps + # its bounded worker slot on subsequent reconciliations. + updated_records = dict(state.records) + updated_records[request.identity] = RequestRecord( + prior_record.request, "stale" + ) + state = type(state)(updated_records, dict(state.latest_sequences)) plan = plan_dispatches( state, [request], @@ -103,7 +131,14 @@ def reconcile_state(state): continue terminal = ( record.request.component == "opencode" - and (has_current_head_approval(pr) or has_current_head_changes_requested(pr)) + and ( + has_current_head_approval(pr) + or has_current_head_changes_requested(pr) + or ( + bool(pr.get("isDraft")) + and has_current_head_draft_review_completion(pr) + ) + ) ) or ( record.request.component == "strix" and strix_evidence_state(pr) == "complete" @@ -2296,6 +2331,28 @@ def has_current_head_changes_requested(pr: dict[str, Any]) -> bool: return current_head_review_state(pr, "CHANGES_REQUESTED") +def has_current_head_draft_review_completion(pr: dict[str, Any]) -> bool: + """Return whether OpenCode completed an exact-head Draft review-only request. + + A clean Draft cannot receive merge approval authority. The central reviewer + therefore publishes a formal COMMENTED review carrying an explicit result + marker and explanatory sentence. Both markers are required so an ordinary + status comment cannot suppress a later explicit Draft review request. + """ + for review in reversed((pr.get("reviews") or {}).get("nodes") or []): + if not is_opencode_review(review) or not review_matches_current_head(review, pr): + continue + body = review.get("body") or "" + if ( + (review.get("state") or "").upper() == "COMMENTED" + and "- Result: DRAFT_REVIEW_COMPLETE" in body + and "Draft review-only request completed without publishing merge approval authority." + in body + ): + return True + return False + + def latest_current_head_coverage_change_request( pr: dict[str, Any], ) -> dict[str, Any] | None: @@ -3862,6 +3919,7 @@ def dispatch_opencode_review(repo: str, workflow: str, pr: dict[str, Any], *, dr "pr_base_sha": base_sha, "pr_head_ref": head_ref, "pr_head_sha": head_sha, + "draft_review_only": bool(pr.get("isDraft")), } complete_paginated_pr_contexts(target_repo, pr) required_run_id = matching_actions_run_id(pr, is_opencode_check_run) @@ -4233,7 +4291,11 @@ def dispatch_draft_review_only( # alone as a verdict would make a failed dispatch attempt permanently # block every later explicit retry. Only an actual current-head formal # review is a verdict. - if has_current_head_approval(pr) or has_current_head_changes_requested(pr): + if ( + has_current_head_approval(pr) + or has_current_head_changes_requested(pr) + or has_current_head_draft_review_completion(pr) + ): return Decision( number, "skip", diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index e53741bd51..e3d4ce826a 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -587,11 +587,12 @@ assert_opencode_review_uses_codegraph_and_contextual_orchestrator() { record_failure "opencode required workflow bootstrap condition detection must survive a job block larger than the pipe buffer" fi rm -f "$large_bootstrap_fixture" - assert_file_contains "$workflow_file" 'needs.validate-pr-metadata.outputs.target_repository' "opencode review scopes concurrency by the live validated target repository" - assert_file_contains "$workflow_file" 'needs.validate-pr-metadata.outputs.pr_number || github.run_id' "opencode review scopes concurrency by the live validated PR with a non-PR fallback" + assert_file_contains "$workflow_file" 'admit-exact-head-dispatch:' "opencode review admits one exact-head receiver before expensive work" + assert_file_contains "$workflow_file" 'admitted: ${{ steps.single_flight.outputs.admitted }}' "opencode review exports its atomic exact-head lease decision" + assert_file_contains "$workflow_file" "needs.admit-exact-head-dispatch.outputs.admitted == 'true'" "opencode review gates expensive work on the exact-head lease" + assert_file_contains "$workflow_file" 'opencode-dispatch-leases' "opencode review uses a durable exact-head lease instead of lossy native concurrency" + assert_file_contains "$workflow_file" "Wake every failed exact-head Required OpenCode workflow" "opencode review preserves distinct same-head admissions for deterministic receipt recovery" assert_file_not_contains "$workflow_file" "format('pr-{0}-{1}'" "opencode review does not keep stale head-specific concurrency groups" - assert_file_contains "$workflow_file" 'opencode-review-${{' "opencode review uses the workflow-repository-PR group prefix" - assert_file_contains "$workflow_file" 'cancel-in-progress: true' "opencode review cancels stale in-progress review attempts when a newer PR event arrives" assert_file_contains "$workflow_file" "Materialize pull request merge tree for coverage measurement" "opencode pull_request coverage execution materializes the exact base/head merge tree" assert_file_contains "$workflow_file" "stale OpenCode run: event head=" "opencode review side effects are skipped for stale heads" assert_file_not_contains "$workflow_file" "github.event.pull_request.head.repo.full_name == github.event.pull_request.base.repo.full_name" "opencode never treats a same-repository pull_request_target head as authorization to execute PR-controlled code" @@ -614,7 +615,18 @@ assert_opencode_review_uses_codegraph_and_contextual_orchestrator() { assert_file_contains "$workflow_file" "actions: read" "opencode review workflow can read failed Actions logs without Actions write scope" assert_file_contains "$workflow_file" "checks: read" "opencode review workflow can read failed check-run annotations for line-specific findings" assert_file_contains "$workflow_file" "contents: read" "opencode review workflow uses read-only repository contents permission" - assert_file_not_contains "$workflow_file" "contents: write" "opencode review workflow does not need repository contents write scope" + local admission_job validation_job + admission_job="$(awk '/^ admit-exact-head-dispatch:$/ { emit=1 } emit && /^ [A-Za-z0-9_-]+:$/ && $0 !~ /^ admit-exact-head-dispatch:$/ { exit } emit { print }' "$workflow_file")" + validation_job="$(awk '/^ validate-pr-metadata:$/ { emit=1 } emit && /^ [A-Za-z0-9_-]+:$/ && $0 !~ /^ validate-pr-metadata:$/ { exit } emit { print }' "$workflow_file")" + if ! grep -Fq -- "contents: write" <<<"$admission_job"; then + record_failure "opencode review scopes repository contents write permission to the atomic lease job" + fi + if grep -Fq -- "contents: write" <<<"$validation_job"; then + record_failure "opencode review metadata validation must not receive repository contents write permission" + fi + if ! grep -Fq -- "contents: read" <<<"$validation_job"; then + record_failure "opencode review metadata validation keeps read-only repository contents permission" + fi assert_file_contains "$workflow_file" "pull-requests: write" "opencode review workflow may use github-actions[bot] for same-repository review-thread, update-branch, auto-merge, and merge follow-up" assert_file_contains "$workflow_file" "issues: write" "opencode review workflow can publish or update overview comments through the job token" assert_file_contains "$workflow_file" "statuses: write" "opencode review workflow can read status contexts and publish the repository_dispatch status evidence it owns" @@ -1593,7 +1605,10 @@ assert_pr_review_merge_scheduler_uses_github_actions_bot_token() { assert_file_contains "$workflow_file" "github.event_name == 'pull_request_target' && format('pr-{0}', github.event.pull_request.number)" "scheduler scopes pull_request_target concurrency to the active PR" assert_file_contains "$workflow_file" "github.event_name == 'schedule' && format('schedule-{0}', github.event.schedule)" "scheduler isolates repository-local recovery from PR runs" assert_file_contains "$workflow_file" "github.event_name == 'repository_dispatch' && github.event.client_payload.target_repository != '' && github.event.client_payload.pr_number != ''" "scheduler scopes targeted manual queue scans to the requested PR" - assert_file_contains "$workflow_file" "cancel-in-progress: \${{ github.event_name == 'pull_request_target' || github.event_name == 'pull_request_review' || github.event_name == 'repository_dispatch' }}" "scheduler cancels stale PR/review/manual queue scans instead of accumulating merge/update attempts" + assert_file_contains "$workflow_file" "queue: max" "scheduler preserves distinct same-head admissions up to the documented pending limit" + assert_file_not_contains "$workflow_file" "cancel-in-progress:" "scheduler avoids native pending-run replacement" + assert_file_contains "$workflow_file" "cancel-superseded-pr-runs:" "scheduler retires only revalidated predecessor-head runs" + assert_file_contains "$workflow_file" "Cancel revalidated predecessor scheduler runs" "scheduler keeps predecessor cleanup metadata-only and explicit" assert_file_not_contains "$workflow_file" 'github.event.workflow_run' "scheduler does not poll required-check completion through follow-up workflow runs" assert_file_contains "$workflow_file" "github.event.client_payload.trigger_reviews != false" "scheduler enables review dispatch by default for default-branch dispatch events" assert_file_contains "$workflow_file" "github.event_name == 'schedule' || github.event_name == 'push'" "scheduler can dispatch a bounded OpenCode review from native or recovery events" diff --git a/scripts/ci/verify_release_maturin_tool_assets.py b/scripts/ci/verify_release_maturin_tool_assets.py index a59507ac83..2aa9767d8a 100644 --- a/scripts/ci/verify_release_maturin_tool_assets.py +++ b/scripts/ci/verify_release_maturin_tool_assets.py @@ -120,6 +120,7 @@ def _download(filename: str) -> bytes: def _binary(raw: bytes, filename: str) -> bytes: + """Extract the single bounded Maturin executable from its reviewed archive.""" if filename.endswith(".zip"): with zipfile.ZipFile(io.BytesIO(raw)) as archive: members = archive.infolist() @@ -168,6 +169,7 @@ def verify_assets(evidence: dict, reader: str, fetch=_download) -> None: def main() -> None: + """Verify reviewed Maturin assets from the network or an explicit local root.""" parser = argparse.ArgumentParser() parser.add_argument("--asset-root", type=Path) args = parser.parse_args() diff --git a/tests/test_close_empty_pr_queue_pressure.py b/tests/test_close_empty_pr_queue_pressure.py index 6da88f63f1..5600a081c3 100644 --- a/tests/test_close_empty_pr_queue_pressure.py +++ b/tests/test_close_empty_pr_queue_pressure.py @@ -29,8 +29,19 @@ def test_closed_pull_request_does_not_allocate_a_noop_runner( assert "closed" in workflow assert "github.event.pull_request.number" in concurrency - assert "github.event.pull_request.head.sha" not in concurrency - assert re.search(r"(?m)^[ \t]+cancel-in-progress:[ \t]+\S", concurrency) - assert "cancel-closed-pr-runs:" not in workflow + if filename == "pr-review-merge-scheduler.yml": + assert "github.event.pull_request.head.sha" in concurrency + assert "queue: max" in concurrency + assert "cancel-in-progress:" not in concurrency + assert "cancel-superseded-pr-runs:" in workflow + cleanup = workflow.split(" cancel-superseded-pr-runs:", 1)[1].split( + " scan-pr-queue:", 1 + )[0] + assert "actions: write" in cleanup + assert "actions/checkout" not in cleanup + else: + assert "github.event.pull_request.head.sha" not in concurrency + assert re.search(r"(?m)^[ \t]+cancel-in-progress:[ \t]+\S", concurrency) + assert "cancel-closed-pr-runs:" not in workflow assert "github.event.action != 'closed'" in workflow assert evidence_job in workflow diff --git a/tests/test_current_head_coalescer_self_cancellation.py b/tests/test_current_head_coalescer_self_cancellation.py index e49193d4df..55de2228f2 100644 --- a/tests/test_current_head_coalescer_self_cancellation.py +++ b/tests/test_current_head_coalescer_self_cancellation.py @@ -9,8 +9,8 @@ ) -def test_current_head_coalescer_shares_pr_scoped_scheduler_admission() -> None: - """The integrated step reuses PR-scoped scheduler admission and its runner.""" +def test_current_head_coalescer_preserves_exact_head_scheduler_admission() -> None: + """The scheduler queues one exact head and retires predecessors explicitly.""" workflow_text = WORKFLOW_PATH.read_text(encoding="utf-8") coalescer = workflow_text.split("\n scan-pr-queue:\n", 1)[1] concurrency_block = workflow_text.split("\nconcurrency:\n", 1)[1].split( @@ -24,11 +24,18 @@ def test_current_head_coalescer_shares_pr_scoped_scheduler_admission() -> None: assert "Retire redundant queued exact-head runs" in coalescer assert "github.repository == 'ContextualWisdomLab/.github'" in coalescer - assert "github.event.pull_request.head.sha" not in concurrency_block + assert "github.event.pull_request.head.sha" in concurrency_block + assert "github.event.client_payload.pr_head_sha" in concurrency_block assert "github.event.pull_request.number" in concurrency_block - assert any( - line.startswith("cancel-in-progress:") - and "github.event_name == 'pull_request_target'" in line - for line in active_lines - ) - assert "queue: max" not in workflow_text + assert not any(line.startswith("cancel-in-progress:") for line in active_lines) + assert "queue: max" in concurrency_block + + cleanup = workflow_text.split("\n cancel-superseded-pr-runs:\n", 1)[1].split( + "\n scan-pr-queue:\n", 1 + )[0] + assert "github.event.action == 'synchronize'" in cleanup + assert "github.event.action == 'closed'" in cleanup + assert "actions: write" in cleanup + assert "actions/checkout" not in cleanup + assert "live_target_matches" in cleanup + assert "did not reach completed/cancelled" in cleanup diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 8a80d65461..0bcc16d49a 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -1225,7 +1225,41 @@ def test_opencode_model_exhaustion_retry_stays_owned_by_central_scheduler(): workflow = Path(".github/workflows/opencode-review-dispatch.yml").read_text(encoding="utf-8") assert "opencode-exhausted-retry:" not in workflow assert "RETRY_DISPATCH_TOKEN" not in workflow - assert "contents: write" not in workflow + admission_job = workflow.split(" admit-exact-head-dispatch:\n", 1)[1].split( + "\n validate-pr-metadata:", 1 + )[0] + validation_job = workflow.split(" validate-pr-metadata:\n", 1)[1].split( + "\n coverage-evidence:", 1 + )[0] + review_job = workflow.split(" opencode-review-target:\n", 1)[1] + assert admission_job.count("contents: write") == 1 + assert "opencode-dispatch-leases" in admission_job + assert "contents: write" not in validation_job + assert "contents: write" not in review_job + + +def test_receiver_rechecks_formal_receipt_after_exact_head_lease(): + """A completed owner must not let a duplicate receiver repeat model work.""" + workflow = Path(".github/workflows/opencode-review-dispatch.yml").read_text( + encoding="utf-8" + ) + admission_job = workflow.split(" admit-exact-head-dispatch:\n", 1)[1].split( + "\n validate-pr-metadata:", 1 + )[0] + + assert "opencode_review_receipt_gate.py?ref=${GITHUB_SHA}" in admission_job + assert "definite_receipt_state()" in admission_job + assert 'gate["evaluate_receipts"](' in admission_job + assert 'echo "admitted=false" >>"$GITHUB_OUTPUT"' in admission_job + assert admission_job.count('echo "admitted=true" >>"$GITHUB_OUTPUT"') == 1 + + lease_acquired = admission_job.index( + 'echo "Central exact-head dispatch ${GITHUB_RUN_ID} acquired the atomic lease."' + ) + receipt_recheck = admission_job.index( + "complete_receipt_admission", lease_acquired + ) + assert lease_acquired < receipt_recheck def test_sandbox_git_config_env_trusts_only_the_validated_worktree(tmp_path): @@ -1854,19 +1888,13 @@ def test_workflow_provisions_sandbox_tool_and_reviewer_agent(): assert "run_opencode_review_model_pool.sh" in workflow assert "rekick_model_pool_on_exhaustion" not in workflow assert "publish stage performs no duplicate model-catalog pass" in workflow - # The review job's own group, addressed by its indentation: the workflow - # also carries a workflow-level admission group (pinned in - # tests/test_required_workflow_queue_contract.py), so splitting on the - # first "concurrency:" would read that one instead of this one. - concurrency_contract = workflow.split("\n concurrency:", 1)[1].split( - "\n runs-on:", 1 - )[0] - assert "needs.validate-pr-metadata.outputs.target_repository" in concurrency_contract - assert "needs.validate-pr-metadata.outputs.pr_number || github.run_id" in concurrency_contract - assert "format('pr-{0}-{1}'" not in concurrency_contract - assert "github.event.client_payload.pr_head_sha" not in concurrency_contract - assert "github.event.client_payload.pr_number" not in concurrency_contract - assert "github.event.pull_request" not in concurrency_contract + # GitHub native concurrency replaces an older pending member even with + # cancel-in-progress disabled. Producer inventory and live-head retirement + # own admission, so the receiver intentionally has no concurrency group. + workflow_header = workflow.split("permissions:", 1)[0] + review_job = workflow.split("\n opencode-review-target:\n", 1)[1] + assert not re.search(r"(?m)^concurrency:", workflow_header) + assert not re.search(r"(?m)^ concurrency:", review_job) assert "OPENCODE_MODEL_CANDIDATES" in workflow model_pool_runner = Path("scripts/ci/run_opencode_review_model_pool.sh").read_text( encoding="utf-8" @@ -2456,10 +2484,13 @@ def test_merge_scheduler_uses_escalating_mutation_credentials(): assert 'check_delay="$((check_attempt * 2))"' in workflow assert "steps.review_followup.outputs.proceed != 'false'" in workflow assert "Native events and the explicit org-sweep recovery remain authoritative." in workflow - assert ( - "github.event_name == 'pull_request_review' || " - "github.event_name == 'repository_dispatch'" in workflow - ) + concurrency = workflow.split("\nconcurrency:\n", 1)[1].split( + "\npermissions:\n", 1 + )[0] + assert "queue: max" in concurrency + assert "cancel-in-progress:" not in concurrency + assert "github.event.pull_request.head.sha" in concurrency + assert "github.event.client_payload.pr_head_sha" in concurrency def test_opencode_runs_merge_scheduler_after_review_without_repo_local_dispatch(): diff --git a/tests/test_opencode_required_rerun_capacity.py b/tests/test_opencode_required_rerun_capacity.py index c85bc24e3c..f2438c46d4 100644 --- a/tests/test_opencode_required_rerun_capacity.py +++ b/tests/test_opencode_required_rerun_capacity.py @@ -7,6 +7,7 @@ import os from pathlib import Path import subprocess +import textwrap from tests.test_opencode_required_verdict_regression import HEAD, fail_closed_script @@ -15,6 +16,28 @@ DISPATCH = Path(".github/workflows/opencode-review-dispatch.yml") +def wake_failed_required_runs_script() -> str: + """Extract the exact production wake step shell.""" + dispatch = DISPATCH.read_text(encoding="utf-8") + step = dispatch.split( + " - name: Wake every failed exact-head Required OpenCode workflow\n", 1 + )[1].split("\n\n - name:", 1)[0] + return textwrap.dedent(step.split(" run: |\n", 1)[1]) + + +def required_run(run_id: int, *, pr_number: int = 7) -> dict[str, object]: + """Build one exact-head required-workflow list record.""" + return { + "id": run_id, + "event": "pull_request_target", + "path": ".github/workflows/opencode-review.yml", + "head_sha": "c" * 40, + "status": "completed", + "conclusion": "failure", + "pull_requests": [{"number": pr_number, "head": {"sha": HEAD}}], + } + + def test_required_job_releases_runner_until_exact_run_wakeup() -> None: required = REQUIRED.read_text(encoding="utf-8") target = required.split(" opencode-review-target:\n", 1)[1].split( @@ -29,18 +52,355 @@ def test_required_job_releases_runner_until_exact_run_wakeup() -> None: assert "will rerun this failed job" in target -def test_dispatch_wakes_only_the_exact_failed_current_head_run() -> None: +def test_dispatch_wakes_every_exact_failed_current_head_run() -> None: dispatch = DISPATCH.read_text(encoding="utf-8") - wake = dispatch.split(" - name: Wake exact-head required OpenCode workflow\n", 1)[1].split( - "\n\n - name:", 1 - )[0] + wake = dispatch.split( + " - name: Wake every failed exact-head Required OpenCode workflow\n", 1 + )[1].split("\n\n - name:", 1)[0] - assert "github.event.client_payload.required_run_id != ''" in wake - assert "select(.id == $run_id)" in wake - assert 'select(.event == "pull_request_target")' in wake - assert 'select(.path == ".github/workflows/opencode-review.yml")' in wake - assert "select(.head_sha == $head)" in wake + assert "github.event.client_payload.required_run_id != ''" not in wake + assert '.event == "pull_request_target"' in wake + assert '.path == ".github/workflows/opencode-review.yml"' in wake + assert "(.head.sha | ascii_downcase) == ($head | ascii_downcase)" in wake + assert ".number == $pr" in wake + assert "unique_by(.id)" in wake assert "rerun-failed-jobs" in wake + assert "actions/workflows/opencode-review.yml/runs" not in wake + assert "actions/runs?event=pull_request_target&created=" in wake + assert "range_total" in wake + assert "expected_total" in wake + assert "live_authority_matches" in wake + + +def test_dispatch_wake_binds_shared_head_runs_to_exact_pull_request( + tmp_path: Path, +) -> None: + """Wake all failures for this PR and none for another PR sharing its head.""" + calls = tmp_path / "calls" + fake_gh = tmp_path / "gh" + inventory = { + "total_count": 3, + "workflow_runs": [ + required_run(41), + required_run(42, pr_number=8), + required_run(43), + ] + } + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +printf '%s\n' "$*" >>"$CALLS" +if [[ "$*" == "api repos/owner/repo/pulls/7" ]]; then + printf '%s' "$LIVE_PR" +elif [[ "$*" == *"repos/owner/repo/actions/runs?"* ]]; then + printf '%s' "$RUN_INVENTORY" +elif [[ "$*" == *"rerun-failed-jobs"* ]]; then + exit 0 +else + exit 97 +fi +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + result = subprocess.run( + ["bash", "-c", wake_failed_required_runs_script()], + env={ + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ['PATH']}", + "CALLS": str(calls), + "GH_TOKEN": "token", + "WAKE_TOKEN_SOURCE": "PR_REVIEW_MERGE_TOKEN", + "GH_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "PR_HEAD_SHA": HEAD, + "LIVE_PR": json.dumps( + { + "state": "open", + "draft": False, + "created_at": "2026-09-30T00:00:00Z", + "head": {"sha": HEAD}, + } + ), + "RUN_INVENTORY": json.dumps(inventory), + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + reruns = [line for line in calls.read_text().splitlines() if "rerun-failed-jobs" in line] + assert reruns == [ + "api -X POST repos/owner/repo/actions/runs/41/rerun-failed-jobs", + "api -X POST repos/owner/repo/actions/runs/43/rerun-failed-jobs", + ] + assert calls.read_text().count("api repos/owner/repo/pulls/7") == 3 + + +def test_dispatch_wake_stops_when_live_authority_moves_between_mutations( + tmp_path: Path, +) -> None: + """Revalidate the live open head immediately before every rerun POST.""" + calls = tmp_path / "calls" + authority_reads = tmp_path / "authority-reads" + fake_gh = tmp_path / "gh" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +printf '%s\n' "$*" >>"$CALLS" +if [[ "$*" == "api repos/owner/repo/pulls/7" ]]; then + count=0 + [[ ! -f "$AUTHORITY_READS" ]] || count="$(cat "$AUTHORITY_READS")" + count=$((count + 1)) + printf '%s' "$count" >"$AUTHORITY_READS" + if [[ "$count" -lt 3 ]]; then head="$PR_HEAD_SHA"; else head="$(printf 'd%.0s' {1..40})"; fi + jq -cn --arg head "$head" '{state:"open",draft:false,created_at:"2026-09-30T00:00:00Z",head:{sha:$head}}' +elif [[ "$*" == *"repos/owner/repo/actions/runs?"* ]]; then + printf '%s' "$RUN_INVENTORY" +elif [[ "$*" == *"rerun-failed-jobs"* ]]; then + exit 0 +else + exit 97 +fi +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + result = subprocess.run( + ["bash", "-c", wake_failed_required_runs_script()], + env={ + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ['PATH']}", + "CALLS": str(calls), + "AUTHORITY_READS": str(authority_reads), + "GH_TOKEN": "token", + "WAKE_TOKEN_SOURCE": "PR_REVIEW_MERGE_TOKEN", + "GH_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "PR_HEAD_SHA": HEAD, + "RUN_INVENTORY": json.dumps( + { + "total_count": 2, + "workflow_runs": [required_run(41), required_run(43)], + } + ), + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 1 + assert "authority changed before Required OpenCode rerun 43" in result.stdout + reruns = [line for line in calls.read_text().splitlines() if "rerun-failed-jobs" in line] + assert reruns == ["api -X POST repos/owner/repo/actions/runs/41/rerun-failed-jobs"] + + +def test_dispatch_wake_reports_partial_rerun_api_failure(tmp_path: Path) -> None: + """A later rerun failure must fail the publisher after recording prior work.""" + calls = tmp_path / "calls" + fake_gh = tmp_path / "gh" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +printf '%s\n' "$*" >>"$CALLS" +if [[ "$*" == "api repos/owner/repo/pulls/7" ]]; then + jq -cn --arg head "$PR_HEAD_SHA" '{state:"open",draft:false,created_at:"2026-09-30T00:00:00Z",head:{sha:$head}}' +elif [[ "$*" == *"repos/owner/repo/actions/runs?"* ]]; then + printf '%s' "$RUN_INVENTORY" +elif [[ "$*" == *"actions/runs/41/rerun-failed-jobs"* ]]; then + exit 0 +elif [[ "$*" == *"actions/runs/43/rerun-failed-jobs"* ]]; then + exit 22 +else + exit 97 +fi +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + result = subprocess.run( + ["bash", "-c", wake_failed_required_runs_script()], + env={ + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ['PATH']}", + "CALLS": str(calls), + "GH_TOKEN": "token", + "WAKE_TOKEN_SOURCE": "PR_REVIEW_MERGE_TOKEN", + "GH_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "PR_HEAD_SHA": HEAD, + "RUN_INVENTORY": json.dumps( + { + "total_count": 2, + "workflow_runs": [required_run(41), required_run(43)], + } + ), + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 22 + reruns = [line for line in calls.read_text().splitlines() if "rerun-failed-jobs" in line] + assert reruns == [ + "api -X POST repos/owner/repo/actions/runs/41/rerun-failed-jobs", + "api -X POST repos/owner/repo/actions/runs/43/rerun-failed-jobs", + ] + + +def test_dispatch_wake_partitions_inventory_above_github_search_cap( + tmp_path: Path, +) -> None: + """Bisect GitHub's overflow sentinel before collecting bounded pages.""" + calls = tmp_path / "calls" + probe_count = tmp_path / "probe-count" + page_count = tmp_path / "page-count" + fake_gh = tmp_path / "gh" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +printf '%s\n' "$*" >>"$CALLS" +if [[ "$*" == "api repos/owner/repo/pulls/7" ]]; then + jq -cn --arg head "$PR_HEAD_SHA" '{state:"open",draft:false,created_at:"2026-09-30T00:00:00Z",head:{sha:$head}}' +elif [[ "$*" == *"actions/runs?"* && "$*" == *"per_page=1" ]]; then + count=0 + [[ ! -f "$PROBE_COUNT" ]] || count="$(cat "$PROBE_COUNT")" + count=$((count + 1)) + printf '%s' "$count" >"$PROBE_COUNT" + case "$count" in + 1) printf '%s' '{"total_count":"2,500+","workflow_runs":[]}' ;; + 2) printf '%s' '{"total_count":1,"workflow_runs":[]}' ;; + 3) printf '%s' '{"total_count":0,"workflow_runs":[]}' ;; + *) exit 96 ;; + esac +elif [[ "$*" == *"api --paginate"* && "$*" == *"per_page=100"* ]]; then + count=0 + [[ ! -f "$PAGE_COUNT" ]] || count="$(cat "$PAGE_COUNT")" + count=$((count + 1)) + printf '%s' "$count" >"$PAGE_COUNT" + if [[ "$count" -eq 1 ]]; then printf '%s' "$ONE_RUN"; else printf '%s' '{"workflow_runs":[]}'; fi +elif [[ "$*" == *"actions/runs/41/rerun-failed-jobs"* ]]; then + exit 0 +else + exit 97 +fi +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + result = subprocess.run( + ["bash", "-c", wake_failed_required_runs_script()], + env={ + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ['PATH']}", + "CALLS": str(calls), + "PROBE_COUNT": str(probe_count), + "PAGE_COUNT": str(page_count), + "GH_TOKEN": "token", + "WAKE_TOKEN_SOURCE": "PR_REVIEW_MERGE_TOKEN", + "GH_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "PR_HEAD_SHA": HEAD, + "ONE_RUN": json.dumps({"workflow_runs": [required_run(41)]}), + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + assert probe_count.read_text() == "3" + assert page_count.read_text() == "2" + assert "actions/runs/41/rerun-failed-jobs" in calls.read_text() + + +def test_dispatch_wake_fails_when_overflow_shares_one_second(tmp_path: Path) -> None: + """A non-partitionable overflow sentinel fails before run selection.""" + fake_gh = tmp_path / "gh" + fake_date = tmp_path / "date" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +if [[ "$*" == "api repos/owner/repo/pulls/7" ]]; then + jq -cn --arg head "$PR_HEAD_SHA" '{state:"open",draft:false,created_at:"2026-09-30T00:00:00Z",head:{sha:$head}}' +elif [[ "$*" == *"actions/runs?"* && "$*" == *"per_page=1" ]]; then + printf '%s' '{"total_count":"2,500+","workflow_runs":[]}' +else + exit 97 +fi +""", + encoding="utf-8", + ) + fake_date.write_text( + """#!/usr/bin/env bash +set -euo pipefail +if [[ "$*" == "-u +%Y-%m-%dT%H:%M:%SZ" ]]; then + printf '%s\n' '2026-09-30T00:00:00Z' +elif [[ "$*" == *"+%s"* ]]; then + printf '%s\n' '1' +else + exit 98 +fi +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + fake_date.chmod(0o755) + result = subprocess.run( + ["bash", "-c", wake_failed_required_runs_script()], + env={ + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ['PATH']}", + "GH_TOKEN": "token", + "WAKE_TOKEN_SOURCE": "PR_REVIEW_MERGE_TOKEN", + "GH_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "PR_HEAD_SHA": HEAD, + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 1 + assert "More than 1,000 pull_request_target runs share one second" in result.stderr + + +def test_dispatch_wake_rejects_paginated_inventory_truncation(tmp_path: Path) -> None: + """Compare API total_count with collected rows before selecting mutations.""" + fake_gh = tmp_path / "gh" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +if [[ "$*" == "api repos/owner/repo/pulls/7" ]]; then + jq -cn --arg head "$PR_HEAD_SHA" '{state:"open",draft:false,created_at:"2026-09-30T00:00:00Z",head:{sha:$head}}' +elif [[ "$*" == *"actions/runs?"* && "$*" == *"per_page=1" ]]; then + printf '%s' '{"total_count":2,"workflow_runs":[]}' +elif [[ "$*" == *"api --paginate"* && "$*" == *"per_page=100"* ]]; then + printf '%s' "$ONE_RUN" +else + exit 97 +fi +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + result = subprocess.run( + ["bash", "-c", wake_failed_required_runs_script()], + env={ + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ['PATH']}", + "GH_TOKEN": "token", + "WAKE_TOKEN_SOURCE": "PR_REVIEW_MERGE_TOKEN", + "GH_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "PR_HEAD_SHA": HEAD, + "ONE_RUN": json.dumps({"workflow_runs": [required_run(41)]}), + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 1 + assert "inventory was truncated: expected=2 collected=1" in result.stderr def test_native_cancellation_runs_before_runner_admission() -> None: diff --git a/tests/test_opencode_required_verdict_regression.py b/tests/test_opencode_required_verdict_regression.py index c764ad0ad2..9a080bb915 100644 --- a/tests/test_opencode_required_verdict_regression.py +++ b/tests/test_opencode_required_verdict_regression.py @@ -52,6 +52,26 @@ def admission_script() -> str: return textwrap.dedent(step.split(" run: |\n", 1)[1].split("\n\n changed-scope:", 1)[0]) +def central_single_flight_script() -> str: + """Extract the receiver's deterministic exact-head winner selection.""" + workflow = DISPATCH_WORKFLOW.read_text(encoding="utf-8") + step = workflow.split( + " - name: Admit one exact-head central dispatch\n", 1 + )[1] + block = step.split(" run: |\n", 1)[1].split("\n\n - name:", 1)[0] + return textwrap.dedent(block) + + +def central_authorization_script() -> str: + """Extract the unprivileged authorization boundary before token exchange.""" + workflow = DISPATCH_WORKFLOW.read_text(encoding="utf-8") + step = workflow.split( + " - name: Authorize repository dispatch envelope\n", 1 + )[1] + block = step.split(" run: |\n", 1)[1].split("\n\n - name:", 1)[0] + return textwrap.dedent(block) + + def test_stale_opencode_event_never_reaches_review_concurrency(tmp_path: Path) -> None: """A delayed old synchronize event is retired by live-head admission.""" fake_gh = tmp_path / "gh" @@ -82,16 +102,677 @@ 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_never_uses_lossy_native_concurrency() -> None: + """The receiver must not let GitHub replace a pending exact-head run.""" 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) - assert dispatched.index("validate-pr-metadata:") < dispatched.index(" concurrency:") + header = dispatched.split("permissions:", 1)[0] + review_job = dispatched.split("\n opencode-review-target:\n", 1)[1] + required_job = required.split("\n opencode-review-target:\n", 1)[1] + assert not re.search(r"(?m)^concurrency:", header) + assert not re.search(r"(?m)^ concurrency:", review_job) + required_permissions = required_job.split(" permissions:\n", 1)[1].split( + " steps:", 1 + )[0] + assert "actions: write" not in required_permissions + admission_job = dispatched.split("\n admit-exact-head-dispatch:\n", 1)[1].split( + "\n validate-pr-metadata:\n", 1 + )[0] + validation_job = dispatched.split("\n validate-pr-metadata:\n", 1)[1].split( + "\n coverage-evidence:\n", 1 + )[0] + assert "contents: write" in admission_job + assert "contents: write" not in validation_job + assert "contents: read" in validation_job + assert "admitted: ${{ steps.single_flight.outputs.admitted }}" in admission_job + assert "needs.admit-exact-head-dispatch.outputs.admitted == 'true'" in dispatched + assert "pull_request_review:" not in required + assert "Wake every failed exact-head Required OpenCode workflow" in dispatched + + +@pytest.mark.parametrize( + ("override", "error_text"), + ( + ({"DISPATCH_SENDER": "untrusted"}, "rejected actor="), + ( + {"TARGET_REPOSITORY": "ContextualWisdomLab/unlisted"}, + "rejected target=", + ), + ({"SUPPLIED_HEAD_SHA": "mutable"}, "malformed PR identity metadata"), + ({"DRAFT_REVIEW_ONLY": "yes"}, "malformed draft-review authority"), + ), +) +def test_central_authorization_rejects_before_any_token_or_lease_access( + tmp_path: Path, override: dict[str, str], error_text: str +) -> None: + """Untrusted envelopes stop before OIDC exchange or Contents API mutation.""" + workflow = DISPATCH_WORKFLOW.read_text(encoding="utf-8") + authorize = workflow.index( + " - name: Authorize repository dispatch envelope\n" + ) + exchange = workflow.index( + " - name: Exchange OpenCode app token for target repository metadata reads\n" + ) + lease = workflow.index(" - name: Admit one exact-head central dispatch\n") + assert authorize < exchange < lease + + calls = tmp_path / "external-calls" + for command in ("curl", "gh"): + fake_command = tmp_path / command + fake_command.write_text( + "#!/usr/bin/env bash\nprintf '%s\\n' \"$0 $*\" >>\"$EXTERNAL_CALLS\"\nexit 99\n", + encoding="utf-8", + ) + fake_command.chmod(0o755) + env = { + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ.get('PATH', '')}", + "EXTERNAL_CALLS": str(calls), + "EVENT_NAME": "repository_dispatch", + "DISPATCH_ACTOR": "opencode-agent[bot]", + "DISPATCH_SENDER": "opencode-agent[bot]", + "ALLOWED_DISPATCH_ACTOR": "opencode-agent[bot]", + "ALLOWED_DISPATCH_TARGETS": "ContextualWisdomLab/example", + "TARGET_REPOSITORY": "ContextualWisdomLab/example", + "PR_NUMBER": "7", + "SUPPLIED_BASE_REF": "main", + "SUPPLIED_BASE_SHA": "b" * 40, + "SUPPLIED_HEAD_REF": "feature", + "SUPPLIED_HEAD_SHA": HEAD, + "DRAFT_REVIEW_ONLY": "false", + **override, + } + result = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", central_authorization_script()], + env=env, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 1 + assert error_text in result.stdout + assert not calls.exists() + + +def test_central_admission_accepts_only_an_exact_authorized_draft_marker( + tmp_path: Path, +) -> None: + """An explicit Draft review reaches lease admission only with its live marker.""" + calls = tmp_path / "calls" + marker_name = f"cwl-draft-review-request-owner-repo-7-{HEAD}" + fake_gh = tmp_path / "gh" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +printf '%s\n' "$*" >>"$CALLS" +if [[ "$*" == "api repos/owner/repo/pulls/7" ]]; then + jq -cn --arg base "$SUPPLIED_BASE_SHA" --arg head "$HEAD_SHA" \ + '{state:"open",draft:true,base:{ref:"main",sha:$base,repo:{full_name:"owner/repo"}},head:{ref:"feature",sha:$head,repo:{full_name:"owner/repo"}}}' +elif [[ "$*" == "api repos/ContextualWisdomLab/.github/actions/artifacts?name=$MARKER_NAME&per_page=100" ]]; then + jq -cn --arg name "$MARKER_NAME" '{total_count:1,artifacts:[{id:91,name:$name,expired:false}]}' +elif [[ "$*" == *"git/ref/heads/opencode-dispatch-leases"* ]]; then + printf '{}' +else + exit 97 +fi +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + result = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", central_single_flight_script()], + env={ + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ.get('PATH', '')}", + "CALLS": str(calls), + "MARKER_NAME": marker_name, + "GITHUB_OUTPUT": str(tmp_path / "github-output"), + "GITHUB_REPOSITORY": "ContextualWisdomLab/.github", + "GITHUB_RUN_ID": "42", + "GITHUB_SHA": "a" * 40, + "TARGET_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "HEAD_SHA": HEAD, + "SUPPLIED_BASE_REF": "main", + "SUPPLIED_BASE_SHA": "b" * 40, + "SUPPLIED_HEAD_REF": "feature", + "SUPPLIED_HEAD_SHA": HEAD, + "DRAFT_REVIEW_ONLY": "true", + "GH_TOKEN": "token", + "TARGET_READ_TOKEN": "target-token", + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 1 + assert "Central OpenCode lease branch identity was malformed" in result.stdout + assert calls.read_text(encoding="utf-8").splitlines()[:3] == [ + "api repos/owner/repo/pulls/7", + ( + "api repos/ContextualWisdomLab/.github/actions/artifacts?" + f"name={marker_name}&per_page=100" + ), + "api repos/ContextualWisdomLab/.github/git/ref/heads/opencode-dispatch-leases", + ] + + +@pytest.mark.parametrize( + "artifact_payload", + ( + '{"total_count":1,"artifacts":[{"id":91,"name":"wrong","expired":false}]}', + '{"total_count":1,"artifacts":[{"id":91,"name":"MARKER","expired":true}]}', + '{"total_count":"1","artifacts":[]}', + ), +) +def test_central_admission_rejects_draft_without_a_valid_live_marker( + tmp_path: Path, artifact_payload: str +) -> None: + """Malformed, mismatched, or expired Draft authority fails before lease access.""" + calls = tmp_path / "calls" + marker_name = f"cwl-draft-review-request-owner-repo-7-{HEAD}" + fake_gh = tmp_path / "gh" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +printf '%s\n' "$*" >>"$CALLS" +if [[ "$*" == "api repos/owner/repo/pulls/7" ]]; then + jq -cn --arg base "$SUPPLIED_BASE_SHA" --arg head "$HEAD_SHA" \ + '{state:"open",draft:true,base:{ref:"main",sha:$base,repo:{full_name:"owner/repo"}},head:{ref:"feature",sha:$head,repo:{full_name:"owner/repo"}}}' +elif [[ "$*" == *"/actions/artifacts?name="* ]]; then + printf '%s' "$ARTIFACT_PAYLOAD" | sed "s/MARKER/$MARKER_NAME/g" +else + exit 97 +fi +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + result = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", central_single_flight_script()], + env={ + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ.get('PATH', '')}", + "CALLS": str(calls), + "MARKER_NAME": marker_name, + "ARTIFACT_PAYLOAD": artifact_payload, + "GITHUB_OUTPUT": str(tmp_path / "github-output"), + "GITHUB_REPOSITORY": "ContextualWisdomLab/.github", + "GITHUB_RUN_ID": "42", + "GITHUB_SHA": "a" * 40, + "TARGET_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "HEAD_SHA": HEAD, + "SUPPLIED_BASE_REF": "main", + "SUPPLIED_BASE_SHA": "b" * 40, + "SUPPLIED_HEAD_REF": "feature", + "SUPPLIED_HEAD_SHA": HEAD, + "DRAFT_REVIEW_ONLY": "true", + "GH_TOKEN": "token", + "TARGET_READ_TOKEN": "target-token", + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 1 + assert "authority changed before atomic OpenCode admission" in result.stdout + assert all("opencode-dispatch-leases" not in call for call in calls.read_text().splitlines()) + + +def test_central_admission_revalidates_draft_marker_before_lease_mutation( + tmp_path: Path, +) -> None: + """A marker that expires after admission cannot authorize a Contents write.""" + calls = tmp_path / "calls" + marker_reads = tmp_path / "marker-reads" + marker_name = f"cwl-draft-review-request-owner-repo-7-{HEAD}" + fake_gh = tmp_path / "gh" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +printf '%s\n' "$*" >>"$CALLS" +if [[ "$*" == "api repos/owner/repo/pulls/7" ]]; then + jq -cn --arg base "$SUPPLIED_BASE_SHA" --arg head "$HEAD_SHA" \ + '{state:"open",draft:true,base:{ref:"main",sha:$base,repo:{full_name:"owner/repo"}},head:{ref:"feature",sha:$head,repo:{full_name:"owner/repo"}}}' +elif [[ "$*" == *"/actions/artifacts?name="* ]]; then + read_count=0 + [[ ! -f "$MARKER_READS" ]] || read_count="$(cat "$MARKER_READS")" + read_count=$((read_count + 1)) + printf '%s' "$read_count" >"$MARKER_READS" + if [[ "$read_count" -eq 1 ]]; then marker_expired=false; else marker_expired=true; fi + jq -cn --arg name "$MARKER_NAME" --argjson expired "$marker_expired" \ + '{total_count:1,artifacts:[{id:91,name:$name,expired:$expired}]}' +elif [[ "$*" == *"git/ref/heads/opencode-dispatch-leases"* ]]; then + jq -cn --arg sha "$GITHUB_SHA" '{object:{type:"commit",sha:$sha}}' +elif [[ "$*" == *"contents/opencode-dispatch-leases/"* && "$*" != *"--method PUT"* ]]; then + exit 1 +else + exit 97 +fi +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + result = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", central_single_flight_script()], + env={ + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ.get('PATH', '')}", + "CALLS": str(calls), + "MARKER_READS": str(marker_reads), + "MARKER_NAME": marker_name, + "GITHUB_OUTPUT": str(tmp_path / "github-output"), + "GITHUB_REPOSITORY": "ContextualWisdomLab/.github", + "GITHUB_RUN_ID": "42", + "GITHUB_SHA": "a" * 40, + "TARGET_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "HEAD_SHA": HEAD, + "SUPPLIED_BASE_REF": "main", + "SUPPLIED_BASE_SHA": "b" * 40, + "SUPPLIED_HEAD_REF": "feature", + "SUPPLIED_HEAD_SHA": HEAD, + "DRAFT_REVIEW_ONLY": "true", + "GH_TOKEN": "token", + "TARGET_READ_TOKEN": "target-token", + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 1 + assert "authority changed before the atomic OpenCode lease mutation" in result.stdout + assert marker_reads.read_text(encoding="utf-8") == "2" + assert all("--method PUT" not in call for call in calls.read_text().splitlines()) + + +@pytest.mark.parametrize( + "live_override", + ( + {"state": "closed"}, + {"draft": True}, + {"base": {"ref": "release", "sha": "b" * 40, "repo": {"full_name": "owner/repo"}}}, + {"base": {"ref": "main", "sha": "d" * 40, "repo": {"full_name": "owner/repo"}}}, + {"head": {"ref": "other", "sha": HEAD, "repo": {"full_name": "owner/repo"}}}, + {"head": {"ref": "feature", "sha": "d" * 40, "repo": {"full_name": "owner/repo"}}}, + ), +) +def test_central_admission_rejects_changed_live_identity_before_contents_mutation( + tmp_path: Path, live_override: dict[str, object] +) -> None: + """Every live state/base/head mismatch fails before the central lease write.""" + live_pr: dict[str, object] = { + "state": "open", + "draft": False, + "base": { + "ref": "main", + "sha": "b" * 40, + "repo": {"full_name": "owner/repo"}, + }, + "head": { + "ref": "feature", + "sha": HEAD, + "repo": {"full_name": "owner/repo"}, + }, + } + live_pr.update(live_override) + calls = tmp_path / "calls" + fake_gh = tmp_path / "gh" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +printf '%s\n' "$*" >>"$CALLS" +if [[ "$*" == "api repos/owner/repo/pulls/7" ]]; then + printf '%s' "$LIVE_PR" + exit 0 +fi +exit 97 +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + result = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", central_single_flight_script()], + env={ + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ.get('PATH', '')}", + "CALLS": str(calls), + "LIVE_PR": json.dumps(live_pr), + "GITHUB_OUTPUT": str(tmp_path / "github-output"), + "GITHUB_RUN_ID": "42", + "GITHUB_SHA": "a" * 40, + "TARGET_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "HEAD_SHA": HEAD, + "SUPPLIED_BASE_REF": "main", + "SUPPLIED_BASE_SHA": "b" * 40, + "SUPPLIED_HEAD_REF": "feature", + "SUPPLIED_HEAD_SHA": HEAD, + "GH_TOKEN": "token", + "TARGET_READ_TOKEN": "target-token", + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 1 + assert "authority changed before atomic OpenCode admission" in result.stdout + assert calls.read_text().splitlines() == ["api repos/owner/repo/pulls/7"] + + +def test_central_admission_rejects_draft_request_after_pr_becomes_ready( + tmp_path: Path, +) -> None: + """Draft-only authority cannot survive a Draft-to-Ready state transition.""" + calls = tmp_path / "calls" + fake_gh = tmp_path / "gh" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +printf '%s\n' "$*" >>"$CALLS" +if [[ "$*" == "api repos/owner/repo/pulls/7" ]]; then + jq -cn --arg base "$SUPPLIED_BASE_SHA" --arg head "$HEAD_SHA" \ + '{state:"open",draft:false,base:{ref:"main",sha:$base,repo:{full_name:"owner/repo"}},head:{ref:"feature",sha:$head,repo:{full_name:"owner/repo"}}}' + exit 0 +fi +exit 97 +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + result = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", central_single_flight_script()], + env={ + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ.get('PATH', '')}", + "CALLS": str(calls), + "GITHUB_OUTPUT": str(tmp_path / "github-output"), + "GITHUB_RUN_ID": "42", + "GITHUB_SHA": "a" * 40, + "TARGET_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "HEAD_SHA": HEAD, + "SUPPLIED_BASE_REF": "main", + "SUPPLIED_BASE_SHA": "b" * 40, + "SUPPLIED_HEAD_REF": "feature", + "SUPPLIED_HEAD_SHA": HEAD, + "DRAFT_REVIEW_ONLY": "true", + "GH_TOKEN": "token", + "TARGET_READ_TOKEN": "target-token", + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 1 + assert "authority changed before atomic OpenCode admission" in result.stdout + assert calls.read_text().splitlines() == ["api repos/owner/repo/pulls/7"] + + +@pytest.mark.parametrize( + ( + "lease_state", + "owner_run_id", + "owner_head", + "owner_status", + "receipt_present", + "expected_admitted", + ), + ( + ("absent", 41, HEAD, "", False, True), + ("present", 41, HEAD, "in_progress", False, False), + ("present", 41, HEAD, "completed", False, True), + ("present", 41, HEAD, "completed", True, False), + ("present", 42, HEAD, "in_progress", False, True), + ("present", 42, HEAD, "in_progress", True, False), + ("present", 41, "d" * 40, "in_progress", False, True), + ("present", 43, "d" * 40, "in_progress", False, True), + ), +) +def test_central_dispatch_single_flight_uses_atomic_contents_lease( + tmp_path: Path, + lease_state: str, + owner_run_id: int, + owner_head: str, + owner_status: str, + receipt_present: bool, + expected_admitted: bool, +) -> None: + """One atomic lease owner reaches expensive work; terminal leases recover.""" + fake_gh = tmp_path / "gh" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +printf '%s\n' "$*" >>"$FAKE_CALLS" +if [[ "$*" == *"git/ref/heads/opencode-dispatch-leases"* ]]; then + printf '{"object":{"type":"commit","sha":"%s"}}' "$GITHUB_SHA" +elif [[ "$*" == *"repos/owner/repo/pulls/7"* && "$*" != *"/reviews"* ]]; then + jq -cn --arg base "$SUPPLIED_BASE_SHA" --arg head "$HEAD_SHA" \ + '{state:"open",draft:false,base:{ref:"main",sha:$base,repo:{full_name:"owner/repo"}},head:{ref:"feature",sha:$head,repo:{full_name:"owner/repo"}}}' +elif [[ "$*" == *"opencode_review_receipt_gate.py?ref="* ]]; then + base64 <"$RECEIPT_HELPER_SOURCE" | tr -d '\n' +elif [[ "$*" == *"repos/owner/repo/pulls/7/reviews"* ]]; then + if [[ "$FAKE_RECEIPT_PRESENT" == "true" ]]; then + jq -cn --arg head "$HEAD_SHA" '[[{id:91,user:{login:"opencode-agent[bot]"},state:"CHANGES_REQUESTED",commit_id:$head,body:"## Pull request overview"}]]' + else + printf '[[]]' + fi +elif [[ "$*" == *"contents/opencode-dispatch-leases/"* && "$*" != *"--method PUT"* ]]; then + if [[ "$FAKE_LEASE_STATE" == "absent" ]]; then exit 1; fi + owner_title="OpenCode Review Dispatch owner/repo#7@${FAKE_OWNER_HEAD}" + content="$(jq -cn --argjson owner "$FAKE_OWNER_ID" --arg title "$owner_title" --arg head "$FAKE_OWNER_HEAD" \ + '{owner_run_id:$owner,exact_title:$title,head_sha:$head}')" + encoded="$(printf '%s' "$content" | base64 | tr -d '\n')" + jq -cn --arg encoded "$encoded" --arg sha "$(printf 'b%.0s' {1..40})" \ + '{sha:$sha,encoding:"base64",content:$encoded}' +elif [[ "$*" == *"actions/runs/"* ]]; then + owner_title="OpenCode Review Dispatch owner/repo#7@${FAKE_OWNER_HEAD}" + jq -cn --argjson owner "$FAKE_OWNER_ID" --arg status "$FAKE_OWNER_STATUS" --arg title "$owner_title" \ + '{id:$owner,path:".github/workflows/opencode-review-dispatch.yml",event:"repository_dispatch",display_title:$title,status:$status}' +elif [[ "$*" == *"contents/opencode-dispatch-leases/"* && "$*" == *"--method PUT"* ]]; then + jq -cn --arg sha "$(printf 'c%.0s' {1..40})" '{content:{sha:$sha}}' +else + printf 'unexpected gh call: %s\n' "$*" >&2 + exit 97 +fi +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + output = tmp_path / "github-output" + calls = tmp_path / "calls" + result = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", central_single_flight_script()], + env={ + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ.get('PATH', '')}", + "FAKE_CALLS": str(calls), + "FAKE_LEASE_STATE": lease_state, + "FAKE_OWNER_ID": str(owner_run_id), + "FAKE_OWNER_HEAD": owner_head, + "FAKE_OWNER_STATUS": owner_status, + "FAKE_RECEIPT_PRESENT": str(receipt_present).lower(), + "RECEIPT_HELPER_SOURCE": str(RECEIPT_HELPER.resolve()), + "GITHUB_OUTPUT": str(output), + "GITHUB_RUN_ID": "42", + "GITHUB_SHA": "a" * 40, + "TARGET_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "HEAD_SHA": HEAD, + "SUPPLIED_BASE_REF": "main", + "SUPPLIED_BASE_SHA": "b" * 40, + "SUPPLIED_HEAD_REF": "feature", + "SUPPLIED_HEAD_SHA": HEAD, + "EXACT_TITLE": f"OpenCode Review Dispatch owner/repo#7@{HEAD}", + "GH_TOKEN": "token", + "TARGET_READ_TOKEN": "target-token", + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + output_lines = output.read_text(encoding="utf-8").splitlines() + assert output_lines[0] == f"admitted={str(expected_admitted).lower()}" + expected_mutation = owner_run_id != 42 and not ( + owner_head == HEAD and owner_status == "in_progress" + ) + assert any("--method PUT" in call for call in calls.read_text().splitlines()) is expected_mutation + + +def test_atomic_contents_lease_admits_one_concurrent_receiver(tmp_path: Path) -> None: + """Two simultaneous cache misses converge on one compare-and-swap owner.""" + fake_gh = tmp_path / "gh" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +if [[ "$*" == *"git/ref/heads/opencode-dispatch-leases"* ]]; then + printf '{"object":{"type":"commit","sha":"%s"}}' "$GITHUB_SHA" +elif [[ "$*" == *"repos/owner/repo/pulls/7"* && "$*" != *"/reviews"* ]]; then + jq -cn --arg base "$SUPPLIED_BASE_SHA" --arg head "$HEAD_SHA" \ + '{state:"open",draft:false,base:{ref:"main",sha:$base,repo:{full_name:"owner/repo"}},head:{ref:"feature",sha:$head,repo:{full_name:"owner/repo"}}}' +elif [[ "$*" == *"opencode_review_receipt_gate.py?ref="* ]]; then + base64 <"$RECEIPT_HELPER_SOURCE" | tr -d '\n' +elif [[ "$*" == *"repos/owner/repo/pulls/7/reviews"* ]]; then + printf '[[]]' +elif [[ "$*" == *"contents/opencode-dispatch-leases/"* && "$*" != *"--method PUT"* ]]; then + [[ -f "$LEASE_OWNER" ]] || exit 1 + owner="$(cat "$LEASE_OWNER")" + content="$(jq -cn --argjson owner "$owner" --arg title "$EXACT_TITLE" --arg head "$HEAD_SHA" \ + '{owner_run_id:$owner,exact_title:$title,head_sha:$head}')" + encoded="$(printf '%s' "$content" | base64 | tr -d '\n')" + jq -cn --arg encoded "$encoded" --arg sha "$(printf 'd%.0s' {1..40})" \ + '{sha:$sha,encoding:"base64",content:$encoded}' +elif [[ "$*" == *"actions/runs/"* ]]; then + [[ "$*" =~ actions/runs/([0-9]+) ]] || exit 96 + owner="${BASH_REMATCH[1]}" + jq -cn --argjson owner "$owner" --arg title "$EXACT_TITLE" \ + '{id:$owner,path:".github/workflows/opencode-review-dispatch.yml",event:"repository_dispatch",display_title:$title,status:"in_progress"}' +elif [[ "$*" == *"contents/opencode-dispatch-leases/"* && "$*" == *"--method PUT"* ]]; then + payload="$(cat)" + if mkdir "$LEASE_MUTEX" 2>/dev/null; then + printf '%s' "$payload" | jq -r '.content' | base64 --decode | jq -r '.owner_run_id' >"$LEASE_OWNER" + jq -cn --arg sha "$(printf 'e%.0s' {1..40})" '{content:{sha:$sha}}' + else + sleep 0.05 + exit 1 + fi +else + exit 97 +fi +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + base_env = { + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ.get('PATH', '')}", + "GITHUB_SHA": "a" * 40, + "TARGET_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "HEAD_SHA": HEAD, + "SUPPLIED_BASE_REF": "main", + "SUPPLIED_BASE_SHA": "b" * 40, + "SUPPLIED_HEAD_REF": "feature", + "SUPPLIED_HEAD_SHA": HEAD, + "EXACT_TITLE": f"OpenCode Review Dispatch owner/repo#7@{HEAD}", + "GH_TOKEN": "token", + "TARGET_READ_TOKEN": "target-token", + "RECEIPT_HELPER_SOURCE": str(RECEIPT_HELPER.resolve()), + "LEASE_MUTEX": str(tmp_path / "lease-mutex"), + "LEASE_OWNER": str(tmp_path / "lease-owner"), + } + processes: list[tuple[subprocess.Popen[str], Path]] = [] + for run_id in (41, 42): + output = tmp_path / f"output-{run_id}" + process = subprocess.Popen( # noqa: S603 + [shutil.which("bash") or "/bin/bash", "-c", central_single_flight_script()], + env={ + **base_env, + "GITHUB_RUN_ID": str(run_id), + "GITHUB_OUTPUT": str(output), + }, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + processes.append((process, output)) + admissions: list[str] = [] + for process, output in processes: + stdout, stderr = process.communicate(timeout=10) + assert process.returncode == 0, f"{stdout}\n{stderr}" + admissions.append(output.read_text(encoding="utf-8").splitlines()[0]) + assert sorted(admissions) == ["admitted=false", "admitted=true"] + + +def test_atomic_lease_branch_initialization_accepts_a_concurrent_creator( + tmp_path: Path, +) -> None: + """A ref-create conflict must re-read the exact central lease branch.""" + fake_gh = tmp_path / "gh" + ref_reads = tmp_path / "ref-reads" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +if [[ "$*" == *"git/ref/heads/opencode-dispatch-leases"* ]]; then + reads=0 + [[ ! -f "$REF_READS" ]] || reads="$(cat "$REF_READS")" + reads=$((reads + 1)) + printf '%s' "$reads" >"$REF_READS" + [[ "$reads" -gt 1 ]] || exit 1 + printf '{"object":{"type":"commit","sha":"%s"}}' "$GITHUB_SHA" +elif [[ "$*" == *"repos/owner/repo/pulls/7"* && "$*" != *"/reviews"* ]]; then + jq -cn --arg base "$SUPPLIED_BASE_SHA" --arg head "$HEAD_SHA" \ + '{state:"open",draft:false,base:{ref:"main",sha:$base,repo:{full_name:"owner/repo"}},head:{ref:"feature",sha:$head,repo:{full_name:"owner/repo"}}}' +elif [[ "$*" == *"opencode_review_receipt_gate.py?ref="* ]]; then + base64 <"$RECEIPT_HELPER_SOURCE" | tr -d '\n' +elif [[ "$*" == *"repos/owner/repo/pulls/7/reviews"* ]]; then + printf '[[]]' +elif [[ "$*" == *"git/refs"* && "$*" == *"--method POST"* ]]; then + cat >/dev/null + exit 1 +elif [[ "$*" == *"contents/opencode-dispatch-leases/"* && "$*" != *"--method PUT"* ]]; then + exit 1 +elif [[ "$*" == *"contents/opencode-dispatch-leases/"* && "$*" == *"--method PUT"* ]]; then + cat >/dev/null + jq -cn --arg sha "$(printf 'f%.0s' {1..40})" '{content:{sha:$sha}}' +else + exit 97 +fi +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + output = tmp_path / "github-output" + result = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", central_single_flight_script()], + env={ + **os.environ, + "PATH": f"{tmp_path}{os.pathsep}{os.environ.get('PATH', '')}", + "REF_READS": str(ref_reads), + "GITHUB_OUTPUT": str(output), + "GITHUB_RUN_ID": "42", + "GITHUB_SHA": "a" * 40, + "TARGET_REPOSITORY": "owner/repo", + "PR_NUMBER": "7", + "HEAD_SHA": HEAD, + "SUPPLIED_BASE_REF": "main", + "SUPPLIED_BASE_SHA": "b" * 40, + "SUPPLIED_HEAD_REF": "feature", + "SUPPLIED_HEAD_SHA": HEAD, + "GH_TOKEN": "token", + "TARGET_READ_TOKEN": "target-token", + "RECEIPT_HELPER_SOURCE": str(RECEIPT_HELPER.resolve()), + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + assert output.read_text(encoding="utf-8").splitlines() == ["admitted=true"] + assert ref_reads.read_text(encoding="utf-8") == "2" def review(*, state: str, commit_id: str = HEAD, body: str = "") -> dict[str, object]: @@ -104,6 +785,22 @@ def review(*, state: str, commit_id: str = HEAD, body: str = "") -> dict[str, ob } +def active_dispatch( + *, status: str, head_sha: str = HEAD, run_id: int = 42 +) -> dict[str, object]: + """Build one complete central OpenCode workflow-run identity.""" + return { + "id": run_id, + "name": "OpenCode Review Dispatch", + "display_title": f"OpenCode Review Dispatch owner/repo#7@{head_sha}", + "path": ".github/workflows/opencode-review-dispatch.yml", + "event": "repository_dispatch", + "status": status, + "head_sha": "c" * 40, + "pull_requests": [], + } + + def runtime_verdict(reviews: list[dict[str, object]], head_sha: str = HEAD) -> str: """Execute the jq program embedded in the required workflow.""" jq = shutil.which("jq") @@ -311,6 +1008,8 @@ def test_required_workflow_cannot_succeed_with_an_echo_only_placeholder() -> Non assert "id-token: write" in target_job.split(" steps:\n", 1)[0] assert 'event_type:"opencode-review"' in workflow assert "required_run_id:$required_run_id" in workflow + assert "--argjson draft_review_only false" in workflow + assert "draft_review_only:$draft_review_only" in workflow dispatch_step = target_job.split( " - name: Request current-head OpenCode review execution", 1 )[1].split(" - name: Fail closed", 1)[0] @@ -669,19 +1368,70 @@ def test_fail_closed_step_checks_once_for_a_non_draft_pr(tmp_path: Path) -> None @pytest.mark.parametrize( - ("reviews", "dispatches"), ( - ([{"id": 7, **review(state="APPROVED", body="## Verdict\nApprove")}], 0), - ([{"id": 8, **review(state="CHANGES_REQUESTED", body="## Verdict\nRequest changes")}], 0), - ([], 1), - ([{"id": 9, **review(state="APPROVED", commit_id="b" * 40, body="## Verdict\nApprove")}], 1), - ([{"id": 10, **review(state="APPROVED", body="## Pull request overview\n\ndeterministic fallback approval")}], 1), + "reviews", + "active_runs", + "later_active_runs", + "revalidated_head", + "lookup_failure", + "expected_returncode", + "dispatches", + ), + ( + ([{"id": 7, **review(state="APPROVED", body="## Verdict\nApprove")}], [], None, HEAD, False, 0, 0), + ([{"id": 8, **review(state="CHANGES_REQUESTED", body="## Verdict\nRequest changes")}], [], None, HEAD, False, 0, 0), + ([], [], None, HEAD, False, 0, 1), + ([], [], None, "e" * 40, False, 0, 0), + ([], [active_dispatch(status="queued")], None, HEAD, False, 0, 0), + ([], [active_dispatch(status="in_progress")], None, HEAD, False, 0, 0), + ([], [active_dispatch(status="requested")], None, HEAD, False, 0, 0), + ([], [active_dispatch(status="waiting")], None, HEAD, False, 0, 0), + ([], [active_dispatch(status="pending")], None, HEAD, False, 0, 0), + ( + [], + [ + active_dispatch(status="queued"), + active_dispatch(status="queued", head_sha="d" * 40, run_id=48), + ], + None, + HEAD, + False, + 0, + 0, + ), + ([], [active_dispatch(status="queued", head_sha="d" * 40)], None, HEAD, False, 0, 1), + ([], [active_dispatch(status="queued", head_sha="d" * 40, run_id=45)], None, HEAD, False, 1, 0), + ([], [active_dispatch(status="queued", head_sha="d" * 40, run_id=46)], None, HEAD, False, 1, 0), + ([], [active_dispatch(status="queued", head_sha="d" * 40, run_id=47)], None, HEAD, False, 1, 0), + ([], [active_dispatch(status="queued", head_sha="d" * 40, run_id=49)], None, HEAD, False, 0, 1), + ([], [active_dispatch(status="queued", head_sha="d" * 40, run_id=50)], None, HEAD, False, 1, 0), + ([], [active_dispatch(status="queued", head_sha="d" * 40, run_id=51)], None, "e" * 40, False, 0, 0), + ([], [], [active_dispatch(status="pending")], HEAD, False, 0, 0), + ( + [], + [{"id": 44, "path": ".github/workflows/opencode-review-dispatch.yml", "event": "repository_dispatch", "status": "queued"}], + None, + HEAD, + False, + 1, + 0, + ), + ([], [], None, HEAD, True, 1, 0), + ([{"id": 9, **review(state="APPROVED", commit_id="b" * 40, body="## Verdict\nApprove")}], [], None, HEAD, False, 0, 1), + ([{"id": 10, **review(state="APPROVED", body="## Pull request overview\n\ndeterministic fallback approval")}], [], None, HEAD, False, 0, 1), ), ) def test_scheduler_wake_reuses_trusted_receipt_predicate( - tmp_path: Path, reviews: list[dict[str, object]], dispatches: int + tmp_path: Path, + reviews: list[dict[str, object]], + active_runs: list[dict[str, object]], + later_active_runs: list[dict[str, object]] | None, + revalidated_head: str, + lookup_failure: bool, + expected_returncode: int, + dispatches: int, ) -> None: - """Only missing, stale, or fallback-only evidence wakes the scheduler.""" + """Only missing, stale, inactive evidence wakes one exact-head execution.""" fake_bin = tmp_path / "bin" fake_bin.mkdir() calls = tmp_path / "dispatches" @@ -690,11 +1440,61 @@ def test_scheduler_wake_reuses_trusted_receipt_predicate( """#!/usr/bin/env bash set -euo pipefail if [[ "$*" == "api repos/owner/repo/pulls/7" ]]; then - printf '%s' "$LIVE_PR_JSON" + count=0 + [[ ! -f "$LIVE_PR_CALLS" ]] || count="$(cat "$LIVE_PR_CALLS")" + count=$((count + 1)) + printf '%s' "$count" >"$LIVE_PR_CALLS" + if [[ "$count" -eq 1 ]]; then + printf '%s' "$LIVE_PR_JSON" + else + printf '%s' "$REVALIDATED_LIVE_PR_JSON" + fi 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" +elif [[ "$*" == *"actions/workflows/opencode-review-dispatch.yml/runs"* ]]; then + if [[ "$FAKE_ACTIVE_LOOKUP_FAILURE" == "true" ]]; then + exit 19 + fi + count=0 + [[ ! -f "$ACTIVE_RUN_CALLS" ]] || count="$(cat "$ACTIVE_RUN_CALLS")" + count=$((count + 1)) + printf '%s' "$count" >"$ACTIVE_RUN_CALLS" + runs_request="${!#}" + active_status="${runs_request#*status=}" + active_status="${active_status%%&*}" + if [[ "$count" -le 5 ]]; then + active_source="$FAKE_ACTIVE_RUNS" + else + active_source="$FAKE_LATER_ACTIVE_RUNS" + fi + active_selection="$(jq -c --arg status "$active_status" '[.[] | select(.status == $status)]' <<<"$active_source")" + printf '{"workflow_runs":%s}' "$active_selection" +elif [[ "$*" == *"repos/ContextualWisdomLab/.github/actions/runs/"*"/cancel"* ]]; then + [[ "$*" =~ actions/runs/([0-9]+) ]] || exit 90 + run_id="${BASH_REMATCH[1]}" + printf '%s\n' "$run_id" >>"$CANCEL_CALLS" + [[ "$run_id" != "45" ]] || exit 19 +elif [[ "$*" == *"repos/ContextualWisdomLab/.github/actions/runs/"* ]]; then + [[ "$*" =~ actions/runs/([0-9]+) ]] || exit 91 + run_id="${BASH_REMATCH[1]}" + count_file="$RUN_STATE_CALLS/$run_id" + count=0 + [[ ! -f "$count_file" ]] || count="$(cat "$count_file")" + count=$((count + 1)) + printf '%s' "$count" >"$count_file" + if [[ "$run_id" == "46" ]]; then + printf 'mystery\t' + elif [[ "$run_id" == "47" ]]; then + printf 'queued\t' + elif [[ "$run_id" == "49" && "$count" -eq 1 ]]; then + printf 'in_progress\t' + elif [[ "$run_id" == "50" ]]; then + printf 'completed\tsuccess' + else + printf 'completed\tcancelled' + fi elif [[ "$*" == *"repos/ContextualWisdomLab/.github/dispatches"* ]]; then cat >/dev/null printf 'dispatch\n' >>"$DISPATCH_CALLS" @@ -711,12 +1511,25 @@ def test_scheduler_wake_reuses_trusted_receipt_predicate( encoding="utf-8", ) fake_curl.chmod(0o755) + fake_sleep = fake_bin / "sleep" + fake_sleep.write_text("#!/usr/bin/env bash\nexit 0\n", encoding="utf-8") + fake_sleep.chmod(0o755) + run_state_calls = tmp_path / "run-state-calls" + run_state_calls.mkdir() env = { **os.environ, "PATH": f"{fake_bin}{os.pathsep}{os.environ['PATH']}", "REAL_RECEIPT_HELPER": str(RECEIPT_HELPER.resolve()), "FAKE_REVIEWS": json.dumps(reviews), + "FAKE_ACTIVE_RUNS": json.dumps(active_runs), + "FAKE_LATER_ACTIVE_RUNS": json.dumps( + active_runs if later_active_runs is None else later_active_runs + ), + "FAKE_ACTIVE_LOOKUP_FAILURE": str(lookup_failure).lower(), + "ACTIVE_RUN_CALLS": str(tmp_path / "active-run-calls"), + "CANCEL_CALLS": str(tmp_path / "cancel-calls"), "DISPATCH_CALLS": str(calls), + "RUN_STATE_CALLS": str(run_state_calls), "ACTIONS_ID_TOKEN_REQUEST_TOKEN": "request", "ACTIONS_ID_TOKEN_REQUEST_URL": "https://token.example", "OIDC_AUDIENCE": "opencode-github-action", @@ -734,13 +1547,40 @@ def test_scheduler_wake_reuses_trusted_receipt_predicate( "LIVE_PR_JSON": json.dumps( {"draft": False, "head": {"sha": HEAD}, "state": "open"} ), + "REVALIDATED_LIVE_PR_JSON": json.dumps( + {"draft": False, "head": {"sha": revalidated_head}, "state": "open"} + ), + "LIVE_PR_CALLS": str(tmp_path / "live-pr-calls"), } result = subprocess.run( ["bash", "-c", request_review_script()], env=env, text=True, capture_output=True ) - assert result.returncode == 0, result.stderr + assert result.returncode == expected_returncode, result.stderr actual = calls.read_text(encoding="utf-8").count("dispatch") if calls.exists() else 0 assert actual == dispatches + cancel_calls = tmp_path / "cancel-calls" + actual_cancel_ids = ( + sorted(set(cancel_calls.read_text(encoding="utf-8").splitlines())) + if cancel_calls.exists() + else [] + ) + expected_cancel_ids = ( + [] + if revalidated_head != HEAD + else sorted( + { + str(run["id"]) + for run in [*active_runs, *(later_active_runs or [])] + if isinstance(run.get("display_title"), str) + and str(run["display_title"]).startswith( + "OpenCode Review Dispatch owner/repo#7@" + ) + and str(run["display_title"]).lower() + != f"OpenCode Review Dispatch owner/repo#7@{HEAD}".lower() + } + ) + ) + assert actual_cancel_ids == expected_cancel_ids def test_formal_receipt_wake_reruns_the_immediately_failed_required_job() -> None: @@ -754,13 +1594,14 @@ def test_formal_receipt_wake_reruns_the_immediately_failed_required_job() -> Non assert "rerun-failed-jobs" in dispatched assert "id: formal_review_receipt" in dispatched assert "steps.formal_review_receipt.outcome == 'success'" in dispatched - assert "github.event.client_payload.required_run_id != ''" in dispatched - assert 'gh api "repos/${GH_REPOSITORY}/actions/runs/${REQUIRED_RUN_ID}"' in dispatched - assert "select(.id == $run_id)" in dispatched - assert 'select(.event == "pull_request_target")' in dispatched - assert 'select(.path == ".github/workflows/opencode-review.yml")' in dispatched - assert "select(.head_sha == $head)" in dispatched - wake_step = dispatched.split("Wake exact-head required OpenCode workflow", 1)[1].split("\n\n - name:", 1)[0] + assert "actions/runs?event=pull_request_target" in dispatched + assert '.event == "pull_request_target"' in dispatched + assert '.path == ".github/workflows/opencode-review.yml"' in dispatched + assert "(.head.sha | ascii_downcase) == ($head | ascii_downcase)" in dispatched + wake_step = dispatched.split( + "Wake every failed exact-head Required OpenCode workflow", 1 + )[1].split("\n\n - name:", 1)[0] + assert "needs.validate-pr-metadata.outputs.is_draft == 'false'" in wake_step target_job = dispatched.split(" opencode-review-target:\n", 1)[1] target_permissions = target_job.split(" env:\n", 1)[0] assert "actions: write" in target_permissions @@ -771,32 +1612,81 @@ def test_formal_receipt_wake_reruns_the_immediately_failed_required_job() -> Non assert "steps.opencode_app_token.outputs.token" not in wake_step assert "WAKE_TOKEN_SOURCE" in wake_step assert '"$WAKE_TOKEN_SOURCE" = "unavailable"' in wake_step - assert "--paginate" not in wake_step - # Identity is the immutable target-repository run id plus event/path/head; - # do not depend on context-specific title or workflow_url rendering. + assert "--paginate" in wake_step + # Inventory identity is event/path/head/PR; do not depend on context-specific + # title or workflow_url rendering. + assert ".number == $pr" in wake_step assert "display_title ==" not in wake_step assert ".name | startswith(" not in wake_step assert 'workflow_url | contains("/actions/required_workflows/")' not in wake_step -def wake_selector(run: dict[str, object], *, head: str = HEAD, run_id: int = 42) -> str: - """Execute the wake step's run-validation jq program in isolation.""" +def test_authorized_draft_review_cannot_publish_approval_or_run_merge_followups() -> None: + """Review-only Draft work publishes prose but cannot create approval authority.""" + dispatched = DISPATCH_WORKFLOW.read_text(encoding="utf-8") + fast_approval = dispatched.split( + " - name: Publish central OpenCode fast approval", 1 + )[1].split(" - name: Publish OpenCode review outcome", 1)[0] + publication = dispatched.split( + " - name: Publish OpenCode review outcome", 1 + )[1].split(" - name: Enforce current-head formal OpenCode review receipt", 1)[0] + formal_receipt = dispatched.split( + " - name: Enforce current-head formal OpenCode review receipt", 1 + )[1].split(" - name: Wake every failed exact-head Required OpenCode workflow", 1)[0] + status_publication = dispatched.split( + " - name: Publish repository_dispatch OpenCode status", 1 + )[1].split(" - name: Dispatch Noema after current-head OpenCode approval", 1)[0] + noema_handoff = dispatched.split( + " - name: Dispatch Noema after current-head OpenCode approval", 1 + )[1].split(" - name: Run merge scheduler after approval", 1)[0] + merge_followup = dispatched.split( + " - name: Run merge scheduler after approval", 1 + )[1] + + assert "needs.validate-pr-metadata.outputs.is_draft == 'false'" in fast_approval + assert "PR_DRAFT: ${{ needs.validate-pr-metadata.outputs.is_draft }}" in publication + assert ( + 'if [ "$event" = "APPROVE" ] && [ "$PR_DRAFT" = "true" ]; then' + in publication + ) + assert 'event="COMMENT"' in publication + assert "needs.validate-pr-metadata.outputs.is_draft == 'false'" in formal_receipt + assert "needs.validate-pr-metadata.outputs.is_draft == 'false'" in status_publication + assert "needs.validate-pr-metadata.outputs.is_draft == 'false'" in noema_handoff + assert "needs.validate-pr-metadata.outputs.is_draft == 'false'" in merge_followup + + +def test_dispatch_preserves_draft_review_authority_type_before_validation() -> None: + """Falsy non-booleans cannot be normalized into Ready review authority.""" + dispatched = DISPATCH_WORKFLOW.read_text(encoding="utf-8") + raw_binding = "toJSON(github.event.client_payload.draft_review_only)" + assert dispatched.count(raw_binding) == 3 + assert "draft_review_only || false" not in dispatched + + +def wake_selector(run: dict[str, object], *, head: str = HEAD) -> str: + """Execute the wake inventory's fail-closed jq program in isolation.""" jq = shutil.which("jq") if jq is None: pytest.skip("jq is required to execute the production wake selector") dispatched = DISPATCH_WORKFLOW.read_text(encoding="utf-8") - marker = """jq -r --arg head "$PR_HEAD_SHA" --argjson run_id "$REQUIRED_RUN_ID" '""" + marker = """jq -c -s --arg head "$PR_HEAD_SHA" --argjson pr "$PR_NUMBER" '""" start = dispatched.index(marker) + len(marker) - end = dispatched.index("\n ')", start) + end = dispatched.index("\n ' <<<\"$run_pages\")", start) result = subprocess.run( - [jq, "-r", "--arg", "head", head, "--argjson", "run_id", str(run_id), dispatched[start:end]], - input=json.dumps(run), + [jq, "-c", "-s", "--arg", "head", head, "--argjson", "pr", "7", dispatched[start:end]], + input=json.dumps({"workflow_runs": [run]}), text=True, capture_output=True, check=False, ) - assert result.returncode == 0, result.stderr - return result.stdout.strip() + if result.returncode != 0: + return "" + inventory = json.loads(result.stdout) + if not inventory: + return "" + selected = inventory[0] + return f"{selected['id']}\t{selected['status']}\t{selected['conclusion']}" def required_run(*, run_id: int = 42, head_sha: str = HEAD, path: str = ".github/workflows/opencode-review.yml") -> dict[str, object]: @@ -810,7 +1700,9 @@ def required_run(*, run_id: int = 42, head_sha: str = HEAD, path: str = ".github """ return { "id": run_id, - "head_sha": head_sha, + # pull_request_target executes the trusted default-branch workflow, so + # run-level head_sha is not the pull request head. + "head_sha": "c" * 40, "event": "pull_request_target", "name": "Required OpenCode Review", "display_title": "Fix an unrelated example bug", @@ -821,11 +1713,12 @@ def required_run(*, run_id: int = 42, head_sha: str = HEAD, path: str = ".github ), "status": "completed", "conclusion": "failure", + "pull_requests": [{"number": 7, "head": {"sha": head_sha}}], } def test_wake_selector_matches_the_referenced_run_without_name_or_display_title() -> None: - """The exact-id, exact-head run is matched using only id/event/path/head_sha.""" + """An exact-head run is matched using only id/event/path/head_sha.""" assert wake_selector(required_run()) == "42\tcompleted\tfailure" @@ -847,7 +1740,9 @@ def test_wake_selector_rejects_a_referenced_run_for_a_different_workflow() -> No def test_formal_receipt_wakes_the_exact_head_failed_required_run(tmp_path: Path) -> None: """Execute the production wake script end-to-end against a fake GitHub API.""" dispatched = DISPATCH_WORKFLOW.read_text(encoding="utf-8") - step = dispatched.split(" - name: Wake exact-head required OpenCode workflow\n", 1)[1] + step = dispatched.split( + " - name: Wake every failed exact-head Required OpenCode workflow\n", 1 + )[1] run_block = step.split(" run: |\n", 1)[1].split("\n\n - name:", 1)[0] script = textwrap.dedent(run_block) calls = tmp_path / "calls" @@ -856,8 +1751,15 @@ def test_formal_receipt_wakes_the_exact_head_failed_required_run(tmp_path: Path) f"""#!/usr/bin/env bash set -euo pipefail printf '%s\\n' "$*" >>"$FAKE_CALLS" +if [[ "$*" == "api repos/ContextualWisdomLab/example/pulls/7" ]]; then + printf '%s' '{{"state":"open","draft":false,"created_at":"2026-09-30T00:00:00Z","head":{{"sha":"{HEAD}"}}}}' + exit 0 +fi if [[ "$*" == *"actions/runs/42/rerun-failed-jobs"* ]]; then exit 0; fi -if [[ "$*" == *"actions/runs/42"* ]]; then printf '%s\\n' '{json.dumps(required_run())}'; exit 0; fi +if [[ "$*" == *"actions/runs?event=pull_request_target"* ]]; then + printf '%s\\n' '{{"total_count":1,"workflow_runs":[{json.dumps(required_run())}]}}' + exit 0 +fi exit 1 """, encoding="utf-8", @@ -870,9 +1772,9 @@ def test_formal_receipt_wakes_the_exact_head_failed_required_run(tmp_path: Path) "PATH": f"{tmp_path}{os.pathsep}{os.environ.get('PATH', '')}", "FAKE_CALLS": str(calls), "GH_REPOSITORY": "ContextualWisdomLab/example", + "PR_NUMBER": "7", "GH_TOKEN": "actions-write-token", "PR_HEAD_SHA": HEAD, - "REQUIRED_RUN_ID": "42", "WAKE_TOKEN_SOURCE": "PR_REVIEW_MERGE_TOKEN", }, capture_output=True, @@ -882,14 +1784,16 @@ def test_formal_receipt_wakes_the_exact_head_failed_required_run(tmp_path: Path) assert result.returncode == 0, result.stderr recorded = calls.read_text(encoding="utf-8") assert "actions/runs/42/rerun-failed-jobs" in recorded - assert "repos/ContextualWisdomLab/example/actions/runs/42" in recorded - assert "--paginate" not in recorded + assert "actions/runs?event=pull_request_target" in recorded + assert "--paginate" in recorded def test_sibling_formal_receipt_fails_closed_without_actions_token() -> None: """A sibling wake without either Actions-capable PAT fails before GitHub I/O.""" dispatched = DISPATCH_WORKFLOW.read_text(encoding="utf-8") - step = dispatched.split(" - name: Wake exact-head required OpenCode workflow\n", 1)[1] + step = dispatched.split( + " - name: Wake every failed exact-head Required OpenCode workflow\n", 1 + )[1] script = textwrap.dedent( step.split(" run: |\n", 1)[1].split("\n\n - name:", 1)[0] ) @@ -900,7 +1804,6 @@ def test_sibling_formal_receipt_fails_closed_without_actions_token() -> None: "GH_TOKEN": "", "GH_REPOSITORY": "ContextualWisdomLab/example", "PR_HEAD_SHA": HEAD, - "REQUIRED_RUN_ID": "42", "WAKE_TOKEN_SOURCE": "unavailable", }, capture_output=True, diff --git a/tests/test_opencode_workflow_shell_syntax.py b/tests/test_opencode_workflow_shell_syntax.py index 3e30633eb7..b18b207ca4 100644 --- a/tests/test_opencode_workflow_shell_syntax.py +++ b/tests/test_opencode_workflow_shell_syntax.py @@ -262,6 +262,7 @@ def test_merge_scheduler_targeted_dispatch_validates_live_exact_pr(tmp_path): "DEFAULT_BRANCH": "main", "TARGET_REPOSITORY_INPUT": "ContextualWisdomLab/naruon", "TARGET_PR_NUMBER": "1179", + "TARGET_HEAD_SHA_INPUT": "4afd4af7ad343660356791873d940aa2846f40c2", "TARGET_BASE_BRANCH_INPUT": "develop", "ALLOWED_TARGET_REPOSITORIES": ( "ContextualWisdomLab/.github, ContextualWisdomLab/naruon" @@ -302,6 +303,22 @@ def test_merge_scheduler_targeted_dispatch_validates_live_exact_pr(tmp_path): assert "absent from the configured exact allowlist" in rejected.stdout assert not output.exists() + stale_head_env = { + **env, + "TARGET_HEAD_SHA_INPUT": "a" * 40, + } + stale_head = subprocess.run( + [bash], + input=script, + text=True, + capture_output=True, + check=False, + env=stale_head_env, + ) + assert stale_head.returncode == 1 + assert "head does not match the live PR" in stale_head.stdout + assert not output.exists() + output.unlink(missing_ok=True) cross_repo_pull = { **pull, diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 1bfc13292e..a702f79f6c 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 = "09bbf8181a443f7a5630ec91ca958440c5dcfc68" +REVIEW_DISPATCH_BLOB_SHA = "ac80537dacf2bb0d20c0716c76b460da0b493f52" def _workflow_text(path: Path) -> str: diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index abe5a411c7..33c9237154 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -5606,11 +5606,45 @@ def fake_run_with_env(args, *, stdin=None, env=None): "pr_base_sha": base_sha, "pr_head_ref": "feature", "pr_head_sha": head_sha, + "draft_review_only": False, "required_run_id": 42, }, } +def test_draft_review_dispatch_carries_explicit_review_only_authority(monkeypatch): + """The receiver can distinguish an authorized Draft review from merge work.""" + monkeypatch.setenv("GITHUB_ACTIONS", "true") + monkeypatch.setenv("GH_TOKEN", "opencode-app-token") + monkeypatch.setattr(sched, "active_opencode_run_refs", lambda *args: ([], [])) + monkeypatch.setattr(sched, "_cancel_revalidated_review_run_refs", lambda *args: ([], [])) + monkeypatch.setattr(sched, "review_dispatch_admitted", lambda *args: True) + monkeypatch.setattr(sched, "live_dispatch_head_matches", lambda *args: True) + monkeypatch.setattr(sched, "complete_paginated_pr_contexts", lambda *args: None) + monkeypatch.setattr(sched, "matching_actions_run_id", lambda *args: None) + monkeypatch.setattr(sched, "discover_opencode_required_run_id", lambda *args: None) + monkeypatch.setattr(sched, "reset_active_workflow_runs_cache", lambda: None) + dispatch_payloads = [] + monkeypatch.setattr( + sched, + "run_github_dispatch", + lambda _args, stdin=None: dispatch_payloads.append(json.loads(stdin)), + ) + pull_request = make_pr( + isDraft=True, + baseRefOid="b" * 40, + headRefOid="a" * 40, + ) + + assert ( + sched.dispatch_opencode_review( + "owner/repo", "OpenCode Review", pull_request, dry_run=False + ) + == "dispatched" + ) + assert dispatch_payloads[0]["client_payload"]["draft_review_only"] is True + + def test_central_required_workflow_waits_without_cross_repo_dispatch_credential(monkeypatch): monkeypatch.setenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", "ContextualWisdomLab/.github") monkeypatch.setenv("SCHEDULER_REQUIRED_WORKFLOW_REF", "main") @@ -8282,6 +8316,67 @@ def test_draft_pr_review_only_dispatch_skips_when_a_current_head_verdict_exists( "draft PR review-only dispatch; current-head OpenCode verdict already exists" ) + draft_completion = { + **opencode_review("COMMENTED", "head"), + "body": ( + "OpenCode review\n\n" + "- Result: DRAFT_REVIEW_COMPLETE\n" + "- Head SHA: `head`\n\n" + "Draft review-only request completed without publishing merge approval authority." + ), + } + completed_draft = make_pr(isDraft=True, reviews={"nodes": [draft_completion]}) + completed_decision = inspect(completed_draft, allow_draft_review_dispatch=True) + assert completed_decision.action == "skip" + assert completed_decision.reason == ( + "draft PR review-only dispatch; current-head OpenCode verdict already exists" + ) + + +def test_draft_review_completion_requires_exact_head_and_both_formal_markers(): + """A stale or generic comment cannot suppress an explicit Draft review.""" + complete_body = ( + "OpenCode review\n\n" + "- Result: DRAFT_REVIEW_COMPLETE\n\n" + "Draft review-only request completed without publishing merge approval authority." + ) + stale = { + **opencode_review("COMMENTED", "old-head"), + "body": complete_body, + } + missing_explanation = { + **opencode_review("COMMENTED", "head"), + "body": "OpenCode review\n\n- Result: DRAFT_REVIEW_COMPLETE", + } + generic = { + **opencode_review("COMMENTED", "head"), + "body": "OpenCode is still running.", + } + + assert not sched.has_current_head_draft_review_completion( + make_pr(isDraft=True, reviews={"nodes": [stale]}) + ) + assert not sched.has_current_head_draft_review_completion( + make_pr(isDraft=True, reviews={"nodes": [missing_explanation]}) + ) + assert not sched.has_current_head_draft_review_completion( + make_pr(isDraft=True, reviews={"nodes": [generic]}) + ) + assert sched.has_current_head_draft_review_completion( + make_pr( + isDraft=True, + reviews={ + "nodes": [ + { + **opencode_review("COMMENTED", "head"), + "body": complete_body, + }, + generic, + ] + }, + ) + ) + def test_draft_pr_review_only_dispatch_retries_a_failed_required_check_with_no_verdict(): """A completed-but-failed required-workflow check is not a posted review: @@ -10989,6 +11084,51 @@ def test_bounded_admission_persists_leases_and_completes_only_current_head( assert [record.status for record in persisted.records.values()].count("dispatched") == 1 +def test_ready_transition_retires_same_head_draft_completion_lease(tmp_path): + """A Draft comment cannot permanently suppress Ready review on the same head.""" + state_path = tmp_path / "admission.json" + gate = sched.SchedulerAdmissionGate(state_path, sequence=78, dispatch_budget=1) + completion = { + **opencode_review("COMMENTED", "a" * 40), + "body": ( + "OpenCode review\n\n" + "- Result: DRAFT_REVIEW_COMPLETE\n\n" + "Draft review-only request completed without publishing merge approval authority." + ), + } + draft_pr = make_pr( + number=7, + isDraft=True, + headRefOid="a" * 40, + reviews={"nodes": [completion]}, + ) + assert gate.admit("opencode", "ContextualWisdomLab/example", draft_pr) + gate.reconcile("ContextualWisdomLab/example", [draft_pr]) + + from scripts.ci.review_admission_controller import load_state_file + + completed = next(iter(load_state_file(state_path).records.values())) + assert completed.status == "complete" + + ready_pr = { + **draft_pr, + "isDraft": False, + "statusCheckRollup": { + "contexts": {"nodes": [opencode_check(status="IN_PROGRESS")]} + }, + } + ready_gate = sched.SchedulerAdmissionGate( + state_path, sequence=79, dispatch_budget=1 + ) + assert ready_gate.admit("opencode", "ContextualWisdomLab/example", ready_pr) + admitted = next(iter(load_state_file(state_path).records.values())) + assert admitted.status == "dispatched" + + ready_gate.reconcile("ContextualWisdomLab/example", [ready_pr]) + preserved = next(iter(load_state_file(state_path).records.values())) + assert preserved.status == "dispatched" + + def test_actual_opencode_dispatch_path_obeys_one_shared_admission_budget( monkeypatch, tmp_path ): diff --git a/tests/test_required_review_runner_image_contract.py b/tests/test_required_review_runner_image_contract.py index ab7ede7aa4..ad80e2ff40 100644 --- a/tests/test_required_review_runner_image_contract.py +++ b/tests/test_required_review_runner_image_contract.py @@ -85,8 +85,8 @@ def test_opencode_review_dispatch_uses_explicit_supported_image(self) -> None: follow-up sweep as still open). """ workflow = OPENCODE_REVIEW_DISPATCH.read_text(encoding="utf-8") - self.assertEqual(workflow.count("group: CWL central OpenCode"), 3) - self.assertEqual(workflow.count("labels: [self-hosted, linux, x64]"), 3) + self.assertEqual(workflow.count("group: CWL central OpenCode"), 4) + self.assertEqual(workflow.count("labels: [self-hosted, linux, x64]"), 4) self.assertNotIn("runs-on: ubuntu-latest", workflow) self.assertNotIn("runs-on: ubuntu-24.04", workflow) diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index a24b3c7d0c..93d0bba8c4 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -25,7 +25,7 @@ def test_central_dispatch_and_control_jobs_use_dedicated_groups() -> None: """Central-only workflows cannot fall back into the general Ubuntu pool.""" for name, group, jobs in ( ("codeql-scan-dispatch.yml", "CWL central CodeQL", 3), - ("opencode-review-dispatch.yml", "CWL central OpenCode", 3), + ("opencode-review-dispatch.yml", "CWL central OpenCode", 4), ("agent-mention-router.yml", "CWL central control", 2), ("hourly-review-repair.yml", "CWL central control", 1), ): @@ -37,7 +37,10 @@ def test_central_dispatch_and_control_jobs_use_dedicated_groups() -> None: def test_reusable_scheduler_keeps_consumer_runner_access() -> None: """Reusable trusted schedulers share control capacity without PR execution.""" text = workflow_text("pr-review-merge-scheduler.yml") - selector = next(line for line in text.splitlines() if line.strip().startswith("runs-on:")) + scan_job = text.split("\n scan-pr-queue:\n", 1)[1] + selector = next( + line for line in scan_job.splitlines() if line.strip().startswith("runs-on:") + ) assert selector.strip() == "runs-on:" assert " runs-on:\n group: CWL central control\n labels: [self-hosted, linux, x64]" in text assert "fromJSON" not in selector @@ -251,12 +254,23 @@ def test_merge_scheduler_uses_native_auto_merge_after_required_checks() -> None: assert "github.event_name == 'repository_dispatch' && github.run_id" not in ( concurrency_contract ) - # Anchored, not a substring: this workflow's value is an expression rather - # than a constant, so it cannot use the boolean helper, but a commented-out - # setting must not satisfy it either. - assert re.search(r"(?m)^[ \t]+cancel-in-progress:[ \t]+\$\{\{", concurrency_contract) + assert "queue: max" in concurrency_contract + assert "cancel-in-progress:" not in concurrency_contract + assert "github.event.pull_request.head.sha" in concurrency_contract + assert "github.event.client_payload.pr_head_sha" in concurrency_contract assert "github.event_name == 'repository_dispatch'" in concurrency_contract + cleanup_job = workflow.split(" cancel-superseded-pr-runs:", 1)[1].split( + " scan-pr-queue:", 1 + )[0] + assert "actions: write" in cleanup_job + assert "actions/checkout" not in cleanup_job + assert "github.event.pull_request.number" in cleanup_job + assert "github.event.pull_request.head.sha" in cleanup_job + assert ".pull_requests[]?" in cleanup_job + assert ".head.sha" in cleanup_job + assert "force-cancel" in cleanup_job + def test_merge_scheduler_provides_same_repository_dispatch_credential() -> None: """Guard the runner-token dispatch credential for central review workflows. @@ -356,8 +370,8 @@ 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. +def test_privileged_review_dispatch_avoids_lossy_native_concurrency() -> None: + """The receiver must never let GitHub replace pending exact-head work. ``opencode-review-dispatch.yml`` carried its concurrency group only on the long ``opencode-review-target`` job. A job-level group is not evaluated @@ -369,25 +383,16 @@ def test_privileged_review_dispatch_coalesces_superseded_runs_before_admission() 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. + GitHub native concurrency retains at most one pending run per group and + replaces an older pending member even when ``cancel-in-progress`` is false. + The receiver therefore uses no native concurrency group. Trusted producer + deduplication and live-head cleanup remain the admission authorities. """ workflow = workflow_text("opencode-review-dispatch.yml") header = workflow.split("permissions:", 1)[0] - concurrency_contract = header.split("concurrency:", 1)[1] - group_value = workflow_level_concurrency_group(workflow) - - assert re.search(r"(?m)^concurrency:", header) - assert "opencode-review-dispatch-" in group_value - assert ( - "github.event.client_payload.target_repository || github.repository" - 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 not re.search(r"(?m)^concurrency:", header) + review_job = workflow.split("\n opencode-review-target:\n", 1)[1] + assert not re.search(r"(?m)^ concurrency:", review_job) @pytest.mark.parametrize( @@ -674,6 +679,8 @@ def test_required_opencode_dispatch_does_not_wait_on_merge_scheduler() -> None: assert 'event_type:"opencode-review"' in dispatch assert 'event_type:"merge-scheduler"' not in dispatch assert 'required_run_id:$required_run_id' in dispatch + assert "--argjson draft_review_only false" in dispatch + assert "draft_review_only:$draft_review_only" in dispatch for field in ( "target_repository", "pr_number", @@ -1131,9 +1138,23 @@ def test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs() - assert "actions: write" in cleanup_job assert "actions/checkout" not in cleanup_job assert "cleanup skipped" not in cleanup_job + elif filename == "pr-review-merge-scheduler.yml": + assert "cancel-closed-pr-runs:" not in workflow + concurrency_contract = workflow.split("concurrency:", 1)[1].split( + "permissions:", 1 + )[0] + assert "github.event.pull_request.number" in concurrency_contract + assert "github.event.pull_request.head.sha" in concurrency_contract + assert "queue: max" in concurrency_contract + assert "cancel-in-progress:" not in concurrency_contract + assert "cancel-superseded-pr-runs:" in workflow + cleanup_job = workflow.split( + " cancel-superseded-pr-runs:", 1 + )[1].split(" scan-pr-queue:", 1)[0] + assert "actions: write" in cleanup_job + assert "actions/checkout" not in cleanup_job elif filename in { "codeql-pr.yml", - "pr-review-merge-scheduler.yml", "python-security.yml", "sast-semgrep.yml", "security-scan.yml", @@ -1517,6 +1538,231 @@ def test_merge_scheduler_has_no_workflow_run_trigger() -> None: assert "workflow_run:" not in workflow.split("workflow_call:", 1)[0] +def _run_merge_scheduler_cleanup( + tmp_path: Path, + pull_states: list[dict[str, object]], + run_states: list[dict[str, object]], + inventory_responses: list[dict[str, object]] | None = None, +) -> tuple[subprocess.CompletedProcess[str], str]: + """Execute predecessor cleanup against stateful GitHub API fixtures.""" + if shutil.which("jq") is None: + pytest.skip("jq is required to execute the production cleanup") + step = workflow_step( + workflow_text("pr-review-merge-scheduler.yml"), + "Cancel revalidated predecessor scheduler runs", + ) + run_block = step.split(" run: |\n", 1)[1].split( + "\n scan-pr-queue:", 1 + )[0] + script = textwrap.dedent(run_block) + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + calls = tmp_path / "calls" + pulls = tmp_path / "pulls" + runs = tmp_path / "runs" + inventories = tmp_path / "inventories" + pulls.write_text( + "\n".join(json.dumps(state) for state in pull_states) + "\n", + encoding="utf-8", + ) + runs.write_text( + "\n".join(json.dumps(state) for state in run_states) + "\n", + encoding="utf-8", + ) + if inventory_responses is None: + default_inventory = { + "total_count": 1, + "workflow_runs": [ + { + "id": 100, + "status": "queued", + "pull_requests": [ + { + "number": 7, + "head": {"sha": "b" * 40}, + } + ], + } + ], + } + inventory_responses = [default_inventory] * 10 + inventories.write_text( + "\n".join(json.dumps(response) for response in inventory_responses) + "\n", + encoding="utf-8", + ) + fake_gh = fake_bin / "gh" + fake_gh.write_text( + '''#!/usr/bin/env bash +set -euo pipefail +printf '%s\n' "$*" >>"$FAKE_CALLS" +next_line() { + local source="$1" count_file="${1}.count" count=0 + [[ ! -f "$count_file" ]] || count="$(cat "$count_file")" + count=$((count + 1)) + printf '%s' "$count" >"$count_file" + sed -n "${count}p" "$source" +} +if [[ "$*" == *"/pulls/7"* ]]; then + next_line "$FAKE_PULLS" +elif [[ "$*" == *"actions/workflows/pr-review-merge-scheduler.yml/runs"* ]]; then + printf '[%s]\n' "$(next_line "$FAKE_INVENTORIES")" +elif [[ "$*" == *"actions/runs/100/force-cancel"* ]]; then + exit 0 +elif [[ "$*" == *"actions/runs/100"* ]]; then + state="$(next_line "$FAKE_RUNS")" + jq -r '[.status // "", .conclusion // ""] | @tsv' <<<"$state" +else + exit 1 +fi +''', + encoding="utf-8", + ) + fake_gh.chmod(0o755) + result = subprocess.run( + ["bash", "-c", script], + env={ + **os.environ, + "PATH": f"{fake_bin}{os.pathsep}{os.environ['PATH']}", + "FAKE_CALLS": str(calls), + "FAKE_PULLS": str(pulls), + "FAKE_RUNS": str(runs), + "FAKE_INVENTORIES": str(inventories), + "GH_TOKEN": "synthetic-actions-token", + "GITHUB_RUN_ID": "999", + "TARGET_REPOSITORY": "owner/repo", + "TARGET_PR_NUMBER": "7", + "TARGET_PR_HEAD_SHA": "a" * 40, + "TARGET_ACTION": "synchronize", + }, + capture_output=True, + text=True, + check=False, + ) + return result, calls.read_text(encoding="utf-8") + + +def _live_scheduler_pull(*, head_sha: str = "a" * 40) -> dict[str, object]: + """Build one live pull-request response for scheduler cleanup evidence.""" + return { + "base": {"repo": {"full_name": "owner/repo"}}, + "number": 7, + "state": "open", + "head": {"sha": head_sha}, + } + + +def test_scheduler_cleanup_revalidates_target_after_run_selection(tmp_path: Path) -> None: + """A concurrent head advance after selection prevents cancellation.""" + result, calls = _run_merge_scheduler_cleanup( + tmp_path, + [_live_scheduler_pull(), _live_scheduler_pull(head_sha="c" * 40)], + [{"status": "completed", "conclusion": "cancelled"}], + ) + assert result.returncode == 0, result.stderr + assert calls.count("/pulls/7") == 2 + assert "/actions/runs/100/force-cancel" not in calls + + +def test_scheduler_cleanup_fails_when_accepted_cancel_never_finishes( + tmp_path: Path, +) -> None: + """An accepted POST is not terminal cancellation evidence.""" + result, calls = _run_merge_scheduler_cleanup( + tmp_path, + [_live_scheduler_pull()] * 8, + [{"status": "in_progress", "conclusion": None}] * 6, + ) + assert result.returncode == 1 + assert calls.count("actions/runs/100 --jq") == 6 + assert "did not reach completed/cancelled" in result.stdout + + +def test_scheduler_cleanup_verifies_accepted_cancelled_state(tmp_path: Path) -> None: + """Finish only after GitHub reports completed/cancelled.""" + result, calls = _run_merge_scheduler_cleanup( + tmp_path, + [_live_scheduler_pull()] * 8, + [ + {"status": "in_progress", "conclusion": None}, + {"status": "completed", "conclusion": "cancelled"}, + ], + ) + assert result.returncode == 0, result.stderr + assert calls.count("actions/runs/100 --jq") == 2 + assert "Verified cancelled scheduler run 100." in result.stdout + + +def test_scheduler_cleanup_second_inventory_pass_catches_state_transition( + tmp_path: Path, +) -> None: + """A run moving between filtered states remains visible on the second pass.""" + empty_inventory = {"total_count": 0, "workflow_runs": []} + transitioned_inventory = { + "total_count": 1, + "workflow_runs": [ + { + "id": 100, + "status": "in_progress", + "pull_requests": [ + {"number": 7, "head": {"sha": "b" * 40}} + ], + } + ], + } + result, calls = _run_merge_scheduler_cleanup( + tmp_path, + [_live_scheduler_pull()] * 8, + [{"status": "completed", "conclusion": "cancelled"}], + [empty_inventory] * 5 + + [empty_inventory] * 4 + + [transitioned_inventory], + ) + assert result.returncode == 0, result.stderr + assert calls.count("actions/workflows/pr-review-merge-scheduler.yml/runs") == 10 + assert "/actions/runs/100/force-cancel" in calls + + +def test_scheduler_cleanup_inventory_covers_review_event_runs(tmp_path: Path) -> None: + """A new head must retire predecessor runs triggered by PR reviews too.""" + review_inventory = { + "total_count": 1, + "workflow_runs": [ + { + "id": 100, + "event": "pull_request_review", + "status": "queued", + "pull_requests": [ + {"number": 7, "head": {"sha": "b" * 40}}, + ], + } + ], + } + result, calls = _run_merge_scheduler_cleanup( + tmp_path, + [_live_scheduler_pull()] * 8, + [{"status": "completed", "conclusion": "cancelled"}], + [review_inventory] * 10, + ) + assert result.returncode == 0, result.stderr + assert "event=pull_request_target" not in calls + assert "/actions/runs/100/force-cancel" in calls + + +def test_scheduler_cleanup_fails_closed_on_truncated_inventory( + tmp_path: Path, +) -> None: + """Never treat GitHub's filtered-search ceiling as a complete snapshot.""" + result, calls = _run_merge_scheduler_cleanup( + tmp_path, + [_live_scheduler_pull()], + [{"status": "completed", "conclusion": "cancelled"}], + [{"total_count": 1001, "workflow_runs": []}], + ) + assert result.returncode == 1 + assert "inventory was incomplete" in result.stdout + assert "/force-cancel" not in calls + + def test_review_events_can_dispatch_after_threads_are_resolved() -> None: """Let the scheduler dispatch OpenCode when a review event clears its last blocker.""" workflow = workflow_text("pr-review-merge-scheduler.yml") diff --git a/tests/test_trusted_uv_materializer_quality_workflow_contract.py b/tests/test_trusted_uv_materializer_quality_workflow_contract.py index ed4ad55681..7c27766c5d 100644 --- a/tests/test_trusted_uv_materializer_quality_workflow_contract.py +++ b/tests/test_trusted_uv_materializer_quality_workflow_contract.py @@ -146,7 +146,7 @@ def test_full_quality_gate_proves_tests_coverage_docstrings_and_compilation() -> assert "python -m coverage report" in workflow assert "python -m coverage run -m pytest tests -q" in workflow assert "unset COVERAGE_RCFILE" in workflow - assert "python -m interrogate --fail-under 100" in workflow + assert "run: python -m interrogate --fail-under 100 scripts/ci\n" in workflow assert "python -m compileall -q" in workflow required_tests = ( diff --git a/tests/test_verify_release_maturin_tool_assets.py b/tests/test_verify_release_maturin_tool_assets.py index 6aa84a0827..85e720152e 100644 --- a/tests/test_verify_release_maturin_tool_assets.py +++ b/tests/test_verify_release_maturin_tool_assets.py @@ -201,11 +201,14 @@ def open(self, _request, timeout): verifier._download("maturin-x86_64-pc-windows-msvc.zip") assert closed == ["response"] + class ErrorBody(io.BytesIO): + def close(self): + closed.append("http-error") + super().close() + + error_body = ErrorBody() transport_error = urllib.error.HTTPError( - "https://github.com/asset", 502, "Bad Gateway", {}, io.BytesIO() - ) - monkeypatch.setattr( - transport_error, "close", lambda: closed.append("http-error") + "https://github.com/asset", 502, "Bad Gateway", {}, error_body ) class ErrorOpener: @@ -221,6 +224,7 @@ def open(self, _request, timeout): with pytest.raises(ValueError, match="HTTP 502"): verifier._download("maturin-x86_64-pc-windows-msvc.zip") assert closed == ["response", "http-error"] + assert error_body.closed def test_maturin_download_rejects_unlisted_name_before_network(monkeypatch): @@ -334,6 +338,12 @@ def verify(evidence, reader, fetch): assert captured["raw"] == b"asset" +def test_maturin_verifier_has_complete_docstrings(): + """Every production entry point explains its trust-boundary responsibility.""" + assert verifier._binary.__doc__ + assert verifier.main.__doc__ + + def test_maturin_process_entrypoint_uses_the_bounded_downloader(monkeypatch): archives, assets, _, evidence = _asset_case() from scripts.ci import scan_release_native_links as scanner