Refactor ancestry checks for cleaner abstraction

Extract is_base_ancestor() for simple ancestry checks that work in all
modes. Rename skip_if_clean to has_squash_commit for clarity - it
returns a boolean, not performs an action.

update_indirect_target now uses is_base_ancestor() since indirect
targets only need to verify their parent is already an ancestor.

has_squash_commit remains for direct targets in squash-merge mode,
where checking both base ancestry AND squash commit presence is needed.

Also add feature4 branch to e2e tests to verify grandchildren are
properly updated during conflict resolution continuation workflow.

Author: claude

tests/test_e2e.sh

@@ -20,7 +20,7 @@
 # Tests the happy path where PRs are merged without conflicts.
 #
 # Setup:
-#   - Create a stack of 3 PRs: main <- feature1 <- feature2 <- feature3
+#   - Create a stack of 4 PRs: main <- feature1 <- feature2 <- feature3 <- feature4
 #   - Each PR modifies line 2 of file.txt (same line, different content)
 #
 # Action Trigger:
@@ -30,14 +30,14 @@
 #   - The action should detect that PR2 (feature2) was based on feature1
 #   - Update PR2's base branch from feature1 to main
 #   - Merge main into feature2 to incorporate the squash commit
-#   - Propagate the merge to feature3 as well
+#   - Propagate the merge to feature3 and feature4 as well
 #   - Delete the merged branch (feature1)
 #
 # Verifications:
 #   - feature1 branch is deleted from remote
 #   - PR2 base branch is updated from feature1 to main
 #   - PR3 base branch remains feature2 (only direct children are updated)
-#   - feature2 and feature3 branches contain the squash merge commit
+#   - feature2, feature3, and feature4 branches contain the squash merge commit
 #   - PR diffs show the correct changes relative to their new bases
 #
 # SCENARIO 2: Conflict Handling (Steps 8-13)
@@ -47,6 +47,7 @@
 # Setup:
 #   - After Scenario 1, modify line 7 on feature3 and push
 #   - Also modify line 7 on main with different content (creating a conflict)
+#   - feature4 (grandchild) exists based on feature3
 #
 # Action Trigger:
 #   - Squash merge PR2 (feature2) into main
@@ -67,13 +68,19 @@
 #   - Conflict label "autorestack-needs-conflict-resolution" exists on PR3
 #   - feature3 branch was NOT updated (still at pre-conflict SHA)
 #
-# Manual Conflict Resolution (Steps 12-14):
+# Manual Conflict Resolution (Steps 12-15):
 #   - Test simulates user resolving the conflict manually
 #   - Merge main into feature3, resolve conflict (keep feature3's changes)
 #   - Push the resolved branch
 #   - The push triggers the 'synchronize' event on PR3
 #   - The action detects the conflict label and removes it
-#   - Verify the label is removed and resolution comment is posted
+#   - The continuation workflow updates feature4 (grandchild) recursively
+#   - Verify the label is removed, resolution comment is posted, and feature4 is updated
+#
+# Grandchild Update (feature4):
+#   - Tests that update_branch_recursive properly handles grandchildren
+#   - Even when SQUASH_COMMIT is undefined (in conflict-resolved mode)
+#   - The skip_if_clean guard must handle the missing SQUASH_COMMIT ref
 #
 # =============================================================================
 set -e # Exit immediately if a command exits with a non-zero status.
