Fix conflict resolutions stranding the PR when its base branch moved

.github/workflows/tests.yml

@@ -18,6 +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_merge_commit_merge.sh
           bash tests/test_rebase_merge_skip.sh
 

git-merge-onto

@@ -4,7 +4,7 @@
 # dependencies = []
 # ///
 #
-# Vendored from https://github.com/scortexio/git-merge-onto (v0.1.0): a single
+# Vendored from https://github.com/scortexio/git-merge-onto (v0.2.0): a single
 # zero-dependency file so the action re-parents a branch without a network
 # download. Do not edit here -- change it upstream, publish, and re-sync.
 """git merge-onto: re-parent HEAD onto <new>, dropping <old>.
@@ -21,6 +21,11 @@ its commit (a squash-merge) -- and a plain merge re-applies <old>, often
 conflicting. Too high -- when the new parent transitively contains HEAD's own
 commit (a reorder) -- and a plain merge fast-forwards, silently dropping HEAD's
 change. Forcing the base to merge-base(HEAD, <old>) is correct in both.
+
+With --absorbed, the merge also records <old>'s tip as a parent: an assertion
+that <new> already carries <old>'s changes even though <old> is not its
+ancestor (squash merge, rebase merge, cherry-picks), so git and GitHub treat
+<old> as merged instead of dropped.
 """
 
 from __future__ import annotations
@@ -32,7 +37,7 @@ import subprocess
 import sys
 from pathlib import Path
 
-__version__ = "0.1.0"  # vendored snapshot; see the header above
+__version__ = "0.2.0"  # vendored snapshot; see the header above
 
 # Point at a specific git binary (used by the test suite; "git" otherwise).
 GIT = os.environ.get("GIT_MERGE_ONTO_GIT", "git")
@@ -113,11 +118,8 @@ def git_dir() -> Path:
     return Path(git("rev-parse", "--absolute-git-dir"))
 
 
-def worktree_dirty(include_untracked: bool = True) -> bool:
-    args = ["status", "--porcelain"]
-    if not include_untracked:
-        args.append("--untracked-files=no")
-    return bool(git(*args))
+def worktree_dirty() -> bool:
+    return bool(git("status", "--porcelain"))
 
 
 def in_progress_merge() -> bool:
@@ -141,43 +143,52 @@ def blocking_operation() -> str | None:
     return None
 
 
-def setup_merge_markers(theirs: str, message: str, head_tip: str) -> None:
+def setup_merge_markers(merge_parents: list[str], message: str, head_tip: str) -> None:
     """Write the in-progress-merge state `git commit` reads to finalize a merge:
-    parents come from HEAD + MERGE_HEAD, the message from MERGE_MSG."""
+    parents come from HEAD + MERGE_HEAD (one per line), the message from MERGE_MSG."""
     gd = git_dir()
-    (gd / "MERGE_HEAD").write_text(theirs + "\n")
+    (gd / "MERGE_HEAD").write_text("".join(p + "\n" for p in merge_parents))
     (gd / "MERGE_MODE").write_text("")
     (gd / "MERGE_MSG").write_text(message + "\n")
     (gd / "ORIG_HEAD").write_text(head_tip + "\n")
 
 
-def merge_with_base(base: str, theirs: str, message: str) -> bool:
+def merge_with_base(base: str, theirs: str, message: str, extra_parent: str | None = None) -> bool:
     """Merge `theirs` into HEAD as if `base` were the merge base -- a `git merge`
     with a caller-chosen base, the one thing git porcelain cannot do.
 
     Clean -> commits with parents [HEAD, theirs] and returns True. Conflict ->
     leaves the merge in progress (MERGE_HEAD set, conflict markers in the worktree)
     and returns False, so the caller (or a human) resolves and `git commit`s normally.
