diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5b6742b..b2b7268 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_already_deleted.sh e2e-tests: name: E2E Tests diff --git a/tests/test_merged_branch_already_deleted.sh b/tests/test_merged_branch_already_deleted.sh new file mode 100755 index 0000000..0eb9e9c --- /dev/null +++ b/tests/test_merged_branch_already_deleted.sh @@ -0,0 +1,94 @@ +#!/bin/bash +# +# Someone can click "Delete branch" on the merged PR while the action is still +# running, so `git push origin :` fails with "remote ref does not +# exist". The deletion is the last step, after the children are updated and +# retargeted, so the run must still end green. + +set -ueo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "$SCRIPT_DIR/../command_utils.sh" + +simulate_push() { + log_cmd git update-ref "refs/remotes/origin/$1" "$1" +} + +MOCK_DIR=$(mktemp -d) +TEST_REPO=$(mktemp -d) +cd "$TEST_REPO" +echo "Created test repo at $TEST_REPO" + +log_cmd git init -b main +log_cmd git config user.email "test@example.com" +log_cmd git config user.name "Test User" + +echo "line" > file.txt +log_cmd git add file.txt +log_cmd git commit -m "Initial commit" +simulate_push main + +log_cmd git checkout -b feature1 +echo "f1" >> file.txt +log_cmd git add file.txt +log_cmd git commit -m "Add feature 1" +simulate_push feature1 + +log_cmd git checkout -b feature2 +echo "f2" >> file.txt +log_cmd git add file.txt +log_cmd git commit -m "Add feature 2" +simulate_push feature2 + +# Squash feature1 into main +log_cmd git checkout main +log_cmd git merge --squash feature1 +log_cmd git commit -m "Add feature 1 (#1)" +SQUASH=$(git rev-parse HEAD) +simulate_push main + +# A git that refuses branch deletions, like the remote does once the branch is gone +cat > "$MOCK_DIR/failing_git.sh" <<'MOCK' +#!/bin/bash +if [[ "$1" == "push" && "$3" == :* ]]; then + echo "error: unable to delete '${3#:}': remote ref does not exist" >&2 + exit 1 +fi +exec "$MOCK_GIT" "$@" +MOCK +chmod +x "$MOCK_DIR/failing_git.sh" + +set +e +OUT=$(env \ + SQUASH_COMMIT="$SQUASH" \ + MERGED_BRANCH=feature1 \ + PR_NUMBER=1 \ + TARGET_BRANCH=main \ + MOCK_GIT="$SCRIPT_DIR/mock_git.sh" \ + GH="$SCRIPT_DIR/mock_gh.sh" \ + GIT="$MOCK_DIR/failing_git.sh" \ + "$SCRIPT_DIR/../update-pr-stack.sh" 2>&1) +RC=$? +set -e +echo "$OUT" + +if [[ "$RC" -ne 0 ]]; then + echo "❌ The run must succeed when the merged branch is already deleted (exit $RC)" + exit 1 +fi +if ! grep -q "Could not delete 'feature1'" <<<"$OUT"; then + echo "❌ Expected a warning about the failed deletion" + exit 1 +fi +if ! grep -q "gh pr edit 2 --base main" <<<"$OUT"; then + echo "❌ The child PR must still be retargeted" + exit 1 +fi +ACTUAL_DIFF=$(git diff main...feature2 | grep '^[+-]' | grep -v '^[+-][+-][+-]') +if [[ "$ACTUAL_DIFF" != "+f2" ]]; then + echo "❌ Diff main...feature2 should show only feature2's change, got:" + echo "$ACTUAL_DIFF" + exit 1 +fi + +echo "✅ Already-deleted merged branch: warned, not failed" diff --git a/update-pr-stack.sh b/update-pr-stack.sh index cdb7c00..0e6d27d 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -152,6 +152,16 @@ post_conflict_comment() { run gh pr edit "$PR_NUMBER" --add-label "$CONFLICT_LABEL" } +# Deleting a PR's base branch closes the PR, so callers must retarget first. +# The branch can already be gone by then: a human clicking "Delete branch" right +# after the merge races this run, and "delete branch on merge" wins it outright. +# Everything that matters is done at that point, so a failed delete is a warning, +# not a red run. +delete_merged_branch() { + try git push origin ":$MERGED_BRANCH" \ + || echo "⚠️ Could not delete '$MERGED_BRANCH' (may already be deleted)" +} + # Args: head branch, base branch, PR number. git commands use the branch; gh # commands use the number, since a head branch can carry several PRs. update_direct_target() { @@ -381,8 +391,7 @@ main() { [[ -n "$BRANCH" ]] || continue 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" + delete_merged_branch return 0 fi @@ -427,10 +436,9 @@ main() { run gh pr edit "$NUMBER" --base "$TARGET_BRANCH" done - # 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. + # Keep the branch for reference while conflicted PRs remain. if [[ "${#CONFLICTED_TARGETS[@]}" -eq 0 ]]; then - run git push origin ":$MERGED_BRANCH" + delete_merged_branch else echo "⚠️ Keeping branch '$MERGED_BRANCH' - still referenced by conflicted PRs: ${CONFLICTED_TARGETS[*]}" fi