@@ -487,6 +494,16 @@ PR3_URL=$(log_cmd gh pr create --repo "$REPO_FULL_NAME" --base feature2 --head f
 PR3_NUM=$(echo "$PR3_URL" | awk -F'/' '{print $NF}')
 echo >&2 "Created PR #$PR3_NUM: $PR3_URL"
 
+# Branch feature4 (base: feature3) - tests grandchildren in conflict resolution
+log_cmd git checkout -b feature4 feature3
+sed -i '2s/.*/Feature 4 content line 2/' file.txt # Edit line 2 again
+log_cmd git add file.txt
+log_cmd git commit -m "Add feature 4"
+log_cmd git push origin feature4
+PR4_URL=$(log_cmd gh pr create --repo "$REPO_FULL_NAME" --base feature3 --head feature4 --title "Feature 4" --body "This is PR 4, based on PR 3 (grandchild for conflict resolution test)")
+PR4_NUM=$(echo "$PR4_URL" | awk -F'/' '{print $NF}')
+echo >&2 "Created PR #$PR4_NUM: $PR4_URL"
+
 # --- Initial Merge Scenario ---
 echo >&2 "--- Testing Initial Merge (PR1) ---"
 
@@ -540,6 +557,8 @@ log_cmd git checkout feature2 # Checkout local branch first
 log_cmd git pull origin feature2 # Pull updates pushed by the action
 log_cmd git checkout feature3
 log_cmd git pull origin feature3
+log_cmd git checkout feature4
+log_cmd git pull origin feature4
 
 # Check ancestry
 if log_cmd git merge-base --is-ancestor "$MERGE_COMMIT_SHA1" feature2; then
@@ -556,6 +575,13 @@ else
     log_cmd git log --graph --oneline feature3 main
     exit 1
 fi
+if log_cmd git merge-base --is-ancestor "$MERGE_COMMIT_SHA1" feature4; then
+    echo >&2 "✅ Verification Passed: feature4 correctly incorporates the squash commit $MERGE_COMMIT_SHA1."
+else
+    echo >&2 "❌ Verification Failed: feature4 does not include the squash commit $MERGE_COMMIT_SHA1."
+    log_cmd git log --graph --oneline feature4 main
+    exit 1
+fi
 # Verify diffs (using triple-dot diff against the *new* base: main)
 echo >&2 "Verifying diff content for updated PRs..."
 # Expected diff for feature2 vs main (should only contain feature2 changes relative to feature1)
@@ -587,6 +613,20 @@ else
     exit 1
 fi
 
+# Expected diff for feature4 vs feature3 (should only contain feature4 changes relative to feature3)
+EXPECTED_DIFF4_CONTENT="Feature 4 content line 2"
+ACTUAL_DIFF4_CONTENT=$(log_cmd gh pr diff "$PR4_URL" --repo "$REPO_FULL_NAME" | grep '^+Feature 4' | sed 's/^+//')
+
+if [[ "$ACTUAL_DIFF4_CONTENT" == "$EXPECTED_DIFF4_CONTENT" ]]; then
+    echo >&2 "✅ Verification Passed: Diff content for PR #$PR4_NUM seems correct."
+else
+    echo >&2 "❌ Verification Failed: Diff content for PR #$PR4_NUM is incorrect."
+    echo "Expected Added Line Content: $EXPECTED_DIFF4_CONTENT"
+    echo "Actual Added Line Content: $ACTUAL_DIFF4_CONTENT"
+    gh pr diff "$PR4_URL" --repo "$REPO_FULL_NAME"
+    exit 1
+fi
+
 echo >&2 "--- Initial Merge Test Completed Successfully ---"
 
 
@@ -765,6 +805,8 @@ echo >&2 "15. Verifying conflict resolution content..."
 log_cmd git fetch origin
 log_cmd git checkout feature3
 log_cmd git pull origin feature3
+log_cmd git checkout feature4
+log_cmd git pull origin feature4
 
 # Verify feature3 now incorporates main (including PR2 merge commit and main's conflict commit)
 if log_cmd git merge-base --is-ancestor origin/main feature3; then
@@ -775,6 +817,16 @@ else
     exit 1
 fi
 
+# Verify feature4 (grandchild) was updated by continuation workflow
+# This tests that update_branch_recursive properly handles grandchildren even when SQUASH_COMMIT is undefined
+if log_cmd git merge-base --is-ancestor origin/feature3 feature4; then
+    echo >&2 "✅ Verification Passed: feature4 (grandchild) correctly incorporates resolved feature3."
+else
+    echo >&2 "❌ Verification Failed: feature4 does not include the resolved feature3."
+    log_cmd git log --graph --oneline feature4 feature3
+    exit 1
+fi
+
 # Verify the final content of file.txt on feature3
 # Line 1: Original base
 # Line 2: From feature 3 commit ("Feature 3 content line 2")

update-pr-stack.sh

@@ -6,6 +6,13 @@
 # SQUASH_COMMIT - The hash of the squash commit that was merged
 # MERGED_BRANCH - The name of the branch that was merged and will be deleted
 # TARGET_BRANCH - The name of the branch that the PR was merged into
+#
+# Design note:
+# This script aims to output a transcript of "plain" git/gh commands that a
+# human could follow through manually. For this reason:
+# - We use git refs (e.g., SQUASH_COMMIT) instead of shell variables where
+#   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
 
@@ -26,12 +33,19 @@ check_env_var() {
     fi
 }
 
-skip_if_clean() {
+# Check if BASE is already an ancestor of BRANCH (simple merge check)
+is_base_ancestor() {
+    local BRANCH="$1"
+    local BASE="$2"
+    git merge-base --is-ancestor "origin/$BASE" "origin/$BRANCH"
+}
+
+# Check if a branch already has the squash commit merged (squash-merge mode only)
+# Requires SQUASH_COMMIT ref to be set via git update-ref
+has_squash_commit() {
     local BRANCH="$1"
     local BASE="$2"
-    # If BASE is already an ancestor of BRANCH *and*
-    # the squash commit is already in history, we're done.
-    git merge-base --is-ancestor "origin/$BASE" "origin/$BRANCH" \
+    is_base_ancestor "$BRANCH" "$BASE" \
         && git merge-base --is-ancestor SQUASH_COMMIT "origin/$BRANCH"
 }
 
@@ -50,7 +64,7 @@ update_direct_target() {
     local BRANCH="$1"
     local BASE_BRANCH="$2"
 
-    if skip_if_clean "$BRANCH" "$TARGET_BRANCH"; then
+    if has_squash_commit "$BRANCH" "$TARGET_BRANCH"; then
         echo "✓ $BRANCH already up-to-date; skipping"
         return
     fi
@@ -108,7 +122,10 @@ update_indirect_target() {
     local BRANCH="$1"
     local BASE_BRANCH="$2"
 
-    if skip_if_clean "$BRANCH" "$BASE_BRANCH"; then
+    # For indirect targets, we only need to check if the parent is already
+    # an ancestor. If so, the branch already has all updates from the parent.
+    # (No SQUASH_COMMIT check needed - that's only for direct targets)
+    if is_base_ancestor "$BRANCH" "$BASE_BRANCH"; then
         echo "✓ $BRANCH already up-to-date with $BASE_BRANCH; skipping"
         return
     fi