Keep the staging branch sweep alive when a pull request is deleted

The scheduled reconciliation sweep has failed on every run since 19 Aug.
`process_pr` assumed every `<login>/advisory-improvement-<N>` branch still
has a pull request `N`, so the unguarded `gh api repos/.../pulls/N` returned
404 and `set -euo pipefail` killed the whole step. Branches are processed in
alphabetical order, so the sweep aborted at the same branch every time --
57 of 1,608 -- and never reconciled the remaining 97%. Runs triggered by
`workflow_run` were unaffected, which is why per-pull-request cleanup kept
working and the breakage went unnoticed.

Distinguish a deleted pull request from a transport failure, and isolate
per-target failures so one unreconcilable pull request no longer stops the
sweep. Failures are still counted and reported, and the step exits non-zero
at the end, so a real problem stays visible.

Resolving all 1,608 branches over REST would have cost ~1,770 requests and
~43 minutes on a 10-minute cron, so triage the branches in batches of 50
over GraphQL first. That brings a full sweep to roughly 55 API calls and
about 90 seconds. The triage only ever narrows the candidate set --
`process_pr` still re-verifies every target over REST before deleting -- and
the join deliberately keeps the branch name observed in the branch listing
rather than the one GraphQL reported, so a pull request retargeted between
the listing and the deletion is still caught.

Add a `concurrency` group so sweeps cannot overlap, keyed per-run for the
per-pull-request triggers so those are never queued behind a sweep.

`set -e` is disabled inside `process_pr` because it is invoked from an
`if !` context. Make the failure paths explicit rather than relying on it:
guard the response parsing, propagate a failed head-branch deletion so the
staging branch is never deleted after it, and stop trailing `rm` cleanups
from masking the exit status of the command they follow. Without that last
one a failed triage batch would have been silently dropped without being
counted, turning "always red and does nothing" into "always green and does
nothing".

Verified against the live repository: 1,608 branches triage to 140
actionable targets in 1m25s with no triage failures, and the run that
previously aborted now completes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: daca0885-6779-4adb-bb02-ff6920d73ab3

Author: taladrane

.github/workflows/delete_staging_and_head_branches_writer.yaml

@@ -17,6 +17,13 @@ permissions:
   contents: write
   pull-requests: read
 
+# Scheduled reconciliation sweeps share a single group so they cannot pile up on
+# top of each other. Per-pull-request runs get their own group so they are never
+# queued behind a sweep.
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event_name == 'schedule' && 'reconcile' || github.run_id }}
+  cancel-in-progress: false
+
 jobs:
   delete-staging-and-head-branches:
     if: ${{ github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || (github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event == 'pull_request') }}
@@ -59,6 +66,53 @@ jobs:
             jq -rn --arg value "$1" '$value | @uri'
           }
 
