Git branch flow improvements (#370)

crates/yaak-git/src/binary.rs

@@ -5,7 +5,15 @@ use std::process::Stdio;
 use tokio::process::Command;
 use yaak_common::command::new_xplatform_command;
 
+/// Create a git command that runs in the specified directory
 pub(crate) async fn new_binary_command(dir: &Path) -> Result<Command> {
+    let mut cmd = new_binary_command_global().await?;
+    cmd.arg("-C").arg(dir);
+    Ok(cmd)
+}
+
+/// Create a git command without a specific directory (for global operations)
+pub(crate) async fn new_binary_command_global() -> Result<Command> {
     // 1. Probe that `git` exists and is runnable
     let mut probe = new_xplatform_command("git");
     probe.arg("--version").stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null());
@@ -17,8 +25,6 @@ pub(crate) async fn new_binary_command(dir: &Path) -> Result<Command> {
     }
 
     // 2. Build the reusable git command
-    let mut cmd = new_xplatform_command("git");
-    cmd.arg("-C").arg(dir);
-
+    let cmd = new_xplatform_command("git");
     Ok(cmd)
 }

crates/yaak-git/src/branch.rs

@@ -1,99 +1,153 @@
+use serde::{Deserialize, Serialize};
+use ts_rs::TS;
+
+use crate::binary::new_binary_command;
 use crate::error::Error::GenericError;
 use crate::error::Result;
-use crate::merge::do_merge;
-use crate::repository::open_repo;
-use crate::util::{bytes_to_string, get_branch_by_name, get_current_branch};
-use git2::BranchType;
-use git2::build::CheckoutBuilder;
-use log::info;
 use std::path::Path;
 
-pub fn git_checkout_branch(dir: &Path, branch_name: &str, force: bool) -> Result<String> {
-    if branch_name.starts_with("origin/") {
-        return git_checkout_remote_branch(dir, branch_name, force);
-    }
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, TS)]
+#[serde(rename_all = "snake_case", tag = "type")]
+#[ts(export, export_to = "gen_git.ts")]
+pub enum BranchDeleteResult {
+    Success { message: String },
+    NotFullyMerged,
+}
 
-    let repo = open_repo(dir)?;
-    let branch = get_branch_by_name(&repo, branch_name)?;
-    let branch_ref = branch.into_reference();
-    let branch_tree = branch_ref.peel_to_tree()?;
+pub async fn git_checkout_branch(dir: &Path, branch_name: &str, force: bool) -> Result<String> {
+    let branch_name = branch_name.trim_start_matches("origin/");
 
-    let mut options = CheckoutBuilder::default();
+    let mut args = vec!["checkout"];
     if force {
-        options.force();
+        args.push("--force");
     }
+    args.push(branch_name);
 
-    repo.checkout_tree(branch_tree.as_object(), Some(&mut options))?;
-    repo.set_head(branch_ref.name().unwrap())?;
+    let out = new_binary_command(dir)
+        .await?
+        .args(&args)
+        .output()
+        .await
+        .map_err(|e| GenericError(format!("failed to run git checkout: {e}")))?;
+
+    let stdout = String::from_utf8_lossy(&out.stdout);
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    let combined = format!("{}{}", stdout, stderr);
+
+    if !out.status.success() {
+        return Err(GenericError(format!("Failed to checkout: {}", combined.trim())));
+    }
 
     Ok(branch_name.to_string())
 }
 
