diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5b6742b..431d256 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -18,7 +18,7 @@ jobs: bash tests/test_rebase_workflow.sh bash tests/test_mixed_workflows.sh bash tests/test_conflict_resolution_resume.sh - bash tests/test_conflict_absorbed_resolution.sh + bash tests/test_conflict_two_step_resolution.sh bash tests/test_merge_commit_merge.sh bash tests/test_rebase_merge.sh diff --git a/tests/test_conflict_absorbed_resolution.sh b/tests/test_conflict_two_step_resolution.sh old mode 100644 new mode 100755 similarity index 65% rename from tests/test_conflict_absorbed_resolution.sh rename to tests/test_conflict_two_step_resolution.sh index cc3cc1d..8078ad3 --- a/tests/test_conflict_absorbed_resolution.sh +++ b/tests/test_conflict_two_step_resolution.sh @@ -1,6 +1,7 @@ #!/bin/bash # -# Replays the incident that motivated --absorbed (scortexio/gh-stack-mv#36): +# Replays the incident that stranded a conflicted PR (scortexio/gh-stack-mv#36) +# and checks the two-step resolution recovers from it: # # 1. main <- feature1 <- feature2, where feature1 advanced AFTER feature2 # forked (in the incident, autorestack itself advanced it when a @@ -8,12 +9,16 @@ # feature2. # 2. feature1 is squash-merged; the action's re-parent of feature2 conflicts, # so the action posts the resolution comment and stops. -# 3. The user resolves by running the posted re-parent with --absorbed. +# 3. The user resolves with two plain merges: `git merge origin/feature1` to +# catch up to the moved base, then `git merge origin/main` to bring in the +# target. No git-merge-onto, no --absorbed. # # The resolution must descend from origin/feature1's tip: feature2's PR is # still based on feature1 at that point, and GitHub creates no pull_request # runs for a PR that conflicts with its base, so without that ancestry the -# resume event may never fire and the conflict label stays stuck forever. +# resume event may never fire and the conflict label stays stuck forever. The +# first merge is what provides it -- it lands feature1's tip in the head's +# ancestry, which the second merge keeps, so no re-parent is needed. set -ueo pipefail @@ -101,18 +106,41 @@ if [[ "$(git rev-parse feature2)" != "$FEATURE2_BEFORE" ]]; then echo "❌ feature2 must not move on a conflict" exit 1 fi -if ! grep -q -- "--absorbed origin/main origin/feature1" "$COMMENT_FILE"; then - echo "❌ The posted resolution command must use --absorbed" +if ! grep -qF "git merge origin/feature1" "$COMMENT_FILE"; then + echo "❌ The posted resolution must merge the updated base branch first" cat "$COMMENT_FILE" exit 1 fi -echo "✅ Conflict detected: comment posted with --absorbed, stack left alone" +if ! grep -qF "git merge origin/main" "$COMMENT_FILE"; then + echo "❌ The posted resolution must merge the target branch" + cat "$COMMENT_FILE" + exit 1 +fi +if grep -qF "git-merge-onto" "$COMMENT_FILE"; then + echo "❌ The posted resolution must be plain git, with no git-merge-onto" + cat "$COMMENT_FILE" + exit 1 +fi +echo "✅ Conflict detected: two-step resolution comment posted, stack left alone" -# Resolve as the comment instructs, with the vendored copy standing in for -# `uvx git-merge-onto` (unit tests have no network). +# Resolve as the comment instructs, with two plain merges. Step 1: merge the +# moved base branch. Both sides rewrote line 2, so this conflicts; resolve to +# feature2's content. log_cmd git checkout feature2 -if python3 "$SCRIPT_DIR/../git-merge-onto" --absorbed origin/main origin/feature1; then - echo "❌ The re-parent should conflict (both sides rewrote line 2)" +if log_cmd git merge --no-edit origin/feature1; then + echo "❌ The base merge should conflict (both sides rewrote line 2)" + exit 1 +fi +sed -i '/^<<<<<<>>>>>>/c\Feature 2 version' file.txt +log_cmd git add file.txt +log_cmd git commit --no-edit + +# Step 2: merge the target branch. feature2 rewrote the same line 2 that the +# squash reshaped, so with the plain merge's true base (before feature1 existed) +# this raises the conflict a second time -- the price of a re-parent-free recipe +# when the child edits its parent's lines. Resolve it again. +if log_cmd git merge --no-edit origin/main; then + echo "❌ The target merge should conflict (feature2 vs main both rewrote line 2)" exit 1 fi sed -i '/^<<<<<<>>>>>>/c\Feature 2 version' file.txt @@ -120,9 +148,10 @@ log_cmd git add file.txt log_cmd git commit --no-edit simulate_push feature2 -# The regression assertion: the resolution descends from the old base's tip. +# The regression assertion: the resolution still descends from the old base's +# tip, provided by step 1's merge (no --absorbed, no re-parent). if log_cmd git merge-base --is-ancestor "$FEATURE1_TIP" feature2; then - echo "✅ Resolution descends from origin/feature1's tip (--absorbed recorded it)" + echo "✅ Resolution descends from origin/feature1's tip (step-1 merge recorded it)" else echo "❌ Resolution does not descend from origin/feature1's tip" log_cmd git log --graph --oneline feature2 feature1 @@ -142,5 +171,5 @@ else fi echo "" -echo "All absorbed-resolution tests passed! 🎉" +echo "All two-step resolution tests passed! 🎉" echo "Test repository remains at: $TEST_REPO for inspection" diff --git a/tests/test_e2e.sh b/tests/test_e2e.sh index 332d0e5..f86383b 100755 --- a/tests/test_e2e.sh +++ b/tests/test_e2e.sh @@ -275,21 +275,33 @@ get_conflict_comment() { printf '%s\n' "$comment" } -# The conflict comment must tell the user to run the re-parent the action tried: -# a single `uvx git-merge-onto --absorbed origin/ origin/`. +# The conflict comment must tell the user to resolve with two plain merges: +# first `git merge origin/` to catch up to the moved base, then +# `git merge origin/` to bring in the target. No git-merge-onto. assert_conflict_comment_reparent() { local comment=$1 local target=$2 local merged=$3 - if echo "$comment" | grep -qxF "uvx 'git-merge-onto>=0.2' --absorbed origin/$target origin/$merged"; then - echo >&2 "✅ Verification Passed: conflict comment re-parents origin/$merged onto origin/$target." - else - echo >&2 "❌ Verification Failed: conflict comment lacks 'uvx 'git-merge-onto>=0.2' --absorbed origin/$target origin/$merged'." + if ! echo "$comment" | grep -qxF "git merge origin/$merged"; then + echo >&2 "❌ Verification Failed: conflict comment lacks 'git merge origin/$merged'." + echo >&2 "--- Full comment ---" + echo >&2 "$comment" + exit 1 + fi + if ! echo "$comment" | grep -qxF "git merge origin/$target"; then + echo >&2 "❌ Verification Failed: conflict comment lacks 'git merge origin/$target'." + echo >&2 "--- Full comment ---" + echo >&2 "$comment" + exit 1 + fi + if echo "$comment" | grep -qF "git-merge-onto"; then + echo >&2 "❌ Verification Failed: conflict comment must be plain git, with no git-merge-onto." echo >&2 "--- Full comment ---" echo >&2 "$comment" exit 1 fi + echo >&2 "✅ Verification Passed: conflict comment merges origin/$merged then origin/$target." } follow_conflict_comment() { @@ -1036,9 +1048,10 @@ fi # The re-parent is one atomic merge, so on a conflict the action commits and # pushes nothing: origin/feature3 must still sit at its pre-conflict head. The -# resume is guaranteed by the resolution itself: its --absorbed re-parent records -# origin/feature2's tip as a parent, so the pushed head descends from its base -# and GitHub creates the synchronize run (it creates none for a PR that +# resume is guaranteed by the resolution itself: its first merge, +# `git merge origin/feature2`, lands origin/feature2's tip in the head's +# ancestry (the second merge keeps it), so the pushed head descends from +# its base and GitHub creates the synchronize run (it creates none for a PR that # conflicts with its base). Asserted after the resolution below. REMOTE_FEATURE3_SHA_BEFORE_RESOLVE=$(log_cmd git rev-parse "refs/remotes/origin/feature3") if [[ "$REMOTE_FEATURE3_SHA_BEFORE_RESOLVE" == "$FEATURE3_CONFLICT_COMMIT_SHA" ]]; then @@ -1056,11 +1069,10 @@ echo >&2 "12. Resolving conflict on feature3 by following the posted comment..." # asserts the resolution descends from this tip. log_cmd git fetch origin FEATURE2_TIP_AT_RESOLUTION=$(log_cmd git rev-parse "refs/remotes/origin/feature2") -# Follow the comment exactly: fetch, fast-forward to origin/feature3, run the -# re-parent (uvx git-merge-onto), resolve the conflict, and push. Following it -# must leave feature3 cleanly mergeable into its new base, or the -# synchronize-triggered continuation can never make progress and the conflict -# label stays stuck. +# Follow the comment exactly: fetch, fast-forward to origin/feature3, merge the +# moved base, merge main, resolve the conflict, and push. Following it must leave +# feature3 cleanly mergeable into its new base, or the synchronize-triggered +# continuation can never make progress and the conflict label stays stuck. follow_conflict_comment "$CONFLICT_COMMENT" file.txt "feature3" 1 echo >&2 "Resolved file.txt content:" cat file.txt @@ -1125,11 +1137,12 @@ else exit 1 fi -# Verify the --absorbed re-parent recorded the old base's tip as a parent. This -# is the structural guarantee that PR3, still based on feature2 when the -# resolution was pushed, read as mergeable so GitHub created the resume run. +# Verify the resolution's first step (git merge origin/feature2) left the old +# base's tip in feature3's ancestry. This is the structural guarantee that PR3, +# still based on feature2 when the resolution was pushed, read as mergeable so +# GitHub created the resume run. if log_cmd git merge-base --is-ancestor "$FEATURE2_TIP_AT_RESOLUTION" feature3; then - echo >&2 "✅ Verification Passed: Resolved feature3 descends from feature2's tip (--absorbed)." + echo >&2 "✅ Verification Passed: Resolved feature3 descends from feature2's tip (step-1 merge)." else echo >&2 "❌ Verification Failed: Resolved feature3 does not descend from feature2's tip ($FEATURE2_TIP_AT_RESOLUTION)." log_cmd git log --graph --oneline feature3 diff --git a/update-pr-stack.sh b/update-pr-stack.sh index 1fb9410..d95a80f 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -167,31 +167,47 @@ See $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" # state so the next push can resume. The label comes last: it is what # re-triggers us. # + # The resolution is two plain merges: the updated base branch, then the + # target. The target already carries the merged branch's content (the squash + # commit, or the rebase copies), so once the head contains the target the + # retargeted diff shows only the child's own changes -- no re-parent needed + # to drop the base. Both commands name branch tips, never the squash commit + # or its parent, so nothing here depends on whether the branch was squash- or + # rebase-merged. + # # The resume rides on a synchronize event, and GitHub creates no pull_request # runs for a PR that conflicts with its base. This PR's base is the merged # branch (kept until the resume retargets it), and the head does not always # descend from its tip: when an earlier run updated that branch after this # head forked (an ancestor PR merged first), GitHub falls back to a textual # merge to decide mergeability, which fails exactly when the resolution - # rewrote the same lines. That would strand the PR: no run, label stuck. - # --absorbed in the posted command is what prevents this: it records the - # merged branch's tip as a parent of the resolution, so the pushed head - # descends from its base again and the resume event is guaranteed to fire. + # rewrote the same lines. That would strand the PR: no run, label stuck. The + # first merge, `git merge origin/$MERGED_BRANCH`, is what prevents this: it + # lands the merged branch's tip in the resolved head's ancestry, so the + # pushed head descends from its base again and the resume event is + # guaranteed to fire. abort_merge_if_in_progress local SQUASH_HASH_FOR_MARKER SQUASH_HASH_FOR_MARKER=$(git rev-parse SQUASH_COMMIT) || die "cannot resolve SQUASH_COMMIT" { echo "### ⚠️ Automatic update blocked by a merge conflict" echo - echo "Resolve it like this:" + echo "Resolve it in two steps. First merge the updated base branch:" echo '```bash' echo "git fetch origin" echo "git switch $BRANCH" echo "git merge --ff-only origin/$BRANCH" - echo "uvx 'git-merge-onto>=0.2' --absorbed origin/$BASE_BRANCH origin/$MERGED_BRANCH" + echo "git merge origin/$MERGED_BRANCH" + echo '```' + echo + echo 'Fix any conflicts (for instance with `git mergetool`), then run `git add -A && git commit` to finish the merge.' + echo + echo "Then merge the target branch \`$BASE_BRANCH\`:" + echo '```bash' + echo "git merge origin/$BASE_BRANCH" echo '```' echo - echo 'Fix the conflicts (for instance with `git mergetool`), then run `git add -A && git commit` to finish the merge.' + echo 'Fix any conflicts, then run `git add -A && git commit` to finish the merge.' echo echo '```bash' echo "git push origin $BRANCH" @@ -306,8 +322,8 @@ continue_after_resolution() { return fi - # Same check for the old base: the resolution command we posted re-parents - # against origin/$OLD_BASE, so if that branch is gone (auto-delete head branches + # Same check for the old base: both resolution commands we posted reference + # origin/$OLD_BASE, so if that branch is gone (auto-delete head branches # left enabled, or deleted manually) the user cannot resolve 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 @@ -316,18 +332,18 @@ continue_after_resolution() { return fi - # The user resolved by re-parenting (the comment's `git-merge-onto`), so the - # head now contains the squash commit. Verify that and finalize -- do NOT re-run - # the merge. Its forced base is the old parent, where the lines the user just - # resolved still differ from the trunk, so a re-merge would re-raise the very - # conflict they fixed. A plain ancestry check is all the resume needs. + # The user resolved by merging the target branch (the comment's second step), + # so the head now contains the squash commit. Verify that and finalize -- do + # NOT re-run the merge. The lines the user just resolved would conflict again + # against the same refs, so a re-merge would re-raise the very conflict they + # fixed. A plain ancestry check is all the resume needs. run git update-ref SQUASH_COMMIT "$SQUASH_HASH" run git checkout "$PR_BRANCH" if ! git merge-base --is-ancestor SQUASH_COMMIT "$PR_BRANCH"; then # Fail loudly rather than silently: the user pushed without finishing the # re-parent, so a red run is the signal they need to look again. echo "❌ '$PR_BRANCH' does not contain the squash commit; the conflict is not resolved." >&2 - echo " Follow the conflict comment on this PR (run its git-merge-onto command), then push again." >&2 + echo " Follow the conflict comment on this PR (run its two resolution steps), then push again." >&2 return 1 fi