Skip to content

Commit 4f46bd6

Browse files
Phlogistiqueclaude
andauthored
Fix branch ancestry check logic in merge script (#15)
* Fix branch ancestry check to use local branches The is_base_ancestor function was checking remote-tracking branches (origin/$BASE and origin/$BRANCH), but merge operations performed by the script haven't been pushed yet. This caused incorrect ancestry detection for child branches. The fix leverages control flow analysis: git checkout creates the local branch from origin if needed, so we checkout first then compare local refs. This eliminates the need for conditional branch creation and ensures idempotence even when pushes haven't occurred yet. Changes: - Remove ensure_local_branch() and is_base_ancestor() helpers - Modify has_squash_commit() to use local refs (caller ensures they exist) - Move git checkout before ancestry checks in update_direct_target() - Inline merge-base check in update_indirect_target() after checkout - Add comments explaining the control flow reasoning * Fix e2e test to checkout feature3 before verifying its content The test was checking file.txt content after checking out feature4, so it was reading feature4's content instead of feature3's. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 986f9a7 commit 4f46bd6

2 files changed

Lines changed: 21 additions & 15 deletions

File tree

tests/test_e2e.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -831,6 +831,7 @@ fi
831831
# Line 1: Original base
832832
# Line 2: From feature 3 commit ("Feature 3 content line 2")
833833
# Line 7: From feature 3 conflict commit, kept during resolution ("Feature 3 conflicting change line 7")
834+
log_cmd git checkout feature3
834835
EXPECTED_CONTENT_LINE1="Base file content line 1"
835836
EXPECTED_CONTENT_LINE2="Feature 3 content line 2"
836837
EXPECTED_CONTENT_LINE7="Feature 3 conflicting change line 7"

update-pr-stack.sh

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,20 +33,16 @@ check_env_var() {
3333
fi
3434
}
3535

36-
# Check if BASE is already an ancestor of BRANCH (simple merge check)
37-
is_base_ancestor() {
38-
local BRANCH="$1"
39-
local BASE="$2"
40-
git merge-base --is-ancestor "origin/$BASE" "origin/$BRANCH"
41-
}
42-
4336
# Check if a branch already has the squash commit merged (squash-merge mode only)
4437
# Requires SQUASH_COMMIT ref to be set via git update-ref
38+
#
39+
# Note: This uses local branch refs. The caller must ensure both branches
40+
# exist locally before calling (e.g., via git checkout).
4541
has_squash_commit() {
4642
local BRANCH="$1"
4743
local BASE="$2"
48-
is_base_ancestor "$BRANCH" "$BASE" \
49-
&& git merge-base --is-ancestor SQUASH_COMMIT "origin/$BRANCH"
44+
git merge-base --is-ancestor "$BASE" "$BRANCH" \
45+
&& git merge-base --is-ancestor SQUASH_COMMIT "$BRANCH"
5046
}
5147

5248
format_branch_list_for_text() {
@@ -64,13 +60,17 @@ update_direct_target() {
6460
local BRANCH="$1"
6561
local BASE_BRANCH="$2"
6662

63+
# Checkout first to ensure the local branch exists (created from origin if
64+
# needed). This allows has_squash_commit to compare local refs, which matters
65+
# for testing where the script may run multiple times in the same repo.
66+
log_cmd git checkout "$BRANCH"
67+
6768
if has_squash_commit "$BRANCH" "$TARGET_BRANCH"; then
6869
echo "$BRANCH already up-to-date; skipping"
6970
return
7071
fi
7172

7273
echo "Updating direct target $BRANCH (from $MERGED_BRANCH to $BASE_BRANCH)"
73-
log_cmd git checkout "$BRANCH"
7474

7575
CONFLICTS=()
7676
log_cmd git update-ref BEFORE_MERGE HEAD
@@ -122,16 +122,21 @@ update_indirect_target() {
122122
local BRANCH="$1"
123123
local BASE_BRANCH="$2"
124124

125-
# For indirect targets, we only need to check if the parent is already
126-
# an ancestor. If so, the branch already has all updates from the parent.
127-
# (No SQUASH_COMMIT check needed - that's only for direct targets)
128-
if is_base_ancestor "$BRANCH" "$BASE_BRANCH"; then
125+
# At this point in the control flow:
126+
# - BASE_BRANCH has already been checked out and potentially modified locally
127+
# - BRANCH has not been touched yet
128+
#
129+
# We checkout BRANCH first (which creates the local branch from origin if
130+
# needed), then compare against the local BASE_BRANCH to check if it's
131+
# already an ancestor.
132+
log_cmd git checkout "$BRANCH"
133+
134+
if git merge-base --is-ancestor "$BASE_BRANCH" "$BRANCH"; then
129135
echo "$BRANCH already up-to-date with $BASE_BRANCH; skipping"
130136
return
131137
fi
132138

133139
echo "Updating indirect target $BRANCH (based on $BASE_BRANCH)"
134-
log_cmd git checkout "$BRANCH"
135140
log_cmd git merge --no-edit "$BASE_BRANCH"
136141
}
137142

0 commit comments

Comments
 (0)