+
+    `extra_parent` is recorded as an additional parent, on the clean path and the
+    conflict path alike (it rides along in MERGE_HEAD, so the resolver's plain
+    `git commit` picks it up). `git commit` drops any parent that is an ancestor
+    of another, so a redundant extra parent is harmless.
     """
     head_tip = git("rev-parse", "HEAD")
+    merge_parents = [theirs] if extra_parent is None else [theirs, extra_parent]
     # 3-way merge into index+worktree with the merge base forced to `base`.
     rc = git_rc("merge-recursive", base, "--", head_tip, theirs)
     # merge-recursive returns 0 = clean, 1 = content conflict, >1 = it refused to run
     # at all (dirty index/worktree, bad arg). Only set the in-progress-merge markers
     # when there is a real merge to finalize; on a refusal, raise so we never fabricate
     # a merge commit or clobber an existing MERGE_HEAD.
     if rc == 0:
-        # A re-parent normally changes the tree; if it doesn't AND `theirs` is already
-        # an ancestor, the merge commit would add nothing (no content, no new ancestor),
-        # so skip it. (Don't skip merely because `theirs` is an ancestor: re-parenting
-        # onto a trunk that is already an ancestor still must drop the old parent's content.)
-        if git("write-tree") == git("rev-parse", "HEAD^{tree}") and git_rc("merge-base", "--is-ancestor", theirs, head_tip) == 0:
+        # A re-parent normally changes the tree; if it doesn't AND every parent to
+        # record is already an ancestor, the merge commit would add nothing (no
+        # content, no new ancestor), so skip it. (Don't skip merely because `theirs`
+        # is an ancestor: re-parenting onto a trunk that is already an ancestor still
+        # must drop the old parent's content.)
+        if git("write-tree") == git("rev-parse", "HEAD^{tree}") and all(
+            git_rc("merge-base", "--is-ancestor", p, head_tip) == 0 for p in merge_parents
+        ):
             return True
-        setup_merge_markers(theirs, message, head_tip)
+        setup_merge_markers(merge_parents, message, head_tip)
         git("commit", "--no-edit")
         return True
     if rc == 1:
-        setup_merge_markers(theirs, message, head_tip)
+        setup_merge_markers(merge_parents, message, head_tip)
         return False
     raise CommandError(
         [GIT, "merge-recursive", base, "--", head_tip, theirs],
@@ -192,10 +203,15 @@ def _resolve_commit(ref: str) -> str | None:
     return rev_parse(ref) or rev_parse(f"origin/{ref}")
 
 
-def merge_onto(new: str, old: str, message: str | None = None) -> bool:
+def merge_onto(new: str, old: str, message: str | None = None, absorbed: bool = False) -> bool:
     """Re-parent HEAD onto `new`, dropping `old`. Returns True on a clean merge
     (committed), False on a conflict (left in progress to resolve and commit).
-    Raises UserError on a precondition failure (dirty tree, bad ref, no ancestor)."""
+    Raises UserError on a precondition failure (dirty tree, bad ref, no ancestor).
+
+    `absorbed` asserts that `new` already carries `old`'s changes without its
+    commits (squash merge, rebase merge, cherry-picks): `old`'s tip is then
+    recorded as an extra parent of the merge, so the result descends from it
+    and git and GitHub treat `old` as merged instead of dropped."""
     # merge-recursive writes straight into the index/worktree, so refuse to run during
     # another git operation or on a dirty tree rather than corrupt either.
     op = blocking_operation()
@@ -215,11 +231,11 @@ def merge_onto(new: str, old: str, message: str | None = None) -> bool:
     if not base:
         raise UserError(f"no common ancestor between HEAD and old parent {old!r}")
     msg = message or f"Merge {new} into HEAD, dropping {old}"
-    return merge_with_base(base, new_sha, msg)
+    return merge_with_base(base, new_sha, msg, extra_parent=old_sha if absorbed else None)
 
 
-def cmd_merge_onto(new: str, old: str, message: str | None) -> int:
-    if merge_onto(new, old, message):
+def cmd_merge_onto(new: str, old: str, message: str | None, absorbed: bool = False) -> int:
+    if merge_onto(new, old, message, absorbed=absorbed):
         print(bold(f"git merge-onto: merged {new} into HEAD, dropping {old}."), file=sys.stderr)
         return 0
     print(
@@ -242,6 +258,15 @@ def build_parser() -> argparse.ArgumentParser:
         ),
     )
     p.add_argument("-m", "--message", help="commit message for a clean merge")
