From 4e85028084fec4aee50f5350f8017643202ce84d Mon Sep 17 00:00:00 2001 From: Solomon Date: Tue, 2 Sep 2025 11:21:39 +0100 Subject: [PATCH] Automated tests for git operations (#598) --- .../src/logs/plain_text_processor.rs | 9 +- crates/services/src/services/git.rs | 328 ++++-- crates/services/src/services/git_cli.rs | 373 ++++--- crates/services/tests/git_ops_safety.rs | 967 ++++++++++++++++++ crates/services/tests/git_workflow.rs | 596 +++++++++++ 5 files changed, 2007 insertions(+), 266 deletions(-) create mode 100644 crates/services/tests/git_ops_safety.rs create mode 100644 crates/services/tests/git_workflow.rs diff --git a/crates/executors/src/logs/plain_text_processor.rs b/crates/executors/src/logs/plain_text_processor.rs index e026aea3..25969344 100644 --- a/crates/executors/src/logs/plain_text_processor.rs +++ b/crates/executors/src/logs/plain_text_processor.rs @@ -119,11 +119,6 @@ impl PlainTextBuffer { result } - /// Return the total number of lines. - pub fn line_count(&self) -> usize { - self.lines.len() - } - /// Return the total length of content. pub fn total_len(&self) -> usize { self.total_len @@ -376,11 +371,11 @@ mod tests { let mut buffer = PlainTextBuffer::new(); buffer.ingest("line1\npartial".to_string()); - assert_eq!(buffer.line_count(), 2); + assert_eq!(buffer.lines().len(), 2); let lines = buffer.flush(); assert_eq!(lines, vec!["line1\n", "partial"]); - assert_eq!(buffer.line_count(), 0); + assert_eq!(buffer.lines().len(), 0); } #[test] diff --git a/crates/services/src/services/git.rs b/crates/services/src/services/git.rs index 6a58117d..db6baae1 100644 --- a/crates/services/src/services/git.rs +++ b/crates/services/src/services/git.rs @@ -38,6 +38,8 @@ pub enum GitServiceError { InvalidFilePaths(String), #[error("No GitHub token available.")] TokenUnavailable, + #[error("Rebase in progress; resolve or abort it before retrying")] + RebaseInProgress, } /// Service for managing Git operations in task execution workflows @@ -97,6 +99,33 @@ impl GitService { Repository::open(repo_path).map_err(GitServiceError::from) } + /// Ensure local (repo-scoped) identity exists for CLI commits. + /// Sets user.name/email only if missing in the repo config. + fn ensure_cli_commit_identity(&self, repo_path: &Path) -> Result<(), GitServiceError> { + let repo = self.open_repo(repo_path)?; + let cfg = repo.config()?; + let has_name = cfg.get_string("user.name").is_ok(); + let has_email = cfg.get_string("user.email").is_ok(); + if !(has_name && has_email) { + let mut cfg = repo.config()?; + cfg.set_str("user.name", "Vibe Kanban")?; + cfg.set_str("user.email", "noreply@vibekanban.com")?; + } + Ok(()) + } + + /// Get a signature for libgit2 commits with a safe fallback identity. + fn signature_with_fallback<'a>( + &self, + repo: &'a Repository, + ) -> Result, GitServiceError> { + match repo.signature() { + Ok(sig) => Ok(sig), + Err(_) => git2::Signature::now("Vibe Kanban", "noreply@vibekanban.com") + .map_err(GitServiceError::from), + } + } + pub fn default_remote_name(&self, repo: &Repository) -> String { if let Ok(repos) = repo.remotes() { repos @@ -147,11 +176,7 @@ impl GitService { } pub fn create_initial_commit(&self, repo: &Repository) -> Result<(), GitServiceError> { - let signature = repo.signature().unwrap_or_else(|_| { - // Fallback if no Git config is set - git2::Signature::now("Vibe Kanban", "noreply@vibekanban.com") - .expect("Failed to create fallback signature") - }); + let signature = self.signature_with_fallback(repo)?; let tree_id = { let tree_builder = repo.treebuilder(None)?; @@ -188,6 +213,8 @@ impl GitService { git.add_all(path) .map_err(|e| GitServiceError::InvalidRepository(format!("git add failed: {e}")))?; + // Only ensure identity once we know we're about to commit + self.ensure_cli_commit_identity(path)?; git.commit(path, message) .map_err(|e| GitServiceError::InvalidRepository(format!("git commit failed: {e}")))?; Ok(true) @@ -561,28 +588,54 @@ impl GitService { base_branch_name: &str, commit_message: &str, ) -> Result { - // Open the worktree repository + // Open the repositories let worktree_repo = self.open_repo(worktree_path)?; let main_repo = self.open_repo(repo_path)?; - // Check if worktree is dirty before proceeding - self.check_worktree_clean(&worktree_repo)?; - self.check_worktree_clean(&main_repo)?; + // If main repo is currently on the base branch, perform a safe CLI + // squash merge directly in the main working tree, provided there are + // no staged changes (to avoid accidental inclusion). + if let Ok(head) = main_repo.head() + && let Some(cur) = head.shorthand() + && cur == base_branch_name + { + let git = GitCli::new(); + if git.has_staged_changes(repo_path).map_err(|e| { + GitServiceError::InvalidRepository(format!("git diff --cached failed: {e}")) + })? { + return Err(GitServiceError::WorktreeDirty( + base_branch_name.to_string(), + "staged changes present".to_string(), + )); + } + // This path updates both ref and working tree safely (git will refuse if unsafe) + // Ensure identity for the CLI commit + self.ensure_cli_commit_identity(repo_path)?; + let sha = git + .merge_squash_commit(repo_path, base_branch_name, branch_name, commit_message) + .map_err(|e| { + GitServiceError::InvalidRepository(format!("git merge --squash failed: {e}")) + })?; + // Also update task branch ref to merged commit for continuity + let task_refname = format!("refs/heads/{branch_name}"); + git.update_ref(repo_path, &task_refname, &sha) + .map_err(|e| { + GitServiceError::InvalidRepository(format!("git update-ref failed: {e}")) + })?; + return Ok(sha); + } - // Verify the task branch exists in the worktree + // Otherwise, fall back to libgit2 in-memory squash commit (no working tree changes) + // Locate branches in the shared repository (common.git across worktrees) let task_branch = Self::find_branch(&worktree_repo, branch_name)?; - - // Get the base branch from the worktree let base_branch = Self::find_branch(&worktree_repo, base_branch_name)?; - // Get commits + // Resolve commits let base_commit = base_branch.get().peel_to_commit()?; let task_commit = task_branch.get().peel_to_commit()?; - // Get the signature for the merge commit - let signature = worktree_repo.signature()?; - - // Perform a squash merge - create a single commit with all changes + // Create the squash commit in-memory (no checkout) and update the base branch ref + let signature = self.signature_with_fallback(&worktree_repo)?; let squash_commit_id = self.perform_squash_merge( &worktree_repo, &base_commit, @@ -592,30 +645,16 @@ impl GitService { base_branch_name, )?; - // Reset the task branch to point to the squash commit - // This allows follow-up work to continue from the merged state without conflicts + // Optionally update the task branch to the new squash commit so follow-up + // work can continue from the merged state without conflicts. let task_refname = format!("refs/heads/{branch_name}"); main_repo.reference( &task_refname, squash_commit_id, true, - "Reset task branch after merge in main repo", + "Reset task branch after squash merge", )?; - // Fix: Update main repo's HEAD if it's pointing to the base branch - let refname = format!("refs/heads/{base_branch_name}"); - - if let Ok(main_head) = main_repo.head() - && let Some(branch_name) = main_head.shorthand() - && branch_name == base_branch_name - { - // Only update main repo's HEAD if it's currently on the base branch - main_repo.set_head(&refname)?; - let mut co = CheckoutBuilder::new(); - co.force(); - main_repo.checkout_head(Some(&mut co))?; - } - Ok(squash_commit_id.to_string()) } fn get_branch_status_inner( @@ -762,6 +801,135 @@ impl GitService { } } + /// Get the commit OID (as hex string) for a given branch without modifying HEAD + pub fn get_branch_oid( + &self, + repo_path: &Path, + branch_name: &str, + ) -> Result { + let repo = self.open_repo(repo_path)?; + let branch = Self::find_branch(&repo, branch_name)?; + let oid = branch.get().peel_to_commit()?.id().to_string(); + Ok(oid) + } + + /// Get the author name and email for the given commit OID (hex) + pub fn get_commit_author( + &self, + repo_path: &Path, + commit_sha: &str, + ) -> Result<(Option, Option), GitServiceError> { + let repo = self.open_repo(repo_path)?; + let oid = git2::Oid::from_str(commit_sha) + .map_err(|_| GitServiceError::InvalidRepository("Invalid commit SHA".into()))?; + let commit = repo.find_commit(oid)?; + let author = commit.author(); + Ok(( + author.name().map(|s| s.to_string()), + author.email().map(|s| s.to_string()), + )) + } + + /// Convenience: Get author of HEAD commit + pub fn get_head_author( + &self, + repo_path: &Path, + ) -> Result<(Option, Option), GitServiceError> { + let head = self.get_head_info(repo_path)?; + self.get_commit_author(repo_path, &head.oid) + } + + /// Configure local user identity for committing via CLI + pub fn configure_user( + &self, + repo_path: &Path, + name: &str, + email: &str, + ) -> Result<(), GitServiceError> { + let repo = self.open_repo(repo_path)?; + let mut cfg = repo.config()?; + cfg.set_str("user.name", name)?; + cfg.set_str("user.email", email)?; + Ok(()) + } + + /// Create a local branch at the current HEAD + pub fn create_branch( + &self, + repo_path: &Path, + branch_name: &str, + ) -> Result<(), GitServiceError> { + let repo = self.open_repo(repo_path)?; + let head_commit = repo.head()?.peel_to_commit()?; + repo.branch(branch_name, &head_commit, true)?; + Ok(()) + } + + /// Checkout a local branch in the given working tree + pub fn checkout_branch( + &self, + repo_path: &Path, + branch_name: &str, + ) -> Result<(), GitServiceError> { + let repo = self.open_repo(repo_path)?; + let refname = format!("refs/heads/{branch_name}"); + repo.set_head(&refname)?; + let mut co = CheckoutBuilder::new(); + co.force(); + repo.checkout_head(Some(&mut co))?; + Ok(()) + } + + /// Add a worktree for a branch, optionally creating the branch + pub fn add_worktree( + &self, + repo_path: &Path, + worktree_path: &Path, + branch: &str, + create_branch: bool, + ) -> Result<(), GitServiceError> { + let git = GitCli::new(); + git.worktree_add(repo_path, worktree_path, branch, create_branch) + .map_err(|e| GitServiceError::InvalidRepository(e.to_string()))?; + Ok(()) + } + + /// Set or add a remote URL + pub fn set_remote( + &self, + repo_path: &Path, + name: &str, + url: &str, + ) -> Result<(), GitServiceError> { + let repo = self.open_repo(repo_path)?; + match repo.find_remote(name) { + Ok(_) => repo.remote_set_url(name, url)?, + Err(_) => { + repo.remote(name, url)?; + } + } + Ok(()) + } + + /// Stage a specific path (wrapper over git add) + pub fn add_path(&self, repo_path: &Path, path: &str) -> Result<(), GitServiceError> { + let git = GitCli::new(); + git.git(repo_path, ["add", path]) + .map(|_| ()) + .map_err(|e| GitServiceError::InvalidRepository(e.to_string())) + } + + /// Detach HEAD to the current commit (for testing commit on detached HEAD) + pub fn detach_head_current(&self, repo_path: &Path) -> Result<(), GitServiceError> { + let repo = self.open_repo(repo_path)?; + let oid = repo + .head()? + .target() + .ok_or_else(|| GitServiceError::InvalidRepository("HEAD has no target".into()))?; + repo.set_head_detached(oid)?; + Ok(()) + } + pub fn get_all_branches(&self, repo_path: &Path) -> Result, git2::Error> { let repo = Repository::open(repo_path)?; let current_branch = self.get_current_branch(repo_path).unwrap_or_default(); @@ -836,8 +1004,11 @@ impl GitService { commit_message: &str, base_branch_name: &str, ) -> Result { - // Attempt an in-memory merge to detect conflicts - let merge_opts = git2::MergeOptions::new(); + // In-memory merge to detect conflicts without touching the working tree + let mut merge_opts = git2::MergeOptions::new(); + // Safety and correctness options + merge_opts.find_renames(true); // improve rename handling + merge_opts.fail_on_conflict(true); // bail out instead of generating conflicted index let mut index = repo.merge_commits(base_commit, task_commit, Some(&merge_opts))?; // If there are conflicts, return an error @@ -885,17 +1056,11 @@ impl GitService { // resetting or cherry-picking over them. Untracked files are allowed. self.check_worktree_clean(&worktree_repo)?; - // Check if there's an existing rebase in progress and abort it - let state = worktree_repo.state(); - if state == git2::RepositoryState::Rebase - || state == git2::RepositoryState::RebaseInteractive - || state == git2::RepositoryState::RebaseMerge - { - tracing::warn!("Existing rebase in progress, aborting it first"); - // Try to abort the existing rebase - if let Ok(mut existing_rebase) = worktree_repo.open_rebase(None) { - let _ = existing_rebase.abort(); - } + // If a rebase is already in progress, refuse to proceed instead of + // aborting (which might destroy user changes mid-rebase). + let git = GitCli::new(); + if git.is_rebase_in_progress(worktree_path).unwrap_or(false) { + return Err(GitServiceError::RebaseInProgress); } // Get the target base branch reference @@ -908,69 +1073,24 @@ impl GitService { .unwrap_or_else(|| "main".to_string()), }; let nbr = Self::find_branch(&main_repo, &new_base_branch_name)?.into_reference(); - let new_base_commit_id = if nbr.is_remote() { + // If the target base is remote, update it first so CLI sees latest + if nbr.is_remote() { let github_token = github_token.ok_or(GitServiceError::TokenUnavailable)?; let remote = self.get_remote_from_branch_ref(&main_repo, &nbr)?; // First, fetch the latest changes from remote self.fetch_from_remote(&main_repo, &github_token, &remote)?; - // Try to find the remote branch after fetch - let remote_branch = Self::find_branch(&main_repo, &new_base_branch_name)?; - remote_branch.into_reference().peel_to_commit()?.id() - // Use the local branch name for rebase - } else { - nbr.peel_to_commit()?.id() - }; - - // Remember the original task-branch commit before we touch anything - let original_head_oid = worktree_repo.head()?.peel_to_commit()?.id(); - // Get the HEAD commit of the worktree (the changes to rebase) - let task_branch_commit_id = worktree_repo.head()?.peel_to_commit()?.id(); - - let signature = worktree_repo.signature()?; - let old_base_commit_id = Self::find_branch(&main_repo, old_base_branch)? - .into_reference() - .peel_to_commit()? - .id(); - - // Find commits unique to the task branch - let unique_commits = Self::find_unique_commits( - &worktree_repo, - task_branch_commit_id, - old_base_commit_id, - new_base_commit_id, - )?; - - // Attempt the rebase operation - let rebase_result = if !unique_commits.is_empty() { - // Reset HEAD to the new base branch - let new_base_commit = worktree_repo.find_commit(new_base_commit_id)?; - worktree_repo.reset(new_base_commit.as_object(), git2::ResetType::Hard, None)?; - - // Cherry-pick the unique commits - Self::cherry_pick_commits(&worktree_repo, &unique_commits, &signature) - } else { - // No unique commits to rebase, just reset to new base - let new_base_commit = worktree_repo.find_commit(new_base_commit_id)?; - worktree_repo.reset(new_base_commit.as_object(), git2::ResetType::Hard, None)?; - Ok(()) - }; - - // Handle rebase failure by restoring original state - if let Err(e) = rebase_result { - // Clean up any cherry-pick state - let _ = worktree_repo.cleanup_state(); - - // Restore original task branch state - if let Ok(orig_commit) = worktree_repo.find_commit(original_head_oid) { - let _ = worktree_repo.reset(orig_commit.as_object(), git2::ResetType::Hard, None); - } - - return Err(e); } - // Get the final commit ID after rebase - let final_commit = worktree_repo.head()?.peel_to_commit()?; + // Ensure identity for any commits produced by rebase + self.ensure_cli_commit_identity(worktree_path)?; + // Use git CLI rebase to carry out the operation safely + git.rebase_onto(worktree_path, &new_base_branch_name, old_base_branch) + .map_err(|e| { + GitServiceError::InvalidRepository(format!("git rebase --onto failed: {e}")) + })?; + // Return resulting HEAD commit + let final_commit = worktree_repo.head()?.peel_to_commit()?; Ok(final_commit.id().to_string()) } @@ -1036,7 +1156,7 @@ impl GitService { index.write()?; // Create a commit for the file deletion - let signature = repo.signature()?; + let signature = self.signature_with_fallback(&repo)?; let tree_id = index.write_tree()?; let tree = repo.find_tree(tree_id)?; diff --git a/crates/services/src/services/git_cli.rs b/crates/services/src/services/git_cli.rs index e4bbc5cf..d5652a1c 100644 --- a/crates/services/src/services/git_cli.rs +++ b/crates/services/src/services/git_cli.rs @@ -1,4 +1,25 @@ -use std::{path::Path, process::Command}; +//! Why we prefer the Git CLI here +//! +//! - Safer working-tree semantics: the `git` CLI refuses to clobber uncommitted +//! tracked changes and untracked files during checkout/merge/rebase unless you +//! explicitly force it. libgit2 does not enforce those protections by default, +//! which means callers must re‑implement a lot of safety checks to avoid data loss. +//! - Sparse‑checkout correctness: the CLI natively respects sparse‑checkout. +//! libgit2 does not yet support sparse‑checkout semantics the same way, which +//! led to incorrect diffs and staging in our workflows. +//! - Cross‑platform stability: we observed libgit2 corrupt repositories shared +//! between WSL and Windows in scenarios where the `git` CLI did not. Delegating +//! working‑tree mutations to the CLI has proven more reliable in practice. +//! +//! Given these reasons, this module centralizes destructive or working‑tree‑ +//! touching operations (rebase, merge, checkout, add/commit, etc.) through the +//! `git` CLI, while keeping libgit2 for read‑only graph queries and credentialed +//! network operations when useful. +use std::{ + ffi::{OsStr, OsString}, + path::Path, + process::Command, +}; use thiserror::Error; use utils::shell::resolve_executable_path; @@ -9,6 +30,8 @@ pub enum GitCliError { NotAvailable, #[error("git command failed: {0}")] CommandFailed(String), + #[error("rebase in progress in this worktree")] + RebaseInProgress, } #[derive(Clone, Default)] @@ -45,20 +68,6 @@ impl GitCli { Self {} } - /// Ensure `git` is available on PATH - pub fn ensure_available(&self) -> Result<(), GitCliError> { - let git = resolve_executable_path("git").ok_or(GitCliError::NotAvailable)?; - let out = Command::new(&git) - .arg("--version") - .output() - .map_err(|_| GitCliError::NotAvailable)?; - if out.status.success() { - Ok(()) - } else { - Err(GitCliError::NotAvailable) - } - } - /// Run `git -C worktree add ` (optionally creating the branch with -b) pub fn worktree_add( &self, @@ -69,31 +78,18 @@ impl GitCli { ) -> Result<(), GitCliError> { self.ensure_available()?; - let git = resolve_executable_path("git").ok_or(GitCliError::NotAvailable)?; - let mut cmd = Command::new(&git); - cmd.arg("-C").arg(repo_path); - cmd.arg("worktree").arg("add"); + let mut args: Vec = vec!["worktree".into(), "add".into()]; if create_branch { - cmd.arg("-b").arg(branch); - } - cmd.arg(worktree_path).arg(branch); - - let out = cmd - .output() - .map_err(|e| GitCliError::CommandFailed(e.to_string()))?; - if !out.status.success() { - let stderr = String::from_utf8_lossy(&out.stderr).trim().to_string(); - return Err(GitCliError::CommandFailed(stderr)); + args.push("-b".into()); + args.push(OsString::from(branch)); } + args.push(worktree_path.as_os_str().into()); + args.push(OsString::from(branch)); + self.git(repo_path, args)?; // Good practice: reapply sparse-checkout in the new worktree to ensure materialization matches // Non-fatal if it fails or not configured. - let _ = Command::new(&git) - .arg("-C") - .arg(worktree_path) - .arg("sparse-checkout") - .arg("reapply") - .output(); + let _ = self.git(worktree_path, ["sparse-checkout", "reapply"]); Ok(()) } @@ -106,59 +102,25 @@ impl GitCli { force: bool, ) -> Result<(), GitCliError> { self.ensure_available()?; - let git = resolve_executable_path("git").ok_or(GitCliError::NotAvailable)?; - let mut cmd = Command::new(&git); - cmd.arg("-C").arg(repo_path); - cmd.arg("worktree").arg("remove"); + let mut args: Vec = vec!["worktree".into(), "remove".into()]; if force { - cmd.arg("--force"); - } - cmd.arg(worktree_path); - - let out = cmd - .output() - .map_err(|e| GitCliError::CommandFailed(e.to_string()))?; - if !out.status.success() { - let stderr = String::from_utf8_lossy(&out.stderr).trim().to_string(); - return Err(GitCliError::CommandFailed(stderr)); + args.push("--force".into()); } + args.push(worktree_path.as_os_str().into()); + self.git(repo_path, args)?; Ok(()) } /// Prune stale worktree metadata pub fn worktree_prune(&self, repo_path: &Path) -> Result<(), GitCliError> { - self.ensure_available()?; - let git = resolve_executable_path("git").ok_or(GitCliError::NotAvailable)?; - let out = Command::new(&git) - .arg("-C") - .arg(repo_path) - .arg("worktree") - .arg("prune") - .output() - .map_err(|e| GitCliError::CommandFailed(e.to_string()))?; - if !out.status.success() { - let stderr = String::from_utf8_lossy(&out.stderr).trim().to_string(); - return Err(GitCliError::CommandFailed(stderr)); - } + self.git(repo_path, ["worktree", "prune"])?; Ok(()) } /// Return true if there are any changes in the working tree (staged or unstaged). pub fn has_changes(&self, worktree_path: &Path) -> Result { - self.ensure_available()?; - let git = resolve_executable_path("git").ok_or(GitCliError::NotAvailable)?; - let out = Command::new(&git) - .arg("-C") - .arg(worktree_path) - .arg("status") - .arg("--porcelain") - .output() - .map_err(|e| GitCliError::CommandFailed(e.to_string()))?; - if !out.status.success() { - let stderr = String::from_utf8_lossy(&out.stderr).trim().to_string(); - return Err(GitCliError::CommandFailed(stderr)); - } - Ok(!out.stdout.is_empty()) + let out = self.git(worktree_path, ["status", "--porcelain"])?; + Ok(!out.is_empty()) } /// Diff status vs a base branch using a temporary index (always includes untracked). @@ -169,56 +131,31 @@ impl GitCli { base_branch: &str, opts: StatusDiffOptions, ) -> Result, GitCliError> { - self.ensure_available()?; - let git = resolve_executable_path("git").ok_or(GitCliError::NotAvailable)?; - // Create a temp index file let tmp_dir = tempfile::TempDir::new() .map_err(|e| GitCliError::CommandFailed(format!("temp dir create failed: {e}")))?; let tmp_index = tmp_dir.path().join("index"); + let envs = vec![( + OsString::from("GIT_INDEX_FILE"), + tmp_index.as_os_str().to_os_string(), + )]; // Use a temp index from HEAD to accurately track renames in untracked files - let seed_out = Command::new(&git) - .env("GIT_INDEX_FILE", &tmp_index) - .arg("-C") - .arg(worktree_path) - .arg("read-tree") - .arg("HEAD") - .output() - .map_err(|e| GitCliError::CommandFailed(e.to_string()))?; - if !seed_out.status.success() { - let stderr = String::from_utf8_lossy(&seed_out.stderr).trim().to_string(); - return Err(GitCliError::CommandFailed(format!( - "git read-tree failed: {stderr}" - ))); - } + let _ = self.git_with_env(worktree_path, ["read-tree", "HEAD"], &envs)?; // Stage all in temp index - let add_out = Command::new(&git) - .env("GIT_INDEX_FILE", &tmp_index) - .arg("-C") - .arg(worktree_path) - .arg("add") - .arg("-A") - .output() - .map_err(|e| GitCliError::CommandFailed(e.to_string()))?; - if !add_out.status.success() { - let stderr = String::from_utf8_lossy(&add_out.stderr).trim().to_string(); - return Err(GitCliError::CommandFailed(stderr)); - } + let _ = self.git_with_env(worktree_path, ["add", "-A"], &envs)?; // git diff --cached - let mut cmd = Command::new(&git); - cmd.env("GIT_INDEX_FILE", &tmp_index) - .arg("-C") - .arg(worktree_path) - .arg("-c") - .arg("core.quotepath=false") - .arg("diff") - .arg("--cached") - .arg("-M") - .arg("--name-status") - .arg(base_branch); + let mut args: Vec = vec![ + "-c".into(), + "core.quotepath=false".into(), + "diff".into(), + "--cached".into(), + "-M".into(), + "--name-status".into(), + OsString::from(base_branch), + ]; if let Some(paths) = &opts.path_filter { let non_empty_paths: Vec<&str> = paths .iter() @@ -226,58 +163,25 @@ impl GitCli { .filter(|p| !p.trim().is_empty()) .collect(); if !non_empty_paths.is_empty() { - cmd.arg("--"); + args.push("--".into()); for p in non_empty_paths { - cmd.arg(p); + args.push(OsString::from(p)); } } } - let diff_out = cmd - .output() - .map_err(|e| GitCliError::CommandFailed(e.to_string()))?; - if !diff_out.status.success() { - let stderr = String::from_utf8_lossy(&diff_out.stderr).trim().to_string(); - return Err(GitCliError::CommandFailed(stderr)); - } - Ok(Self::parse_name_status(&String::from_utf8_lossy( - &diff_out.stdout, - ))) + let out = self.git_with_env(worktree_path, args, &envs)?; + Ok(Self::parse_name_status(&out)) } /// Stage all changes in the working tree (respects sparse-checkout semantics). pub fn add_all(&self, worktree_path: &Path) -> Result<(), GitCliError> { - self.ensure_available()?; - let git = resolve_executable_path("git").ok_or(GitCliError::NotAvailable)?; - let out = Command::new(&git) - .arg("-C") - .arg(worktree_path) - .arg("add") - .arg("-A") - .output() - .map_err(|e| GitCliError::CommandFailed(e.to_string()))?; - if !out.status.success() { - let stderr = String::from_utf8_lossy(&out.stderr).trim().to_string(); - return Err(GitCliError::CommandFailed(stderr)); - } + self.git(worktree_path, ["add", "-A"])?; Ok(()) } /// Commit staged changes with the given message. pub fn commit(&self, worktree_path: &Path, message: &str) -> Result<(), GitCliError> { - self.ensure_available()?; - let git = resolve_executable_path("git").ok_or(GitCliError::NotAvailable)?; - let out = Command::new(&git) - .arg("-C") - .arg(worktree_path) - .arg("commit") - .arg("-m") - .arg(message) - .output() - .map_err(|e| GitCliError::CommandFailed(e.to_string()))?; - if !out.status.success() { - let stderr = String::from_utf8_lossy(&out.stderr).trim().to_string(); - return Err(GitCliError::CommandFailed(stderr)); - } + self.git(worktree_path, ["commit", "-m", message])?; Ok(()) } @@ -326,4 +230,163 @@ impl GitCli { } out } + + /// Perform `git rebase --onto ` on the current branch in `worktree_path`. + pub fn rebase_onto( + &self, + worktree_path: &Path, + new_base: &str, + old_base: &str, + ) -> Result<(), GitCliError> { + // If a rebase is in progress, refuse to proceed. The caller can + // choose to abort or continue; we avoid destructive actions here. + if self.is_rebase_in_progress(worktree_path).unwrap_or(false) { + return Err(GitCliError::RebaseInProgress); + } + self.git(worktree_path, ["rebase", "--onto", new_base, old_base])?; + Ok(()) + } + + /// Return true if there is a rebase in progress in this worktree. + pub fn is_rebase_in_progress(&self, worktree_path: &Path) -> Result { + match self.git(worktree_path, ["rev-parse", "--verify", "REBASE_HEAD"]) { + Ok(_) => Ok(true), + Err(GitCliError::CommandFailed(_)) => Ok(false), + Err(e) => Err(e), + } + } + + /// Return true if there are staged changes (index differs from HEAD) + pub fn has_staged_changes(&self, repo_path: &Path) -> Result { + // `git diff --cached --quiet` returns exit code 1 if there are differences + let out = Command::new(resolve_executable_path("git").ok_or(GitCliError::NotAvailable)?) + .arg("-C") + .arg(repo_path) + .arg("diff") + .arg("--cached") + .arg("--quiet") + .output() + .map_err(|e| GitCliError::CommandFailed(e.to_string()))?; + match out.status.code() { + Some(0) => Ok(false), + Some(1) => Ok(true), + _ => Err(GitCliError::CommandFailed( + String::from_utf8_lossy(&out.stderr).trim().to_string(), + )), + } + } + + /// Reset index to HEAD (mixed reset). Does not modify working tree. + pub fn reset(&self, repo_path: &Path) -> Result<(), GitCliError> { + self.git(repo_path, ["reset"]).map(|_| ()) + } + + /// Checkout base branch, squash-merge from_branch, and commit with message. Returns new HEAD sha. + pub fn merge_squash_commit( + &self, + repo_path: &Path, + base_branch: &str, + from_branch: &str, + message: &str, + ) -> Result { + self.git(repo_path, ["checkout", base_branch]).map(|_| ())?; + self.git(repo_path, ["merge", "--squash", "--no-commit", from_branch]) + .map(|_| ())?; + self.git(repo_path, ["commit", "-m", message]).map(|_| ())?; + let sha = self + .git(repo_path, ["rev-parse", "HEAD"])? + .trim() + .to_string(); + Ok(sha) + } + + /// Update a ref to a specific sha in the repo. + pub fn update_ref( + &self, + repo_path: &Path, + refname: &str, + sha: &str, + ) -> Result<(), GitCliError> { + self.git(repo_path, ["update-ref", refname, sha]) + .map(|_| ()) + } +} + +// Private methods +impl GitCli { + /// Ensure `git` is available on PATH + fn ensure_available(&self) -> Result<(), GitCliError> { + let git = resolve_executable_path("git").ok_or(GitCliError::NotAvailable)?; + let out = Command::new(&git) + .arg("--version") + .output() + .map_err(|_| GitCliError::NotAvailable)?; + if out.status.success() { + Ok(()) + } else { + Err(GitCliError::NotAvailable) + } + } + + /// Run `git -C ` and return stdout on success. + /// Caller may ignore the output; errors surface via Result. + /// + /// About `OsStr`/`OsString` usage: + /// - `Command` and `Path` operate on `OsStr` to support non‑UTF‑8 paths and + /// arguments across platforms. Using `String` would force lossy conversion + /// or partial failures. This API accepts anything that implements + /// `AsRef` so typical call sites can still pass `&str` literals or + /// owned `String`s without friction. + pub fn git(&self, repo_path: &Path, args: I) -> Result + where + I: IntoIterator, + S: AsRef, + { + self.ensure_available()?; + let git = resolve_executable_path("git").ok_or(GitCliError::NotAvailable)?; + let mut cmd = Command::new(&git); + cmd.arg("-C").arg(repo_path); + for a in args { + cmd.arg(a); + } + let out = cmd + .output() + .map_err(|e| GitCliError::CommandFailed(e.to_string()))?; + if !out.status.success() { + let stderr = String::from_utf8_lossy(&out.stderr).trim().to_string(); + return Err(GitCliError::CommandFailed(stderr)); + } + Ok(String::from_utf8_lossy(&out.stdout).to_string()) + } + + /// Like `git`, but allows passing additional environment variables. + fn git_with_env( + &self, + repo_path: &Path, + args: I, + envs: &[(OsString, OsString)], + ) -> Result + where + I: IntoIterator, + S: AsRef, + { + self.ensure_available()?; + let git = resolve_executable_path("git").ok_or(GitCliError::NotAvailable)?; + let mut cmd = Command::new(&git); + cmd.arg("-C").arg(repo_path); + for (k, v) in envs { + cmd.env(k, v); + } + for a in args { + cmd.arg(a); + } + let out = cmd + .output() + .map_err(|e| GitCliError::CommandFailed(e.to_string()))?; + if !out.status.success() { + let stderr = String::from_utf8_lossy(&out.stderr).trim().to_string(); + return Err(GitCliError::CommandFailed(stderr)); + } + Ok(String::from_utf8_lossy(&out.stdout).to_string()) + } } diff --git a/crates/services/tests/git_ops_safety.rs b/crates/services/tests/git_ops_safety.rs new file mode 100644 index 00000000..e0b2b64d --- /dev/null +++ b/crates/services/tests/git_ops_safety.rs @@ -0,0 +1,967 @@ +use std::{ + fs, + io::Write, + path::{Path, PathBuf}, +}; + +use git2::{Repository, build::CheckoutBuilder}; +use services::services::git::GitService; +use services::services::git_cli::GitCli; // used only to set up sparse-checkout +use tempfile::TempDir; +// Avoid direct git CLI usage in tests; exercise GitService instead. + +fn write_file>(base: P, rel: &str, content: &str) { + let path = base.as_ref().join(rel); + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).unwrap(); + } + let mut f = fs::File::create(&path).unwrap(); + f.write_all(content.as_bytes()).unwrap(); +} + +fn commit_all(repo: &Repository, message: &str) { + let mut index = repo.index().unwrap(); + index + .add_all(["*"].iter(), git2::IndexAddOption::DEFAULT, None) + .unwrap(); + index.write().unwrap(); + let tree_id = index.write_tree().unwrap(); + let tree = repo.find_tree(tree_id).unwrap(); + let sig = repo.signature().unwrap(); + let parents: Vec = match repo.head() { + Ok(h) => vec![h.peel_to_commit().unwrap()], + Err(e) if e.code() == git2::ErrorCode::UnbornBranch => vec![], + Err(e) => panic!("failed to read HEAD: {e}"), + }; + let parent_refs: Vec<&git2::Commit> = parents.iter().collect(); + let update_ref = if repo.head().is_ok() { + Some("HEAD") + } else { + None + }; + repo.commit(update_ref, &sig, &sig, message, &tree, &parent_refs) + .unwrap(); +} + +fn checkout_branch(repo: &Repository, name: &str) { + repo.set_head(&format!("refs/heads/{name}")).unwrap(); + let mut co = CheckoutBuilder::new(); + co.force(); + repo.checkout_head(Some(&mut co)).unwrap(); +} + +fn create_branch_from_head(repo: &Repository, name: &str) { + let head = repo.head().unwrap().peel_to_commit().unwrap(); + let _ = repo.branch(name, &head, true).unwrap(); +} + +fn configure_user(repo: &Repository) { + let mut cfg = repo.config().unwrap(); + cfg.set_str("user.name", "Test User").unwrap(); + cfg.set_str("user.email", "test@example.com").unwrap(); +} + +use services::services::git::DiffTarget; + +// Non-conflicting setup used by several tests +fn setup_repo_with_worktree(root: &TempDir) -> (PathBuf, PathBuf) { + let repo_path = root.path().join("repo"); + let worktree_path = root.path().join("wt-feature"); + + let service = GitService::new(); + service + .initialize_repo_with_main_branch(&repo_path) + .expect("init repo"); + + let repo = Repository::open(&repo_path).unwrap(); + configure_user(&repo); + checkout_branch(&repo, "main"); + + write_file(&repo_path, "common.txt", "base\n"); + commit_all(&repo, "initial main commit"); + + create_branch_from_head(&repo, "old-base"); + checkout_branch(&repo, "old-base"); + write_file(&repo_path, "base.txt", "from old-base\n"); + commit_all(&repo, "old-base commit"); + + checkout_branch(&repo, "main"); + create_branch_from_head(&repo, "new-base"); + checkout_branch(&repo, "new-base"); + write_file(&repo_path, "base.txt", "from new-base\n"); + commit_all(&repo, "new-base commit"); + + checkout_branch(&repo, "old-base"); + create_branch_from_head(&repo, "feature"); + + let svc = GitService::new(); + svc.add_worktree(&repo_path, &worktree_path, "feature", false) + .expect("create worktree"); + + write_file(&worktree_path, "feat.txt", "feat change\n"); + let wt_repo = Repository::open(&worktree_path).unwrap(); + commit_all(&wt_repo, "feature commit"); + + (repo_path, worktree_path) +} + +// Conflicting setup to simulate interactive rebase interruption +fn setup_conflict_repo_with_worktree(root: &TempDir) -> (PathBuf, PathBuf) { + let repo_path = root.path().join("repo"); + let worktree_path = root.path().join("wt-feature"); + + let service = GitService::new(); + service + .initialize_repo_with_main_branch(&repo_path) + .expect("init repo"); + + let repo = Repository::open(&repo_path).unwrap(); + configure_user(&repo); + checkout_branch(&repo, "main"); + + write_file(&repo_path, "conflict.txt", "base\n"); + commit_all(&repo, "initial main commit"); + + // old-base modifies conflict.txt one way + create_branch_from_head(&repo, "old-base"); + checkout_branch(&repo, "old-base"); + write_file(&repo_path, "conflict.txt", "old-base version\n"); + commit_all(&repo, "old-base change"); + + // feature builds on old-base and modifies same lines differently + create_branch_from_head(&repo, "feature"); + + // new-base modifies in a conflicting way + checkout_branch(&repo, "main"); + create_branch_from_head(&repo, "new-base"); + checkout_branch(&repo, "new-base"); + write_file(&repo_path, "conflict.txt", "new-base version\n"); + commit_all(&repo, "new-base change"); + + // add a worktree for feature and create the conflicting commit + let svc = GitService::new(); + svc.add_worktree(&repo_path, &worktree_path, "feature", false) + .expect("create worktree"); + let wt_repo = Repository::open(&worktree_path).unwrap(); + write_file(&worktree_path, "conflict.txt", "feature version\n"); + commit_all(&wt_repo, "feature conflicting change"); + + (repo_path, worktree_path) +} + +// Setup where feature has no unique commits (feature == old-base) +fn setup_no_unique_feature_repo(root: &TempDir) -> (PathBuf, PathBuf) { + let repo_path = root.path().join("repo"); + let worktree_path = root.path().join("wt-feature"); + + let service = GitService::new(); + service + .initialize_repo_with_main_branch(&repo_path) + .expect("init repo"); + + let repo = Repository::open(&repo_path).unwrap(); + configure_user(&repo); + checkout_branch(&repo, "main"); + + write_file(&repo_path, "base.txt", "main base\n"); + commit_all(&repo, "initial main commit"); + + // Create old-base at this point + create_branch_from_head(&repo, "old-base"); + // Create new-base diverging + checkout_branch(&repo, "main"); + create_branch_from_head(&repo, "new-base"); + checkout_branch(&repo, "new-base"); + write_file(&repo_path, "advance.txt", "new base\n"); + commit_all(&repo, "advance new-base"); + + // Create feature equal to old-base (no unique commits) + checkout_branch(&repo, "old-base"); + create_branch_from_head(&repo, "feature"); + let svc = GitService::new(); + svc.add_worktree(&repo_path, &worktree_path, "feature", false) + .expect("create worktree"); + + (repo_path, worktree_path) +} + +// Simple two-way conflict between main and feature on the same file +fn setup_direct_conflict_repo(root: &TempDir) -> (PathBuf, PathBuf) { + let repo_path = root.path().join("repo"); + let worktree_path = root.path().join("wt-feature"); + + let service = GitService::new(); + service + .initialize_repo_with_main_branch(&repo_path) + .expect("init repo"); + + let repo = Repository::open(&repo_path).unwrap(); + configure_user(&repo); + checkout_branch(&repo, "main"); + + write_file(&repo_path, "conflict.txt", "base\n"); + commit_all(&repo, "initial main commit"); + + // Create feature and commit conflicting change + create_branch_from_head(&repo, "feature"); + let svc = GitService::new(); + svc.add_worktree(&repo_path, &worktree_path, "feature", false) + .expect("create worktree"); + let wt_repo = Repository::open(&worktree_path).unwrap(); + write_file(&worktree_path, "conflict.txt", "feature change\n"); + commit_all(&wt_repo, "feature change"); + + // Change main in a conflicting way + checkout_branch(&repo, "main"); + write_file(&repo_path, "conflict.txt", "main change\n"); + commit_all(&repo, "main change"); + + (repo_path, worktree_path) +} + +#[test] +fn rebase_preserves_untracked_files() { + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + + write_file(&worktree_path, "scratch/untracked.txt", "temporary note\n"); + + let service = GitService::new(); + let res = service.rebase_branch( + &repo_path, + &worktree_path, + Some("new-base"), + "old-base", + None, + ); + assert!(res.is_ok(), "rebase should succeed: {res:?}"); + + let scratch = worktree_path.join("scratch/untracked.txt"); + let content = fs::read_to_string(&scratch).expect("untracked file exists"); + assert_eq!(content, "temporary note\n"); +} + +#[test] +fn rebase_aborts_on_uncommitted_tracked_changes() { + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + + write_file(&worktree_path, "feat.txt", "feat change (edited)\n"); + + let service = GitService::new(); + let res = service.rebase_branch( + &repo_path, + &worktree_path, + Some("new-base"), + "old-base", + None, + ); + assert!(res.is_err(), "rebase should fail on dirty worktree"); + + let edited = fs::read_to_string(worktree_path.join("feat.txt")).unwrap(); + assert_eq!(edited, "feat change (edited)\n"); +} + +#[test] +fn rebase_aborts_if_untracked_would_be_overwritten_by_base() { + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + + write_file(&worktree_path, "base.txt", "my scratch note\n"); + + let service = GitService::new(); + let res = service.rebase_branch( + &repo_path, + &worktree_path, + Some("new-base"), + "old-base", + None, + ); + assert!( + res.is_err(), + "rebase should fail due to untracked overwrite risk" + ); + + let content = std::fs::read_to_string(worktree_path.join("base.txt")).unwrap(); + assert_eq!(content, "my scratch note\n"); +} + +#[test] +fn merge_does_not_overwrite_main_repo_untracked_files() { + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + + write_file(&worktree_path, "danger.txt", "tracked from feature\n"); + let wt_repo = Repository::open(&worktree_path).unwrap(); + commit_all(&wt_repo, "add danger.txt in feature"); + + write_file(&repo_path, "danger.txt", "my untracked data\n"); + let main_repo = Repository::open(&repo_path).unwrap(); + checkout_branch(&main_repo, "main"); + + let service = GitService::new(); + let res = service.merge_changes( + &repo_path, + &worktree_path, + "feature", + "main", + "squash merge", + ); + assert!( + res.is_err(), + "merge should refuse due to untracked conflict" + ); + + // Untracked file remains untouched + let content = std::fs::read_to_string(repo_path.join("danger.txt")).unwrap(); + assert_eq!(content, "my untracked data\n"); +} + +#[test] +fn merge_does_not_touch_tracked_uncommitted_changes_in_base_worktree() { + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + + // Prepare: modify a tracked file in the base worktree (main) without committing + let _main_repo = Repository::open(&repo_path).unwrap(); + // Base branch commits will be advanced by the merge operation; record before via service + let g = GitService::new(); + let before_oid = g.get_branch_oid(&repo_path, "main").unwrap(); + + // Create a tracked file that will also be added by feature branch to simulate overlap + write_file(&repo_path, "danger2.txt", "my staged change\n"); + { + // stage and then unstage to leave WT_MODIFIED? Simpler: just modify an existing tracked file + // Use common.txt which is tracked + write_file(&repo_path, "common.txt", "edited locally\n"); + } + + // Feature adds a change and is committed in worktree + write_file(&worktree_path, "danger2.txt", "feature tracked\n"); + let wt_repo = Repository::open(&worktree_path).unwrap(); + commit_all(&wt_repo, "feature adds danger2.txt"); + + // Merge via service (squash into main) should not modify files in the main worktree + let service = GitService::new(); + let res = service.merge_changes( + &repo_path, + &worktree_path, + "feature", + "main", + "squash merge", + ); + assert!( + res.is_ok(), + "merge should succeed without touching worktree" + ); + + // Confirm the local edit to tracked file remains + let content = std::fs::read_to_string(repo_path.join("common.txt")).unwrap(); + assert_eq!(content, "edited locally\n"); + + // Confirm the main branch ref advanced + let after_oid = g.get_branch_oid(&repo_path, "main").unwrap(); + assert_ne!(before_oid, after_oid, "main ref should be updated by merge"); +} + +#[test] +fn merge_refuses_with_staged_changes_on_base() { + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + let s = GitService::new(); + // ensure main is checked out + s.checkout_branch(&repo_path, "main").unwrap(); + // feature adds change and commits + write_file(&worktree_path, "m.txt", "feature\n"); + let wt_repo = Repository::open(&worktree_path).unwrap(); + commit_all(&wt_repo, "feat change"); + // main has staged change + write_file(&repo_path, "staged.txt", "staged\n"); + s.add_path(&repo_path, "staged.txt").unwrap(); + let res = s.merge_changes(&repo_path, &worktree_path, "feature", "main", "squash"); + assert!(res.is_err(), "should refuse merge due to staged changes"); + // staged file remains + let content = std::fs::read_to_string(repo_path.join("staged.txt")).unwrap(); + assert_eq!(content, "staged\n"); +} + +#[test] +fn merge_preserves_unstaged_changes_on_base() { + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + let s = GitService::new(); + s.checkout_branch(&repo_path, "main").unwrap(); + // baseline local tracked file and commit + write_file(&repo_path, "local.txt", "base\n"); + let repo = Repository::open(&repo_path).unwrap(); + commit_all(&repo, "add local"); + // modify unstaged + write_file(&repo_path, "local.txt", "local edited\n"); + // feature modifies a different file + write_file(&worktree_path, "merged.txt", "merged content\n"); + let wt_repo = Repository::open(&worktree_path).unwrap(); + commit_all(&wt_repo, "feature merged"); + let _sha = s + .merge_changes(&repo_path, &worktree_path, "feature", "main", "squash") + .unwrap(); + // local edit preserved + let loc = std::fs::read_to_string(repo_path.join("local.txt")).unwrap(); + assert_eq!(loc, "local edited\n"); + // merged file updated + let m = std::fs::read_to_string(repo_path.join("merged.txt")).unwrap(); + assert_eq!(m, "merged content\n"); +} + +#[test] +fn update_ref_does_not_destroy_feature_worktree_dirty_state() { + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + let s = GitService::new(); + // ensure main is checked out + s.checkout_branch(&repo_path, "main").unwrap(); + // feature makes an initial change and commits + write_file(&worktree_path, "f.txt", "feat\n"); + let wt_repo = Repository::open(&worktree_path).unwrap(); + commit_all(&wt_repo, "feat commit"); + // dirty change in feature worktree (uncommitted) + write_file(&worktree_path, "dirty.txt", "unstaged\n"); + // merge from feature into main (CLI path updates task ref via update-ref) + let sha = s + .merge_changes(&repo_path, &worktree_path, "feature", "main", "squash") + .unwrap(); + // uncommitted change in feature worktree preserved + let dirty = std::fs::read_to_string(worktree_path.join("dirty.txt")).unwrap(); + assert_eq!(dirty, "unstaged\n"); + // feature branch ref updated to the squash commit in main repo + let feature_oid = s.get_branch_oid(&repo_path, "feature").unwrap(); + assert_eq!(feature_oid, sha); + // and the feature worktree HEAD now points to that commit + let head = s.get_head_info(&worktree_path).unwrap(); + assert_eq!(head.branch, "feature"); + assert_eq!(head.oid, sha); +} + +#[test] +fn libgit2_merge_updates_base_ref_in_both_repos() { + // Ensure we hit the libgit2 path by NOT checking out the base branch in main repo + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + let s = GitService::new(); + + // Record current main OID from both main repo and worktree repo; they should match pre-merge + let before_main_repo = s.get_branch_oid(&repo_path, "main").unwrap(); + let before_main_wt = s.get_branch_oid(&worktree_path, "main").unwrap(); + assert_eq!(before_main_repo, before_main_wt); + + // Perform merge (squash) while main repo is NOT on base branch (libgit2 path) + let sha = s + .merge_changes(&repo_path, &worktree_path, "feature", "main", "squash") + .expect("merge should succeed via libgit2 path"); + + // Base branch ref advanced in both main and worktree repositories + let after_main_repo = s.get_branch_oid(&repo_path, "main").unwrap(); + let after_main_wt = s.get_branch_oid(&worktree_path, "main").unwrap(); + assert_eq!(after_main_repo, sha); + assert_eq!(after_main_wt, sha); +} + +#[test] +fn libgit2_merge_updates_task_ref_and_feature_head_preserves_dirty() { + // Hit libgit2 path (main repo not on base) and verify task ref + HEAD update safely + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + let s = GitService::new(); + + // Make an uncommitted change in the feature worktree to ensure it's preserved + write_file(&worktree_path, "dirty2.txt", "keep me\n"); + + // Perform merge (squash) from feature into main; this path uses libgit2 + let sha = s + .merge_changes(&repo_path, &worktree_path, "feature", "main", "squash") + .expect("merge should succeed via libgit2 path"); + + // Dirty file preserved in worktree + let dirty = std::fs::read_to_string(worktree_path.join("dirty2.txt")).unwrap(); + assert_eq!(dirty, "keep me\n"); + + // Task branch (feature) updated to squash commit in both repos + let feat_main_repo = s.get_branch_oid(&repo_path, "feature").unwrap(); + let feat_worktree = s.get_branch_oid(&worktree_path, "feature").unwrap(); + assert_eq!(feat_main_repo, sha); + assert_eq!(feat_worktree, sha); + + // Feature worktree HEAD points to the new squash commit + let head = s.get_head_info(&worktree_path).unwrap(); + assert_eq!(head.branch, "feature"); + assert_eq!(head.oid, sha); +} + +#[test] +fn rebase_refuses_to_abort_existing_rebase() { + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_conflict_repo_with_worktree(&td); + + // Start a rebase via GitService that will pause/conflict + let svc = GitService::new(); + let _ = svc + .rebase_branch( + &repo_path, + &worktree_path, + Some("new-base"), + "old-base", + None, + ) + .err() + .expect("first rebase should error and leave in-progress state"); + + // Our service should refuse to proceed and not abort the user's rebase + let service = GitService::new(); + let res = service.rebase_branch( + &repo_path, + &worktree_path, + Some("new-base"), + "old-base", + None, + ); + assert!(res.is_err(), "should error because rebase is in progress"); + // Note: We do not auto-abort; user should resolve or abort explicitly +} + +#[test] +fn rebase_fast_forwards_when_no_unique_commits() { + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_no_unique_feature_repo(&td); + let g = GitService::new(); + let before = g.get_head_info(&worktree_path).unwrap().oid; + let new_base_oid = g.get_branch_oid(&repo_path, "new-base").unwrap(); + + let _res = g + .rebase_branch( + &repo_path, + &worktree_path, + Some("new-base"), + "old-base", + None, + ) + .expect("rebase should succeed"); + let after_oid = g.get_head_info(&worktree_path).unwrap().oid; + assert_ne!(before, after_oid, "HEAD should move after rebase"); + assert_eq!(after_oid, new_base_oid, "fast-forward onto new-base"); +} + +#[test] +fn rebase_applies_multiple_commits_onto_ahead_base() { + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + let repo = Repository::open(&repo_path).unwrap(); + // Advance new-base further + checkout_branch(&repo, "new-base"); + write_file(&repo_path, "base_more.txt", "nb more\n"); + commit_all(&repo, "advance new-base more"); + + // Add another commit to feature + write_file(&worktree_path, "feat2.txt", "second change\n"); + let wt_repo = Repository::open(&worktree_path).unwrap(); + commit_all(&wt_repo, "feature second commit"); + + // Rebase feature onto new-base + let service = GitService::new(); + let _ = service + .rebase_branch( + &repo_path, + &worktree_path, + Some("new-base"), + "old-base", + None, + ) + .expect("rebase should succeed"); + + // Verify both files exist with expected content in the rebased worktree + let feat = std::fs::read_to_string(worktree_path.join("feat.txt")).unwrap(); + let feat2 = std::fs::read_to_string(worktree_path.join("feat2.txt")).unwrap(); + assert_eq!(feat, "feat change\n"); + assert_eq!(feat2, "second change\n"); +} + +#[test] +fn merge_when_base_ahead_and_feature_ahead_succeeds() { + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + let repo = Repository::open(&repo_path).unwrap(); + // Advance base (main) after feature was created + checkout_branch(&repo, "main"); + write_file(&repo_path, "base_ahead.txt", "base ahead\n"); + commit_all(&repo, "base ahead commit"); + + // Feature adds its own file (already has feat.txt from setup) and commit another + write_file(&worktree_path, "another.txt", "feature ahead\n"); + let wt_repo = Repository::open(&worktree_path).unwrap(); + commit_all(&wt_repo, "feature ahead extra"); + + let g = GitService::new(); + let before_main = g.get_branch_oid(&repo_path, "main").unwrap(); + // Merge (squash) into main + let service = GitService::new(); + let merge_sha = service + .merge_changes( + &repo_path, + &worktree_path, + "feature", + "main", + "squash merge", + ) + .expect("merge should succeed"); + + let after_main = g.get_branch_oid(&repo_path, "main").unwrap(); + assert_ne!(before_main, after_main, "main should advance"); + assert_eq!(after_main, merge_sha); + + // Verify squash commit introduced feature files via commit diff + let diffs = g + .get_diffs( + DiffTarget::Commit { + repo_path: Path::new(&repo_path), + commit_sha: &after_main, + }, + None, + ) + .unwrap(); + let has_feat = diffs.iter().any(|d| { + d.new_path.as_deref() == Some("feat.txt") + && d.new_content.as_deref() == Some("feat change\n") + }); + let has_another = diffs.iter().any(|d| { + d.new_path.as_deref() == Some("another.txt") + && d.new_content.as_deref() == Some("feature ahead\n") + }); + assert!(has_feat && has_another); +} + +#[test] +fn merge_conflict_does_not_move_base_ref() { + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_direct_conflict_repo(&td); + + // Record main ref before + let _repo = Repository::open(&repo_path).unwrap(); + let g = GitService::new(); + let before = g.get_branch_oid(&repo_path, "main").unwrap(); + + let service = GitService::new(); + let res = service.merge_changes( + &repo_path, + &worktree_path, + "feature", + "main", + "squash merge", + ); + + assert!(res.is_err(), "conflicting merge should fail"); + + let after = g.get_branch_oid(&repo_path, "main").unwrap(); + assert_eq!(before, after, "main ref must remain unchanged on conflict"); +} + +#[test] +fn merge_delete_vs_modify_conflict_behaves_safely() { + // main modifies file, feature deletes it -> conflict + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + let repo = Repository::open(&repo_path).unwrap(); + + // start from main with a file + checkout_branch(&repo, "main"); + write_file(&repo_path, "conflict_dm.txt", "base\n"); + commit_all(&repo, "add conflict file"); + let g = GitService::new(); + let before = g.get_branch_oid(&repo_path, "main").unwrap(); + + // feature deletes it and commits + let wt_repo = Repository::open(&worktree_path).unwrap(); + let path = worktree_path.join("conflict_dm.txt"); + if path.exists() { + std::fs::remove_file(&path).unwrap(); + } + commit_all(&wt_repo, "delete in feature"); + + // main modifies same file + write_file(&repo_path, "conflict_dm.txt", "main modify\n"); + commit_all(&repo, "modify in main"); + + let service = GitService::new(); + let res = service.merge_changes( + &repo_path, + &worktree_path, + "feature", + "main", + "squash merge", + ); + match res { + Err(_) => { + // On failure, ensure base ref unchanged + let after = g.get_branch_oid(&repo_path, "main").unwrap(); + assert_eq!(before, after, "main ref must remain unchanged on failure"); + } + Ok(merge_sha) => { + // On success, verify the resulting commit exists and the working tree was not touched + let after_oid = g.get_branch_oid(&repo_path, "main").unwrap(); + assert_eq!(after_oid, merge_sha); + // File either preserved (modify wins) or deleted (delete wins); both are acceptable, but no crash + let path = repo_path.join("conflict_dm.txt"); + if path.exists() { + let content = std::fs::read_to_string(&path).unwrap(); + assert_eq!(content, "main modify\n"); + } + } + } +} + +#[test] +fn rebase_preserves_rename_changes() { + // feature renames a file; rebase onto new-base preserves rename + let td = TempDir::new().unwrap(); + let (repo_path, worktree_path) = setup_repo_with_worktree(&td); + + // feature: rename feat.txt -> feat_renamed.txt + std::fs::rename( + worktree_path.join("feat.txt"), + worktree_path.join("feat_renamed.txt"), + ) + .unwrap(); + let wt_repo = Repository::open(&worktree_path).unwrap(); + commit_all(&wt_repo, "rename feat"); + + // rebase onto new-base + let service = GitService::new(); + let _ = service + .rebase_branch( + &repo_path, + &worktree_path, + Some("new-base"), + "old-base", + None, + ) + .expect("rebase should succeed"); + + // after rebase, renamed file present; original absent + assert!(worktree_path.join("feat_renamed.txt").exists()); + assert!(!worktree_path.join("feat.txt").exists()); +} + +#[test] +fn merge_refreshes_main_worktree_when_on_base() { + let td = TempDir::new().unwrap(); + // Initialize repo and ensure main is checked out + let repo_path = td.path().join("repo_refresh"); + let s = GitService::new(); + s.initialize_repo_with_main_branch(&repo_path).unwrap(); + s.configure_user(&repo_path, "Test User", "test@example.com") + .unwrap(); + s.checkout_branch(&repo_path, "main").unwrap(); + // Baseline file + write_file(&repo_path, "file.txt", "base\n"); + let _ = s.commit(&repo_path, "add base").unwrap(); + + // Create feature branch and worktree + s.create_branch(&repo_path, "feature").unwrap(); + let wt = td.path().join("wt_refresh"); + s.add_worktree(&repo_path, &wt, "feature", false).unwrap(); + // Modify file in worktree and commit + write_file(&wt, "file.txt", "feature change\n"); + let _ = s.commit(&wt, "feature change").unwrap(); + + // Merge into main (squash) and ensure main worktree is updated since it is on base + let merge_sha = s + .merge_changes(&repo_path, &wt, "feature", "main", "squash") + .unwrap(); + // Since main is on base branch and we use safe CLI merge, both working tree + // and ref should reflect the merged content. + let content = std::fs::read_to_string(repo_path.join("file.txt")).unwrap(); + assert_eq!(content, "feature change\n"); + let oid = s.get_branch_oid(&repo_path, "main").unwrap(); + assert_eq!(oid, merge_sha); +} + +#[test] +fn sparse_checkout_respected_in_worktree_diffs_and_commit() { + let td = TempDir::new().unwrap(); + let repo_path = td.path().join("repo_sparse"); + let s = GitService::new(); + s.initialize_repo_with_main_branch(&repo_path).unwrap(); + s.configure_user(&repo_path, "Test User", "test@example.com") + .unwrap(); + s.checkout_branch(&repo_path, "main").unwrap(); + // baseline content + write_file(&repo_path, "included/a.txt", "A\n"); + write_file(&repo_path, "excluded/b.txt", "B\n"); + let _ = s.commit(&repo_path, "baseline").unwrap(); + + // enable sparse-checkout for 'included' only + let cli = GitCli::new(); + cli.git(&repo_path, ["sparse-checkout", "init", "--cone"]) + .unwrap(); + cli.git(&repo_path, ["sparse-checkout", "set", "included"]) + .unwrap(); + + // create feature branch and worktree + s.create_branch(&repo_path, "feature").unwrap(); + let wt = td.path().join("wt_sparse"); + s.add_worktree(&repo_path, &wt, "feature", false).unwrap(); + + // materialization check: included exists, excluded does not + assert!(wt.join("included/a.txt").exists()); + assert!(!wt.join("excluded/b.txt").exists()); + + // modify included file + write_file(&wt, "included/a.txt", "A-mod\n"); + // get worktree diffs vs main, ensure excluded/b.txt is NOT reported deleted + let diffs = s + .get_diffs( + DiffTarget::Worktree { + worktree_path: Path::new(&wt), + branch_name: "feature", + base_branch: "main", + }, + None, + ) + .unwrap(); + assert!( + diffs + .iter() + .any(|d| d.new_path.as_deref() == Some("included/a.txt")) + ); + assert!( + !diffs + .iter() + .any(|d| d.old_path.as_deref() == Some("excluded/b.txt") + || d.new_path.as_deref() == Some("excluded/b.txt")) + ); + + // commit and verify commit diffs also only include included/ changes + let _ = s.commit(&wt, "modify included").unwrap(); + let head_sha = s.get_head_info(&wt).unwrap().oid; + let commit_diffs = s + .get_diffs( + DiffTarget::Commit { + repo_path: Path::new(&wt), + commit_sha: &head_sha, + }, + None, + ) + .unwrap(); + assert!( + commit_diffs + .iter() + .any(|d| d.new_path.as_deref() == Some("included/a.txt")) + ); + assert!( + commit_diffs + .iter() + .all(|d| d.new_path.as_deref() != Some("excluded/b.txt") + && d.old_path.as_deref() != Some("excluded/b.txt")) + ); +} + +// Helper: initialize a repo with main, configure user via service +fn init_repo_only_service(root: &TempDir) -> PathBuf { + let repo_path = root.path().join("repo_svc"); + let s = GitService::new(); + s.initialize_repo_with_main_branch(&repo_path).unwrap(); + s.configure_user(&repo_path, "Test User", "test@example.com") + .unwrap(); + s.checkout_branch(&repo_path, "main").unwrap(); + repo_path +} + +#[test] +fn merge_binary_conflict_does_not_move_ref() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_only_service(&td); + let s = GitService::new(); + // seed + let _ = s.commit(&repo_path, "seed").unwrap(); + // create feature branch and worktree + s.create_branch(&repo_path, "feature").unwrap(); + let worktree_path = td.path().join("wt_bin"); + s.add_worktree(&repo_path, &worktree_path, "feature", false) + .unwrap(); + + // feature adds/commits binary file + let mut f = fs::File::create(worktree_path.join("bin.dat")).unwrap(); + f.write_all(&[0, 1, 2, 3]).unwrap(); + let _ = s.commit(&worktree_path, "feature bin").unwrap(); + + // main adds conflicting binary content + let mut f2 = fs::File::create(repo_path.join("bin.dat")).unwrap(); + f2.write_all(&[9, 8, 7, 6]).unwrap(); + let _ = s.commit(&repo_path, "main bin").unwrap(); + + let before = s.get_branch_oid(&repo_path, "main").unwrap(); + let res = s.merge_changes(&repo_path, &worktree_path, "feature", "main", "merge bin"); + assert!(res.is_err(), "binary conflict should fail"); + let after = s.get_branch_oid(&repo_path, "main").unwrap(); + assert_eq!(before, after, "main ref unchanged on conflict"); +} + +#[test] +fn merge_rename_vs_modify_conflict_does_not_move_ref() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_only_service(&td); + let s = GitService::new(); + // base file + fs::write(repo_path.join("conflict.txt"), b"base\n").unwrap(); + let _ = s.commit(&repo_path, "base").unwrap(); + s.create_branch(&repo_path, "feature").unwrap(); + let worktree_path = td.path().join("wt_ren"); + s.add_worktree(&repo_path, &worktree_path, "feature", false) + .unwrap(); + + // feature renames file + std::fs::rename( + worktree_path.join("conflict.txt"), + worktree_path.join("conflict_renamed.txt"), + ) + .unwrap(); + let _ = s.commit(&worktree_path, "rename").unwrap(); + + // main modifies original path + fs::write(repo_path.join("conflict.txt"), b"main change\n").unwrap(); + let _ = s.commit(&repo_path, "modify main").unwrap(); + + let before = s.get_branch_oid(&repo_path, "main").unwrap(); + let res = s.merge_changes( + &repo_path, + &worktree_path, + "feature", + "main", + "merge rename", + ); + match res { + Err(_) => { + let after = s.get_branch_oid(&repo_path, "main").unwrap(); + assert_eq!(before, after, "main unchanged on conflict"); + } + Ok(sha) => { + // ensure main advanced and result contains either renamed or modified content + let after = s.get_branch_oid(&repo_path, "main").unwrap(); + assert_eq!(after, sha); + let diffs = s + .get_diffs( + DiffTarget::Commit { + repo_path: Path::new(&repo_path), + commit_sha: &after, + }, + None, + ) + .unwrap(); + let has_renamed = diffs + .iter() + .any(|d| d.new_path.as_deref() == Some("conflict_renamed.txt")); + let has_modified = diffs.iter().any(|d| { + d.new_path.as_deref() == Some("conflict.txt") + && d.new_content.as_deref() == Some("main change\n") + }); + assert!(has_renamed || has_modified); + } + } +} diff --git a/crates/services/tests/git_workflow.rs b/crates/services/tests/git_workflow.rs new file mode 100644 index 00000000..bb3a00ab --- /dev/null +++ b/crates/services/tests/git_workflow.rs @@ -0,0 +1,596 @@ +use std::{ + fs, + io::Write, + path::{Path, PathBuf}, +}; + +use services::services::git::{DiffTarget, GitService}; +use tempfile::TempDir; +use utils::diff::DiffChangeKind; + +fn write_file>(base: P, rel: &str, content: &str) { + let path = base.as_ref().join(rel); + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).unwrap(); + } + let mut f = fs::File::create(&path).unwrap(); + f.write_all(content.as_bytes()).unwrap(); +} + +fn init_repo_main(root: &TempDir) -> PathBuf { + let path = root.path().join("repo"); + let s = GitService::new(); + s.initialize_repo_with_main_branch(&path).unwrap(); + s.configure_user(&path, "Test User", "test@example.com") + .unwrap(); + s.checkout_branch(&path, "main").unwrap(); + path +} + +#[test] +fn commit_empty_message_behaviour() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + write_file(&repo_path, "x.txt", "x\n"); + let s = GitService::new(); + let res = s.commit(&repo_path, ""); + // Some environments disallow empty commit messages by default. + // Accept either success or a clear error. + if let Err(e) = &res { + let msg = format!("{e}"); + assert!(msg.contains("empty commit message") || msg.contains("git commit failed")); + } +} + +fn has_global_git_identity() -> bool { + if let Ok(cfg) = git2::Config::open_default() { + let has_name = cfg.get_string("user.name").is_ok(); + let has_email = cfg.get_string("user.email").is_ok(); + return has_name && has_email; + } + false +} + +#[test] +fn initialize_repo_without_user_creates_initial_commit() { + let td = TempDir::new().unwrap(); + let repo_path = td.path().join("repo_no_user_init"); + let s = GitService::new(); + // No configure_user call; rely on fallback signature for initial commit + s.initialize_repo_with_main_branch(&repo_path).unwrap(); + let head = s.get_head_info(&repo_path).unwrap(); + assert_eq!(head.branch, "main"); + assert!(!head.oid.is_empty()); + // Verify author is set: either global identity (if configured) or fallback + let (name, email) = s.get_head_author(&repo_path).unwrap(); + if has_global_git_identity() { + assert!(name.is_some() && email.is_some()); + } else { + assert_eq!(name.as_deref(), Some("Vibe Kanban")); + assert_eq!(email.as_deref(), Some("noreply@vibekanban.com")); + } +} + +#[test] +fn commit_without_user_config_succeeds() { + let td = TempDir::new().unwrap(); + let repo_path = td.path().join("repo_no_user"); + let s = GitService::new(); + s.initialize_repo_with_main_branch(&repo_path).unwrap(); + write_file(&repo_path, "f.txt", "x\n"); + // No configure_user call here + let res = s.commit(&repo_path, "no user config"); + assert!(res.is_ok()); +} + +#[test] +fn commit_fails_when_index_locked() { + use std::fs::File; + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + write_file(&repo_path, "y.txt", "y\n"); + // Simulate index lock + let git_dir = repo_path.join(".git"); + let _lock = File::create(git_dir.join("index.lock")).unwrap(); + let s = GitService::new(); + let res = s.commit(&repo_path, "should fail"); + assert!(res.is_err()); +} + +#[test] +fn staged_but_uncommitted_changes_is_dirty() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + let s = GitService::new(); + // seed tracked file + write_file(&repo_path, "t1.txt", "a\n"); + let _ = s.commit(&repo_path, "seed").unwrap(); + // modify and stage + write_file(&repo_path, "t1.txt", "b\n"); + s.add_path(&repo_path, "t1.txt").unwrap(); + assert!(!s.is_worktree_clean(&repo_path).unwrap()); +} + +#[test] +fn delete_nonexistent_file_creates_noop_commit() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + // baseline commit first so we have HEAD + write_file(&repo_path, "seed.txt", "s\n"); + let s = GitService::new(); + let _ = s.commit(&repo_path, "seed").unwrap(); + let before = s.get_head_info(&repo_path).unwrap().oid; + let res = s.delete_file_and_commit(&repo_path, "nope.txt").unwrap(); + let after = s.get_head_info(&repo_path).unwrap().oid; + assert_ne!(before, after); + assert_eq!(after, res); +} + +#[test] +fn delete_directory_path_errors() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + // create and commit a file so repo has history + write_file(&repo_path, "dir/file.txt", "z\n"); + let s = GitService::new(); + let _ = s.commit(&repo_path, "add file").unwrap(); + // directory path should cause an error + let s = GitService::new(); + let res = s.delete_file_and_commit(&repo_path, "dir"); + assert!(res.is_err()); +} + +#[test] +fn worktree_clean_detects_staged_deleted_and_renamed() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + write_file(&repo_path, "t1.txt", "1\n"); + write_file(&repo_path, "t2.txt", "2\n"); + let s = GitService::new(); + let _ = s.commit(&repo_path, "seed").unwrap(); + + // delete tracked file + std::fs::remove_file(repo_path.join("t2.txt")).unwrap(); + assert!(!s.is_worktree_clean(&repo_path).unwrap()); + + // restore and test rename + write_file(&repo_path, "t2.txt", "2\n"); + let _ = s.commit(&repo_path, "restore t2").unwrap(); + std::fs::rename(repo_path.join("t2.txt"), repo_path.join("t2-renamed.txt")).unwrap(); + assert!(!s.is_worktree_clean(&repo_path).unwrap()); +} + +#[test] +fn diff_added_binary_file_has_no_content() { + // ensure binary file content is not loaded (null byte guard) + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + // base + let s = GitService::new(); + let _ = s.commit(&repo_path, "base").unwrap(); + // branch with binary file + s.create_branch(&repo_path, "feature").unwrap(); + s.checkout_branch(&repo_path, "feature").unwrap(); + // write binary with null byte + let mut f = fs::File::create(repo_path.join("bin.dat")).unwrap(); + f.write_all(&[0u8, 1, 2, 3]).unwrap(); + let _ = s.commit(&repo_path, "add binary").unwrap(); + + let s = GitService::new(); + let diffs = s + .get_diffs( + DiffTarget::Branch { + repo_path: Path::new(&repo_path), + branch_name: "feature", + base_branch: "main", + }, + None, + ) + .unwrap(); + let bin = diffs + .iter() + .find(|d| d.new_path.as_deref() == Some("bin.dat")) + .expect("binary diff present"); + assert!(bin.new_content.is_none()); +} + +#[test] +fn initialize_and_default_branch_and_head_info() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + + // Default branch should be main + let s = GitService::new(); + let def = s.get_default_branch_name(&repo_path).unwrap(); + assert_eq!(def, "main"); + + // Head info branch should be main + let head = s.get_head_info(&repo_path).unwrap(); + assert_eq!(head.branch, "main"); + + // Repo has an initial commit (OID parsable) + assert!(!head.oid.is_empty()); +} + +#[test] +fn commit_and_is_worktree_clean() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + write_file(&repo_path, "foo.txt", "hello\n"); + + let s = GitService::new(); + let committed = s.commit(&repo_path, "add foo").unwrap(); + assert!(committed); + assert!(s.is_worktree_clean(&repo_path).unwrap()); + + // Verify commit contains file + let diffs = s + .get_diffs( + DiffTarget::Commit { + repo_path: Path::new(&repo_path), + commit_sha: &s.get_head_info(&repo_path).unwrap().oid, + }, + None, + ) + .unwrap(); + assert!( + diffs + .iter() + .any(|d| d.new_path.as_deref() == Some("foo.txt")) + ); +} + +#[test] +fn commit_in_detached_head_succeeds_via_service() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + // initial parent + write_file(&repo_path, "a.txt", "a\n"); + let s = GitService::new(); + let _ = s.commit(&repo_path, "add a").unwrap(); + // detach via service + s.detach_head_current(&repo_path).unwrap(); + // commit while detached + write_file(&repo_path, "b.txt", "b\n"); + let ok = s.commit(&repo_path, "detached commit").unwrap(); + assert!(ok); +} + +#[test] +fn branch_status_ahead_and_behind() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + let s = GitService::new(); + + // main: initial commit + write_file(&repo_path, "base.txt", "base\n"); + let _ = s.commit(&repo_path, "base").unwrap(); + + // create feature from main + s.create_branch(&repo_path, "feature").unwrap(); + // advance feature by 1 + s.checkout_branch(&repo_path, "feature").unwrap(); + write_file(&repo_path, "feature.txt", "f1\n"); + let _ = s.commit(&repo_path, "f1").unwrap(); + + // advance main by 1 + s.checkout_branch(&repo_path, "main").unwrap(); + write_file(&repo_path, "main.txt", "m1\n"); + let _ = s.commit(&repo_path, "m1").unwrap(); + + let s = GitService::new(); + let (ahead, behind) = s.get_branch_status(&repo_path, "feature", "main").unwrap(); + assert_eq!((ahead, behind), (1, 1)); + + // advance feature by one more (ahead 2, behind 1) + s.checkout_branch(&repo_path, "feature").unwrap(); + write_file(&repo_path, "feature2.txt", "f2\n"); + let _ = s.commit(&repo_path, "f2").unwrap(); + let (ahead2, behind2) = s.get_branch_status(&repo_path, "feature", "main").unwrap(); + assert_eq!((ahead2, behind2), (2, 1)); +} + +#[test] +fn get_all_branches_lists_current_and_others() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + let s = GitService::new(); + s.create_branch(&repo_path, "feature").unwrap(); + + let s = GitService::new(); + let branches = s.get_all_branches(&repo_path).unwrap(); + let names: Vec<_> = branches.iter().map(|b| b.name.as_str()).collect(); + assert!(names.contains(&"main")); + assert!(names.contains(&"feature")); + // current should be main + let main_entry = branches.iter().find(|b| b.name == "main").unwrap(); + assert!(main_entry.is_current); +} + +#[test] +fn delete_file_and_commit_creates_new_commit() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + write_file(&repo_path, "to_delete.txt", "bye\n"); + let s = GitService::new(); + let _ = s.commit(&repo_path, "add to_delete").unwrap(); + let before = s.get_head_info(&repo_path).unwrap().oid; + + let new_commit = s + .delete_file_and_commit(&repo_path, "to_delete.txt") + .unwrap(); + let after = s.get_head_info(&repo_path).unwrap().oid; + assert_ne!(before, after); + assert_eq!(after, new_commit); + assert!(!repo_path.join("to_delete.txt").exists()); +} + +#[test] +fn get_github_repo_info_parses_origin() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + let s = GitService::new(); + s.set_remote(&repo_path, "origin", "https://github.com/foo/bar.git") + .unwrap(); + let info = s.get_github_repo_info(&repo_path).unwrap(); + assert_eq!(info.owner, "foo"); + assert_eq!(info.repo_name, "bar"); +} + +#[test] +fn get_branch_diffs_between_branches() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + let s = GitService::new(); + // base commit on main + write_file(&repo_path, "a.txt", "a\n"); + let _ = s.commit(&repo_path, "add a").unwrap(); + + // create branch and add new file + s.create_branch(&repo_path, "feature").unwrap(); + s.checkout_branch(&repo_path, "feature").unwrap(); + write_file(&repo_path, "b.txt", "b\n"); + let _ = s.commit(&repo_path, "add b").unwrap(); + + let s = GitService::new(); + let diffs = s + .get_diffs( + DiffTarget::Branch { + repo_path: Path::new(&repo_path), + branch_name: "feature", + base_branch: "main", + }, + None, + ) + .unwrap(); + assert!(diffs.iter().any(|d| d.new_path.as_deref() == Some("b.txt"))); +} + +#[test] +fn worktree_diff_respects_path_filter() { + // Use git CLI status diff under the hood + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + + // main baseline + write_file(&repo_path, "src/keep.txt", "k\n"); + write_file(&repo_path, "other/skip.txt", "s\n"); + let s = GitService::new(); + let _ = s.commit(&repo_path, "baseline").unwrap(); + + // create feature and work in place (worktree is repo_path) + s.create_branch(&repo_path, "feature").unwrap(); + + // modify files without committing + write_file(&repo_path, "src/only.txt", "only\n"); + write_file(&repo_path, "other/skip2.txt", "skip\n"); + + let s = GitService::new(); + let diffs = s + .get_diffs( + DiffTarget::Worktree { + worktree_path: Path::new(&repo_path), + branch_name: "feature", + base_branch: "main", + }, + Some(&["src"]), + ) + .unwrap(); + assert!( + diffs + .iter() + .any(|d| d.new_path.as_deref() == Some("src/only.txt")) + ); + assert!( + !diffs + .iter() + .any(|d| d.new_path.as_deref() == Some("other/skip2.txt")) + ); +} + +#[test] +fn get_branch_oid_nonexistent_errors() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + let s = GitService::new(); + let res = s.get_branch_oid(&repo_path, "no-such-branch"); + assert!(res.is_err()); +} + +#[test] +fn create_unicode_branch_and_list() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + let s = GitService::new(); + // base commit + write_file(&repo_path, "file.txt", "ok\n"); + let _ = s.commit(&repo_path, "base"); + // unicode/slash branch name (valid ref) + let bname = "feature/ünicode"; + s.create_branch(&repo_path, bname).unwrap(); + let names: Vec<_> = s + .get_all_branches(&repo_path) + .unwrap() + .into_iter() + .map(|b| b.name) + .collect(); + assert!(names.iter().any(|n| n == bname)); +} + +#[cfg(unix)] +#[test] +fn worktree_diff_permission_only_change() { + use std::os::unix::fs::PermissionsExt; + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + let s = GitService::new(); + // baseline commit + write_file(&repo_path, "p.sh", "echo hi\n"); + let _ = s.commit(&repo_path, "add p.sh").unwrap(); + // create a feature branch baseline at HEAD + s.create_branch(&repo_path, "feature").unwrap(); + + // change only the permission (chmod +x) + let mut perms = std::fs::metadata(repo_path.join("p.sh")) + .unwrap() + .permissions(); + perms.set_mode(perms.mode() | 0o111); + std::fs::set_permissions(repo_path.join("p.sh"), perms).unwrap(); + + // Compute worktree diff vs main on feature + let diffs = s + .get_diffs( + DiffTarget::Worktree { + worktree_path: Path::new(&repo_path), + branch_name: "feature", + base_branch: "main", + }, + None, + ) + .unwrap(); + let d = diffs + .into_iter() + .find(|d| d.new_path.as_deref() == Some("p.sh")) + .expect("p.sh diff present"); + assert!(matches!(d.change, DiffChangeKind::PermissionChange)); + assert_eq!(d.old_content, d.new_content); +} + +#[test] +fn delete_with_uncommitted_changes_succeeds() { + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + let s = GitService::new(); + // baseline file and commit + write_file(&repo_path, "d.txt", "v1\n"); + let _ = s.commit(&repo_path, "add d").unwrap(); + let before = s.get_head_info(&repo_path).unwrap().oid; + // uncommitted change + write_file(&repo_path, "d.txt", "v2\n"); + // delete and commit + let new_sha = s.delete_file_and_commit(&repo_path, "d.txt").unwrap(); + assert_eq!(s.get_head_info(&repo_path).unwrap().oid, new_sha); + assert!(!repo_path.join("d.txt").exists()); + assert_ne!(before, new_sha); +} + +#[cfg(unix)] +#[test] +fn delete_symlink_and_commit() { + use std::os::unix::fs::symlink; + let td = TempDir::new().unwrap(); + let repo_path = init_repo_main(&td); + let s = GitService::new(); + // Create target and symlink, commit + write_file(&repo_path, "target.txt", "t\n"); + let _ = s.commit(&repo_path, "add target").unwrap(); + symlink(repo_path.join("target.txt"), repo_path.join("link.txt")).unwrap(); + let _ = s.commit(&repo_path, "add symlink").unwrap(); + let before = s.get_head_info(&repo_path).unwrap().oid; + // Delete symlink + let new_sha = s.delete_file_and_commit(&repo_path, "link.txt").unwrap(); + assert_eq!(s.get_head_info(&repo_path).unwrap().oid, new_sha); + assert!(!repo_path.join("link.txt").exists()); + assert_ne!(before, new_sha); +} + +#[test] +fn delete_file_commit_has_author_without_user() { + // Verify libgit2 path uses fallback author when no config exists + let td = TempDir::new().unwrap(); + let repo_path = td.path().join("repo_fallback_delete"); + let s = GitService::new(); + // No configure_user call; initial commit uses fallback signature too + s.initialize_repo_with_main_branch(&repo_path).unwrap(); + + // Create then delete an untracked file via service + write_file(&repo_path, "q.txt", "temp\n"); + let sha = s.delete_file_and_commit(&repo_path, "q.txt").unwrap(); + + // Author should be present: either global identity or fallback + let (name, email) = s.get_commit_author(&repo_path, &sha).unwrap(); + if has_global_git_identity() { + assert!(name.is_some() && email.is_some()); + } else { + assert_eq!(name.as_deref(), Some("Vibe Kanban")); + assert_eq!(email.as_deref(), Some("noreply@vibekanban.com")); + } +} + +#[test] +fn squash_merge_libgit2_sets_author_without_user() { + // Verify merge_changes (libgit2 path) uses fallback author when no config exists + use git2::Repository; + + let td = TempDir::new().unwrap(); + let repo_path = td.path().join("repo_fallback_merge"); + let worktree_path = td.path().join("wt_feature"); + let s = GitService::new(); + + // Init repo without user config + s.initialize_repo_with_main_branch(&repo_path).unwrap(); + + // Create feature branch and worktree + s.create_branch(&repo_path, "feature").unwrap(); + s.add_worktree(&repo_path, &worktree_path, "feature", false) + .unwrap(); + + // Make a feature commit in the worktree via libgit2 using an explicit signature + write_file(&worktree_path, "f.txt", "feat\n"); + { + let repo = Repository::open(&worktree_path).unwrap(); + // stage all + let mut index = repo.index().unwrap(); + index + .add_all(["*"].iter(), git2::IndexAddOption::DEFAULT, None) + .unwrap(); + index.write().unwrap(); + let tree_id = index.write_tree().unwrap(); + let tree = repo.find_tree(tree_id).unwrap(); + let sig = git2::Signature::now("Other Author", "other@example.com").unwrap(); + let parent = repo.head().unwrap().peel_to_commit().unwrap(); + let _cid = repo + .commit(Some("HEAD"), &sig, &sig, "feat", &tree, &[&parent]) + .unwrap(); + } + + // Ensure main repo is NOT on base branch so merge_changes takes libgit2 path + s.create_branch(&repo_path, "dev").unwrap(); + s.checkout_branch(&repo_path, "dev").unwrap(); + + // Merge feature -> main (libgit2 squash) + let merge_sha = s + .merge_changes(&repo_path, &worktree_path, "feature", "main", "squash") + .unwrap(); + + // The squash commit author should not be the feature commit's author, and must be present. + let (name, email) = s.get_commit_author(&repo_path, &merge_sha).unwrap(); + assert_ne!(name.as_deref(), Some("Other Author")); + assert_ne!(email.as_deref(), Some("other@example.com")); + if has_global_git_identity() { + assert!(name.is_some() && email.is_some()); + } else { + assert_eq!(name.as_deref(), Some("Vibe Kanban")); + assert_eq!(email.as_deref(), Some("noreply@vibekanban.com")); + } +}