Skip to content

Commit cd0089c

Browse files
committed
Fix "Specified HEAD didn't match actual HEAD^{tree}" error on branch apply
Telemetry showed users hitting this error when applying a branch. It's caused by a race condition: `apply()` reads `prev_head_id` from the workspace graph and passes it to `safe_checkout`, but between those two calls the file watcher can run `update_workspace_commit`, which moves the `gitbutler/workspace` ref to a new commit with a different tree. Inside `merge_worktree_changes_into_destination_or_keep_snapshot`, the caller's stale `source_tree_id` was compared against `repo.head_tree_id_or_empty()` and the mismatch triggered a `bail!()`. The fix removes that assertion and always reads the tree from `repo.head_tree_id_or_empty()`. This is correct because worktree diffs (via the git index) are always relative to the actual HEAD tree, not whatever the caller cached — so the snapshot base must match. Includes regression test: `checkout_succeeds_when_source_tree_diverges_from_head` Remove unused source_tree_id parameter Remove the now-unused source_tree_id parameter from merge_worktree_changes_into_destination_or_keep_snapshot and its callers. The parameter was unused in the function body and documentation, so eliminating it cleans up the function signature, related docs, and the call site in function.rs to avoid passing an unnecessary value. This simplifies the API and reduces confusion about the expected inputs.
1 parent 577e759 commit cd0089c

3 files changed

Lines changed: 88 additions & 10 deletions

File tree

