diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index a15cdf36e1..fe5cf4206f 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -1087,37 +1087,40 @@ jobs: fi fi - # Queue hygiene, part 1: cancel every queued/in-progress PR run whose - # head SHA no longer matches its open PR's Current HEAD, plus default- - # branch push/schedule runs superseded by a newer default HEAD. PR - # concurrency normally does this on synchronize/close events, but it - # cannot repair runs left behind by an outage or a manual dispatch. - # Compare live refs on every sweep instead of waiting for an age - # threshold: previous-head checks are never useful merge evidence. + # Queue hygiene, part 1: classify queued/in-progress runs against a + # bounded PR/default-branch snapshot. The snapshot is intentionally + # cheap and may race with a subsequent head move; every destructive + # cancellation is therefore revalidated against live run/PR/ref state + # immediately before the mutation by the production helper below. queue_hygiene_ready=true - if ! open_pr_heads_json="$( + open_pr_heads_json="{}" + if open_pr_payload_json="$( gh api \ -H "Accept: application/vnd.github+json" \ "/repos/${repo_full_name}/pulls?state=open&per_page=100" \ --paginate \ - | jq -sc ' - add - | map( - select( - .head.repo.full_name != null and - .head.ref != null and - .head.sha != null - ) - | { - key: "\(.head.repo.full_name):\(.head.ref)", - value: .head.sha - } - ) - | from_entries - ' + | jq -sc '[.[] | .[]]' )"; then + if ! jq -e ' + all(.[]; + (.head.repo.full_name | type) == "string" and (.head.repo.full_name | length) > 0 and + (.head.ref | type) == "string" and (.head.ref | length) > 0 and + (.head.sha | type) == "string" and (.head.sha | test("^[0-9a-fA-F]{40}$")) + ) + ' <<<"$open_pr_payload_json" >/dev/null; then + echo "::warning::Current-HEAD cancellation skipped for ${repo_full_name}: an open PR has malformed head repository/ref/SHA metadata. No run will be cancelled from incomplete evidence." + queue_hygiene_ready=false + else + open_pr_heads_json="$( + jq -c ' + reduce .[] as $pr ({}; + . + {(($pr.head.repo.full_name + ":" + $pr.head.ref)): $pr.head.sha} + ) + ' <<<"$open_pr_payload_json" + )" + fi + else echo "::warning::Current-HEAD cancellation skipped for ${repo_full_name}: open PR head refs could not be read safely. No run will be cancelled from incomplete evidence." - open_pr_heads_json="{}" queue_hygiene_ready=false fi if ! current_default_sha="$( @@ -1129,6 +1132,10 @@ jobs: echo "::warning::Current-HEAD cancellation skipped for ${repo_full_name}: default-branch HEAD could not be read safely. No run will be cancelled from incomplete evidence." current_default_sha="" queue_hygiene_ready=false + elif ! [[ "$current_default_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + echo "::warning::Current-HEAD cancellation skipped for ${repo_full_name}: default-branch HEAD is malformed. No run will be cancelled from incomplete evidence." + current_default_sha="" + queue_hygiene_ready=false fi if ! active_runs_json="$( for active_status in queued in_progress; do @@ -1187,13 +1194,17 @@ jobs: fi superseded_count="$(jq 'length' <<<"$superseded_runs_json")" if [ "$superseded_count" -gt 0 ]; then - echo "Cancelling ${superseded_count} queued/in-progress run(s) that do not match an open PR or default-branch Current HEAD:" - jq -r '.[] | " run \(.id) [\(.name)] status=\(.status) event=\(.event) branch=\(.head_branch) run_head=\(.run_head) current_head=\(.current_head // "closed-or-no-open-pr")"' <<<"$superseded_runs_json" + echo "Revalidating ${superseded_count} queued/in-progress run(s) classified as not matching an open PR or default-branch Current HEAD:" + jq -r '.[] | " run \(.id) [\(.name)] status=\(.status) event=\(.event) branch=\(.head_branch) run_head=\(.run_head) classified_head=\(.current_head // "closed-or-no-open-pr")"' <<<"$superseded_runs_json" if [ "$DRY_RUN" != "true" ]; then while IFS= read -r run_id; do - if ! gh api -X POST "/repos/${repo_full_name}/actions/runs/${run_id}/cancel" >/dev/null; then - echo "Could not cancel superseded run ${run_id} in ${repo_full_name}; it may have finished already." - fi + scripts/ci/revalidate_queue_cancellation.sh \ + "$repo_full_name" \ + "$run_id" \ + "$default_branch" \ + "$current_default_sha" \ + "$open_pr_heads_json" \ + "superseded" done < <(jq -r '.[].id' <<<"$superseded_runs_json") fi fi @@ -1202,6 +1213,7 @@ jobs: # runs that are not tied to a currently open PR head. This catches # orphaned manual/workflow-chain runs without cancelling a valid # current-head PR check merely because runner capacity was scarce. + # The helper re-checks late PR association/live refs before mutation. stale_runs_json="[]" if [ "$queue_hygiene_ready" = "true" ]; then stale_cutoff="$(date -u -d "${ORG_SWEEP_STALE_QUEUE_HOURS} hours ago" +%Y-%m-%dT%H:%M:%SZ)" @@ -1224,13 +1236,17 @@ jobs: fi stale_count="$(jq 'length' <<<"$stale_runs_json")" if [ "$stale_count" -gt 0 ]; then - echo "Cancelling ${stale_count} queued run(s) older than ${ORG_SWEEP_STALE_QUEUE_HOURS}h:" + echo "Revalidating ${stale_count} queued run(s) older than ${ORG_SWEEP_STALE_QUEUE_HOURS}h:" jq -r '.[] | " run \(.id) [\(.name)] on \(.head_branch) queued since \(.created_at)"' <<<"$stale_runs_json" if [ "$DRY_RUN" != "true" ]; then while IFS= read -r run_id; do - if ! gh api -X POST "/repos/${repo_full_name}/actions/runs/${run_id}/cancel" >/dev/null; then - echo "Could not cancel run ${run_id} in ${repo_full_name}; it may have started or finished already." - fi + scripts/ci/revalidate_queue_cancellation.sh \ + "$repo_full_name" \ + "$run_id" \ + "$default_branch" \ + "$current_default_sha" \ + "$open_pr_heads_json" \ + "aged-orphan" done < <(jq -r '.[].id' <<<"$stale_runs_json") fi fi diff --git a/docs/doctoring/queue-hygiene-live-ref-race.md b/docs/doctoring/queue-hygiene-live-ref-race.md new file mode 100644 index 0000000000..029cdf31f3 --- /dev/null +++ b/docs/doctoring/queue-hygiene-live-ref-race.md @@ -0,0 +1,25 @@ +# Queue-hygiene live-ref race doctoring + +## Incident + +The organization queue sweep classified queued/in-progress Actions runs against a pull-request list snapshot and later cancelled the selected run IDs. A PR head can advance after that snapshot but before the destructive cancellation. GitHub's run and PR payloads may also lag the branch ref. Trusting either predecessor snapshot as final authority can therefore cancel the sole current-head review/check evidence and amplify Actions-capacity saturation. + +## Owner and boundary + +`ContextualWisdomLab/.github` owns this defect because the destructive organization queue hygiene and required review/merge scheduler are central control-plane behavior. Leaf repositories must not duplicate cancellation policy. The scheduler may use cheap PR payloads to classify candidates, but every destructive cancellation must revalidate the live run and its authoritative current ref immediately before the mutation. + +## Contract + +The repaired scheduler keeps a bounded initial snapshot and delegates every selected cancellation to `scripts/ci/revalidate_queue_cancellation.sh`. The helper fails closed when run/PR/ref evidence cannot be read or is malformed. For an attached PR it re-fetches the PR and resolves the head branch through the Git ref endpoint. For an Actions PR run whose `pull_requests` association is still empty, it re-fetches open PRs only to discover a matching head repository/ref and then resolves that branch ref; the payload SHA is explicitly non-authoritative. If the live ref equals the run head, the run is preserved. Default-branch push/schedule candidates are similarly revalidated against the live protected-branch head. + +The final design intentionally removes the earlier serial live-ref lookup for every open PR and its repository-wide lookup ceiling. Live-ref traffic is proportional to destructive candidates, so a large open-PR queue cannot disable all cleanup merely by exceeding a fanout cap. + +## Reconciliation and one-shot retirement + +PR #1348 diverged while protected `main` advanced. The reconciliation tree is based on the live protected-main tree and preserves the later scheduler fixes: hourly organization sweep cadence, explicit Ubuntu 24.04 queue-draining runners, and review-event dispatch after thread updates. The obsolete `_temp_pr1348_final_revalidation_repair.yml` source-fix workflow is not carried forward. The production helper is executable in the Git tree and is covered by focused executable regressions, including the stale-PR-payload/live-ref race. + +## Evidence + +`tests/test_queue_cancellation_revalidation.py` covers post-classification head movement, current-head preservation, fail-closed API/ref failures, predecessor cancellation, and aged-orphan behavior. `tests/test_queue_cancellation_open_pr_revalidation.py` specifically proves that a stale open-PR payload SHA cannot authorize cancellation when the authoritative live branch ref still points at the queued run. `tests/test_queue_cancellation_scheduler_contract.py` proves the scheduler routes both cancellation modes through the helper, removes serial upfront ref fanout and the lookup ceiling, preserves current-main scheduler fixes, keeps the helper executable, and retires the temporary writer workflow. + +Hosted exact-head CI, security, coverage and review evidence remain authoritative before merge; this doctoring note does not substitute for those gates. diff --git a/scripts/ci/revalidate_queue_cancellation.sh b/scripts/ci/revalidate_queue_cancellation.sh new file mode 100755 index 0000000000..14bf5d2eca --- /dev/null +++ b/scripts/ci/revalidate_queue_cancellation.sh @@ -0,0 +1,178 @@ +#!/usr/bin/env bash +set -euo pipefail + +if [ "$#" -ne 6 ]; then + echo "usage: $0 " >&2 + exit 2 +fi + +repo_full_name="$1" +run_id="$2" +default_branch="$3" +classified_default_sha="$4" +classified_open_pr_heads_json="$5" +cancellation_mode="$6" + +case "$cancellation_mode" in + superseded|aged-orphan) ;; + *) + echo "invalid cancellation mode: ${cancellation_mode}" >&2 + exit 2 + ;; +esac + +warn_preserve() { + echo "::warning::Preserving run ${run_id} in ${repo_full_name}: $1" + exit 0 +} + +encode_ref_path() { + jq -rn --arg value "$1" '$value | split("/") | map(@uri) | join("/")' +} + +if ! run_json="$(gh api -H "Accept: application/vnd.github+json" "/repos/${repo_full_name}/actions/runs/${run_id}")"; then + warn_preserve "live run metadata could not be re-fetched before cancellation." +fi + +event="$(jq -r '.event // empty' <<<"$run_json")" +status="$(jq -r '.status // empty' <<<"$run_json")" +run_head="$(jq -r '.head_sha // empty' <<<"$run_json")" +run_branch="$(jq -r '.head_branch // empty' <<<"$run_json")" +run_head_repo="$(jq -r '.head_repository.full_name // empty' <<<"$run_json")" +if ! [[ "$run_head" =~ ^[0-9a-fA-F]{40}$ ]]; then + warn_preserve "live run head is malformed." +fi + +if [ "$cancellation_mode" = "aged-orphan" ]; then + if [ "$status" != "queued" ]; then + warn_preserve "aged-orphan candidate is no longer queued (status=${status:-})." + fi +elif [ "$status" != "queued" ] && [ "$status" != "in_progress" ]; then + warn_preserve "superseded candidate is no longer queued or in progress (status=${status:-})." +fi + +case "$event" in + pull_request|pull_request_target) + pr_number="$(jq -r '.pull_requests[0].number // empty' <<<"$run_json")" + if ! [[ "$pr_number" =~ ^[1-9][0-9]*$ ]]; then + if [ "$cancellation_mode" = "aged-orphan" ]; then + # Association metadata on an Actions run can lag the PR itself. Re-read + # open PRs immediately before destructive cancellation, but use that + # payload only to discover the authoritative head repository/ref. The + # payload SHA itself can be stale, so resolve a matching branch through + # the Git reference endpoint before deciding whether the run is current. + if [ -z "$run_head_repo" ] || [ -z "$run_branch" ]; then + warn_preserve "unassociated PR run has no authoritative head repository/ref." + fi + if ! fresh_open_pr_refs_json="$( + gh api \ + -H "Accept: application/vnd.github+json" \ + "/repos/${repo_full_name}/pulls?state=open&per_page=100" \ + --paginate \ + | jq -sc '[.[] | .[] | { + repo: (.head.repo.full_name // null), + ref: (.head.ref // null) + }]' + )"; then + warn_preserve "open PR heads could not be re-fetched for an unassociated PR run." + fi + if ! jq -e ' + all(.[]; + (.repo | type) == "string" and (.repo | length) > 0 and + (.ref | type) == "string" and (.ref | length) > 0 + ) + ' <<<"$fresh_open_pr_refs_json" >/dev/null; then + warn_preserve "fresh open PR head evidence is malformed." + fi + if jq -e \ + --arg repo "$run_head_repo" \ + --arg ref "$run_branch" \ + 'any(.[]; .repo == $repo and .ref == $ref)' \ + <<<"$fresh_open_pr_refs_json" >/dev/null; then + encoded_run_ref="$(encode_ref_path "$run_branch")" + if ! final_ref_sha="$( + gh api \ + -H "Accept: application/vnd.github+json" \ + "/repos/${run_head_repo}/git/ref/heads/${encoded_run_ref}" \ + --jq '.object.sha // empty' + )"; then + warn_preserve "live ref for newly associated PR head could not be re-fetched before cancellation." + fi + if ! [[ "$final_ref_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + warn_preserve "live ref for newly associated PR head is malformed." + fi + if [ "$run_head" = "$final_ref_sha" ]; then + warn_preserve "run became associated with an open PR at its authoritative current head after queue classification." + fi + fi + else + warn_preserve "no authoritative PR identity is attached to the live run." + fi + else + if ! pr_json="$(gh api -H "Accept: application/vnd.github+json" "/repos/${repo_full_name}/pulls/${pr_number}")"; then + warn_preserve "live PR ${pr_number} could not be re-fetched before cancellation." + fi + live_state="$(jq -r '.state // empty' <<<"$pr_json")" + if [ "$live_state" = "open" ]; then + live_head_repo="$(jq -r '.head.repo.full_name // empty' <<<"$pr_json")" + live_head_ref="$(jq -r '.head.ref // empty' <<<"$pr_json")" + live_head_sha="$(jq -r '.head.sha // empty' <<<"$pr_json")" + if [ -z "$live_head_repo" ] || [ -z "$live_head_ref" ] || ! [[ "$live_head_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + warn_preserve "live PR ${pr_number} head metadata is malformed." + fi + encoded_head_ref="$(encode_ref_path "$live_head_ref")" + if ! final_ref_sha="$(gh api -H "Accept: application/vnd.github+json" "/repos/${live_head_repo}/git/ref/heads/${encoded_head_ref}" --jq '.object.sha // empty')"; then + warn_preserve "live ref for PR ${pr_number} could not be re-fetched before cancellation." + fi + if ! [[ "$final_ref_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + warn_preserve "live ref for PR ${pr_number} is malformed." + fi + classified_sha="$(jq -r --arg key "${live_head_repo}:${live_head_ref}" '.[$key] // empty' <<<"$classified_open_pr_heads_json")" + if ! [[ "$classified_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + warn_preserve "the classification snapshot has no valid head for PR ${pr_number}." + fi + if [ "$live_head_sha" != "$classified_sha" ] || [ "$final_ref_sha" != "$classified_sha" ]; then + warn_preserve "PR ${pr_number} moved after queue classification." + fi + if [ "$run_head" = "$final_ref_sha" ]; then + echo "Preserving run ${run_id} in ${repo_full_name}: authoritative current-head evidence for PR ${pr_number}." + exit 0 + fi + elif [ "$live_state" != "closed" ]; then + warn_preserve "live PR ${pr_number} state is malformed." + fi + # A closed PR cannot supply current merge evidence. If the run is still + # active and was selected from the trusted snapshot, closure remains an + # authoritative reason to retire it. + fi + ;; + push|schedule) + if [ "$run_branch" = "$default_branch" ] || [ "$cancellation_mode" = "superseded" ]; then + if ! live_default_sha="$(gh api -H "Accept: application/vnd.github+json" "/repos/${repo_full_name}/commits/${default_branch}" --jq '.sha // empty')"; then + warn_preserve "live default-branch HEAD could not be re-fetched before cancellation." + fi + if ! [[ "$live_default_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + warn_preserve "live default-branch HEAD is malformed." + fi + if [ "$live_default_sha" != "$classified_default_sha" ]; then + warn_preserve "default branch moved after queue classification." + fi + if [ "$run_head" = "$live_default_sha" ]; then + echo "Preserving run ${run_id} in ${repo_full_name}: authoritative current default-branch evidence." + exit 0 + fi + fi + ;; + *) + if [ "$cancellation_mode" = "superseded" ]; then + warn_preserve "event ${event:-} is outside the authoritative superseded-run contract." + fi + # Aged-orphan mode intentionally retains the legacy cleanup contract for + # workflow_dispatch, workflow_run, repository_dispatch, and other queued + # events that the trusted initial snapshot proved were not current PR heads. + ;; +esac + +if ! gh api -X POST "/repos/${repo_full_name}/actions/runs/${run_id}/cancel" >/dev/null; then + echo "Could not cancel ${cancellation_mode} run ${run_id} in ${repo_full_name}; it may have started or finished already." +fi diff --git a/tests/test_queue_cancellation_open_pr_revalidation.py b/tests/test_queue_cancellation_open_pr_revalidation.py new file mode 100644 index 0000000000..72ecc9b9e9 --- /dev/null +++ b/tests/test_queue_cancellation_open_pr_revalidation.py @@ -0,0 +1,129 @@ +"""Regressions for aged PR-run cancellation after late PR association.""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + + +REPO_ROOT = Path(__file__).resolve().parents[1] +SCRIPT = REPO_ROOT / "scripts" / "ci" / "revalidate_queue_cancellation.sh" + + +def _run_late_association_case( + tmp_path: Path, *, payload_sha: str, live_ref_sha: str, fail_ref: bool = False +) -> tuple[subprocess.CompletedProcess[str], bool]: + if shutil.which("jq") is None: + pytest.skip("jq is required for the queue-cancellation regression") + + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + cancelled = tmp_path / "cancelled" + current = "b" * 40 + run_payload = json.dumps( + { + "event": "pull_request", + "status": "queued", + "head_sha": current, + "head_branch": "feature/late-pr", + "head_repository": {"full_name": "ContextualWisdomLab/example"}, + "pull_requests": [], + }, + separators=(",", ":"), + ) + # Deliberately include a payload SHA that may lag the authoritative branch + # ref. The helper must use this response only to discover repo/ref identity. + open_prs = json.dumps( + [ + { + "state": "open", + "head": { + "repo": {"full_name": "ContextualWisdomLab/example"}, + "ref": "feature/late-pr", + "sha": payload_sha, + }, + } + ], + separators=(",", ":"), + ) + fake_gh = bin_dir / "gh" + fake_gh.write_text( + f"""#!/usr/bin/env bash +set -euo pipefail +args="$*" +if [[ "$args" == *"/actions/runs/77/cancel"* ]]; then + : > {cancelled!s} + exit 0 +fi +if [[ "$args" == *"/actions/runs/77"* ]]; then + printf '%s\\n' '{run_payload}' + exit 0 +fi +if [[ "$args" == *"/pulls?state=open&per_page=100"* ]]; then + printf '%s\\n' '{open_prs}' + exit 0 +fi +if [[ "$args" == *"/git/ref/heads/feature/late-pr"* ]]; then + {'exit 74' if fail_ref else f"printf '%s\\n' '{live_ref_sha}'"} + exit 0 +fi +exit 79 +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + env = os.environ.copy() + env["PATH"] = f"{bin_dir}{os.pathsep}{env['PATH']}" + result = subprocess.run( + [ + "bash", + str(SCRIPT), + "ContextualWisdomLab/example", + "77", + "main", + "d" * 40, + "{}", + "aged-orphan", + ], + capture_output=True, + text=True, + env=env, + check=False, + ) + return result, cancelled.exists() + + +def test_aged_unassociated_pr_run_resolves_authoritative_live_ref(tmp_path: Path) -> None: + """A stale PR payload cannot authorize cancellation of the live current head.""" + current = "b" * 40 + stale_payload = "a" * 40 + result, cancelled = _run_late_association_case( + tmp_path, + payload_sha=stale_payload, + live_ref_sha=current, + ) + + assert result.returncode == 0, result.stderr + assert "authoritative current head" in result.stdout + assert not cancelled + + +def test_aged_unassociated_pr_run_fails_closed_when_live_ref_is_unreadable( + tmp_path: Path, +) -> None: + """A matching late PR with unreadable ref must preserve the queued run.""" + result, cancelled = _run_late_association_case( + tmp_path, + payload_sha="a" * 40, + live_ref_sha="b" * 40, + fail_ref=True, + ) + + assert result.returncode == 0, result.stderr + assert "could not be re-fetched" in result.stdout + assert not cancelled diff --git a/tests/test_queue_cancellation_revalidation.py b/tests/test_queue_cancellation_revalidation.py new file mode 100644 index 0000000000..23ac824d59 --- /dev/null +++ b/tests/test_queue_cancellation_revalidation.py @@ -0,0 +1,364 @@ +"""Executable regressions for destructive queue-cancellation revalidation.""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + + +REPO_ROOT = Path(__file__).resolve().parents[1] +SCRIPT = REPO_ROOT / "scripts" / "ci" / "revalidate_queue_cancellation.sh" + + +def _run_case( + tmp_path: Path, + *, + snapshot_sha: str, + pr_sha: str, + ref_sha: str, + run_sha: str, + fail_lookup: str | None = None, +) -> tuple[subprocess.CompletedProcess[str], bool]: + """Run the production shell helper against a deterministic fake GitHub CLI.""" + if shutil.which("jq") is None: + pytest.skip("jq is required for the queue-cancellation regression") + + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + cancelled = tmp_path / "cancelled" + pr_payload = json.dumps( + { + "state": "open", + "head": { + "repo": {"full_name": "ContextualWisdomLab/example"}, + "ref": "feature/race", + "sha": pr_sha, + }, + }, + separators=(",", ":"), + ) + run_payload = json.dumps( + { + "event": "pull_request", + "status": "queued", + "head_sha": run_sha, + "pull_requests": [{"number": 12}], + }, + separators=(",", ":"), + ) + fake_gh = bin_dir / "gh" + fake_gh.write_text( + f"""#!/usr/bin/env bash +set -euo pipefail +args="$*" +if [[ "$args" == *"/actions/runs/77/cancel"* ]]; then + : > {cancelled!s} + exit 0 +fi +if [[ "$args" == *"/actions/runs/77"* ]]; then + printf '%s\\n' '{run_payload}' + exit 0 +fi +if [[ "$args" == *"/pulls/12"* ]]; then + {'exit 73' if fail_lookup == 'pr' else f"printf '%s\\n' '{pr_payload}'"} + exit 0 +fi +if [[ "$args" == *"/git/ref/heads/feature/race"* ]]; then + {'exit 74' if fail_lookup == 'ref' else f"printf '%s\\n' '{ref_sha}'"} + exit 0 +fi +exit 79 +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + env = os.environ.copy() + env["PATH"] = f"{bin_dir}{os.pathsep}{env['PATH']}" + snapshot = json.dumps( + {"ContextualWisdomLab/example:feature/race": snapshot_sha}, + separators=(",", ":"), + ) + result = subprocess.run( + [ + "bash", + str(SCRIPT), + "ContextualWisdomLab/example", + "77", + "main", + "d" * 40, + snapshot, + "superseded", + ], + capture_output=True, + text=True, + env=env, + check=False, + ) + return result, cancelled.exists() + + +def _run_aged_orphan_case( + tmp_path: Path, *, event: str, status: str = "queued" +) -> tuple[subprocess.CompletedProcess[str], bool]: + """Run an aged orphan candidate that has no current PR/default-branch authority.""" + if shutil.which("jq") is None: + pytest.skip("jq is required for the queue-cancellation regression") + + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + cancelled = tmp_path / "cancelled" + run_payload = json.dumps( + { + "event": event, + "status": status, + "head_sha": "a" * 40, + "pull_requests": [], + }, + separators=(",", ":"), + ) + fake_gh = bin_dir / "gh" + fake_gh.write_text( + f"""#!/usr/bin/env bash +set -euo pipefail +args="$*" +if [[ "$args" == *"/actions/runs/77/cancel"* ]]; then + : > {cancelled!s} + exit 0 +fi +if [[ "$args" == *"/actions/runs/77"* ]]; then + printf '%s\\n' '{run_payload}' + exit 0 +fi +exit 79 +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + env = os.environ.copy() + env["PATH"] = f"{bin_dir}{os.pathsep}{env['PATH']}" + result = subprocess.run( + [ + "bash", + str(SCRIPT), + "ContextualWisdomLab/example", + "77", + "main", + "d" * 40, + "{}", + "aged-orphan", + ], + capture_output=True, + text=True, + env=env, + check=False, + ) + return result, cancelled.exists() + + +def _run_unassociated_pr_aged_orphan_case( + tmp_path: Path, + *, + listed_sha: str, + ref_sha: str, + run_sha: str, + fail_ref_lookup: bool = False, +) -> tuple[subprocess.CompletedProcess[str], bool]: + """Run an unassociated aged PR run against stale listing and live-ref evidence.""" + if shutil.which("jq") is None: + pytest.skip("jq is required for the queue-cancellation regression") + + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + cancelled = tmp_path / "cancelled" + run_payload = json.dumps( + { + "event": "pull_request", + "status": "queued", + "head_sha": run_sha, + "head_branch": "feature/race", + "head_repository": {"full_name": "ContextualWisdomLab/example"}, + "pull_requests": [], + }, + separators=(",", ":"), + ) + open_pr_payload = json.dumps( + [ + { + "head": { + "repo": {"full_name": "ContextualWisdomLab/example"}, + "ref": "feature/race", + "sha": listed_sha, + } + } + ], + separators=(",", ":"), + ) + fake_gh = bin_dir / "gh" + fake_gh.write_text( + f"""#!/usr/bin/env bash +set -euo pipefail +args="$*" +if [[ "$args" == *"/actions/runs/77/cancel"* ]]; then + : > {cancelled!s} + exit 0 +fi +if [[ "$args" == *"/actions/runs/77"* ]]; then + printf '%s\\n' '{run_payload}' + exit 0 +fi +if [[ "$args" == *"/pulls?state=open&per_page=100"* ]]; then + printf '%s\\n' '{open_pr_payload}' + exit 0 +fi +if [[ "$args" == *"/git/ref/heads/feature/race"* ]]; then + {'exit 74' if fail_ref_lookup else f"printf '%s\\n' '{ref_sha}'"} + exit 0 +fi +exit 79 +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + env = os.environ.copy() + env["PATH"] = f"{bin_dir}{os.pathsep}{env['PATH']}" + result = subprocess.run( + [ + "bash", + str(SCRIPT), + "ContextualWisdomLab/example", + "77", + "main", + "d" * 40, + "{}", + "aged-orphan", + ], + capture_output=True, + text=True, + env=env, + check=False, + ) + return result, cancelled.exists() + + +def test_post_classification_head_movement_fails_closed(tmp_path: Path) -> None: + """A new exact head arriving after classification must never be cancelled.""" + old = "a" * 40 + new = "b" * 40 + result, cancelled = _run_case( + tmp_path, + snapshot_sha=old, + pr_sha=new, + ref_sha=new, + run_sha=new, + ) + assert result.returncode == 0, result.stderr + assert "moved after queue classification" in result.stdout + assert not cancelled + + +@pytest.mark.parametrize("failed_lookup", ["pr", "ref"]) +def test_final_lookup_failure_fails_closed( + tmp_path: Path, failed_lookup: str +) -> None: + """Unavailable final authoritative PR/ref state must preserve the candidate.""" + current = "b" * 40 + result, cancelled = _run_case( + tmp_path, + snapshot_sha=current, + pr_sha=current, + ref_sha=current, + run_sha="a" * 40, + fail_lookup=failed_lookup, + ) + assert result.returncode == 0, result.stderr + assert "could not be re-fetched" in result.stdout + assert not cancelled + + +def test_current_head_is_preserved(tmp_path: Path) -> None: + """Final live-ref validation must preserve sole current-head evidence.""" + current = "b" * 40 + result, cancelled = _run_case( + tmp_path, + snapshot_sha=current, + pr_sha=current, + ref_sha=current, + run_sha=current, + ) + assert result.returncode == 0, result.stderr + assert "authoritative current-head evidence" in result.stdout + assert not cancelled + + +def test_proven_predecessor_is_cancelled(tmp_path: Path) -> None: + """An unchanged final live ref may cancel a proven predecessor run.""" + current = "b" * 40 + result, cancelled = _run_case( + tmp_path, + snapshot_sha=current, + pr_sha=current, + ref_sha=current, + run_sha="a" * 40, + ) + assert result.returncode == 0, result.stderr + assert cancelled + + +def test_unassociated_aged_pr_uses_live_ref_not_stale_listing_sha( + tmp_path: Path, +) -> None: + """A stale PR payload cannot authorize cancelling the live branch head.""" + listed = "a" * 40 + current = "b" * 40 + result, cancelled = _run_unassociated_pr_aged_orphan_case( + tmp_path, + listed_sha=listed, + ref_sha=current, + run_sha=current, + ) + assert result.returncode == 0, result.stderr + assert "authoritative current-head evidence" in result.stdout + assert not cancelled + + +def test_unassociated_aged_pr_live_ref_lookup_failure_fails_closed( + tmp_path: Path, +) -> None: + """Missing final ref evidence must preserve an unassociated PR candidate.""" + result, cancelled = _run_unassociated_pr_aged_orphan_case( + tmp_path, + listed_sha="a" * 40, + ref_sha="b" * 40, + run_sha="b" * 40, + fail_ref_lookup=True, + ) + assert result.returncode == 0, result.stderr + assert "live ref" in result.stdout + assert "could not be re-fetched" in result.stdout + assert not cancelled + + +@pytest.mark.parametrize( + "event", + ["workflow_dispatch", "workflow_run", "repository_dispatch", "issues"], +) +def test_aged_orphan_events_remain_cancellable(tmp_path: Path, event: str) -> None: + """Final revalidation must not disable legacy aged-orphan queue cleanup.""" + result, cancelled = _run_aged_orphan_case(tmp_path, event=event) + assert result.returncode == 0, result.stderr + assert cancelled + + +def test_aged_orphan_that_started_running_is_preserved(tmp_path: Path) -> None: + """Aged-orphan mode applies only while the candidate is still queued.""" + result, cancelled = _run_aged_orphan_case( + tmp_path, event="workflow_dispatch", status="in_progress" + ) + assert result.returncode == 0, result.stderr + assert "no longer queued" in result.stdout + assert not cancelled diff --git a/tests/test_queue_cancellation_scheduler_contract.py b/tests/test_queue_cancellation_scheduler_contract.py new file mode 100644 index 0000000000..16c291a6f1 --- /dev/null +++ b/tests/test_queue_cancellation_scheduler_contract.py @@ -0,0 +1,52 @@ +"""Structural contracts for final-state queue cancellation revalidation.""" + +from __future__ import annotations + +import os +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +WORKFLOW = ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" +HELPER = ROOT / "scripts" / "ci" / "revalidate_queue_cancellation.sh" +TEMP_WRITER = ROOT / ".github" / "workflows" / "_temp_pr1348_final_revalidation_repair.yml" + + +def test_scheduler_revalidates_each_destructive_candidate() -> None: + workflow = WORKFLOW.read_text(encoding="utf-8") + + assert workflow.count("scripts/ci/revalidate_queue_cancellation.sh") == 2 + assert '"superseded"' in workflow + assert '"aged-orphan"' in workflow + assert 'gh api -X POST "/repos/${repo_full_name}/actions/runs/${run_id}/cancel"' not in workflow + + +def test_initial_snapshot_is_bounded_without_serial_live_ref_fanout() -> None: + workflow = WORKFLOW.read_text(encoding="utf-8") + queue_block = workflow.split("# Queue hygiene, part 1:", 1)[1].split( + "# Queue hygiene, part 2:", 1 + )[0] + + assert "/pulls?state=open&per_page=100" in queue_block + assert "all(.[];" in queue_block + assert 'test("^[0-9a-fA-F]{40}$")' in queue_block + assert "/git/ref/heads/" not in queue_block + assert "ORG_QUEUE_HYGIENE_MAX_REF_LOOKUPS" not in workflow + + +def test_revalidation_helper_is_executable_and_temp_writer_is_retired() -> None: + assert HELPER.is_file() + assert os.access(HELPER, os.X_OK) + assert not TEMP_WRITER.exists() + + +def test_reconciled_scheduler_preserves_current_main_control_plane_fixes() -> None: + workflow = WORKFLOW.read_text(encoding="utf-8") + + assert '- cron: "0 * * * *"' in workflow + assert '*/15 * * * *' not in workflow + assert workflow.count("runs-on: ubuntu-24.04") >= 3 + scan_job = workflow.split(" scan-pr-queue:", 1)[1].split(" org-queue-sweep:", 1)[0] + assert "github.event_name == 'pull_request_review'" in scan_job.split( + "TRIGGER_REVIEWS:", 1 + )[1].splitlines()[0]