diff --git a/.github/workflows/delete_staging_and_head_branches_writer.yaml b/.github/workflows/delete_staging_and_head_branches_writer.yaml index e87a95ef7580..1437391f0622 100644 --- a/.github/workflows/delete_staging_and_head_branches_writer.yaml +++ b/.github/workflows/delete_staging_and_head_branches_writer.yaml @@ -113,6 +113,53 @@ jobs: rm -f "${body_file}" } + request_open_prs_for_head() { + local body_file="$1" + local encoded_branch="$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?state=open&per_page=100&head=${REPOSITORY%%/*}:${encoded_branch}" + } + + # Prints the number of an open pull request that currently uses the branch as its + # head ref. Returns 0 when one exists, 2 when none does, and 1 when the lookup + # failed. A failed lookup must never be read as "nothing is using this branch". + open_pr_using_branch() { + local branch="$1" + local body_file encoded_branch pr_number status + + encoded_branch="$(encode_ref "${branch}")" + body_file="$(mktemp)" + if ! status="$(request_open_prs_for_head "${body_file}" "${encoded_branch}")"; then + rm -f "${body_file}" + echo "::error::Failed to look up open pull requests for branch ${branch}." >&2 + return 1 + fi + if [[ "${status}" != "200" ]]; then + cat "${body_file}" >&2 + rm -f "${body_file}" + echo "::error::Failed to look up open pull requests for branch ${branch}: GitHub API returned ${status}." >&2 + return 1 + fi + + # The head filter is re-checked locally so that an unexpected response shape + # reports "none found" rather than silently authorising a deletion. + if ! pr_number="$(jq -er --arg branch "${branch}" --arg repo "${REPOSITORY}" \ + 'map(select(.head.ref == $branch and .head.repo.full_name == $repo)) | first | .number' \ + "${body_file}")"; then + rm -f "${body_file}" + return 2 + fi + rm -f "${body_file}" + printf '%s\n' "${pr_number}" + } + request_ref() { local body_file="$1" local method="$2" @@ -136,7 +183,7 @@ jobs: delete_branch() { local branch="$1" local expected_sha="${2:-}" - local body_file current_sha encoded_branch status + local blocking_pr body_file current_sha encoded_branch lookup_status status encoded_branch="$(encode_ref "${branch}")" body_file="$(mktemp)" @@ -165,7 +212,24 @@ jobs: 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." + # The branch moved after the pull request closed. That is only safe to ignore + # when another open pull request is using it: deleting it would break that + # pull request, and it is collected anyway once that pull request closes in + # turn. Any other cause -- a push after closure, or a ref deleted and + # recreated -- leaves an orphan that nothing else will ever collect, so it has + # to be surfaced rather than silently reported as a self-healing skip. + lookup_status=0 + blocking_pr="$(open_pr_using_branch "${branch}")" || lookup_status=$? + if (( lookup_status == 0 )); then + BLOCKING_PR="${blocking_pr}" + echo "::warning::Branch ${branch} now points to ${current_sha}, not ${expected_sha}, because open pull request #${blocking_pr} is using it; it and the staging branch are left in place." + return 3 + fi + if (( lookup_status == 2 )); then + echo "::error::Branch ${branch} now points to ${current_sha}, not ${expected_sha}, and no open pull request is using it. Leaving it in place because the change is unexplained; this branch needs manual investigation." + return 1 + fi + echo "::error::Could not determine whether branch ${branch} is still in use, so it was left in place." return 1 fi @@ -192,9 +256,16 @@ jobs: return 1 } + # Records a branch that was deliberately left in place, along with the open pull + # request that is using it, so the job summary can report it without the run + # having to fail. + record_skip() { + printf '%s\t%s\t%s\n' "$1" "$2" "$3" >> "${SKIPPED_FILE}" + } + process_pr() { local advisory_file_pages base_ref base_repo expected_staging_branch head_ref head_repo head_sha - local fetch_status=0 pr_json pr_number="$1" state + local delete_status=0 fetch_status=0 pr_json pr_number="$1" state expected_staging_branch="${2:-}" if ! is_pr_number "${pr_number}"; then @@ -274,7 +345,12 @@ jobs: fi if [[ "${head_ref}" == "${base_ref}" ]]; then - delete_branch "${base_ref}" "${head_sha}" || return 1 + delete_branch "${base_ref}" "${head_sha}" || delete_status=$? + if (( delete_status == 3 )); then + record_skip "${pr_number}" "${base_ref}" "${BLOCKING_PR}" + return 0 + fi + (( delete_status == 0 )) || return 1 return 0 fi @@ -284,7 +360,14 @@ jobs: fi # Never delete the staging branch when the head branch could not be removed. - delete_branch "${head_ref}" "${head_sha}" || return 1 + delete_branch "${head_ref}" "${head_sha}" || delete_status=$? + if (( delete_status == 3 )); then + record_skip "${pr_number}" "${head_ref}" "${BLOCKING_PR}" + return 0 + fi + if (( delete_status != 0 )); then + return 1 + fi delete_branch "${base_ref}" } @@ -393,17 +476,67 @@ jobs: "${candidates_file}" "${pairs_file}" || join_status=$? fi - echo "Inspected $(wc -l < "${pairs_file}" | tr -d ' ') staging branches." >&2 + INSPECTED_COUNT="$(wc -l < "${pairs_file}" | tr -d ' ')" + echo "Inspected ${INSPECTED_COUNT} 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}" } + # Branches left in place are expected rather than exceptional, so they are + # reported in the run summary instead of being buried in the log. + write_job_summary() { + local blocking_pr branch pr skipped_count=0 + + [[ -n "${GITHUB_STEP_SUMMARY:-}" ]] || return 0 + if [[ -s "${SKIPPED_FILE}" ]]; then + skipped_count="$(wc -l < "${SKIPPED_FILE}" | tr -d ' ')" + fi + # A per-pull-request run with nothing to report should not add an empty section. + if (( ! SWEEP && skipped_count == 0 )); then + return 0 + fi + + { + echo "### Staging branch cleanup" + echo + if (( SWEEP )); then + echo "| Metric | Count |" + echo "| --- | ---: |" + echo "| Staging branches inspected | ${INSPECTED_COUNT} |" + echo "| Pull requests reconciled | ${RECONCILE_COUNT} |" + echo "| Left in place (branch reused by an open pull request) | ${skipped_count} |" + echo "| Pull request failures | ${PROCESS_FAILURES} |" + echo "| Triage batches failed | ${TRIAGE_FAILURES} |" + echo + fi + + if (( skipped_count > 0 )); then + echo "
Head branch reused by a newer pull request (${skipped_count})" + echo + echo "Each of these branches was verified to still be the head ref of an open pull request, so deleting it would break that pull request and it was left alone. They are collected automatically once that pull request is closed. No action is needed." + echo + while IFS=$'\t' read -r pr branch blocking_pr; do + echo "- \`${branch}\` — left over from #${pr}, still in use by open pull request #${blocking_pr}" + done < "${SKIPPED_FILE}" + echo + echo "
" + fi + } >> "${GITHUB_STEP_SUMMARY}" + } + TRIAGE_CHUNK_SIZE=50 TRIAGE_FAILURES=0 PROCESS_FAILURES=0 MISSING_PR_IS_ERROR=0 + INSPECTED_COUNT=0 + RECONCILE_COUNT=0 + SWEEP=0 + BLOCKING_PR="" + SKIPPED_FILE="$(mktemp)" + # An EXIT trap keeps the summary accurate even when the run ends in failure. + trap 'write_job_summary || true; rm -f "${SKIPPED_FILE}"' EXIT if [[ "${GITHUB_EVENT_NAME}" == "workflow_run" ]]; then PR_NUMBER="${WORKFLOW_RUN_PR_NUMBER}" @@ -416,9 +549,11 @@ jobs: MISSING_PR_IS_ERROR=1 process_pr "${DISPATCH_PR_NUMBER}" else + SWEEP=1 TARGETS_FILE="$(mktemp)" collect_reconciliation_targets > "${TARGETS_FILE}" - echo "Reconciling $(wc -l < "${TARGETS_FILE}" | tr -d ' ') staging branch(es)." + RECONCILE_COUNT="$(wc -l < "${TARGETS_FILE}" | tr -d ' ')" + echo "Reconciling ${RECONCILE_COUNT} staging branch(es)." # A single unreconcilable pull request must not stop the sweep, otherwise # every branch after it is never reconciled.