Abort the run when a git/gh command fails unexpectedly

.github/workflows/tests.yml

@@ -13,6 +13,7 @@ jobs:
 
       - name: Run unit tests
         run: |
+          bash tests/test_command_utils.sh
           bash tests/test_update_pr_stack.sh
           bash tests/test_rebase_workflow.sh
           bash tests/test_mixed_workflows.sh

command_utils.sh

@@ -7,3 +7,26 @@ log_cmd() {
     printf "\n" >&2
     "$@"
 }
+
+die() {
+    echo "❌ $*" >&2
+    exit 1
+}
+
+# Log and execute a command, aborting the run if it fails. The explicit exit
+# in die aborts from any context; `set -e` does not, because it is suppressed
+# inside if/&&/|| conditions and everything they call, including the whole
+# body of a function invoked as a condition.
+#
+# Note: inside a command substitution, exit only leaves the subshell, so
+# `VAR=$(run ...)` does not abort the script. Use `VAR=$(try ...) || die ...`
+# instead.
+run() {
+    log_cmd "$@" || die "command failed (exit $?): $*"
+}
+
+# Log and execute a command whose failure is an expected outcome (e.g. a
+# merge that may conflict), handing the exit status to the caller.
+try() {
+    log_cmd "$@"
+}

tests/test_command_utils.sh

