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

When moving a commit or branch to a different stack, both rebases are
computed against the ODB first — no refs, no snapshot, no worktree
state is touched. Their outputs are then inspected for newly-conflicted
commits before any state is written.

This means each stack is rebased exactly once per operation.

Source check: if removing the moved commit(s) would cause remaining
commits in the source stack to conflict, the move is rejected — those
commits depend on the changes being moved away.

Destination check: if inserting the moved commit(s) onto the destination
stack would cause a conflict, the move is rejected — the changes do not
apply cleanly at the target location.

Pre-existing conflicts in the source stack are exempted: if a commit was
already conflicted before the move, it is not counted as a new conflict
caused by the move.

Moves that are now prevented:
- Moving a commit whose removal would leave dependent commits in the
  source stack that can no longer rebase cleanly.
- Moving a commit onto a destination stack that already modified the
  same content, causing a conflict at the insertion point.
- Moving a branch whose removal would strand dependent branches in the
  source stack.
- Moving a branch onto a destination that has already modified the
  same content.

Moves that are not affected:
- Intra-stack moves (source == destination); these cannot produce
  inter-stack conflicts and skip the check entirely.
- Cross-stack moves where source and destination are truly independent
  (disjoint file changes).
- Moves where the source stack has pre-existing conflicts unrelated to
  the commit being moved.

Author: mtsgrd

crates/gitbutler-branch-actions/src/actions.rs

@@ -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,
@@ -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,

crates/gitbutler-branch-actions/src/move_branch.rs

@@ -3,13 +3,17 @@ 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};
 
 #[derive(Debug, Clone, Serialize)]
 #[serde(rename_all = "camelCase")]
@@ -22,55 +26,141 @@ 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> {
     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)?;
+
+        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)?;
+
+        // We initialize the new StackBranch with the target branch's current head so that
+        // StackBranch::new's range validation passes (the rebased subject head is not yet
+        // "in range" because the stack head hasn't been updated). The correct commit is
+        // applied by set_heads_from_rebase_output below.
+        let target_branch_reference = dest_output
+            .references
+            .iter()
+            .find(|r| r.reference.to_string() == target_branch_name)
+            .context("target branch not found in dest 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 = 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 {
@@ -88,17 +178,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,
     )?;
@@ -126,7 +216,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();
@@ -144,63 +234,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> {
@@ -211,7 +250,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
@@ -223,7 +263,11 @@ 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)?;
     }
@@ -287,7 +331,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();