Report skips that could not be recorded as failures

record_skip appends to the file the job summary is built from, but every
caller runs under "if ! process_pr", which disables set -e for the
function and everything it calls. The "return 0" that followed each call
overwrote the append's exit status, so a failed write left the run green
while silently dropping the branch from the summary.

Two of the four call sites are added by this branch, and the summary is
the only record of which branches were left in place, so a skip that
cannot be recorded is now surfaced as a failure instead of being lost.
Nothing has been deleted at any of these points, so failing there cannot
leave a branch half-processed.

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

@@ -319,8 +319,15 @@ jobs:
           # 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.
+          # Callers run under "if ! process_pr", which disables set -e for everything they
+          # call, so a failed write here has to be returned and propagated by hand. A skip
+          # that cannot be recorded is reported as a failure rather than dropped, because
+          # the job summary is the only record of which branches were left in place.
           record_skip() {
-            printf '%s\t%s\t%s\n' "$1" "$2" "$3" >> "${SKIPPED_FILE}"
+            if ! printf '%s\t%s\t%s\n' "$1" "$2" "$3" >> "${SKIPPED_FILE}"; then
+              echo "::error::Could not record that branch $2 was left in place for pull request $1."
+              return 1
+            fi
           }
 
           # Removes the staging branch a closed pull request targeted. The branch always
@@ -349,7 +356,7 @@ jobs:
             blocking_pr="$(open_pr_using_ref "${branch}")" || lookup_status=$?
             if (( lookup_status == 0 )); then
               echo "::warning::Staging branch ${branch} is still in use by open pull request #${blocking_pr}; leaving it in place."
-              record_skip "${pr_number}" "${branch}" "${blocking_pr}"
+              record_skip "${pr_number}" "${branch}" "${blocking_pr}" || return 1
               return 0
             fi
             if (( lookup_status != 1 )); then
@@ -362,7 +369,7 @@ jobs:
             # in use by an open pull request" skip into a run failure.
             delete_branch "${branch}" || delete_status=$?
             if (( delete_status == 3 )); then
-              record_skip "${pr_number}" "${branch}" "${BLOCKING_PR}"
+              record_skip "${pr_number}" "${branch}" "${BLOCKING_PR}" || return 1
               return 0
             fi
             (( delete_status == 0 )) || return 1
@@ -462,7 +469,7 @@ jobs:
             if [[ "${head_ref}" == "${base_ref}" ]]; then
               delete_branch "${base_ref}" "${head_sha}" || delete_status=$?
               if (( delete_status == 3 )); then
-                record_skip "${pr_number}" "${base_ref}" "${BLOCKING_PR}"
+                record_skip "${pr_number}" "${base_ref}" "${BLOCKING_PR}" || return 1
                 return 0
               fi
               (( delete_status == 0 )) || return 1
@@ -477,7 +484,7 @@ jobs:
             # Never delete the staging branch when the head branch could not be removed.
             delete_branch "${head_ref}" "${head_sha}" || delete_status=$?
             if (( delete_status == 3 )); then
-              record_skip "${pr_number}" "${head_ref}" "${BLOCKING_PR}"
+              record_skip "${pr_number}" "${head_ref}" "${BLOCKING_PR}" || return 1
               return 0
             fi
             if (( delete_status != 0 )); then