+    p.add_argument(
+        "--absorbed",
+        action="store_true",
+        help=(
+            "assert that <new> already carries <old>'s changes (squash merge, rebase "
+            "merge, cherry-picks): record <old> as an extra parent of the merge, so "
+            "git and GitHub treat <old> as merged instead of dropped"
+        ),
+    )
     p.add_argument("--quiet", action="store_true", help="do not echo executed git commands")
     p.add_argument("--version", action="version", version=f"git-merge-onto {__version__}")
     p.add_argument("new", help="the new parent to merge into HEAD")
@@ -255,7 +280,7 @@ def main(argv: list[str] | None = None) -> int:
     if args.quiet:
         VERBOSE = False
     try:
-        return cmd_merge_onto(args.new, args.old, args.message)
+        return cmd_merge_onto(args.new, args.old, args.message, args.absorbed)
     except UserError as e:
         print(red(f"git merge-onto: error: {e}"), file=sys.stderr)
         return 2

tests/mock_gh.sh

@@ -22,6 +22,11 @@ elif [[ "$1" == "pr" && "$2" == "edit" ]]; then
     # Just log the edit command
     echo "Mock: gh pr edit $3 --base $5"
 elif [[ "$1" == "pr" && "$2" == "comment" ]]; then
+    # Consume the body when it comes on stdin (-F -); keep a copy for tests
+    # that assert on the comment's content.
+    if [[ " $* " == *" -F "* ]]; then
+        cat > "${MOCK_COMMENT_FILE:-/dev/null}"
+    fi
     # Just log the comment command
     echo "Mock: gh pr comment $3"
 elif [[ "$1" == "api" && "$2" == repos/*/commits/*/pulls ]]; then

tests/test_conflict_absorbed_resolution.sh