-pub(crate) fn git_checkout_remote_branch(
-    dir: &Path,
-    branch_name: &str,
-    force: bool,
-) -> Result<String> {
-    let branch_name = branch_name.trim_start_matches("origin/");
-    let repo = open_repo(dir)?;
-
-    let refname = format!("refs/remotes/origin/{}", branch_name);
-    let remote_ref = repo.find_reference(&refname)?;
-    let commit = remote_ref.peel_to_commit()?;
-
-    let mut new_branch = repo.branch(branch_name, &commit, false)?;
-    let upstream_name = format!("origin/{}", branch_name);
-    new_branch.set_upstream(Some(&upstream_name))?;
-
-    git_checkout_branch(dir, branch_name, force)
-}
-
-pub fn git_create_branch(dir: &Path, name: &str) -> Result<()> {
-    let repo = open_repo(dir)?;
-    let head = match repo.head() {
-        Ok(h) => h,
-        Err(e) if e.code() == git2::ErrorCode::UnbornBranch => {
-            let msg = "Cannot create branch when there are no commits";
-            return Err(GenericError(msg.into()));
-        }
-        Err(e) => return Err(e.into()),
-    };
-    let head = head.peel_to_commit()?;
-
-    repo.branch(name, &head, false)?;
-
-    Ok(())
-}
-
-pub fn git_delete_branch(dir: &Path, name: &str) -> Result<()> {
-    let repo = open_repo(dir)?;
-    let mut branch = get_branch_by_name(&repo, name)?;
-
-    if branch.is_head() {
-        info!("Deleting head branch");
-        let branches = repo.branches(Some(BranchType::Local))?;
-        let other_branch = branches.into_iter().filter_map(|b| b.ok()).find(|b| !b.0.is_head());
-        let other_branch = match other_branch {
-            None => return Err(GenericError("Cannot delete only branch".into())),
-            Some(b) => bytes_to_string(b.0.name_bytes()?)?,
-        };
-
-        git_checkout_branch(dir, &other_branch, true)?;
+pub async fn git_create_branch(dir: &Path, name: &str, base: Option<&str>) -> Result<()> {
+    let mut cmd = new_binary_command(dir).await?;
+    cmd.arg("branch").arg(name);
+    if let Some(base_branch) = base {
+        cmd.arg(base_branch);
     }
 
-    branch.delete()?;
+    let out =
+        cmd.output().await.map_err(|e| GenericError(format!("failed to run git branch: {e}")))?;
+
+    let stdout = String::from_utf8_lossy(&out.stdout);
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    let combined = format!("{}{}", stdout, stderr);
+
+    if !out.status.success() {
+        return Err(GenericError(format!("Failed to create branch: {}", combined.trim())));
+    }
 
     Ok(())
 }
 
-pub fn git_merge_branch(dir: &Path, name: &str, _force: bool) -> Result<()> {
-    let repo = open_repo(dir)?;
-    let local_branch = get_current_branch(&repo)?.unwrap();
+pub async fn git_delete_branch(dir: &Path, name: &str, force: bool) -> Result<BranchDeleteResult> {
+    let mut cmd = new_binary_command(dir).await?;
 
-    let commit_to_merge = get_branch_by_name(&repo, name)?.into_reference();
-    let commit_to_merge = repo.reference_to_annotated_commit(&commit_to_merge)?;
+    let out =
+        if force { cmd.args(["branch", "-D", name]) } else { cmd.args(["branch", "-d", name]) }
+            .output()
+            .await
+            .map_err(|e| GenericError(format!("failed to run git branch -d: {e}")))?;
 
-    do_merge(&repo, &local_branch, &commit_to_merge)?;
+    let stdout = String::from_utf8_lossy(&out.stdout);
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    let combined = format!("{}{}", stdout, stderr);
+
+    if !out.status.success() && stderr.to_lowercase().contains("not fully merged") {
+        return Ok(BranchDeleteResult::NotFullyMerged);
+    }
+
+    if !out.status.success() {
+        return Err(GenericError(format!("Failed to delete branch: {}", combined.trim())));
+    }
+
+    Ok(BranchDeleteResult::Success { message: combined })
+}
+
+pub async fn git_merge_branch(dir: &Path, name: &str) -> Result<()> {
+    let out = new_binary_command(dir)
+        .await?
+        .args(["merge", name])
+        .output()
+        .await
+        .map_err(|e| GenericError(format!("failed to run git merge: {e}")))?;
+
+    let stdout = String::from_utf8_lossy(&out.stdout);
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    let combined = format!("{}{}", stdout, stderr);
+
+    if !out.status.success() {
+        // Check for merge conflicts
+        if combined.to_lowercase().contains("conflict") {
+            return Err(GenericError(
+                "Merge conflicts detected. Please resolve them manually.".to_string(),
+            ));
+        }
+        return Err(GenericError(format!("Failed to merge: {}", combined.trim())));
+    }
+
+    Ok(())
+}
+
+pub async fn git_delete_remote_branch(dir: &Path, name: &str) -> Result<()> {
+    // Remote branch names come in as "origin/branch-name", extract the branch name
+    let branch_name = name.trim_start_matches("origin/");
+
+    let out = new_binary_command(dir)
+        .await?
+        .args(["push", "origin", "--delete", branch_name])
+        .output()
+        .await
+        .map_err(|e| GenericError(format!("failed to run git push --delete: {e}")))?;
+
+    let stdout = String::from_utf8_lossy(&out.stdout);
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    let combined = format!("{}{}", stdout, stderr);
+
+    if !out.status.success() {
+        return Err(GenericError(format!("Failed to delete remote branch: {}", combined.trim())));
+    }
+
+    Ok(())
+}
+
+pub async fn git_rename_branch(dir: &Path, old_name: &str, new_name: &str) -> Result<()> {
+    let out = new_binary_command(dir)
+        .await?
+        .args(["branch", "-m", old_name, new_name])
+        .output()
+        .await
+        .map_err(|e| GenericError(format!("failed to run git branch -m: {e}")))?;
+
+    let stdout = String::from_utf8_lossy(&out.stdout);
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    let combined = format!("{}{}", stdout, stderr);
+
+    if !out.status.success() {
+        return Err(GenericError(format!("Failed to rename branch: {}", combined.trim())));
+    }
 
     Ok(())
 }

crates/yaak-git/src/clone.rs

@@ -0,0 +1,53 @@
+use crate::binary::new_binary_command;
+use crate::error::Error::GenericError;
+use crate::error::Result;
+use log::info;
+use serde::{Deserialize, Serialize};
+use std::fs;
+use std::path::Path;
+use ts_rs::TS;
+
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, TS)]
+#[serde(rename_all = "snake_case", tag = "type")]
+#[ts(export, export_to = "gen_git.ts")]
+pub enum CloneResult {
+    Success,
+    Cancelled,
+    NeedsCredentials { url: String, error: Option<String> },
+}
+
+pub async fn git_clone(url: &str, dir: &Path) -> Result<CloneResult> {
+    let parent = dir.parent().ok_or_else(|| GenericError("Invalid clone directory".to_string()))?;
+    fs::create_dir_all(parent)
+        .map_err(|e| GenericError(format!("Failed to create directory: {e}")))?;
+    let mut cmd = new_binary_command(parent).await?;
+    cmd.args(["clone", url]).arg(dir).env("GIT_TERMINAL_PROMPT", "0");
+
+    let out =
+        cmd.output().await.map_err(|e| GenericError(format!("failed to run git clone: {e}")))?;
+
+    let stdout = String::from_utf8_lossy(&out.stdout);
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    let combined = format!("{}{}", stdout, stderr);
+    let combined_lower = combined.to_lowercase();
+
+    info!("Cloned status={}: {combined}", out.status);
+
+    if !out.status.success() {
+        // Check for credentials error
+        if combined_lower.contains("could not read") {
+            return Ok(CloneResult::NeedsCredentials { url: url.to_string(), error: None });
+        }
+        if combined_lower.contains("unable to access")
+            || combined_lower.contains("authentication failed")
+        {
+            return Ok(CloneResult::NeedsCredentials {
+                url: url.to_string(),
+                error: Some(combined.to_string()),
+            });
+        }
+        return Err(GenericError(format!("Failed to clone: {}", combined.trim())));
+    }
+
+    Ok(CloneResult::Success)
+}

crates/yaak-git/src/credential.rs

@@ -1,24 +1,18 @@
-use crate::binary::new_binary_command;
+use crate::binary::new_binary_command_global;
 use crate::error::Error::GenericError;
 use crate::error::Result;
-use std::path::Path;
 use std::process::Stdio;
 use tokio::io::AsyncWriteExt;
 use url::Url;
 
-pub async fn git_add_credential(
-    dir: &Path,
-    remote_url: &str,
-    username: &str,
-    password: &str,
-) -> Result<()> {
+pub async fn git_add_credential(remote_url: &str, username: &str, password: &str) -> Result<()> {
     let url = Url::parse(remote_url)
         .map_err(|e| GenericError(format!("Failed to parse remote url {remote_url}: {e:?}")))?;
     let protocol = url.scheme();
     let host = url.host_str().unwrap();
     let path = Some(url.path());
 
-    let mut child = new_binary_command(dir)
+    let mut child = new_binary_command_global()
         .await?
         .args(["credential", "approve"])
         .stdin(Stdio::piped())

crates/yaak-git/src/lib.rs

@@ -1,13 +1,14 @@
 mod add;
 mod binary;
 mod branch;
+mod clone;
 mod commit;
 mod credential;
 pub mod error;
 mod fetch;
 mod init;
 mod log;
-mod merge;
+
 mod pull;
 mod push;
 mod remotes;
@@ -18,7 +19,11 @@ mod util;
 
 // Re-export all git functions for external use
 pub use add::git_add;
-pub use branch::{git_checkout_branch, git_create_branch, git_delete_branch, git_merge_branch};
+pub use branch::{
+    BranchDeleteResult, git_checkout_branch, git_create_branch, git_delete_branch,
+    git_delete_remote_branch, git_merge_branch, git_rename_branch,
+};
+pub use clone::{CloneResult, git_clone};
 pub use commit::git_commit;
 pub use credential::git_add_credential;
 pub use fetch::git_fetch_all;

crates/yaak-git/src/merge.rs

@@ -1,135 +0,0 @@
-use crate::error::Error::MergeConflicts;
-use crate::util::bytes_to_string;
-use git2::{AnnotatedCommit, Branch, IndexEntry, Reference, Repository};
-use log::{debug, info};
-
-pub(crate) fn do_merge(
-    repo: &Repository,
-    local_branch: &Branch,
-    commit_to_merge: &AnnotatedCommit,
-) -> crate::error::Result<()> {
-    debug!("Merging remote branches");
-    let analysis = repo.merge_analysis(&[&commit_to_merge])?;
-
-    if analysis.0.is_fast_forward() {
-        let refname = bytes_to_string(local_branch.get().name_bytes())?;
-        match repo.find_reference(&refname) {
-            Ok(mut r) => {
-                merge_fast_forward(repo, &mut r, &commit_to_merge)?;
-            }
-            Err(_) => {
-                // The branch doesn't exist, so set the reference to the commit directly. Usually
-                // this is because you are pulling into an empty repository.
-                repo.reference(
-                    &refname,
-                    commit_to_merge.id(),
-                    true,
-                    &format!("Setting {} to {}", refname, commit_to_merge.id()),
-                )?;
-                repo.set_head(&refname)?;
-                repo.checkout_head(Some(
-                    git2::build::CheckoutBuilder::default()
-                        .allow_conflicts(true)
-                        .conflict_style_merge(true)
-                        .force(),
-                ))?;
-            }
-        };
-    } else if analysis.0.is_normal() {
-        let head_commit = repo.reference_to_annotated_commit(&repo.head()?)?;
-        merge_normal(repo, &head_commit, commit_to_merge)?;
-    } else {
-        debug!("Skipping merge. Nothing to do")
-    }
-
-    Ok(())
-}
-
-pub(crate) fn merge_fast_forward(
-    repo: &Repository,
-    local_reference: &mut Reference,
-    remote_commit: &AnnotatedCommit,
-) -> crate::error::Result<()> {
-    info!("Performing fast forward");
-    let name = match local_reference.name() {
-        Some(s) => s.to_string(),
-        None => String::from_utf8_lossy(local_reference.name_bytes()).to_string(),
-    };
-    let msg = format!("Fast-Forward: Setting {} to id: {}", name, remote_commit.id());
-    local_reference.set_target(remote_commit.id(), &msg)?;
-    repo.set_head(&name)?;
-    repo.checkout_head(Some(
-        git2::build::CheckoutBuilder::default()
-            // For some reason, the force is required to make the working directory actually get
-            // updated I suspect we should be adding some logic to handle dirty working directory
-            // states, but this is just an example so maybe not.
-            .force(),
-    ))?;
-    Ok(())
-}
-
-pub(crate) fn merge_normal(
-    repo: &Repository,
-    local: &AnnotatedCommit,
-    remote: &AnnotatedCommit,
-) -> crate::error::Result<()> {
-    info!("Performing normal merge");
-    let local_tree = repo.find_commit(local.id())?.tree()?;
-    let remote_tree = repo.find_commit(remote.id())?.tree()?;
-    let ancestor = repo.find_commit(repo.merge_base(local.id(), remote.id())?)?.tree()?;
-
-    let mut idx = repo.merge_trees(&ancestor, &local_tree, &remote_tree, None)?;
-
-    if idx.has_conflicts() {
-        let conflicts = idx.conflicts()?;
-        for conflict in conflicts {
-            if let Ok(conflict) = conflict {
-                print_conflict(&conflict);
-            }
-        }
-        return Err(MergeConflicts);
-    }
-
-    let result_tree = repo.find_tree(idx.write_tree_to(repo)?)?;
-    // now create the merge commit
-    let msg = format!("Merge: {} into {}", remote.id(), local.id());
-    let sig = repo.signature()?;
-    let local_commit = repo.find_commit(local.id())?;
-    let remote_commit = repo.find_commit(remote.id())?;
-
-    // Do our merge commit and set current branch head to that commit.
-    let _merge_commit = repo.commit(
-        Some("HEAD"),
-        &sig,
-        &sig,
-        &msg,
-        &result_tree,
-        &[&local_commit, &remote_commit],
-    )?;
-
-    // Set working tree to match head.
-    repo.checkout_head(None)?;
-
-    Ok(())
-}
-
-fn print_conflict(conflict: &git2::IndexConflict) {
-    let ancestor = conflict.ancestor.as_ref().map(path_from_index_entry);
-    let ours = conflict.our.as_ref().map(path_from_index_entry);
-    let theirs = conflict.their.as_ref().map(path_from_index_entry);
-
-    println!("Conflict detected:");
-    if let Some(path) = ancestor {
-        println!("  Common ancestor: {:?}", path);
-    }
-    if let Some(path) = ours {
-        println!("  Ours: {:?}", path);
-    }
-    if let Some(path) = theirs {
-        println!("  Theirs: {:?}", path);
-    }
-}
-
-fn path_from_index_entry(entry: &IndexEntry) -> String {
-    String::from_utf8_lossy(entry.path.as_slice()).into_owned()
-}

crates/yaak-git/src/util.rs

@@ -47,10 +47,6 @@ pub(crate) fn remote_branch_names(repo: &Repository) -> Result<Vec<String>> {
     Ok(branches)
 }
 
-pub(crate) fn get_branch_by_name<'s>(repo: &'s Repository, name: &str) -> Result<Branch<'s>> {
-    Ok(repo.find_branch(name, BranchType::Local)?)
-}
-
 pub(crate) fn bytes_to_string(bytes: &[u8]) -> Result<String> {
     Ok(String::from_utf8(bytes.to_vec())?)
 }