From 340ff80cd2f26a6a3f78baa48fb363823b7b043e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 21:37:01 +0000 Subject: [PATCH 1/4] Give up cleanly when the kept parent branch is gone during conflict resolution If origin/ was deleted while a child PR sat in conflict (auto-delete head branches left enabled despite the README, or manual deletion), the resume merge failed with "not something we can merge" and git merge --abort failed too ("There is no merge to abort"). The run then exited nonzero after re-posting the misleading conflict comment and the label, repeating on every push. (Not a mid-function set -e kill: update_direct_target is called in an if condition, which suppresses errexit; observed via the new test scenario.) Check origin/ up front like #39 did for the target branch, and only run git merge --abort when MERGE_HEAD exists. https://claude.ai/code/session_01STkeSJ7cLrmrNn4aTDYkwH --- tests/test_conflict_resolution_resume.sh | 30 ++++++++++++++++++++++++ update-pr-stack.sh | 24 +++++++++++++++++-- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/tests/test_conflict_resolution_resume.sh b/tests/test_conflict_resolution_resume.sh index 3f0956a..933611d 100644 --- a/tests/test_conflict_resolution_resume.sh +++ b/tests/test_conflict_resolution_resume.sh @@ -167,5 +167,35 @@ grep -q -- "--base" "$CALLS" && fail "D: base must NOT be edited" [[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "D: child was pushed" ok "D: missing target detected, no branch mutation, label removed" +# --------------------------------------------------------------------------- +echo "### Scenario E: recorded base branch is gone -> give up cleanly, no crash" +setup_repo +# Advance main with the squash commit so the child is not already up to date and +# the resume would actually reach the merge step. +git checkout -q main +echo squash > s.txt && git add s.txt && git commit -qm squash +SQUASH2=$(git rev-parse main) +git push -q origin main +git checkout -q child +# The kept parent branch was deleted (auto-delete head branches left enabled, or +# manual deletion). Before the up-front check, the resume tried to merge the +# missing ref, failed `git merge --abort` because no merge was in progress, and +# exited nonzero after re-posting a misleading conflict comment and the label, +# repeating on every push. +git push -q origin ":parent" +MOCK_LABELS="autorestack-needs-conflict-resolution" +MOCK_BASE="parent" # matches marker -> not a manual retarget +MOCK_COMMENTS_FILE="$WORK/comments.txt" +{ echo "### conflict"; echo; marker parent main "$SQUASH2"; } > "$MOCK_COMMENTS_FILE" +run_resume + +grep -q "EXIT=" "$WORK/out.log" && fail "E: script exited nonzero: $(cat "$WORK/out.log")" +grep -q "remove-label autorestack-needs-conflict-resolution" "$CALLS" || fail "E: label not removed" +grep -q -- "add-label" "$CALLS" && fail "E: conflict label must NOT be re-added" +grep -q "gh pr comment" "$CALLS" || fail "E: no explanatory comment posted" +grep -q -- "--base" "$CALLS" && fail "E: base must NOT be edited" +[[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "E: child was pushed" +ok "E: missing base branch detected, no crash, label removed" + echo echo "All conflict-resume tests passed 🎉 ($PASS scenarios)" diff --git a/update-pr-stack.sh b/update-pr-stack.sh index a2d8ca5..94c271e 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -74,6 +74,16 @@ has_squash_commit() { && git merge-base --is-ancestor SQUASH_COMMIT "$BRANCH" } +# A failed git merge does not always leave a merge in progress: when the ref to +# merge does not exist ("not something we can merge"), there is no MERGE_HEAD, +# and `git merge --abort` itself fails ("There is no merge to abort"). Only +# abort when a merge is actually in progress. +abort_merge_if_in_progress() { + if git rev-parse --verify --quiet MERGE_HEAD >/dev/null; then + log_cmd git merge --abort + fi +} + update_direct_target() { local BRANCH="$1" local BASE_BRANCH="$2" @@ -96,14 +106,14 @@ update_direct_target() { if ! log_cmd git merge --no-edit "origin/$MERGED_BRANCH"; then CONFLICTS+=("origin/$MERGED_BRANCH") BASE_MERGE_CLEAN=false - log_cmd git merge --abort + abort_merge_if_in_progress fi # Only try merging the pre-squash target state if it's not already # included in the merged branch — otherwise the first merge covers it. if ! git merge-base --is-ancestor SQUASH_COMMIT~ "origin/$MERGED_BRANCH"; then if ! log_cmd git merge --no-edit SQUASH_COMMIT~; then CONFLICTS+=( "$(git rev-parse SQUASH_COMMIT~) # $TARGET_BRANCH just before $MERGED_BRANCH was merged" ) - log_cmd git merge --abort + abort_merge_if_in_progress fi fi @@ -256,6 +266,16 @@ continue_after_resolution() { return fi + # Same check for the old base: the resume re-merges origin/$OLD_BASE, so if + # that branch is gone (auto-delete head branches left enabled, or deleted + # manually) the merge can never succeed and the label would re-trigger a + # failing run on every push. Give up cleanly instead. + if ! git rev-parse --verify --quiet "origin/$OLD_BASE" >/dev/null; then + echo "⚠ī¸ Recorded base branch '$OLD_BASE' no longer exists; abandoning resume of $PR_BRANCH." + abandon_resume "$PR_BRANCH" "ℹī¸ The branch this PR was based on (\`$OLD_BASE\`) no longer exists, so autorestack stepped back. If this PR still needs its base updated, update its base manually." + return + fi + # The squash-merge run pushed the base merge and asked the user to resolve the # pre-squash merge, but it never recorded the squash itself. Finish that now: # re-run the same merge sequence as the squash-merge path. With the user's From 24c6c99ebdf16dd97fb3b37d208b5f02dd2cf201 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 22:02:21 +0000 Subject: [PATCH 2/4] Abort the run when a git/gh command fails unexpectedly set -e is suppressed inside if/&&/|| conditions and everything they call, including the whole body of update_direct_target, which both entry points invoke as a condition. So most failures there fell through silently: a failed git checkout let the merges and the final git reset --hard run on the previously checked-out branch (corrupting it, in main's loop over several child PRs), and a failed git commit-tree let the function return success, after which main retargeted the PR onto a base its head does not contain. A failed conflict comment still added the label, leaving a resume with no state marker. Every log_cmd call site now states its failure policy: run (log, execute, die on failure; die's explicit exit aborts from any context) or try (log, execute, hand the status to the caller) for the merges whose failure is an expected outcome. set -e stays as a backstop only. https://claude.ai/code/session_01STkeSJ7cLrmrNn4aTDYkwH --- command_utils.sh | 23 ++++++++++++ tests/test_command_utils.sh | 56 +++++++++++++++++++++++++++++ update-pr-stack.sh | 70 ++++++++++++++++++++++--------------- 3 files changed, 121 insertions(+), 28 deletions(-) create mode 100755 tests/test_command_utils.sh diff --git a/command_utils.sh b/command_utils.sh index a78d278..56212df 100644 --- a/command_utils.sh +++ b/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 "$@" +} diff --git a/tests/test_command_utils.sh b/tests/test_command_utils.sh new file mode 100755 index 0000000..aa07d43 --- /dev/null +++ b/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)" diff --git a/update-pr-stack.sh b/update-pr-stack.sh index 94c271e..2b1a0b7 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -14,7 +14,11 @@ # possible, so the logged commands are self-contained and reproducible # - We strive to keep commands as simple as possible -set -ueo pipefail # Exit on error, undefined var, or pipeline failure +# set -u and pipefail do the real work here; set -e is only a backstop. It is +# suppressed inside if/&&/|| conditions and everything they call, including +# the whole body of update_direct_target (always invoked as a condition), so +# failure handling is explicit instead: run/try/die from command_utils.sh. +set -ueo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "$SCRIPT_DIR/command_utils.sh" @@ -80,7 +84,7 @@ has_squash_commit() { # abort when a merge is actually in progress. abort_merge_if_in_progress() { if git rev-parse --verify --quiet MERGE_HEAD >/dev/null; then - log_cmd git merge --abort + run git merge --abort fi } @@ -91,7 +95,7 @@ update_direct_target() { # Checkout first to ensure the local branch exists (created from origin if # needed). This allows has_squash_commit to compare local refs, which matters # for testing where the script may run multiple times in the same repo. - log_cmd git checkout "$BRANCH" + run git checkout "$BRANCH" if has_squash_commit "$BRANCH" "$TARGET_BRANCH"; then echo "✓ $BRANCH already up-to-date; skipping" @@ -102,8 +106,8 @@ update_direct_target() { CONFLICTS=() local BASE_MERGE_CLEAN=true - log_cmd git update-ref BEFORE_MERGE HEAD - if ! log_cmd git merge --no-edit "origin/$MERGED_BRANCH"; then + run git update-ref BEFORE_MERGE HEAD + if ! try git merge --no-edit "origin/$MERGED_BRANCH"; then CONFLICTS+=("origin/$MERGED_BRANCH") BASE_MERGE_CLEAN=false abort_merge_if_in_progress @@ -111,8 +115,10 @@ update_direct_target() { # Only try merging the pre-squash target state if it's not already # included in the merged branch — otherwise the first merge covers it. if ! git merge-base --is-ancestor SQUASH_COMMIT~ "origin/$MERGED_BRANCH"; then - if ! log_cmd git merge --no-edit SQUASH_COMMIT~; then - CONFLICTS+=( "$(git rev-parse SQUASH_COMMIT~) # $TARGET_BRANCH just before $MERGED_BRANCH was merged" ) + if ! try git merge --no-edit SQUASH_COMMIT~; then + local PRE_SQUASH_HASH + PRE_SQUASH_HASH=$(git rev-parse SQUASH_COMMIT~) || die "cannot resolve SQUASH_COMMIT~" + CONFLICTS+=( "$PRE_SQUASH_HASH # $TARGET_BRANCH just before $MERGED_BRANCH was merged" ) abort_merge_if_in_progress fi fi @@ -129,8 +135,10 @@ update_direct_target() { # Note: ordering is important here: if we label before pushing, we # re-trigger ourselves immediately. if [[ "$BASE_MERGE_CLEAN" == true ]]; then - log_cmd git push origin "$BRANCH" + run git push origin "$BRANCH" fi + local SQUASH_HASH_FOR_MARKER + SQUASH_HASH_FOR_MARKER=$(git rev-parse SQUASH_COMMIT) || die "cannot resolve SQUASH_COMMIT" { echo "### ⚠ī¸ Automatic update blocked by merge conflicts" echo @@ -153,23 +161,25 @@ update_direct_target() { echo echo "Once you push, this action will resume and finish updating this pull request." echo - format_state_marker "$MERGED_BRANCH" "$TARGET_BRANCH" "$(git rev-parse SQUASH_COMMIT)" - } | log_cmd gh pr comment "$BRANCH" -F - + format_state_marker "$MERGED_BRANCH" "$TARGET_BRANCH" "$SQUASH_HASH_FOR_MARKER" + } | try gh pr comment "$BRANCH" -F - \ + || die "could not post the conflict-resolution comment on $BRANCH" # Create the label if it doesn't exist, then add it to the PR gh label create "$CONFLICT_LABEL" --description "PR needs manual conflict resolution" --color "d73a4a" 2>/dev/null || true - log_cmd gh pr edit "$BRANCH" --add-label "$CONFLICT_LABEL" + run gh pr edit "$BRANCH" --add-label "$CONFLICT_LABEL" return 1 else - log_cmd git merge --no-edit -s ours SQUASH_COMMIT - log_cmd git update-ref MERGE_RESULT "HEAD^{tree}" + run git merge --no-edit -s ours SQUASH_COMMIT + run git update-ref MERGE_RESULT "HEAD^{tree}" COMMIT_MSG="Merge updates from $BASE_BRANCH and squash commit" if [[ "${GITHUB_ACTIONS:-}" == "true" ]]; then COMMIT_MSG="$COMMIT_MSG See $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" fi - CUSTOM_COMMIT=$(log_cmd git commit-tree MERGE_RESULT -p BEFORE_MERGE -p "origin/$MERGED_BRANCH" -p SQUASH_COMMIT -m "$COMMIT_MSG") - log_cmd git reset --hard "$CUSTOM_COMMIT" + CUSTOM_COMMIT=$(try git commit-tree MERGE_RESULT -p BEFORE_MERGE -p "origin/$MERGED_BRANCH" -p SQUASH_COMMIT -m "$COMMIT_MSG") \ + || die "could not create the merge-record commit for $BRANCH" + run git reset --hard "$CUSTOM_COMMIT" fi return 0 @@ -208,8 +218,9 @@ has_sibling_conflicts() { abandon_resume() { local PR_BRANCH="$1" local MESSAGE="$2" - echo "$MESSAGE" | log_cmd gh pr comment "$PR_BRANCH" -F - - log_cmd gh pr edit "$PR_BRANCH" --remove-label "$CONFLICT_LABEL" + echo "$MESSAGE" | try gh pr comment "$PR_BRANCH" -F - \ + || die "could not comment on $PR_BRANCH" + run gh pr edit "$PR_BRANCH" --remove-label "$CONFLICT_LABEL" } # Continue processing after user manually resolved conflicts @@ -249,7 +260,8 @@ continue_after_resolution() { # any mutation: once the base diverges, the recorded target is stale and a # merge built against it would be wrong. local CURRENT_BASE - CURRENT_BASE=$(gh pr view "$PR_BRANCH" --json baseRefName --jq '.baseRefName') + CURRENT_BASE=$(gh pr view "$PR_BRANCH" --json baseRefName --jq '.baseRefName') \ + || die "could not read the base branch of $PR_BRANCH" if [[ "$CURRENT_BASE" != "$OLD_BASE" ]]; then echo "⚠ī¸ Base of $PR_BRANCH changed manually ($OLD_BASE -> $CURRENT_BASE); not updating the stack." abandon_resume "$PR_BRANCH" "ℹī¸ The base branch of this PR was changed manually, so autorestack stepped back and will not update it automatically." @@ -282,7 +294,7 @@ continue_after_resolution() { # resolution in place the base merge and pre-squash merge are no-ops; only the # "-s ours" squash record gets applied, keeping the diff against the new base # clean. has_squash_commit makes this idempotent. - log_cmd git update-ref SQUASH_COMMIT "$SQUASH_HASH" + run git update-ref SQUASH_COMMIT "$SQUASH_HASH" MERGED_BRANCH="$OLD_BASE" TARGET_BRANCH="$NEW_TARGET" if ! update_direct_target "$PR_BRANCH" "$NEW_TARGET"; then @@ -295,16 +307,16 @@ continue_after_resolution() { # Push the cleaned-up head before retargeting so the head already contains # NEW_TARGET when the base flips to it, keeping the PR mergeable (GitHub # suppresses CI on a PR that conflicts with its base). - log_cmd git push origin "$PR_BRANCH" - log_cmd gh pr edit "$PR_BRANCH" --base "$NEW_TARGET" - log_cmd gh pr edit "$PR_BRANCH" --remove-label "$CONFLICT_LABEL" + run git push origin "$PR_BRANCH" + run gh pr edit "$PR_BRANCH" --base "$NEW_TARGET" + run gh pr edit "$PR_BRANCH" --remove-label "$CONFLICT_LABEL" # Check if old base branch should be deleted if has_sibling_conflicts "$OLD_BASE" "$PR_BRANCH"; then echo "⚠ī¸ Keeping branch '$OLD_BASE' - still referenced by other conflicted PRs" else echo "Deleting old base branch '$OLD_BASE' (no other PRs depend on it)" - log_cmd git push origin ":$OLD_BASE" || echo "⚠ī¸ Could not delete '$OLD_BASE' (may already be deleted)" + try git push origin ":$OLD_BASE" || echo "⚠ī¸ Could not delete '$OLD_BASE' (may already be deleted)" fi } @@ -314,10 +326,12 @@ main() { check_env_var "MERGED_BRANCH" check_env_var "TARGET_BRANCH" - log_cmd git update-ref SQUASH_COMMIT "$SQUASH_COMMIT" + run git update-ref SQUASH_COMMIT "$SQUASH_COMMIT" # Find all PRs directly targeting the merged PR's head - INITIAL_TARGETS=($(log_cmd gh pr list --base "$MERGED_BRANCH" --json headRefName --jq '.[].headRefName')) + INITIAL_TARGET_LIST=$(try gh pr list --base "$MERGED_BRANCH" --json headRefName --jq '.[].headRefName') \ + || die "could not list PRs based on $MERGED_BRANCH" + INITIAL_TARGETS=($INITIAL_TARGET_LIST) # Track successfully updated vs conflicted branches separately UPDATED_TARGETS=() @@ -333,17 +347,17 @@ main() { # Only update base branches for successfully updated PRs for BRANCH in "${UPDATED_TARGETS[@]}"; do - log_cmd gh pr edit "$BRANCH" --base "$TARGET_BRANCH" + run gh pr edit "$BRANCH" --base "$TARGET_BRANCH" done # Push updated branches; only delete merged branch if no conflicts if [[ "${#CONFLICTED_TARGETS[@]}" -eq 0 ]]; then # No conflicts - safe to delete merged branch - log_cmd git push origin ":$MERGED_BRANCH" "${UPDATED_TARGETS[@]}" + run git push origin ":$MERGED_BRANCH" "${UPDATED_TARGETS[@]}" else # Some conflicts - keep merged branch for reference during manual resolution if [[ "${#UPDATED_TARGETS[@]}" -gt 0 ]]; then - log_cmd git push origin "${UPDATED_TARGETS[@]}" + run git push origin "${UPDATED_TARGETS[@]}" fi echo "⚠ī¸ Keeping branch '$MERGED_BRANCH' - still referenced by conflicted PRs: ${CONFLICTED_TARGETS[*]}" fi From 2384bbba790c78aa1c53761fb3449f8faf943108 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 22:06:20 +0000 Subject: [PATCH 3/4] Stop treating gh API failures as negative answers Two helpers swallowed gh errors with `|| echo ""` and returned the negative answer instead (verified against the previous script version with the new test scenarios): - has_sibling_conflicts: a failed `gh pr list` read as "no siblings", so the resume deleted the kept parent branch while another conflicted PR may still need it for its resolution. - pr_has_conflict_label: a failed `gh pr view` read as "no label", ending the run green without resuming anything. Both now die on command failure, so the branch and label survive and the next push retries. read_state_marker gets the same explicit handling; there the gh failure already aborted the run (pipefail propagated it to an errexit-active caller), but silently, with the error message hidden by 2>/dev/null. Also reorders main() to push the updated heads before retargeting their PRs, matching the conflict-resolved path: if the run fails between the two steps, a retargeted PR with a stale head sits conflicting with its new base, GitHub suppresses its CI, and no label exists to resume it. https://claude.ai/code/session_01STkeSJ7cLrmrNn4aTDYkwH --- tests/test_conflict_resolution_resume.sh | 63 +++++++++++++++++++++++- update-pr-stack.sh | 46 +++++++++++------ 2 files changed, 93 insertions(+), 16 deletions(-) diff --git a/tests/test_conflict_resolution_resume.sh b/tests/test_conflict_resolution_resume.sh index 933611d..5740352 100644 --- a/tests/test_conflict_resolution_resume.sh +++ b/tests/test_conflict_resolution_resume.sh @@ -23,6 +23,8 @@ ok() { echo "✅ $1"; PASS=$((PASS+1)); } # MOCK_LABELS newline-separated labels returned by `pr view --json labels` # MOCK_BASE base branch returned by `pr view --json baseRefName` # 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 +# MOCK_PR_LIST_FAIL set to 1 to make `pr list` fail make_mock_gh() { local dir="$1" cat > "$dir/mock_gh.sh" <<'EOF' @@ -31,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\ baseRefName*) printf '%s\n' "${MOCK_BASE:-}";; *--json\ comments*) cat "${MOCK_COMMENTS_FILE:-/dev/null}";; *) echo "unhandled pr view: $*" >&2; exit 1;; @@ -41,6 +45,7 @@ elif [[ "$1 $2" == "pr comment" ]]; then elif [[ "$1 $2" == "pr edit" ]]; then : elif [[ "$1 $2" == "pr list" ]]; then + [[ "${MOCK_PR_LIST_FAIL:-}" == 1 ]] && { echo "mock gh: pr list API down" >&2; exit 1; } : # no sibling conflicts elif [[ "$1 $2" == "label create" ]]; then : @@ -92,6 +97,7 @@ run_resume() { GH="$MOCK_DIR/mock_gh.sh" GIT="$MOCK_DIR/mock_git.sh" \ MOCK_LABELS="$MOCK_LABELS" MOCK_BASE="$MOCK_BASE" \ MOCK_COMMENTS_FILE="$MOCK_COMMENTS_FILE" CALLS="$CALLS" \ + MOCK_LABELS_FAIL="${MOCK_LABELS_FAIL:-}" MOCK_PR_LIST_FAIL="${MOCK_PR_LIST_FAIL:-}" \ bash "$ROOT_DIR/update-pr-stack.sh" >"$WORK/out.log" 2>&1 || echo "EXIT=$?" >>"$WORK/out.log" } @@ -197,5 +203,60 @@ grep -q -- "--base" "$CALLS" && fail "E: base must NOT be edited" [[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "E: child was pushed" ok "E: missing base branch detected, no crash, label removed" +# --------------------------------------------------------------------------- +echo "### Scenario F: reading the PR comments fails -> fail the run, keep the label" +setup_repo +# A transient API failure while reading the comments must not pass for "no +# state marker": that path removes the conflict label and permanently cancels +# the auto-resume. The run must fail loudly instead, so the next push retries. +MOCK_LABELS="autorestack-needs-conflict-resolution" +MOCK_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 "F: run should have failed" +grep -q "remove-label" "$CALLS" && fail "F: label must NOT be removed on an API failure" +grep -q "gh pr comment" "$CALLS" && fail "F: no comment must be posted on an API failure" +[[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "F: child was pushed" +ok "F: API failure fails the run and keeps the resume armed" + +# --------------------------------------------------------------------------- +echo "### Scenario G: 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 +MOCK_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 "G: run should have failed, not ended green" +grep -q "remove-label" "$CALLS" && fail "G: label must NOT be touched on an API failure" +[[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "G: child was pushed" +ok "G: labels API failure fails the run instead of skipping the resume" + +# --------------------------------------------------------------------------- +echo "### Scenario H: listing sibling conflicts fails -> keep the old base branch" +setup_repo +# Same successful-resume setup as scenario C, but the sibling listing that +# decides whether the old base branch can be deleted fails. Answering "no +# siblings" there would delete a branch another conflicted PR still needs. +git -C "$WORK" merge -q --no-edit main +git -C "$WORK" push -q origin child +MOCK_LABELS="autorestack-needs-conflict-resolution" +MOCK_LABELS_FAIL="" +MOCK_BASE="parent" +MOCK_PR_LIST_FAIL=1 +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" +git -C "$ORIGIN" rev-parse --verify -q parent >/dev/null || fail "H: old base branch was deleted on an API failure" +grep -q -- "push origin :parent" "$CALLS" && fail "H: deletion must not be attempted" +ok "H: sibling-listing API failure keeps the old base branch" + echo echo "All conflict-resume tests passed 🎉 ($PASS scenarios)" diff --git a/update-pr-stack.sh b/update-pr-stack.sh index 2b1a0b7..4769067 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -39,10 +39,15 @@ format_state_marker() { } # Echoes the most recent state-marker line found in our PR comments, or nothing. +# Dies when the comments cannot be read at all: an API failure must not pass +# for "no marker", which the caller treats as a reason to give up the resume +# and remove the conflict label for good. read_state_marker() { local PR_BRANCH="$1" - gh pr view "$PR_BRANCH" --json comments --jq '.comments[].body' 2>/dev/null \ - | { grep -F "$STATE_MARKER_PREFIX" || true; } | tail -n1 + local COMMENTS + COMMENTS=$(gh pr view "$PR_BRANCH" --json comments --jq '.comments[].body') \ + || die "could not read the comments of $PR_BRANCH" + grep -F "$STATE_MARKER_PREFIX" <<<"$COMMENTS" | tail -n1 || true } # Args: a marker line. Echoes "base target squash". @@ -185,23 +190,29 @@ See $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" return 0 } -# Check if a PR has the conflict resolution label +# Check if a PR has the conflict resolution label. Dies when the labels cannot +# be read, rather than answering "no" and letting the run end green without +# resuming anything. pr_has_conflict_label() { local BRANCH="$1" local LABELS - LABELS=$(gh pr view "$BRANCH" --json labels --jq '.labels[].name' 2>/dev/null || echo "") - echo "$LABELS" | grep -q "^${CONFLICT_LABEL}$" + LABELS=$(gh pr view "$BRANCH" --json labels --jq '.labels[].name') \ + || die "could not read the labels of $BRANCH" + grep -q "^${CONFLICT_LABEL}$" <<<"$LABELS" } # Check if any other PRs with conflict label still depend on a given base branch # Returns 0 (true) if siblings exist, 1 (false) if no siblings +# Dies when the PRs cannot be listed: answering "no siblings" on an API failure +# makes the caller delete a branch a sibling may still need for its resolution. has_sibling_conflicts() { local BASE_BRANCH="$1" local EXCLUDE_BRANCH="$2" # Find all open PRs with the conflict label that are based on BASE_BRANCH local CONFLICTED_SIBLINGS - CONFLICTED_SIBLINGS=$(gh pr list --base "$BASE_BRANCH" --label "$CONFLICT_LABEL" --json headRefName --jq '.[].headRefName' 2>/dev/null || echo "") + CONFLICTED_SIBLINGS=$(gh pr list --base "$BASE_BRANCH" --label "$CONFLICT_LABEL" --json headRefName --jq '.[].headRefName') \ + || die "could not list conflicted PRs based on $BASE_BRANCH" for SIBLING in $CONFLICTED_SIBLINGS; do if [[ "$SIBLING" != "$EXCLUDE_BRANCH" ]]; then @@ -242,7 +253,7 @@ continue_after_resolution() { # Recover them from the marker the squash-merge run left in the conflict # comment. local MARKER - MARKER=$(read_state_marker "$PR_BRANCH") + MARKER=$(read_state_marker "$PR_BRANCH") || die "could not read the state marker of $PR_BRANCH" if [[ -z "$MARKER" ]]; then echo "⚠ī¸ No autorestack state marker on $PR_BRANCH; cannot resume safely. Removing the label." abandon_resume "$PR_BRANCH" "ℹī¸ autorestack could not find its state marker on this PR, so it will not update the stack automatically. If this PR still needs its base updated, update its base manually." @@ -345,20 +356,25 @@ main() { fi done - # Only update base branches for successfully updated PRs + # Push the updated heads before retargeting their PRs, so each head already + # contains TARGET_BRANCH when the base flips to it and the PR stays + # mergeable (GitHub suppresses CI on a PR that conflicts with its base). + # Same ordering as the conflict-resolved path; it also keeps a failure + # between the two steps recoverable, since a retargeted PR with a stale + # head would sit conflicting with its new base with no label to resume it. + if [[ "${#UPDATED_TARGETS[@]}" -gt 0 ]]; then + run git push origin "${UPDATED_TARGETS[@]}" + fi + for BRANCH in "${UPDATED_TARGETS[@]}"; do run gh pr edit "$BRANCH" --base "$TARGET_BRANCH" done - # Push updated branches; only delete merged branch if no conflicts + # Delete the merged branch last: open PRs must no longer be based on it + # when it goes away, and conflicted PRs still need it for their resolution. if [[ "${#CONFLICTED_TARGETS[@]}" -eq 0 ]]; then - # No conflicts - safe to delete merged branch - run git push origin ":$MERGED_BRANCH" "${UPDATED_TARGETS[@]}" + run git push origin ":$MERGED_BRANCH" else - # Some conflicts - keep merged branch for reference during manual resolution - if [[ "${#UPDATED_TARGETS[@]}" -gt 0 ]]; then - run git push origin "${UPDATED_TARGETS[@]}" - fi echo "⚠ī¸ Keeping branch '$MERGED_BRANCH' - still referenced by conflicted PRs: ${CONFLICTED_TARGETS[*]}" fi } From e28769e4ec359cb13d3aed6a4047e51d9f0cec3f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 13:21:22 +0000 Subject: [PATCH 4/4] Run the command_utils tests in CI https://claude.ai/code/session_01STkeSJ7cLrmrNn4aTDYkwH --- .github/workflows/tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 856d69c..761e2cf 100644 --- a/.github/workflows/tests.yml +++ b/.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