From 5049ca702a2317ae6d0f20fd6832646bbcea52f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 20:49:24 +0000 Subject: [PATCH 1/3] Push updated heads before retargeting PR bases in the fan-out path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The squash-merge fan-out retargeted every updated child PR onto the target branch and only afterwards pushed the new heads, batched into a single non-atomic push together with the merged-branch deletion. If the push failed (e.g. someone pushed to a child mid-run, rejecting the plain push) or a pr edit died partway through the loop, set -e aborted the run with PRs already retargeted but their heads stale - and unlike the conflict-resume path there is no label to re-trigger the action, so nothing ever repaired them. Apply the ordering the resume path already uses: push the updated heads first, then flip the bases, and delete the merged branch last (deleting a PR's base branch closes the PR, so every child must be off it first). A failed push now leaves the PRs untouched on their old base. The unit test captures the run transcript and asserts the push -> retarget -> delete order; it fails against the previous code. Also corrects the README: pushes are plain, not forced, and branch deletion is its own final step. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JHvKryT4QUpHYdNq9YEQxX --- README.md | 5 +++-- tests/test_update_pr_stack.sh | 20 ++++++++++++++++++-- update-pr-stack.sh | 22 ++++++++++++++-------- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index f55449e..9b6094e 100644 --- a/README.md +++ b/README.md @@ -19,8 +19,9 @@ This action tries to fix that in a transparent way. Install it, and hopefully th 1. Triggers when a PR is squash merged 2. Finds PRs that were based on the merged branch (direct children only) 3. Creates a synthetic merge commit with three parents (child tip, deleted branch tip, squash commit) to preserve history without re-introducing code -4. Updates the direct child PRs to base on trunk now that the bottom change has landed -5. Force-pushes updated branches and deletes the merged branch +4. Pushes the updated branches +5. Updates the direct child PRs to base on trunk now that the bottom change has landed +6. Deletes the merged branch **Note:** Indirect descendants (grandchildren, etc.) are intentionally not modified. Their PR diffs remain correct because the merge-base calculation still works—the synthetic merge commit includes the original parent commit as an ancestor. When their direct parent is eventually merged, they become direct children and get updated at that point. diff --git a/tests/test_update_pr_stack.sh b/tests/test_update_pr_stack.sh index 4317f5e..2930aac 100755 --- a/tests/test_update_pr_stack.sh +++ b/tests/test_update_pr_stack.sh @@ -1,6 +1,6 @@ #!/bin/bash -set -e +set -eo pipefail # Get script directory (needed for static mock files) SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -71,6 +71,8 @@ echo "Simulated Squash commit (via cherry-pick): $SQUASH_COMMIT" echo "Running update-pr-stack.sh..." # The update script sources command_utils.sh itself +# Capture stdout+stderr interleaved so command ordering can be asserted. +RUN_LOG="$TEST_REPO/update_run.log" run_update_pr_stack() { log_cmd \ env \ @@ -79,10 +81,24 @@ run_update_pr_stack() { TARGET_BRANCH=main \ GH="$SCRIPT_DIR/mock_gh.sh" \ GIT="$SCRIPT_DIR/mock_git.sh" \ - $SCRIPT_DIR/../update-pr-stack.sh + $SCRIPT_DIR/../update-pr-stack.sh 2>&1 | tee "$RUN_LOG" } run_update_pr_stack +# The head must be pushed before the PR is retargeted (a failed push must leave +# the PR untouched on its old base), and the merged branch deleted only after +# the retarget (deleting a PR's base branch closes the PR). +push_line=$(grep -n "git push origin feature2" "$RUN_LOG" | head -1 | cut -d: -f1 || true) +edit_line=$(grep -n "pr edit feature2 --base main" "$RUN_LOG" | head -1 | cut -d: -f1 || true) +delete_line=$(grep -n "git push origin :feature1" "$RUN_LOG" | head -1 | cut -d: -f1 || true) +if [[ -n "$push_line" && -n "$edit_line" && -n "$delete_line" \ + && "$push_line" -lt "$edit_line" && "$edit_line" -lt "$delete_line" ]]; then + echo "✅ Ordering: push head, then retarget base, then delete merged branch" +else + echo "❌ Wrong ordering (push=$push_line edit=$edit_line delete=$delete_line)" + exit 1 +fi + # Verify the results cd "$TEST_REPO" diff --git a/update-pr-stack.sh b/update-pr-stack.sh index a2d8ca5..0669f82 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -311,20 +311,26 @@ main() { fi done - # Only update base branches for successfully updated PRs + # Push the updated heads before retargeting, for the same reasons as the + # conflict-resume path: the head already contains TARGET_BRANCH when the + # base flips to it, keeping the PR mergeable (GitHub suppresses CI on a PR + # that conflicts with its base), and a failed push leaves the PR untouched + # on its old base instead of retargeted onto a stale head with nothing to + # re-trigger this action. + if [[ "${#UPDATED_TARGETS[@]}" -gt 0 ]]; then + log_cmd git push origin "${UPDATED_TARGETS[@]}" + fi + for BRANCH in "${UPDATED_TARGETS[@]}"; do log_cmd gh pr edit "$BRANCH" --base "$TARGET_BRANCH" done - # Push updated branches; only delete merged branch if no conflicts + # Delete the merged branch last: deleting a PR's base branch closes the PR, + # so every child must be retargeted off it first. Keep it while conflicted + # PRs remain so it can be referenced during manual resolution. if [[ "${#CONFLICTED_TARGETS[@]}" -eq 0 ]]; then - # No conflicts - safe to delete merged branch - log_cmd git push origin ":$MERGED_BRANCH" "${UPDATED_TARGETS[@]}" + log_cmd git push origin ":$MERGED_BRANCH" else - # Some conflicts - keep merged branch for reference during manual resolution - if [[ "${#UPDATED_TARGETS[@]}" -gt 0 ]]; then - log_cmd git push origin "${UPDATED_TARGETS[@]}" - fi echo "⚠️ Keeping branch '$MERGED_BRANCH' - still referenced by conflicted PRs: ${CONFLICTED_TARGETS[*]}" fi } From f1f2a0e8b907932b8a05afa89f924efedd833f33 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 21:07:27 +0000 Subject: [PATCH 2/3] Fix e2e merge-command assertion broken by the ff-only step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since #40 the conflict comment's fast-forward step reads `git merge --ff-only origin/`, which assert_conflict_comment_merges picks up with its `^git merge` grep, so the extracted commands never match the expected conflict merges. Skip the --ff-only line when extracting. Also trim the new comments in the fan-out push/retarget/delete sequence. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JHvKryT4QUpHYdNq9YEQxX --- tests/test_e2e.sh | 2 +- update-pr-stack.sh | 14 +++++--------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/test_e2e.sh b/tests/test_e2e.sh index 1da4439..420f2cd 100755 --- a/tests/test_e2e.sh +++ b/tests/test_e2e.sh @@ -291,7 +291,7 @@ assert_conflict_comment_merges() { expected+="git merge $conflict"$'\n' done expected=${expected%$'\n'} - actual=$(echo "$comment" | grep -E '^git merge' | sed 's/ *#.*//' || true) + actual=$(echo "$comment" | grep -E '^git merge' | grep -v -- '--ff-only' | sed 's/ *#.*//' || true) if [[ "$actual" == "$expected" ]]; then echo >&2 "✅ Verification Passed: conflict comment lists expected merge command(s)." diff --git a/update-pr-stack.sh b/update-pr-stack.sh index 0669f82..aa08cc7 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -311,12 +311,9 @@ main() { fi done - # Push the updated heads before retargeting, for the same reasons as the - # conflict-resume path: the head already contains TARGET_BRANCH when the - # base flips to it, keeping the PR mergeable (GitHub suppresses CI on a PR - # that conflicts with its base), and a failed push leaves the PR untouched - # on its old base instead of retargeted onto a stale head with nothing to - # re-trigger this action. + # Push the heads before retargeting: a failed push then leaves each PR + # intact on its old base, and the head already contains TARGET_BRANCH when + # the base flips to it. if [[ "${#UPDATED_TARGETS[@]}" -gt 0 ]]; then log_cmd git push origin "${UPDATED_TARGETS[@]}" fi @@ -325,9 +322,8 @@ main() { log_cmd gh pr edit "$BRANCH" --base "$TARGET_BRANCH" done - # Delete the merged branch last: deleting a PR's base branch closes the PR, - # so every child must be retargeted off it first. Keep it while conflicted - # PRs remain so it can be referenced during manual resolution. + # Deleting a PR's base branch closes the PR, so this must come after the + # retargets. Keep the branch for reference while conflicted PRs remain. if [[ "${#CONFLICTED_TARGETS[@]}" -eq 0 ]]; then log_cmd git push origin ":$MERGED_BRANCH" else From 972442ab373a1e3c9792ac788394bcae2b135482 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 21:31:02 +0000 Subject: [PATCH 3/3] Drop the e2e assertion fix, split out into its own PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix for the --ff-only line breaking assert_conflict_comment_merges moved to a separate PR; the e2e job here stays red until that lands and main is merged back in. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JHvKryT4QUpHYdNq9YEQxX --- tests/test_e2e.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_e2e.sh b/tests/test_e2e.sh index 420f2cd..1da4439 100755 --- a/tests/test_e2e.sh +++ b/tests/test_e2e.sh @@ -291,7 +291,7 @@ assert_conflict_comment_merges() { expected+="git merge $conflict"$'\n' done expected=${expected%$'\n'} - actual=$(echo "$comment" | grep -E '^git merge' | grep -v -- '--ff-only' | sed 's/ *#.*//' || true) + actual=$(echo "$comment" | grep -E '^git merge' | sed 's/ *#.*//' || true) if [[ "$actual" == "$expected" ]]; then echo >&2 "✅ Verification Passed: conflict comment lists expected merge command(s)."