Skip to content

Commit 6feea4a

Browse files
taladraneCopilot
andcommitted
Do not fail the sweep when a head branch is reused by a newer pull request
Now that the sweep runs to completion it surfaces a condition that was previously unreachable. When a contributor closes a pull request and then reuses its head branch for a new one, the head branch no longer points at the SHA the closed pull request recorded. `delete_branch` correctly refuses to delete it -- deleting it would break the newer pull request -- but it reported that refusal as a failure, so the whole run went red. That state is permanent for as long as the newer pull request stays open, so the workflow would have been red on every run indefinitely. That is the same alert fatigue the previous fix set out to remove, just from a different cause. `jdeniau-GHSA-xvcm-6775-5m9r` is a live example: pull request 9129 was closed, one more commit was pushed to its head branch, and pull request 9226 was opened from it and is still open. Reusing a branch this way is a normal contributor pattern here, not an operational fault, and it needs no human intervention: once the newer pull request is closed its own cleanup removes the branch, the older pull request's head then reads as already absent, and the leftover staging branch is collected on the next sweep. Treat it the way the other "nothing safe to do here" states are already treated -- still open, fork-headed, deleted -- and skip it instead of failing. `delete_branch` now returns 3 for this case specifically, so a genuine deletion failure is still a failure and still stops the staging branch from being deleted after it. Nothing that was previously deleted is deleted any differently; only the reporting changes. Record the skipped branches and write them to the job summary along with the sweep counters, so they stay visible without being buried in a 1,600-line log and without holding the run red. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: daca0885-6779-4adb-bb02-ff6920d73ab3
1 parent a92e708 commit 6feea4a

1 file changed

Lines changed: 80 additions & 7 deletions

File tree

‎.github/workflows/delete_staging_and_head_branches_writer.yaml‎

