Skip to content

Add pre-flight conflict detection for cross-stack commit and branch moves #13086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mtsgrd merged 1 commit into from
Apr 8, 2026
Merged

Add pre-flight conflict detection for cross-stack commit and branch moves #13086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions crates/gitbutler-branch-actions/src/actions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,10 +396,6 @@ pub fn move_commit(
ctx.verify(guard.write_permission())?;
ensure_open_workspace_mode(ctx, guard.read_permission())
.context("Moving a commit requires open workspace mode")?;
let _ = ctx.create_snapshot(
SnapshotDetails::new(OperationKind::MoveCommit),
guard.write_permission(),
);
move_commits::move_commit(
ctx,
target_stack_id,
Expand All @@ -420,10 +416,6 @@ pub fn move_branch(
ctx.verify(guard.write_permission())?;
ensure_open_workspace_mode(ctx, guard.read_permission())
.context("Moving a branch requires open workspace mode")?;
let _ = ctx.create_snapshot(
SnapshotDetails::new(OperationKind::MoveBranch),
guard.write_permission(),
);
crate::move_branch::move_branch(
ctx,
target_stack_id,
Expand Down
242 changes: 148 additions & 94 deletions crates/gitbutler-branch-actions/src/move_branch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@ use but_core::ref_metadata::StackId;
use but_ctx::{Context, access::RepoExclusive};
use but_rebase::{Rebase, RebaseStep};
use but_workspace::legacy::stack_ext::StackExt;
use gitbutler_oplog::{
OplogExt,
entry::{OperationKind, SnapshotDetails},
};
use gitbutler_reference::{LocalRefname, Refname};
use gitbutler_stack::{StackBranch, VirtualBranchesHandle};
use gitbutler_stack::StackBranch;
use gitbutler_workspace::branch_trees::{WorkspaceState, update_uncommitted_changes};
use gix::refs::transaction::PreviousValue;
use serde::Serialize;

use crate::BranchManagerExt;
use crate::{BranchManagerExt, VirtualBranchesExt as _, move_commits::bail_on_new_conflicts};
use anyhow::bail;

#[derive(Debug, Clone, Serialize)]
#[serde(rename_all = "camelCase")]
Expand All @@ -22,55 +27,143 @@ pub struct MoveBranchResult {
}

pub(crate) fn move_branch(
ctx: &Context,
ctx: &mut Context,
target_stack_id: StackId,
target_branch_name: &str,
source_stack_id: StackId,
subject_branch_name: &str,
perm: &mut RepoExclusive,
) -> Result<MoveBranchResult> {
if source_stack_id == target_stack_id {
bail!("Cannot move a branch within the same stack; use the reorder operation instead.");
}
let old_workspace = WorkspaceState::create(ctx, perm.read_permission())?;
let repo = ctx.repo.get()?;
let mut vb_state = VirtualBranchesHandle::new(ctx.project_data_dir());

let source_stack = vb_state.get_stack_in_workspace(source_stack_id)?;
let source_merge_base = source_stack.merge_base(ctx)?;
let (source_merge_base, dest_merge_base, source_branch_pr_number) = {
let vb_state = ctx.virtual_branches();
let source_stack = vb_state.get_stack_in_workspace(source_stack_id)?;
let dest_stack = vb_state.get_stack_in_workspace(target_stack_id)?;
let pr_number = source_stack
.branches()
.into_iter()
.find(|b| b.name == subject_branch_name)
.context("Subject branch not found in source stack")?
.pr_number;
(
source_stack.merge_base(ctx)?,
dest_stack.merge_base(ctx)?,
pr_number,
)
};

// Cross-stack move: compute both rebases (ODB-only writes), check for
// conflicts, then snapshot, then apply state changes.
let (source_output, dest_output, source_will_be_deleted) = {
let repo = ctx.repo.get()?;
let vb_state = ctx.virtual_branches();
let source_stack = vb_state.get_stack_in_workspace(source_stack_id)?;
let dest_stack = vb_state.get_stack_in_workspace(target_stack_id)?;
Comment thread
mtsgrd marked this conversation as resolved.

let (subject_branch_steps, remaining_steps) =
extract_branch_steps(ctx, &repo, &source_stack, subject_branch_name)?;

let source_will_be_deleted = remaining_steps.is_empty();

// Source: rebase remaining commits without the moved branch (if any remain).
let source_output = if !source_will_be_deleted {
let mut src_rebase = Rebase::new(&repo, source_merge_base, None)?;
src_rebase.steps(remaining_steps)?;
src_rebase.rebase_noops(false);
Some(src_rebase.rebase(&*ctx.cache.get_cache()?)?)
} else {
None
};

// Dest: rebase dest stack with the moved branch injected.
let new_dest_steps = inject_branch_steps(
ctx,
&repo,
&dest_stack,
target_branch_name,
subject_branch_steps,
)?;
let mut dst_rebase = Rebase::new(&repo, dest_merge_base, None)?;
dst_rebase.steps(new_dest_steps)?;
dst_rebase.rebase_noops(false);
let dest_output = dst_rebase.rebase(&*ctx.cache.get_cache()?)?;

// Conflict check — bail before any state is written.
if let Some(ref src_out) = source_output {
bail_on_new_conflicts(
&repo,
src_out,
"This move would cause a conflict in the source stack: \
other commits depend on the changes being moved.",
)?;
}
bail_on_new_conflicts(
&repo,
&dest_output,
"This move would cause a conflict in the destination stack: \
the branch does not apply cleanly at the target location.",
)?;

let source_branch = source_stack
.branches()
.into_iter()
.find(|b| b.name == subject_branch_name)
.context("Subject branch not found in source stack")?;
(source_output, dest_output, source_will_be_deleted)
};

let destination_stack = vb_state.get_stack_in_workspace(target_stack_id)?;
let destination_merge_base = destination_stack.merge_base(ctx)?;
// Snapshot after the conflict check, but before any state writes.
let _ = ctx.create_snapshot(SnapshotDetails::new(OperationKind::MoveBranch), perm);

let (subject_branch_steps, deleted_stacks) = extract_and_rebase_source_branch(
ctx,
source_stack_id,
subject_branch_name,
&repo,
&mut vb_state,
source_stack,
source_merge_base,
)?;
// Apply source changes.
let mut deleted_stacks = Vec::new();
{
let repo = ctx.repo.get()?;
let mut vb_state = ctx.virtual_branches();
if source_will_be_deleted {
vb_state.delete_branch_entry(&source_stack_id)?;
deleted_stacks.push(source_stack_id);
} else if let Some(src_out) = source_output {
let mut source_stack = vb_state.get_stack_in_workspace(source_stack_id)?;
let new_source_head = repo.find_commit(src_out.top_commit)?;
source_stack.remove_branch(ctx, subject_branch_name)?;
source_stack.set_stack_head(&mut vb_state, &repo, new_source_head.id().detach())?;
source_stack.set_heads_from_rebase_output(ctx, src_out.references)?;
}
}

// Inject the extracted branch steps into the destination stack and rebase the stack
inject_branch_steps_into_destination(
ctx,
target_branch_name,
subject_branch_name,
&repo,
&mut vb_state,
destination_stack,
destination_merge_base,
subject_branch_steps,
source_branch.pr_number,
)?;
// Apply dest changes.
{
let repo = ctx.repo.get()?;
let mut vb_state = ctx.virtual_branches();
let mut destination_stack = vb_state.get_stack_in_workspace(target_stack_id)?;
let new_destination_head = repo.find_commit(dest_output.top_commit)?;

// StackBranch::new validates that the supplied commit is within the current stack
// range. The rebased subject head isn't "in range" yet because the stack head hasn't
// been updated, so we seed the new branch with the anchor branch's current head as a
// placeholder. set_heads_from_rebase_output corrects it to the proper commit below.
let anchor_ref = dest_output
.references
.iter()
.find(|r| r.reference.to_string() == target_branch_name)
.context("target branch not found in dest rebase output")?;

let mut new_head =
StackBranch::new(anchor_ref.commit_id, subject_branch_name.to_string(), &repo)?;
new_head.pr_number = source_branch_pr_number;

destination_stack.add_series(ctx, new_head, Some(target_branch_name.to_string()))?;
destination_stack.set_stack_head(
&mut vb_state,
&repo,
new_destination_head.id().detach(),
)?;
destination_stack.set_heads_from_rebase_output(ctx, dest_output.references)?;
}

let new_workspace = WorkspaceState::create(ctx, perm.read_permission())?;
let _ = update_uncommitted_changes(ctx, old_workspace, new_workspace, perm);
crate::integration::update_workspace_commit_with_vb_state(&vb_state, ctx, false)
crate::integration::update_workspace_commit_with_vb_state(&ctx.virtual_branches(), ctx, false)
.context("failed to update gitbutler workspace")?;

Ok(MoveBranchResult {
Expand All @@ -88,17 +181,17 @@ pub(crate) fn tear_off_branch(
) -> Result<MoveBranchResult> {
let old_workspace = WorkspaceState::create(ctx, perm.read_permission())?;
let repo = ctx.repo.get()?;
let mut vb_state = VirtualBranchesHandle::new(ctx.project_data_dir());

let source_stack = vb_state.get_stack_in_workspace(source_stack_id)?;
let source_stack = ctx
.virtual_branches()
.get_stack_in_workspace(source_stack_id)?;
let source_merge_base = source_stack.merge_base(ctx)?;

let (subject_branch_steps, deleted_stacks) = extract_and_rebase_source_branch(
ctx,
source_stack_id,
subject_branch_name,
&repo,
&mut vb_state,
source_stack,
source_merge_base,
)?;
Expand Down Expand Up @@ -126,7 +219,7 @@ pub(crate) fn tear_off_branch(

let new_workspace = WorkspaceState::create(ctx, perm.read_permission())?;
let _ = update_uncommitted_changes(ctx, old_workspace, new_workspace, perm);
crate::integration::update_workspace_commit_with_vb_state(&vb_state, ctx, false)
crate::integration::update_workspace_commit_with_vb_state(&ctx.virtual_branches(), ctx, false)
.context("failed to update gitbutler workspace")?;

let branch_manager = ctx.branch_manager();
Expand All @@ -144,63 +237,12 @@ pub(crate) fn tear_off_branch(
})
}

#[expect(clippy::too_many_arguments)]
/// Injects the extracted branch steps into the destination stack and rebases it.
fn inject_branch_steps_into_destination(
ctx: &Context,
target_branch_name: &str,
subject_branch_name: &str,
repo: &gix::Repository,
vb_state: &mut VirtualBranchesHandle,
destination_stack: gitbutler_stack::Stack,
destination_merge_base: gix::ObjectId,
subject_branch_steps: Vec<RebaseStep>,
subject_branch_pr_number: Option<usize>,
) -> Result<(), anyhow::Error> {
let new_destination_steps = inject_branch_steps(
ctx,
repo,
&destination_stack,
target_branch_name,
subject_branch_steps,
)?;

let mut destination_stack_rebase = Rebase::new(repo, destination_merge_base, None)?;
destination_stack_rebase.steps(new_destination_steps)?;
destination_stack_rebase.rebase_noops(false);
let destination_rebase_result = destination_stack_rebase.rebase(&*ctx.cache.get_cache()?)?;
let new_destination_head = repo.find_commit(destination_rebase_result.top_commit)?;
let mut destination_stack = destination_stack;

let target_branch_reference = destination_rebase_result
.clone()
.references
.into_iter()
.find(|r| r.reference.to_string() == target_branch_name)
.context("subject branch not found in rebase output")?;

let target_branch_head = target_branch_reference.commit_id;

let mut new_head = StackBranch::new(target_branch_head, subject_branch_name.to_string(), repo)?;

new_head.pr_number = subject_branch_pr_number;

destination_stack.add_series(ctx, new_head, Some(target_branch_name.to_string()))?;

destination_stack.set_stack_head(vb_state, repo, new_destination_head.id().detach())?;

destination_stack
.set_heads_from_rebase_output(ctx, destination_rebase_result.clone().references)?;
Ok(())
}

/// Extracts the steps corresponding to the branch to move, and rebases the source stack without those steps.
fn extract_and_rebase_source_branch(
ctx: &Context,
source_stack_id: StackId,
subject_branch_name: &str,
repository: &gix::Repository,
vb_state: &mut VirtualBranchesHandle,
source_stack: gitbutler_stack::Stack,
source_merge_base: gix::ObjectId,
) -> Result<(Vec<RebaseStep>, Vec<StackId>), anyhow::Error> {
Expand All @@ -211,7 +253,8 @@ fn extract_and_rebase_source_branch(

if new_source_steps.is_empty() {
// If there are no other branches left in the source stack, delete the stack.
vb_state.delete_branch_entry(&source_stack_id)?;
ctx.virtual_branches()
.delete_branch_entry(&source_stack_id)?;
deleted_stacks.push(source_stack_id);
} else {
// Rebase the source stack without the extracted branch steps
Expand All @@ -223,13 +266,24 @@ fn extract_and_rebase_source_branch(

source_stack.remove_branch(ctx, subject_branch_name)?;

source_stack.set_stack_head(vb_state, repository, new_source_head.id().detach())?;
source_stack.set_stack_head(
&mut ctx.virtual_branches(),
repository,
new_source_head.id().detach(),
)?;

source_stack.set_heads_from_rebase_output(ctx, source_rebase_result.clone().references)?;
}
Ok((subject_branch_steps, deleted_stacks))
}

/// Splits the source stack's rebase steps into two groups: those belonging to
/// `subject_branch_name` and those that remain.
///
/// Steps are partitioned by scanning for a `Reference` marker whose name matches
/// the subject branch (either as a Git ref or a virtual ref). All steps between
/// consecutive Reference markers are considered part of that branch. Returns
/// `(subject_steps, remaining_steps)`, both in execution order (oldest first).
fn extract_branch_steps(
ctx: &Context,
repository: &gix::Repository,
Expand Down Expand Up @@ -287,7 +341,7 @@ fn inject_branch_steps(
branch_steps: Vec<RebaseStep>,
) -> Result<Vec<RebaseStep>> {
let destination_steps = destination_stack.as_rebase_steps_rev(ctx)?;
let mut branch_steps = branch_steps.clone();
let mut branch_steps = branch_steps;
branch_steps.reverse();

let mut new_destination_steps = Vec::new();
Expand Down
Loading
Loading