crates/but-core/src/worktree/checkout/function.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ pub fn safe_checkout(
7474
let snapshot_tree = merge_worktree_changes_into_destination_or_keep_snapshot(
7575
&changed_files,
7676
repo,
77-
source_tree.id,
7877
destination_tree.id,
7978
&mut opts,
8079
conflicting_worktree_changes_opts,

crates/but-core/src/worktree/checkout/utils.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,6 @@ use crate::{
3636
/// * `repo` - Repository used to inspect the current worktree, read the current `HEAD` tree, create
3737
/// snapshot trees, and persist any in-memory objects needed by the adjusted destination tree if one is written.
3838
/// No changes will be observable if there is no intersecting worktree changes.
39-
/// * `source_tree_id` - The tree expected to match the repository's current `HEAD`. It is used as the
40-
/// base for the snapshot of local worktree changes, and is verified against the actual `HEAD` before
41-
/// preserving changes.
4239
/// * `destination_tree_id` - The tree the checkout originally intends to write into the worktree. If
4340
/// preserved worktree changes can be cleanly reapplied, they are resolved onto this tree and may
4441
/// produce a replacement destination tree in the return value.
@@ -64,7 +61,6 @@ use crate::{
6461
pub fn merge_worktree_changes_into_destination_or_keep_snapshot(
6562
files_to_checkout: &[(ChangeKind, BString)],
6663
repo: &gix::Repository,
67-
source_tree_id: gix::ObjectId,
6864
destination_tree_id: gix::ObjectId,
6965
checkout_opts: &mut git2::build::CheckoutBuilder,
7066
uncommitted_changes: UncommitedWorktreeChanges,
@@ -76,11 +72,11 @@ pub fn merge_worktree_changes_into_destination_or_keep_snapshot(
7672
let worktree_changes = crate::diff::worktree_changes_no_renames(repo)?;
7773
if !worktree_changes.changes.is_empty() || !worktree_changes.ignored_changes.is_empty() {
7874
let actual_head_tree_id = repo.head_tree_id_or_empty()?;
79-
if actual_head_tree_id != source_tree_id {
80-
bail!(
81-
"Specified HEAD {source_tree_id} didn't match actual HEAD^{{tree}} {actual_head_tree_id}"
82-
)
83-
}
75+
// Worktree changes are always relative to the actual HEAD tree (via the index),
76+
// so the snapshot must use it as its base. If the caller's source tree diverges
77+
// (e.g. due to a concurrent workspace commit update), we use the actual HEAD tree
78+
// to keep the snapshot consistent with the worktree diff.
79+
let source_tree_id = actual_head_tree_id.detach();
8480
let mut checkout_deletions_lut = super::tree::Lut::default();
8581
let mut checkout_writes_lut = super::tree::Lut::default();
8682
for (kind, path) in files_to_checkout {

crates/but-core/tests/core/worktree/checkout.rs

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,6 +1086,89 @@ fn partial_commit_with_deletion_plus_insertion_conflicts_on_checkout() -> anyhow
10861086
Ok(())
10871087
}
10881088

1089+
#[test]
1090+
fn checkout_succeeds_when_source_tree_diverges_from_head() -> anyhow::Result<()> {
1091+
let (repo, _tmp) = writable_scenario("mixed-hunk-modifications");
1092+
// This scenario has uncommitted worktree changes — the condition that
1093+
// triggers the HEAD tree consistency check.
1094+
insta::assert_snapshot!(git_status(&repo)?, @r"
1095+
M file
1096+
M file-in-index
1097+
RM file-to-be-renamed-in-index -> file-renamed-in-index
1098+
D file-to-be-renamed
1099+
?? file-renamed
1100+
");
1101+
1102+
// Build a target commit that adds a new independent file, so the checkout
1103+
// doesn't conflict with the existing worktree changes.
1104+
let (head_commit, new_commit) = build_commit(
1105+
&repo,
1106+
|tree| {
1107+
let blob_id = repo.write_blob(b"new-content\n")?;
1108+
tree.upsert("new-file", EntryKind::Blob, blob_id)?;
1109+
Ok(())
1110+
},
1111+
"add 'new-file'",
1112+
)?;
1113+
1114+
// Create a "stale" commit with a different tree to simulate stale workspace
1115+
// metadata causing `entrypoint_commit()` to return a commit whose tree
1116+
// differs from actual HEAD^{tree}. This reproduces the "Specified HEAD
1117+
// didn't match actual HEAD^{tree}" error from production telemetry.
1118+
let stale_tree = {
1119+
let mut editor = head_commit.tree()?.edit()?;
1120+
let blob_id = repo.write_blob(b"stale marker\n")?;
1121+
editor.upsert("stale-marker", EntryKind::Blob, blob_id)?;
1122+
editor.write()?.detach()
1123+
};
1124+
let stale_commit_id = repo
1125+
.write_object(gix::objs::Commit {
1126+
tree: stale_tree,
1127+
parents: [head_commit.id].into(),
1128+
message: "stale workspace commit".into(),
1129+
..head_commit.decode()?.to_owned()?
1130+
})?
1131+
.detach();
1132+
1133+
// Pass the stale commit as current_head_id. Its tree differs from what
1134+
// HEAD^{tree} actually is, but the checkout should still succeed — the
1135+
// worktree changes are relative to the real HEAD, not the stale commit.
1136+
let out = safe_checkout(
1137+
stale_commit_id,
1138+
new_commit.id,
1139+
&repo,
1140+
checkout::Options {
1141+
skip_head_update: true,
1142+
..Default::default()
1143+
},
1144+
)?;
1145+
insta::assert_debug_snapshot!(out, @r#"
1146+
Outcome {
1147+
snapshot_tree: None,
1148+
num_deleted_files: 1,
1149+
num_added_or_updated_files: 1,
1150+
head_update: "None",
1151+
}
1152+
"#);
1153+
1154+
// The new file was checked out.
1155+
let new_file = repo.workdir_path("new-file").unwrap();
1156+
assert!(new_file.exists(), "new-file should have been checked out");
1157+
assert_eq!(std::fs::read_to_string(new_file)?, "new-content\n");
1158+
1159+
// The existing worktree changes are preserved.
1160+
insta::assert_snapshot!(git_status(&repo)?, @r"
1161+
M file
1162+
M file-in-index
1163+
RM file-to-be-renamed-in-index -> file-renamed-in-index
1164+
D file-to-be-renamed
1165+
A new-file
1166+
?? file-renamed
1167+
");
1168+
1169+
Ok(())
1170+
}
1171+
10891172
fn overwrite_options() -> checkout::Options {
10901173
checkout::Options {
10911174
uncommitted_changes: UncommitedWorktreeChanges::KeepConflictingInSnapshotAndOverwrite,

0 commit comments

Comments
 (0)