fabrica

hanna/fabrica

feat(git): surface merge/rebase conflict details

c55d70c · hanna committed on 2026-07-25

MergeConflict now carries the conflicting paths (from merge-tree) or the rebase
output, and the PR merge error shows them, so a failed merge/rebase explains
itself instead of a bare "could not be merged cleanly". Add a diverged
non-conflicting merge/squash/rebase test (previously only fast-forward was
covered) confirming all three strategies succeed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed by hannaSSH key fingerprint: SHA256:4g9cWhkAAw8gwqhJUcVbSnGEvdGwklW+1Aa/fMUu59k
7 files changed · +76 −8UnifiedSplit
crates/git/src/lib.rs +4 −3
@@ -100,9 +100,10 @@ pub enum GitError {
100 Merge(String),100 Merge(String),
101101
102 /// A server-side merge could not apply cleanly: the three-way merge or the102 /// A server-side merge could not apply cleanly: the three-way merge or the
103 /// rebase replay hit conflicts.103 /// rebase replay hit conflicts. The string names the conflicting paths (or
104 #[error("merge has conflicts")]104 /// the git output) when available.
105 MergeConflict,105 #[error("merge has conflicts: {0}")]
106 MergeConflict(String),
106107
107 /// `git init` (used for SHA-256 repositories) failed.108 /// `git init` (used for SHA-256 repositories) failed.
108 #[error("git init failed: {0}")]109 #[error("git init failed: {0}")]
crates/git/src/merge.rs +65 −3
@@ -239,7 +239,13 @@ fn plumbing_commit(req: &MergeRequest, base: &str, parents: &[&str]) -> Result<S
239 })?;239 })?;
240 match tree_out.status.code() {240 match tree_out.status.code() {
241 Some(0) => {}241 Some(0) => {}
242 Some(1) => return Err(GitError::MergeConflict),242 Some(1) => {
243 // On a conflict, stdout is the (partial) tree oid followed by the
244 // conflicted-file information; surface that as the detail.
245 let out = String::from_utf8_lossy(&tree_out.stdout);
246 let detail = out.lines().skip(1).collect::<Vec<_>>().join("; ");
247 return Err(GitError::MergeConflict(detail.trim().to_string()));
248 }
243 _ => {249 _ => {
244 return Err(GitError::Merge(250 return Err(GitError::Merge(
245 String::from_utf8_lossy(&tree_out.stderr).trim().to_string(),251 String::from_utf8_lossy(&tree_out.stderr).trim().to_string(),
@@ -317,13 +323,19 @@ fn rebase_in_worktree(req: &MergeRequest, base: &str, head: &str) -> Result<Stri
317 source,323 source,
318 })?;324 })?;
319 if !out.status.success() {325 if !out.status.success() {
326 // Capture the reason before aborting (git prints it to stdout/stderr).
327 let mut detail = String::from_utf8_lossy(&out.stdout).trim().to_string();
328 let err = String::from_utf8_lossy(&out.stderr);
329 if !err.trim().is_empty() {
330 detail.push_str(err.trim());
331 }
320 // Leave no half-applied rebase state in the worktree.332 // Leave no half-applied rebase state in the worktree.
321 let _ = identified(req)333 let _ = identified(req)
322 .current_dir(req.workdir)334 .current_dir(req.workdir)
323 .env_remove("GIT_DIR")335 .env_remove("GIT_DIR")
324 .args(["rebase", "--abort"])336 .args(["rebase", "--abort"])
325 .status();337 .status();
326 return Err(GitError::MergeConflict);338 return Err(GitError::MergeConflict(detail));
327 }339 }
328340
329 let tip = identified(req)341 let tip = identified(req)
@@ -350,7 +362,7 @@ fn identified(req: &MergeRequest) -> Command {
350362
351#[cfg(test)]363#[cfg(test)]
352mod tests {364mod tests {
353 #![allow(clippy::unwrap_used, clippy::expect_used)]365 #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
354366
355 use std::path::{Path, PathBuf};367 use std::path::{Path, PathBuf};
356368
@@ -457,6 +469,56 @@ mod tests {
457 }469 }
458470
459 #[test]471 #[test]
472 fn merge_and_rebase_diverged_non_conflicting() {
473 if !git_present() {
474 return;
475 }
476 let fx = fixture();
477 let raw = Repository::open_bare(&fx.path).unwrap();
478 // Advance main with a *different* file, so main and feature diverge from
479 // their common base but do not conflict.
480 let base = raw
481 .find_reference("refs/heads/main")
482 .unwrap()
483 .target()
484 .unwrap();
485 commit(
486 &raw,
487 "main",
488 &[base],
489 &[("README.md", "hello\n"), ("main.txt", "m\n")],
490 "advance main",
491 3,
492 );
493
494 // analyze must agree it is clean...
495 assert_eq!(
496 analyze("git", &fx.path, fx.home.path(), "main", "feature").unwrap(),
497 Mergeability::Clean,
498 );
499 // ...and each strategy must succeed, not report a phantom conflict.
500 for (strategy, dir) in [
501 (MergeStrategy::Merge, "wt-div-merge"),
502 (MergeStrategy::Squash, "wt-div-squash"),
503 (MergeStrategy::Rebase, "wt-div-rebase"),
504 ] {
505 let wt = fx.path.parent().unwrap().join(dir);
506 let sha = merge(&request(&fx, strategy, &wt))
507 .unwrap_or_else(|e| panic!("{strategy:?} failed: {e:?}"));
508 let raw = Repository::open_bare(&fx.path).unwrap();
509 assert!(
510 raw.find_commit(git2::Oid::from_str(&sha).unwrap())
511 .unwrap()
512 .tree()
513 .unwrap()
514 .get_name("feature.txt")
515 .is_some(),
516 "{strategy:?}: merged tree keeps feature.txt"
517 );
518 }
519 }
520
521 #[test]
460 fn analyze_reports_clean_and_already_merged() {522 fn analyze_reports_clean_and_already_merged() {
461 if !git_present() {523 if !git_present() {
462 return;524 return;
crates/web/src/pulls.rs +4 −1
@@ -617,9 +617,12 @@ async fn run_merge(
617 .map_err(AppError::internal)?;617 .map_err(AppError::internal)?;
618618
619 sha.map_err(|e| match e {619 sha.map_err(|e| match e {
620 git::GitError::MergeConflict => {620 git::GitError::MergeConflict(detail) if detail.is_empty() => {
621 AppError::BadRequest("The branch could not be merged cleanly.".to_string())621 AppError::BadRequest("The branch could not be merged cleanly.".to_string())
622 }622 }
623 git::GitError::MergeConflict(detail) => {
624 AppError::BadRequest(format!("The branch could not be merged cleanly: {detail}"))
625 }
623 other => AppError::from(other),626 other => AppError::from(other),
624 })627 })
625}628}
data/repos/01/01kyc0j4dzrnx6afv3wwfvxgn4.git/logs/HEAD +1 −0
@@ -1,2 +1,3 @@
10000000000000000000000000000000000000000 f09bd3e7f54bc32b967fdffb126b533e1a487f4d hanna <me@hanna.lol> 1784962339 -0400 push10000000000000000000000000000000000000000 f09bd3e7f54bc32b967fdffb126b533e1a487f4d hanna <me@hanna.lol> 1784962339 -0400 push
2f09bd3e7f54bc32b967fdffb126b533e1a487f4d 1570ae1be5f4b82bf24e1115681290e6d68808bf Hanna <me@hanna.lol> 1785020132 -04002f09bd3e7f54bc32b967fdffb126b533e1a487f4d 1570ae1be5f4b82bf24e1115681290e6d68808bf Hanna <me@hanna.lol> 1785020132 -0400
31570ae1be5f4b82bf24e1115681290e6d68808bf 94ffb4ef79a86c7e24d038233859149ba61439aa Hanna <me@hanna.lol> 1785020888 -0400
data/repos/01/01kyc0j4dzrnx6afv3wwfvxgn4.git/logs/refs/heads/main +1 −0
@@ -1,2 +1,3 @@
10000000000000000000000000000000000000000 f09bd3e7f54bc32b967fdffb126b533e1a487f4d hanna <me@hanna.lol> 1784962339 -0400 push10000000000000000000000000000000000000000 f09bd3e7f54bc32b967fdffb126b533e1a487f4d hanna <me@hanna.lol> 1784962339 -0400 push
2f09bd3e7f54bc32b967fdffb126b533e1a487f4d 1570ae1be5f4b82bf24e1115681290e6d68808bf Hanna <me@hanna.lol> 1785020132 -04002f09bd3e7f54bc32b967fdffb126b533e1a487f4d 1570ae1be5f4b82bf24e1115681290e6d68808bf Hanna <me@hanna.lol> 1785020132 -0400
31570ae1be5f4b82bf24e1115681290e6d68808bf 94ffb4ef79a86c7e24d038233859149ba61439aa Hanna <me@hanna.lol> 1785020888 -0400
data/repos/01/01kyc0j4dzrnx6afv3wwfvxgn4.git/objects/94/ffb4ef79a86c7e24d038233859149ba61439aa +0 −0

Binary file not shown.

data/repos/01/01kyc0j4dzrnx6afv3wwfvxgn4.git/refs/heads/main +1 −1
@@ -1 +1 @@
11570ae1be5f4b82bf24e1115681290e6d68808bf194ffb4ef79a86c7e24d038233859149ba61439aa