@@ -0,0 +1,56 @@
+#!/bin/bash
+#
+# Tests for the run/try/die wrappers. The property that matters: run aborts
+# the whole script even where `set -e` is suppressed, i.e. inside an `if`
+# condition, including the body of a function invoked as the condition. That
+# is exactly where update-pr-stack.sh does most of its work, so `set -e`
+# alone cannot be relied on there.
+
+set -ueo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+ROOT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)"
+
+PASS=0
+fail() { echo "❌ $1"; exit 1; }
+ok() { echo "✅ $1"; PASS=$((PASS+1)); }
+
+# Baseline first, to document the trap that motivates run: with plain set -e,
+# a failure inside a function called as an if-condition does NOT stop it.
+OUT=$(bash -c '
+    set -ueo pipefail
+    f() { false; echo "after-false"; }
+    if f; then :; fi
+' 2>&1)
+grep -q "after-false" <<<"$OUT" || fail "baseline: expected set -e to be suppressed in condition context"
+ok "baseline: set -e is suppressed inside an if-condition function"
+
+# run must abort both the function and the script from that same context.
+STATUS=0
+OUT=$(ROOT_DIR="$ROOT_DIR" bash -c '
+    set -ueo pipefail
+    source "$ROOT_DIR/command_utils.sh"
+    f() { run false; echo "after-run"; }
+    if f; then :; fi
+    echo "survived"
+' 2>&1) || STATUS=$?
+[[ "$STATUS" -ne 0 ]] || fail "run: script should exit nonzero"
+grep -q "after-run" <<<"$OUT" && fail "run: function continued after the failure"
+grep -q "survived" <<<"$OUT" && fail "run: script continued after the failure"
+grep -q "command failed" <<<"$OUT" || fail "run: no failure message printed"
+ok "run aborts the script from a condition context"
+
+# try hands the status back without aborting.
+OUT=$(ROOT_DIR="$ROOT_DIR" bash -c '
+    set -ueo pipefail
+    source "$ROOT_DIR/command_utils.sh"
+    if ! try false; then echo "handled"; fi
+    try true || exit 9
+    echo "done"
+' 2>&1)
+grep -q "handled" <<<"$OUT" || fail "try: failure status not handed to caller"
+grep -q "done" <<<"$OUT" || fail "try: success path broken"
+ok "try returns the status to the caller"
+
+echo
+echo "All command_utils tests passed 🎉 ($PASS)"

tests/test_conflict_resolution_resume.sh

@@ -22,6 +22,7 @@ ok() { echo "✅ $1"; PASS=$((PASS+1)); }
 # $CALLS and is driven by env vars set per scenario:
 #   MOCK_LABELS         newline-separated labels returned by `pr view --json labels`
 #   MOCK_COMMENTS_FILE  file whose contents are returned by `pr view --json comments`
+#   MOCK_LABELS_FAIL    set to 1 to make `pr view --json labels` fail
 # The PR's base branch is not mocked: the script must take it from PR_BASE
 # (event payload), so a baseRefName query is an unhandled call and fails.
 make_mock_gh() {
@@ -32,7 +33,9 @@ set -ueo pipefail
 echo "gh $*" >> "$CALLS"
 if [[ "$1 $2" == "pr view" ]]; then
     case "$*" in
-        *--json\ labels*)   printf '%s\n' "${MOCK_LABELS:-}";;
+        *--json\ labels*)
+            [[ "${MOCK_LABELS_FAIL:-}" == 1 ]] && { echo "mock gh: labels API down" >&2; exit 1; }
+            printf '%s\n' "${MOCK_LABELS:-}";;
         *--json\ comments*)
             # The comments file stands for our own comments only, so the query
             # must restrict itself to those.
@@ -94,7 +97,7 @@ setup_repo() {
 run_resume() {
     env ACTION_MODE=conflict-resolved PR_BRANCH=child PR_NUMBER=5 PR_BASE="$PR_BASE" \
         GH="$MOCK_DIR/mock_gh.sh" GIT="$MOCK_DIR/mock_git.sh" \
-        MOCK_LABELS="$MOCK_LABELS" \
+        MOCK_LABELS="$MOCK_LABELS" MOCK_LABELS_FAIL="${MOCK_LABELS_FAIL:-}" \
         MOCK_COMMENTS_FILE="$MOCK_COMMENTS_FILE" CALLS="$CALLS" \
         bash "$ROOT_DIR/update-pr-stack.sh" >"$WORK/out.log" 2>&1 || echo "EXIT=$?" >>"$WORK/out.log"
 }
@@ -217,5 +220,39 @@ grep -q -- "--base" "$CALLS" && fail "F: base must NOT be edited"
 [[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "F: child was pushed"
 ok "F: missing base branch detected, no crash, label removed"
 
+# ---------------------------------------------------------------------------
+echo "### Scenario G: reading the PR comments fails -> fail the run, keep the label"
+setup_repo
+# An API failure must not pass for "no state marker": that path removes the
+# conflict label and permanently cancels the auto-resume. Fail loudly instead,
+# so the label stays on and the next push retries.
+MOCK_LABELS="autorestack-needs-conflict-resolution"
+PR_BASE="parent"
+MOCK_COMMENTS_FILE="$WORK/does-not-exist"   # makes the mock gh fail on pr view --json comments
+run_resume
+
+grep -q "EXIT=" "$WORK/out.log" || fail "G: run should have failed"
+grep -q "remove-label" "$CALLS" && fail "G: label must NOT be removed on an API failure"
+grep -q "gh pr comment" "$CALLS" && fail "G: no comment must be posted on an API failure"
+[[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "G: child was pushed"
+ok "G: comments API failure fails the run and keeps the resume armed"
+
+# ---------------------------------------------------------------------------
+echo "### Scenario H: reading the labels fails -> fail the run, do nothing"
+setup_repo
+# An API failure must not pass for "no conflict label": that ends the run
+# green without resuming anything.
+MOCK_LABELS=""
+MOCK_LABELS_FAIL=1
+PR_BASE="parent"
+MOCK_COMMENTS_FILE="$WORK/comments.txt"
+{ echo "### conflict"; echo; marker parent main "$SQUASH"; } > "$MOCK_COMMENTS_FILE"
+run_resume
+
+grep -q "EXIT=" "$WORK/out.log" || fail "H: run should have failed, not ended green"
+grep -q "remove-label" "$CALLS" && fail "H: label must NOT be touched on an API failure"
+[[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "H: child was pushed"
+ok "H: labels API failure fails the run instead of skipping the resume"
+
 echo
 echo "All conflict-resume tests passed 🎉 ($PASS scenarios)"