Lines changed: 80 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,12 @@ jobs:
165165
fi
166166
if [[ -n "${expected_sha}" && "${current_sha}" != "${expected_sha}" ]]; then
167167
rm -f "${body_file}"
168-
echo "::error::Head branch ${branch} now points to ${current_sha}, not ${expected_sha}; leaving it and the staging branch in place."
169-
return 1
168+
# The branch moved after the pull request closed, which normally means a newer
169+
# pull request reused it. Deleting it would break that pull request, so report
170+
# this as a skip rather than a failure: it resolves itself once the newer pull
171+
# request is closed and cleaned up in turn.
172+
echo "::warning::Head branch ${branch} now points to ${current_sha}, not ${expected_sha}; it is in use elsewhere, so it and the staging branch are left in place."
173+
return 3
170174
fi
171175
172176
if ! status="$(request_ref "${body_file}" DELETE "${encoded_branch}")"; then
@@ -192,9 +196,15 @@ jobs:
192196
return 1
193197
}
194198
199+
# Records a branch that was deliberately left in place, so the job summary can
200+
# report it without the run having to fail.
201+
record_skip() {
202+
printf '%s\t%s\n' "$1" "$2" >> "${SKIPPED_FILE}"
203+
}
204+
195205
process_pr() {
196206
local advisory_file_pages base_ref base_repo expected_staging_branch head_ref head_repo head_sha
197-
local fetch_status=0 pr_json pr_number="$1" state
207+
local delete_status=0 fetch_status=0 pr_json pr_number="$1" state
198208
expected_staging_branch="${2:-}"
199209
200210
if ! is_pr_number "${pr_number}"; then
@@ -274,7 +284,12 @@ jobs:
274284
fi
275285
276286
if [[ "${head_ref}" == "${base_ref}" ]]; then
277-
delete_branch "${base_ref}" "${head_sha}" || return 1
287+
delete_branch "${base_ref}" "${head_sha}" || delete_status=$?
288+
if (( delete_status == 3 )); then
289+
record_skip "${pr_number}" "${base_ref}"
290+
return 0
291+
fi
292+
(( delete_status == 0 )) || return 1
278293
return 0
279294
fi
280295
@@ -284,7 +299,14 @@ jobs:
284299
fi
285300
286301
# Never delete the staging branch when the head branch could not be removed.
287-
delete_branch "${head_ref}" "${head_sha}" || return 1
302+
delete_branch "${head_ref}" "${head_sha}" || delete_status=$?
303+
if (( delete_status == 3 )); then
304+
record_skip "${pr_number}" "${head_ref}"
305+
return 0
306+
fi
307+
if (( delete_status != 0 )); then
308+
return 1
309+
fi
288310
delete_branch "${base_ref}"
289311
}
290312
@@ -393,17 +415,66 @@ jobs:
393415
"${candidates_file}" "${pairs_file}" || join_status=$?
394416
fi
395417
396-
echo "Inspected $(wc -l < "${pairs_file}" | tr -d ' ') staging branches." >&2
418+
INSPECTED_COUNT="$(wc -l < "${pairs_file}" | tr -d ' ')"
419+
echo "Inspected ${INSPECTED_COUNT} staging branches." >&2
397420
rm -rf "${pairs_file}" "${candidates_file}" "${chunk_dir}"
398421
# The cleanup above must not mask a failed join, otherwise the sweep would
399422
# report success while having reconciled nothing.
400423
return "${join_status}"
401424
}
402425
426+
# Branches left in place are expected rather than exceptional, so they are
427+
# reported in the run summary instead of being buried in the log.
428+
write_job_summary() {
429+
local branch pr skipped_count=0
430+
431+
[[ -n "${GITHUB_STEP_SUMMARY:-}" ]] || return 0
432+
if [[ -s "${SKIPPED_FILE}" ]]; then
433+
skipped_count="$(wc -l < "${SKIPPED_FILE}" | tr -d ' ')"
434+
fi
435+
# A per-pull-request run with nothing to report should not add an empty section.
436+
if (( ! SWEEP && skipped_count == 0 )); then
437+
return 0
438+
fi
439+
440+
{
441+
echo "### Staging branch cleanup"
442+
echo
443+
if (( SWEEP )); then
444+
echo "| Metric | Count |"
445+
echo "| --- | ---: |"
446+
echo "| Staging branches inspected | ${INSPECTED_COUNT} |"
447+
echo "| Pull requests reconciled | ${RECONCILE_COUNT} |"
448+
echo "| Left in place (head branch in use) | ${skipped_count} |"
449+
echo "| Pull request failures | ${PROCESS_FAILURES} |"
450+
echo "| Triage batches failed | ${TRIAGE_FAILURES} |"
451+
echo
452+
fi
453+
454+
if (( skipped_count > 0 )); then
455+
echo "<details><summary>Head branch reused by a newer pull request (${skipped_count})</summary>"
456+
echo
457+
echo "Deleting these branches would break the newer pull request that now uses them, so they were left alone. They are cleaned up automatically once that pull request is closed. No action is needed."
458+
echo
459+
while IFS=$'\t' read -r pr branch; do
460+
echo "- \`${branch}\` — left over from #${pr}"
461+
done < "${SKIPPED_FILE}"
462+
echo
463+
echo "</details>"
464+
fi
465+
} >> "${GITHUB_STEP_SUMMARY}"
466+
}
467+
403468
TRIAGE_CHUNK_SIZE=50
404469
TRIAGE_FAILURES=0
405470
PROCESS_FAILURES=0
406471
MISSING_PR_IS_ERROR=0
472+
INSPECTED_COUNT=0
473+
RECONCILE_COUNT=0
474+
SWEEP=0
475+
SKIPPED_FILE="$(mktemp)"
476+
# An EXIT trap keeps the summary accurate even when the run ends in failure.
477+
trap 'write_job_summary || true; rm -f "${SKIPPED_FILE}"' EXIT
407478
408479
if [[ "${GITHUB_EVENT_NAME}" == "workflow_run" ]]; then
409480
PR_NUMBER="${WORKFLOW_RUN_PR_NUMBER}"
@@ -416,9 +487,11 @@ jobs:
416487
MISSING_PR_IS_ERROR=1
417488
process_pr "${DISPATCH_PR_NUMBER}"
418489
else
490+
SWEEP=1
419491
TARGETS_FILE="$(mktemp)"
420492
collect_reconciliation_targets > "${TARGETS_FILE}"
421-
echo "Reconciling $(wc -l < "${TARGETS_FILE}" | tr -d ' ') staging branch(es)."
493+
RECONCILE_COUNT="$(wc -l < "${TARGETS_FILE}" | tr -d ' ')"
494+
echo "Reconciling ${RECONCILE_COUNT} staging branch(es)."
422495
423496
# A single unreconcilable pull request must not stop the sweep, otherwise
424497
# every branch after it is never reconciled.

0 commit comments

Comments
 (0)