@@ -0,0 +1,146 @@
+#!/bin/bash
+#
+# Replays the incident that motivated --absorbed (scortexio/gh-stack-mv#36):
+#
+#   1. main <- feature1 <- feature2, where feature1 advanced AFTER feature2
+#      forked (in the incident, autorestack itself advanced it when a
+#      grandparent PR merged). feature1's tip is therefore NOT an ancestor of
+#      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.
+#
+# 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.
+
+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"
+}
+
+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"
+
+for i in $(seq 1 10); do echo "line $i" >> file.txt; done
+log_cmd git add file.txt
+log_cmd git commit -m "Initial commit"
+simulate_push main
+
+# feature1: modify line 2
+log_cmd git checkout -b feature1
+sed -i '2s/.*/Feature 1 version A/' file.txt
+log_cmd git add file.txt
+log_cmd git commit -m "feature1: commit A"
+simulate_push feature1
+
+# feature2 forks here and modifies the same line 2: its resolution will rewrite
+# the very lines the squash reshapes, which is what made the incident's PR read
+# as conflicting with its base.
+log_cmd git checkout -b feature2
+sed -i '2s/.*/Feature 2 version/' file.txt
+log_cmd git add file.txt
+log_cmd git commit -m "feature2: change line 2"
+simulate_push feature2
+
+# feature1 advances after the fork: its tip is no longer an ancestor of feature2.
+log_cmd git checkout feature1
+sed -i '2s/.*/Feature 1 version B/' file.txt
+log_cmd git add file.txt
+log_cmd git commit -m "feature1: commit B"
+simulate_push feature1
+FEATURE1_TIP=$(git rev-parse feature1)
+
+if git merge-base --is-ancestor "$FEATURE1_TIP" feature2; then
+    echo "❌ Setup broken: feature1's tip must not be an ancestor of feature2"
+    exit 1
+fi
+
+# Squash-merge feature1 into main
+log_cmd git checkout main
+log_cmd git merge --squash feature1
+log_cmd git commit -m "Squash merge feature1"
+SQUASH_COMMIT=$(git rev-parse HEAD)
+simulate_push main
+
+FEATURE2_BEFORE=$(git rev-parse feature2)
+# Outside the test repo: an untracked file makes git-merge-onto refuse to run.
+COMMENT_FILE=$(mktemp)
+
+# The action must hit the conflict path: comment, label, no push, branch kept.
+OUT=$(env \
+    SQUASH_COMMIT="$SQUASH_COMMIT" \
+    MERGED_BRANCH=feature1 \
+    PR_NUMBER=1 \
+    TARGET_BRANCH=main \
+    GH="$SCRIPT_DIR/mock_gh.sh" \
+    GIT="$SCRIPT_DIR/mock_git.sh" \
+    MOCK_COMMENT_FILE="$COMMENT_FILE" \
+    "$SCRIPT_DIR/../update-pr-stack.sh" 2>&1)
+echo "$OUT"
+
+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
+    echo "❌ The merged branch must be kept while the child is conflicted"
+    exit 1
+fi
+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"
+    cat "$COMMENT_FILE"
+    exit 1
+fi
+echo "✅ Conflict detected: comment posted with --absorbed, stack left alone"
+
+# Resolve as the comment instructs, with the vendored copy standing in for
+# `uvx git-merge-onto` (unit tests have no network).
+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)"
+    exit 1
+fi
+sed -i '/^<<<<<<</,/^>>>>>>>/c\Feature 2 version' file.txt
+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.
+if log_cmd git merge-base --is-ancestor "$FEATURE1_TIP" feature2; then
+    echo "✅ Resolution descends from origin/feature1's tip (--absorbed recorded it)"
+else
+    echo "❌ Resolution does not descend from origin/feature1's tip"
+    log_cmd git log --graph --oneline feature2 feature1
+    exit 1
+fi
+if ! log_cmd git merge-base --is-ancestor "$SQUASH_COMMIT" feature2; then
+    echo "❌ Resolution must contain the squash commit (resume checks this)"
+    exit 1
+fi
+
+# And the content is the user's resolution on top of main.
+if [[ "$(sed -n '2p' file.txt)" == "Feature 2 version" ]]; then
+    echo "✅ Resolved content kept"
+else
+    echo "❌ Resolved content lost: $(sed -n '2p' file.txt)"
+    exit 1
+fi
+
+echo ""
+echo "All absorbed-resolution tests passed! 🎉"
+echo "Test repository remains at: $TEST_REPO for inspection"

tests/test_e2e.sh

@@ -276,16 +276,16 @@ get_conflict_comment() {
 }
 
 # The conflict comment must tell the user to run the re-parent the action tried:
-# a single `uvx git-merge-onto origin/<target> origin/<merged branch>`.
+# a single `uvx git-merge-onto --absorbed origin/<target> origin/<merged branch>`.
 assert_conflict_comment_reparent() {
     local comment=$1
     local target=$2
     local merged=$3
 
-    if echo "$comment" | grep -qxF "uvx git-merge-onto origin/$target origin/$merged"; then
+    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 origin/$target origin/$merged'."
+        echo >&2 "❌ Verification Failed: conflict comment lacks 'uvx 'git-merge-onto>=0.2' --absorbed origin/$target origin/$merged'."
         echo >&2 "--- Full comment ---"
         echo >&2 "$comment"
         exit 1
@@ -1035,9 +1035,11 @@ else
 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. That
-# unchanged head stays a descendant of its base (feature2), so the PR is mergeable
-# and the synchronize event that resumes the action keeps firing.
+# 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
+# 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
     echo >&2 "✅ Verification Passed: action pushed nothing on the conflict; origin/feature3 is unchanged."
@@ -1050,6 +1052,10 @@ fi
 
 # 12. Resolve the conflict by following the comment the action posted.
 echo >&2 "12. Resolving conflict on feature3 by following the posted comment..."
+# Record the base branch's tip now: the resume deletes feature2, and step 15
+# 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
@@ -1119,6 +1125,17 @@ 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.
+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)."
+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
+    exit 1
+fi
+
 # Note: feature4 is NOT updated (indirect children are not modified).
 # It will be updated when feature3 is merged (becoming a direct child at that point).
 echo >&2 "✅ feature4 intentionally not updated (indirect child of resolved PR)"