Skip to content

Test integrate upstream with conflicting local branch#11897

Draft
mtsgrd wants to merge 1 commit into
masterfrom
test-integrate-upstream-conflict
Draft

Test integrate upstream with conflicting local branch#11897
mtsgrd wants to merge 1 commit into
masterfrom
test-integrate-upstream-conflict

Conversation

@mtsgrd
Copy link
Copy Markdown
Contributor

@mtsgrd mtsgrd commented Jan 19, 2026

The test integrate_upstream_with_conflicting_local_branch is located in crates/gitbutler-branch-actions/tests/virtual_branches/apply_virtual_branch.rs (line 558).

What it tests:

  1. Sets up a scenario where:
    • Remote has commits A → B (B modifies file.txt)
    • Local base is at A
    • Local branch has commit C that also modifies file.txt(creating a conflict with B)
  2. Verifies that upstream_integration_statuses returns BranchStatus::Conflicted { rebasable: false }
  3. Performs integrate_upstream with merge strategy
  4. Currently failing assertion: Expects the stack to have exactly 1 branch after integration, but it currently has 2 branches: ["conflicting-branch", "master"]

The test expresses your expectation that there should be only one branch in the stack after integration. The test is failing, which indicates there's a bug in the integration logic that's creating an extra "master" branch.

@vercel
Copy link
Copy Markdown

vercel Bot commented Jan 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Review Updated (UTC)
gitbutler-web Ignored Ignored Preview Jan 23, 2026 2:47pm

Request Review

@github-actions github-actions Bot added the rust Pull requests that update Rust code label Jan 19, 2026
@mtsgrd mtsgrd force-pushed the test-integrate-upstream-conflict branch from 1462eb2 to 5976a16 Compare January 19, 2026 23:40
@mtsgrd
Copy link
Copy Markdown
Contributor Author

mtsgrd commented Jan 19, 2026

I asked claude to help me understand why the test is failing and got this reasonable response:

After the merge integration, the commit graph looks like this:

                conflicting-branch (HEAD)
                       |
                       v
                [Merge Commit]
                  /         \
        (1st parent)      (2nd parent)
                /               \
               v                 v
[commit C - local change]    [commit B - remote change]  <-- origin/master
               |                 |
               v                 v
         [commit A - initial]----+
               |
               v
        [Initial commit]

The merge commit has two parents:

  1. First parent: commit C (the local conflicting change) - this is the "main line" of the branch
  2. Second parent: commit B (the remote change, i.e., origin/master) - this is the branch being merged into the current branch

What the graph traversal currently does:

When building stack segments, the traversal follows all parents. So it sees:

Segment 1: conflicting-branch → Merge Commit → commit C (following first parent)
Segment 2: master → commit B → commit A → Initial commit (following second parent from the merge)

What it should do:

For determining the stack structure (i.e., which branches belong to this stack), the traversal should only follow the first parent. The second parent represents the branch that was merged in, not a branch that's part of this stack.

In other words: master (the target branch / upstream) should not be considered part of the conflicting-branch stack just because we merged it into conflicting-branch. The merge commit's second parent points to origin/master, but that's external to the stack - it's the upstream we integrated, not a branch we own.

The fix should:

When building a stack's segments, recognize that commits reachable only via the second (or later) parent of a merge commit, where those commits belong to the target branch, should be excluded from the stack's segment list. They are "merged-in history", not "stack history".

@mtsgrd mtsgrd force-pushed the test-integrate-upstream-conflict branch 2 times, most recently from 9c3af3c to 47c3e8f Compare January 20, 2026 00:08
@mtsgrd
Copy link
Copy Markdown
Contributor Author

mtsgrd commented Jan 20, 2026

AI generated fix to added test also added as a second commit.

Copy link
Copy Markdown
Collaborator

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this!

I only glimpsed at it and believe this one will take some time to review, a test in but-graph that shows the issue to solve, and a proper implementation of the fix.
Meanwhile, let's be sure this doesn't get merged -I will try to get to it properly soon.

@mtsgrd
Copy link
Copy Markdown
Contributor Author

mtsgrd commented Jan 20, 2026

Meanwhile, let's be sure this doesn't get merged

Ofc, I only put this up to demonstrate the problem ✌️

Add a new integration test that simulates a remote change and a local
conflicting commit to verify upstream integration behavior when
conflicts exist. The test sets up a repo with commit A, a remote commit
B, and a local conflicting commit C on a virtual branch, checks that the
status reports the branch as Conflicted, runs integrate_upstream with a
Merge resolution, and asserts the stack remains intact and contains the
original branch after merge integration.
@Byron
Copy link
Copy Markdown
Collaborator

Byron commented Jan 23, 2026

As getting into this could be a huge time-sink, I had to put it onto my backlog to be able to focus on unapply(). The only consolation I can give is that I let it jump the queue and put it first.

@mtsgrd mtsgrd force-pushed the test-integrate-upstream-conflict branch from 47c3e8f to 401e3d9 Compare January 23, 2026 14:47
@mtsgrd
Copy link
Copy Markdown
Contributor Author

mtsgrd commented Jan 23, 2026

No worries. I force pushed to this branch to remove that second commit, so this pr is now just adding a test that demonstrates the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants