From dad62de8f21dfbee4938311896dae7292f6918c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9=20Rubinstein?= Date: Tue, 3 Nov 2026 10:29:04 +0100 Subject: [PATCH] Keep commits pushed to a branch after it was merged If someone pushed to a branch after GitHub squash-merged it but before autorestack ran, autorestack recorded the new commit as a parent of the child without its changes, then deleted the branch. Reproduced locally, not seen in production. Now the child is re-parented from the commit GitHub merged, and the branch is deleted only if it has not moved. Otherwise the run fails and the branch stays. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/tests.yml | 1 + action.yml | 1 + tests/test_conflict_absorbed_resolution.sh | 3 +- tests/test_merge_commit_merge.sh | 3 +- tests/test_merged_branch_moved.sh | 61 ++++++++++++++++++++++ tests/test_mixed_workflows.sh | 1 + tests/test_rebase_merge.sh | 3 +- tests/test_rebase_workflow.sh | 1 + tests/test_update_pr_stack.sh | 6 ++- update-pr-stack.sh | 13 +++-- 10 files changed, 84 insertions(+), 9 deletions(-) create mode 100644 tests/test_merged_branch_moved.sh diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b1aec69..52fd93e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -21,6 +21,7 @@ jobs: bash tests/test_conflict_absorbed_resolution.sh bash tests/test_merge_commit_merge.sh bash tests/test_rebase_merge.sh + bash tests/test_merged_branch_moved.sh e2e-tests: name: E2E Tests diff --git a/action.yml b/action.yml index 176bd4a..e90f875 100644 --- a/action.yml +++ b/action.yml @@ -52,6 +52,7 @@ runs: ACTION_MODE: ${{ steps.check.outputs.mode }} SQUASH_COMMIT: ${{ github.event.pull_request.merge_commit_sha }} MERGED_BRANCH: ${{ github.event.pull_request.head.ref }} + MERGED_TIP: ${{ github.event.pull_request.head.sha }} TARGET_BRANCH: ${{ github.event.pull_request.base.ref }} PR_BRANCH: ${{ github.event.pull_request.head.ref }} PR_NUMBER: ${{ github.event.pull_request.number }} diff --git a/tests/test_conflict_absorbed_resolution.sh b/tests/test_conflict_absorbed_resolution.sh index a289654..7a4f7f5 100644 --- a/tests/test_conflict_absorbed_resolution.sh +++ b/tests/test_conflict_absorbed_resolution.sh @@ -81,6 +81,7 @@ COMMENT_FILE=$(mktemp) OUT=$(env \ SQUASH_COMMIT="$SQUASH_COMMIT" \ MERGED_BRANCH=feature1 \ + MERGED_TIP=$(git rev-parse feature1) \ PR_NUMBER=1 \ TARGET_BRANCH=main \ GH="$SCRIPT_DIR/mock_gh.sh" \ @@ -93,7 +94,7 @@ if ! grep -q "Mock: gh pr comment 2" <<<"$OUT"; then echo "❌ Expected a conflict comment on the child PR" exit 1 fi -if grep -q "push origin :feature1" <<<"$OUT"; then +if grep -q "origin :feature1" <<<"$OUT"; then echo "❌ The merged branch must be kept while the child is conflicted" exit 1 fi diff --git a/tests/test_merge_commit_merge.sh b/tests/test_merge_commit_merge.sh index af5b57a..2e7c7d7 100755 --- a/tests/test_merge_commit_merge.sh +++ b/tests/test_merge_commit_merge.sh @@ -50,6 +50,7 @@ FEATURE2_BEFORE=$(git rev-parse feature2) OUT=$(env \ SQUASH_COMMIT="$MERGE_COMMIT" \ MERGED_BRANCH=feature1 \ + MERGED_TIP=$(git rev-parse feature1) \ PR_NUMBER=1 \ TARGET_BRANCH=main \ GH="$SCRIPT_DIR/mock_gh.sh" \ @@ -69,7 +70,7 @@ fi # Children must be retargeted before the merged branch is deleted (deleting a # PR's base branch closes the PR). EDIT_LINE=$(grep -n "pr edit 2 --base main" <<<"$OUT" | cut -d: -f1 | head -1 || true) -DELETE_LINE=$(grep -n "push origin :feature1" <<<"$OUT" | cut -d: -f1 | head -1 || true) +DELETE_LINE=$(grep -n "origin :feature1" <<<"$OUT" | cut -d: -f1 | head -1 || true) if [[ -z "$EDIT_LINE" ]]; then echo "❌ Child PR must be retargeted onto main" exit 1 diff --git a/tests/test_merged_branch_moved.sh b/tests/test_merged_branch_moved.sh new file mode 100644 index 0000000..602af46 --- /dev/null +++ b/tests/test_merged_branch_moved.sh @@ -0,0 +1,61 @@ +#!/bin/bash +# +# A commit pushed to the merged branch after GitHub squash-merged it is in +# neither the squash commit nor the children: the child must not absorb it, and +# the merged branch must not be deleted. + +set -ueo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ROOT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" + +fail() { echo "❌ $1"; exit 1; } +ok() { echo "✅ $1"; } + +WORK=$(mktemp -d) +ORIGIN=$(mktemp -d) +MOCK_DIR=$(mktemp -d) +git init -q --bare "$ORIGIN" +git init -q -b main "$WORK" +cd "$WORK" +git config user.email t@e.com && git config user.name t +git remote add origin "$ORIGIN" + +echo base > f.txt && git add f.txt && git commit -qm initial +git checkout -q -b parent +echo parent > p.txt && git add p.txt && git commit -qm parent +MERGED_TIP=$(git rev-parse parent) +git checkout -q -b child +echo child > c.txt && git add c.txt && git commit -qm child +git checkout -q main +git merge -q --squash parent && git commit -qm "Squash merge parent" +SQUASH_COMMIT=$(git rev-parse main) +git push -q origin main parent child + +# Someone pushes to the parent branch after the merge, before the action runs. +git checkout -q parent +echo late > late.txt && git add late.txt && git commit -qm late +LATE=$(git rev-parse parent) +git push -q origin parent +git checkout -q main +git fetch -q origin + +cat > "$MOCK_DIR/gh" <<'EOF' +#!/bin/bash +if [[ "$1" == "api" ]]; then echo "2 child"; fi +EOF +chmod +x "$MOCK_DIR/gh" + +RC=0 +env SQUASH_COMMIT="$SQUASH_COMMIT" MERGED_BRANCH=parent MERGED_TIP="$MERGED_TIP" \ + TARGET_BRANCH=main PR_NUMBER=1 GH="$MOCK_DIR/gh" \ + bash "$ROOT_DIR/update-pr-stack.sh" >"$MOCK_DIR/out.log" 2>&1 || RC=$? + +git fetch -q origin +git merge-base --is-ancestor "$SQUASH_COMMIT" origin/child || fail "child must be re-parented onto main" +git merge-base --is-ancestor "$LATE" origin/child && fail "child must not absorb the late commit" +ok "child re-parented without absorbing the late commit" + +[[ "$(git rev-parse origin/parent 2>/dev/null)" == "$LATE" ]] || fail "parent branch must be kept at the late commit" +[[ "$RC" -ne 0 ]] || fail "the run must fail when it cannot delete the merged branch" +ok "parent branch kept, run failed" diff --git a/tests/test_mixed_workflows.sh b/tests/test_mixed_workflows.sh index 4fb8452..cc7d02d 100755 --- a/tests/test_mixed_workflows.sh +++ b/tests/test_mixed_workflows.sh @@ -90,6 +90,7 @@ run_update_pr_stack() { env \ SQUASH_COMMIT=$SQUASH_COMMIT \ MERGED_BRANCH=feature1 \ + MERGED_TIP=$(git rev-parse feature1) \ PR_NUMBER=1 \ TARGET_BRANCH=main \ GH="$SCRIPT_DIR/mock_gh.sh" \ diff --git a/tests/test_rebase_merge.sh b/tests/test_rebase_merge.sh index 6403994..3a8379d 100644 --- a/tests/test_rebase_merge.sh +++ b/tests/test_rebase_merge.sh @@ -59,6 +59,7 @@ simulate_push main OUT=$(env \ SQUASH_COMMIT="$MERGE_COMMIT" \ MERGED_BRANCH=feature1 \ + MERGED_TIP=$(git rev-parse feature1) \ PR_NUMBER=1 \ TARGET_BRANCH=main \ GH="$SCRIPT_DIR/mock_gh.sh" \ @@ -75,7 +76,7 @@ if ! grep -q "Mock: gh pr edit 2 --base main" <<<"$OUT"; then echo "❌ The child PR must be retargeted to main" exit 1 fi -if ! grep -q "push origin :feature1" <<<"$OUT"; then +if ! grep -q "origin :feature1" <<<"$OUT"; then echo "❌ The merged branch must be deleted" exit 1 fi diff --git a/tests/test_rebase_workflow.sh b/tests/test_rebase_workflow.sh index 4b72418..ca7de1d 100755 --- a/tests/test_rebase_workflow.sh +++ b/tests/test_rebase_workflow.sh @@ -85,6 +85,7 @@ run_update_pr_stack() { env \ SQUASH_COMMIT=$SQUASH_COMMIT \ MERGED_BRANCH=feature1 \ + MERGED_TIP=$(git rev-parse feature1) \ PR_NUMBER=1 \ TARGET_BRANCH=main \ GH="$SCRIPT_DIR/mock_gh.sh" \ diff --git a/tests/test_update_pr_stack.sh b/tests/test_update_pr_stack.sh index 7689141..be4cd0d 100755 --- a/tests/test_update_pr_stack.sh +++ b/tests/test_update_pr_stack.sh @@ -82,6 +82,7 @@ run_update_pr_stack() { env \ SQUASH_COMMIT=$SQUASH_COMMIT \ MERGED_BRANCH=feature1 \ + MERGED_TIP=$(git rev-parse feature1) \ PR_NUMBER=1 \ TARGET_BRANCH=main \ GH="$SCRIPT_DIR/mock_gh.sh" \ @@ -95,7 +96,7 @@ run_update_pr_stack # 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 2 --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) +delete_line=$(grep -n "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" @@ -220,6 +221,7 @@ FAIL_LOG="$TEST_REPO/update_fail_run.log" if log_cmd env \ SQUASH_COMMIT=$SQUASH_COMMIT \ MERGED_BRANCH=feature1 \ + MERGED_TIP=$(git rev-parse feature1) \ PR_NUMBER=1 \ TARGET_BRANCH=main \ MOCK_PR_LIST_FAIL=1 \ @@ -230,7 +232,7 @@ if log_cmd env \ cat "$FAIL_LOG" exit 1 fi -if grep -q "git push origin :feature1" "$FAIL_LOG"; then +if grep -q "origin :feature1" "$FAIL_LOG"; then echo "❌ merged branch must not be deleted when the children cannot be listed" cat "$FAIL_LOG" exit 1 diff --git a/update-pr-stack.sh b/update-pr-stack.sh index cdb7c00..6313ccf 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -6,6 +6,7 @@ # SQUASH_COMMIT - The merged PR's merge_commit_sha: the squash commit, or the # last copied commit of a rebase merge # MERGED_BRANCH - The name of the branch that was merged and will be deleted +# MERGED_TIP - The merged PR's head commit, as GitHub merged it # TARGET_BRANCH - The name of the branch that the PR was merged into # PR_NUMBER - The number of the PR that was merged # @@ -178,7 +179,7 @@ See $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" fi # Re-parent the child onto the target in a single merge: merge the squash - # commit with the base forced to merge-base(HEAD, origin/$MERGED_BRANCH). That + # commit with the base forced to merge-base(HEAD, MERGED_TIP). That # drops the merged branch's content (now carried by the target via the squash) # while keeping the child's own changes -- the merge equivalent of # `git rebase --onto`, done by the vendored git-merge-onto. --absorbed records @@ -186,7 +187,7 @@ See $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" # from its current base (the merged branch) even when that base moved after # the head forked, so the PR reads as mergeable until it is retargeted. local RC=0 - try python3 "$SCRIPT_DIR/git-merge-onto" --absorbed -m "$MERGE_MSG" SQUASH_COMMIT "origin/$MERGED_BRANCH" || RC=$? + try python3 "$SCRIPT_DIR/git-merge-onto" --absorbed -m "$MERGE_MSG" SQUASH_COMMIT MERGED_TIP || RC=$? if [[ "$RC" -eq 0 ]]; then return 0 fi @@ -365,10 +366,12 @@ main() { # Check required environment variables check_env_var "SQUASH_COMMIT" check_env_var "MERGED_BRANCH" + check_env_var "MERGED_TIP" check_env_var "TARGET_BRANCH" check_env_var "PR_NUMBER" run git update-ref SQUASH_COMMIT "$SQUASH_COMMIT" + run git update-ref MERGED_TIP "$MERGED_TIP" # A merge-commit merge does not rewrite history: each child's head already # contains the merged branch's commits, and the merge commit carries them @@ -382,7 +385,7 @@ main() { run gh pr edit "$NUMBER" --base "$TARGET_BRANCH" done <<<"$CHILDREN" # Deleting a PR's base branch closes the PR, so the retargets come first. - run git push origin ":$MERGED_BRANCH" + run git push --force-with-lease="$MERGED_BRANCH:MERGED_TIP" origin ":$MERGED_BRANCH" return 0 fi @@ -429,8 +432,10 @@ main() { # 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. + # The lease keeps commits pushed to the branch after the merge: they are in + # neither the squash commit nor the children. if [[ "${#CONFLICTED_TARGETS[@]}" -eq 0 ]]; then - run git push origin ":$MERGED_BRANCH" + run git push --force-with-lease="$MERGED_BRANCH:MERGED_TIP" origin ":$MERGED_BRANCH" else echo "⚠️ Keeping branch '$MERGED_BRANCH' - still referenced by conflicted PRs: ${CONFLICTED_TARGETS[*]}" fi