+          request_pull_request() {
+            local body_file="$1"
+            local pr_number="$2"
+
+            curl --silent --show-error \
+              --request GET \
+              --output "${body_file}" \
+              --write-out '%{http_code}' \
+              --header "Accept: application/vnd.github+json" \
+              --header "Authorization: Bearer ${GH_TOKEN}" \
+              --header "X-GitHub-Api-Version: 2022-11-28" \
+              "https://api.github.com/repos/${REPOSITORY}/pulls/${pr_number}"
+          }
+
+          # Prints the pull request JSON on stdout. Returns 0 when the pull request
+          # was read, 2 when it does not exist, and 1 when it could not be read.
+          fetch_pr_json() {
+            local pr_number="$1"
+            local body_file status
+
+            body_file="$(mktemp)"
+            if ! status="$(request_pull_request "${body_file}" "${pr_number}")"; then
+              rm -f "${body_file}"
+              echo "::error::Failed to read pull request ${pr_number}." >&2
+              return 1
+            fi
+
+            if [[ "${status}" == "404" || "${status}" == "410" ]]; then
+              rm -f "${body_file}"
+              return 2
+            fi
+
+            if [[ "${status}" != "200" ]]; then
+              cat "${body_file}" >&2
+              rm -f "${body_file}"
+              echo "::error::Failed to read pull request ${pr_number}: GitHub API returned ${status}." >&2
+              return 1
+            fi
+
+            if ! cat "${body_file}"; then
+              rm -f "${body_file}"
+              echo "::error::Failed to read the response body for pull request ${pr_number}." >&2
+              return 1
+            fi
+            rm -f "${body_file}"
+          }
+
           request_ref() {
             local body_file="$1"
             local method="$2"
@@ -104,7 +158,11 @@ jobs:
               return 1
             fi
 
-            current_sha="$(jq -er '.object.sha' "${body_file}")"
+            if ! current_sha="$(jq -er '.object.sha' "${body_file}")"; then
+              rm -f "${body_file}"
+              echo "::error::Could not read the current SHA of branch ${branch}."
+              return 1
+            fi
             if [[ -n "${expected_sha}" && "${current_sha}" != "${expected_sha}" ]]; then
               rm -f "${body_file}"
               echo "::error::Head branch ${branch} now points to ${current_sha}, not ${expected_sha}; leaving it and the staging branch in place."
@@ -136,15 +194,36 @@ jobs:
 
           process_pr() {
             local advisory_file_pages base_ref base_repo expected_staging_branch head_ref head_repo head_sha
-            local pr_json pr_number="$1" state
+            local fetch_status=0 pr_json pr_number="$1" state
             expected_staging_branch="${2:-}"
 
             if ! is_pr_number "${pr_number}"; then
               echo "::error::Unexpected pull request number: ${pr_number}"
               return 1
             fi
 
-            pr_json="$(gh api "repos/${REPOSITORY}/pulls/${pr_number}")"
+            pr_json="$(fetch_pr_json "${pr_number}")" || fetch_status=$?
+            if (( fetch_status == 2 )); then
+              # A manual run names one pull request explicitly, so a missing one is an
+              # operator error. During a sweep, or after a close event, it just means the
+              # pull request was deleted and there is nothing left to verify against.
+              if (( MISSING_PR_IS_ERROR )); then
+                echo "::error::Pull request ${pr_number} does not exist."
+                return 1
+              fi
+              echo "::warning::Pull request ${pr_number} no longer exists; leaving its branches in place."
+              return 0
+            fi
+            if (( fetch_status != 0 )); then
+              return 1
+            fi
+            # set -e is disabled inside this function because it is called from an "if !"
+            # context, so an unusable response body would otherwise turn into a silent
+            # skip on a green run rather than a reported failure.
+            if ! jq -e 'type == "object" and has("state")' >/dev/null 2>&1 <<<"${pr_json}"; then
+              echo "::error::Could not parse the API response for pull request ${pr_number}."
+              return 1
+            fi
             state="$(jq -r '.state' <<<"${pr_json}")"
             base_ref="$(jq -r '.base.ref' <<<"${pr_json}")"
             base_repo="$(jq -r '.base.repo.full_name' <<<"${pr_json}")"
@@ -182,15 +261,20 @@ jobs:
               return 1
             fi
 
-            advisory_file_pages="$(gh api --paginate "repos/${REPOSITORY}/pulls/${pr_number}/files?per_page=100" \
-              --jq 'any(.[]; .filename | startswith("advisories/"))')"
+            # process_pr is invoked from a conditional in the reconciliation loop, which
+            # disables `set -e` inside this function, so every failure is explicit below.
+            if ! advisory_file_pages="$(gh api --paginate "repos/${REPOSITORY}/pulls/${pr_number}/files?per_page=100" \
+              --jq 'any(.[]; .filename | startswith("advisories/"))')"; then
+              echo "::error::Failed to list the files changed by pull request ${pr_number}."
+              return 1
+            fi
             if ! grep -qx 'true' <<<"${advisory_file_pages}"; then
               echo "Pull request ${pr_number} does not modify advisories/; skipping."
               return 0
             fi
 
             if [[ "${head_ref}" == "${base_ref}" ]]; then
-              delete_branch "${base_ref}" "${head_sha}"
+              delete_branch "${base_ref}" "${head_sha}" || return 1
               return 0
             fi
 
@@ -199,22 +283,115 @@ jobs:
               return 1
             fi
 
-            delete_branch "${head_ref}" "${head_sha}"
+            # Never delete the staging branch when the head branch could not be removed.
+            delete_branch "${head_ref}" "${head_sha}" || return 1
             delete_branch "${base_ref}"
           }
 
+          # Resolves a batch of pull requests in a single GraphQL call and prints the
+          # ones that still look like cleanup candidates as "<number>\t<base branch>".
+          # Deleted pull requests come back as null alongside a NOT_FOUND error, so a
+          # partial response is expected and is not treated as a failure.
+          triage_chunk() {
+            local chunk_file="$1"
+            local errors pr_number query response
+
+            # shellcheck disable=SC2016 # $owner and $name are GraphQL variables, not shell ones.
+            query='query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) {'
+            while IFS=$'\t' read -r pr_number _; do
+              is_pr_number "${pr_number}" || continue
+              query+=" pr${pr_number}: pullRequest(number: ${pr_number}) { number state baseRefName headRepository { nameWithOwner } }"
+            done < "${chunk_file}"
+            query+=' } }'
+
+            response="$(mktemp)"
+            errors="$(mktemp)"
+            gh api graphql \
+              -F owner="${REPOSITORY%%/*}" \
+              -F name="${REPOSITORY#*/}" \
+              -f query="${query}" > "${response}" 2>"${errors}" || true
+
+            if ! jq -e '.data.repository' "${response}" >/dev/null 2>&1; then
+              # Only transport-level failures reach here; a response that merely reports
+              # deleted pull requests still carries .data.repository. Surface the reason
+              # so a persistently failing triage is diagnosable from the run log.
+              head -n 10 "${errors}" | sed 's/^/graphql: /' >&2 || true
+              rm -f "${response}" "${errors}"
+              return 1
+            fi
+
+            # A failing jq must not be masked by the trailing cleanup, otherwise the
+            # batch would be silently dropped without being counted as a failure.
+            if ! jq -r --arg repo "${REPOSITORY}" '
+              .data.repository
+              | to_entries[]
+              | .value
+              | select(. != null)
+              | select(.state == "CLOSED" or .state == "MERGED")
+              | select(.headRepository != null and .headRepository.nameWithOwner == $repo)
+              | [(.number | tostring), .baseRefName]
+              | @tsv
+            ' "${response}"; then
+              rm -f "${response}" "${errors}"
+              return 1
+            fi
+            rm -f "${response}" "${errors}"
+          }
+
           collect_reconciliation_targets() {
-            local branch branches
+            local branch branches candidates_file chunk chunk_dir join_status=0 pairs_file
+
+            if ! branches="$(gh api --paginate "repos/${REPOSITORY}/branches?per_page=100" --jq '.[].name')"; then
+              echo "::error::Failed to list the branches of ${REPOSITORY}." >&2
+              return 1
+            fi
+
+            pairs_file="$(mktemp)"
+            candidates_file="$(mktemp)"
+            chunk_dir="$(mktemp -d)"
 
-            branches="$(gh api --paginate "repos/${REPOSITORY}/branches?per_page=100" --jq '.[].name')"
             while IFS= read -r branch; do
               if [[ "${branch}" =~ ^[^/]+/advisory-improvement-([0-9]+)$ ]] &&
                  git check-ref-format "refs/heads/${branch}" >/dev/null; then
                 printf '%s\t%s\n' "${BASH_REMATCH[1]}" "${branch}"
               fi
-            done <<<"${branches}"
+            done <<<"${branches}" > "${pairs_file}"
+
+            if [[ ! -s "${pairs_file}" ]]; then
+              rm -rf "${pairs_file}" "${candidates_file}" "${chunk_dir}"
+              return 0
+            fi
+
+            # Resolving every staging branch over REST costs one request per branch.
+            # Batching the triage keeps a full sweep to a couple of dozen requests.
+            split -l "${TRIAGE_CHUNK_SIZE}" "${pairs_file}" "${chunk_dir}/chunk_"
+            for chunk in "${chunk_dir}"/chunk_*; do
+              if ! triage_chunk "${chunk}" >> "${candidates_file}"; then
+                TRIAGE_FAILURES=$(( TRIAGE_FAILURES + 1 ))
+                echo "::warning::Could not triage a batch of staging branches; they will be retried on the next run." >&2
+              fi
+            done
+
+            # Keep the branch name observed in the branch listing rather than the one
+            # reported by GraphQL, so process_pr still detects a pull request that was
+            # retargeted between the listing and the deletion.
+            if [[ -s "${candidates_file}" ]]; then
+              awk -F'\t' 'NR==FNR { keep[$0] = 1; next } ($1 FS $2) in keep' \
+                "${candidates_file}" "${pairs_file}" || join_status=$?
+            fi
+
+            echo "Inspected $(wc -l < "${pairs_file}" | tr -d ' ') staging branches." >&2
+            rm -rf "${pairs_file}" "${candidates_file}" "${chunk_dir}"
+            # The cleanup above must not mask a failed join, otherwise the sweep would
+            # report success while having reconciled nothing.
+            return "${join_status}"
           }
 
+          TRIAGE_CHUNK_SIZE=50
+          TRIAGE_FAILURES=0
+          PROCESS_FAILURES=0
+          MISSING_PR_IS_ERROR=0
+
           if [[ "${GITHUB_EVENT_NAME}" == "workflow_run" ]]; then
             PR_NUMBER="${WORKFLOW_RUN_PR_NUMBER}"
             if ! is_pr_number "${PR_NUMBER:-}"; then
@@ -223,14 +400,27 @@ jobs:
             fi
             process_pr "${PR_NUMBER}"
           elif [[ "${GITHUB_EVENT_NAME}" == "workflow_dispatch" ]]; then
+            MISSING_PR_IS_ERROR=1
             process_pr "${DISPATCH_PR_NUMBER}"
           else
-            TARGETS="$(collect_reconciliation_targets)"
-            if [[ -z "${TARGETS}" ]]; then
-              echo "No staging branches need reconciliation."
-              exit 0
-            fi
+            TARGETS_FILE="$(mktemp)"
+            collect_reconciliation_targets > "${TARGETS_FILE}"
+            echo "Reconciling $(wc -l < "${TARGETS_FILE}" | tr -d ' ') staging branch(es)."
+
+            # A single unreconcilable pull request must not stop the sweep, otherwise
+            # every branch after it is never reconciled.
             while IFS=$'\t' read -r PR_NUMBER STAGING_BRANCH; do
-              process_pr "${PR_NUMBER}" "${STAGING_BRANCH}"
-            done <<<"${TARGETS}"
+              [[ -n "${PR_NUMBER}" ]] || continue
+              if ! process_pr "${PR_NUMBER}" "${STAGING_BRANCH}"; then
+                PROCESS_FAILURES=$(( PROCESS_FAILURES + 1 ))
+                echo "::error::Failed to reconcile pull request ${PR_NUMBER} (${STAGING_BRANCH})."
+              fi
+            done < "${TARGETS_FILE}"
+            rm -f "${TARGETS_FILE}"
+
+            if (( TRIAGE_FAILURES > 0 || PROCESS_FAILURES > 0 )); then
+              echo "::error::Reconciliation finished with ${PROCESS_FAILURES} pull request failure(s) and ${TRIAGE_FAILURES} triage failure(s)."
+              exit 1
+            fi
+            echo "Reconciliation completed."
           fi