-
Notifications
You must be signed in to change notification settings - Fork 0
fix(scheduler): resolve live refs before cancelling runs #1348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
dca874b
7348cf6
50ceb75
5025350
7c69378
b99f7f1
6fb6303
25aac56
10ece3e
b61c4c8
1eebe1b
af519b7
03f87fa
5cfc2ff
4651a12
c2c2324
d820181
59f374e
4aa4b15
7483507
64eef29
e417a0c
c8b086c
0f390a8
0d7b7a1
0338b17
db86866
79d4461
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| if [ "$#" -ne 6 ]; then | ||
| echo "usage: $0 <repo> <run-id> <default-branch> <classified-default-sha> <classified-open-pr-heads-json> <superseded|aged-orphan>" >&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:-<missing>})." | ||
| fi | ||
| elif [ "$status" != "queued" ] && [ "$status" != "in_progress" ]; then | ||
| warn_preserve "superseded candidate is no longer queued or in progress (status=${status:-<missing>})." | ||
| fi | ||
|
|
||
| case "$event" in | ||
| pull_request|pull_request_target) | ||
| pr_number="$(jq -r '.pull_requests[0].number // empty' <<<"$run_json")" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Secondary pull requests lose current checks For runs associated with several pull requests, Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
| 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 | ||
|
Comment on lines
+67
to
+106
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| 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 | ||
|
Comment on lines
+134
to
+136
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| 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:-<missing>} 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 | ||
|
Comment on lines
+176
to
+178
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
Uh oh!
There was an error while loading. Please reload this page.