From 6985f8a550b03dd9821133a71e9fbc3aa14a42fd 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] Reject conflict resolutions that bring back lines the parent removed If someone resolved an autorestack conflict with a plain `git merge origin/` instead of the posted command, lines removed by the parent's later commits came back in the child, and the resume accepted it. Reproduced locally, not seen in production. Co-Authored-By: Claude Opus 5.5 --- tests/test_conflict_resolution_resume.sh | 34 ++++++++++++++++++++++++ update-pr-stack.sh | 27 +++++++++++-------- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/tests/test_conflict_resolution_resume.sh b/tests/test_conflict_resolution_resume.sh index dc0bd88..ed65947 100644 --- a/tests/test_conflict_resolution_resume.sh +++ b/tests/test_conflict_resolution_resume.sh @@ -306,5 +306,39 @@ grep -q -- "--base" "$CALLS" && fail "J: base must NOT be edited" [[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "J: child was pushed" ok "J: unresolved head re-posts the conflict comment and keeps the label armed" +# --------------------------------------------------------------------------- +echo "### Scenario K: head merged the target but not the old base's tip -> re-post, keep the label" +setup_repo +# The parent got a review commit after the child forked, removing a line. The +# user resolved with a plain merge of main: the head contains the squash commit +# but not the parent's tip, and the removed line is back in the child. +git checkout -q parent +echo kept > p.txt && echo removed-in-review > r.txt && git add p.txt r.txt && git commit -qm "parent" +git push -q origin parent +git checkout -q child +git merge -q --no-edit parent +git checkout -q parent +git rm -q r.txt && git commit -qm "review: remove line" +git push -q origin parent +git checkout -q main +git merge -q --squash parent && git commit -qm "Squash merge parent" +SQUASH2=$(git rev-parse main) +git push -q origin main +git checkout -q child +git merge -q --no-edit main +git cat-file -e child:r.txt || fail "K: setup must bring the removed line back" +git push -q origin child +MOCK_LABELS="autorestack-needs-conflict-resolution" +PR_BASE="parent" +MOCK_COMMENTS_FILE="$WORK/comments.txt" +{ echo "### conflict"; echo; marker parent main "$SQUASH2"; } > "$MOCK_COMMENTS_FILE" +run_resume + +grep -q "EXIT=1" "$WORK/out.log" || fail "K: run must fail while the PR is unresolved" +grep -q "gh pr comment" "$CALLS" || fail "K: conflict comment not re-posted" +grep -q "remove-label" "$CALLS" && fail "K: label must NOT be removed" +grep -q -- "--base" "$CALLS" && fail "K: base must NOT be edited" +ok "K: plain merge of the target is not accepted as a resolution" + echo echo "All conflict-resume tests passed 🎉 ($PASS scenarios)" diff --git a/update-pr-stack.sh b/update-pr-stack.sh index cdb7c00..c713617 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -326,19 +326,24 @@ continue_after_resolution() { 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. + # head now contains the squash commit and, through --absorbed, the old base's + # tip. 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. + # + # The squash commit alone is not enough: a plain `git merge origin/` + # also brings it in, but merges from the natural merge base, which predates + # the old base's later commits. Lines those commits removed come back. run git update-ref SQUASH_COMMIT "$SQUASH_HASH" run git checkout "$PR_BRANCH" - if ! git merge-base --is-ancestor SQUASH_COMMIT "$PR_BRANCH"; then - # The head does not contain the squash commit: the user pushed without - # finishing the re-parent, or followed an old-format conflict comment (a - # prior action version) whose resolution never folds the squash in. Re-post - # the current instructions and keep the label on so the next push resumes. - # Still return 1: a red run flags that the PR is not resolved yet. - echo "⚠️ '$PR_BRANCH' does not contain the squash commit yet; re-posting the conflict comment." >&2 + if ! git merge-base --is-ancestor SQUASH_COMMIT "$PR_BRANCH" \ + || ! git merge-base --is-ancestor "origin/$OLD_BASE" "$PR_BRANCH"; then + # The user pushed without finishing the re-parent, resolved with a plain + # merge, or followed an old-format conflict comment (a prior action + # version). Re-post the current instructions and keep the label on so the + # next push resumes. Still return 1: a red run flags that the PR is not + # resolved yet. + echo "⚠️ '$PR_BRANCH' does not contain both the squash commit and the tip of '$OLD_BASE' yet; re-posting the conflict comment." >&2 post_conflict_comment "$PR_BRANCH" "$OLD_BASE" "$NEW_TARGET" "$SQUASH_HASH" "$PR_NUMBER" return 1 fi