diff --git a/src-tauri/config.example.toml b/src-tauri/config.example.toml index 42e5d3d..73424c5 100644 --- a/src-tauri/config.example.toml +++ b/src-tauri/config.example.toml @@ -20,9 +20,6 @@ uiTextScale = 1.0 # Whether to wrap long lines in diff views. wrapDiffLines = false -# Experimental: show the commit graph toolbar button. -showCommitGraphButton = false - # Experimental: enable Local Copy in the Clone window and CLI. enableLocalCopy = false diff --git a/src-tauri/src/ai/commands.rs b/src-tauri/src/ai/commands.rs index cab520f..4dd6ed6 100644 --- a/src-tauri/src/ai/commands.rs +++ b/src-tauri/src/ai/commands.rs @@ -1915,7 +1915,7 @@ fn build_commit_context( if repository_operation(repo_path)? != workflow.expected_repository_operation() { return Err(AiError::new("operationInProgress")); } - validate_commit_control(existing_message, 4096)?; + validate_existing_commit_message(existing_message)?; let paths = staged_paths(repo_path)?; if paths.is_empty() { return Err(AiError::new("noStagedChanges")); @@ -2466,6 +2466,17 @@ fn validate_commit_control(value: &str, maximum_length: usize) -> Result<(), AiE Ok(()) } +fn validate_existing_commit_message(message: &str) -> Result<(), AiError> { + if message.len() > 4096 + || message + .chars() + .any(|character| character.is_control() && !matches!(character, '\t' | '\n' | '\r')) + { + return Err(AiError::new("invalidCommitControl")); + } + Ok(()) +} + fn commit_system_prompt( configuration: &EffectiveAiConfiguration, request: &GenerateAiCommitMessagesRequest, @@ -2475,7 +2486,7 @@ fn commit_system_prompt( validate_commit_control(&request.language, 64)?; validate_commit_control(&request.issue_key, 64)?; validate_commit_control(&request.additional_instruction, 1000)?; - validate_commit_control(&request.existing_message, 4096)?; + validate_existing_commit_message(&request.existing_message)?; let mut prompt = configuration.commit_message_prompt.clone(); if request.subject_limit > 0 { prompt.push_str(&format!( @@ -4538,6 +4549,64 @@ mod tests { assert!(validate_commit_message("x".repeat(73), 72).is_err()); } + #[test] + fn commit_existing_message_retains_length_and_control_limits() { + assert!(validate_existing_commit_message(&"x".repeat(4096)).is_ok()); + for message in [ + "x".repeat(4097), + "Subject\0body".to_string(), + "Subject\u{1b}body".to_string(), + ] { + assert_eq!( + validate_existing_commit_message(&message).unwrap_err().code, + "invalidCommitControl" + ); + } + assert_eq!( + validate_commit_control("scope\nvalue", 64) + .unwrap_err() + .code, + "invalidCommitControl" + ); + } + + #[test] + fn commit_existing_message_accepts_line_breaks_in_preview_and_generation() { + let repository = tempfile::tempdir().unwrap(); + run_git(repository.path(), &["init", "-q"]); + std::fs::write(repository.path().join("notes.txt"), "Updated notes\n").unwrap(); + run_git(repository.path(), &["add", "notes.txt"]); + let configuration = + provider_configuration(AiProvider::OpenAi, "https://api.openai.com/v1".to_string()); + + for message in [ + "Existing subject\n\nExisting body\n\tIndented detail", + "Existing subject\r\n\r\nExisting body", + ] { + let context = build_commit_context( + repository.path().to_str().unwrap(), + 72, + false, + &[], + AiCommitWorkflow::Normal, + message, + ) + .unwrap(); + assert_eq!(context.existing_message, message); + assert!(render_commit_context(&context).contains(message)); + + let request: GenerateAiCommitMessagesRequest = serde_json::from_value(json!({ + "repoPath": repository.path().to_str().unwrap(), + "subjectLimit": 72, + "operationId": "replacement-test", + "candidateCount": 1, + "existingMessage": message + })) + .unwrap(); + assert!(commit_system_prompt(&configuration, &request).is_ok()); + } + } + #[test] fn commit_context_contains_only_the_staged_version() { let repository = tempfile::tempdir().unwrap(); diff --git a/src-tauri/src/avatar/gitlab.rs b/src-tauri/src/avatar/gitlab.rs new file mode 100644 index 0000000..52a1e96 --- /dev/null +++ b/src-tauri/src/avatar/gitlab.rs @@ -0,0 +1,543 @@ +use super::conditional::ConditionalProvider; +use base64::{Engine, engine::general_purpose::STANDARD}; +use url::Url; + +pub struct GitLabProvider { + client: reqwest::blocking::Client, +} + +impl GitLabProvider { + pub fn new() -> Self { + Self { + client: reqwest::blocking::Client::builder() + .timeout(std::time::Duration::from_secs(5)) + .user_agent("gitmun/0.1") + .build() + .unwrap_or_else(|_| reqwest::blocking::Client::new()), + } + } + + fn remote_project(remote: &str) -> Option<(Url, String)> { + let mut url = if remote.contains("://") { + Url::parse(remote).ok()? + } else { + let (authority, path) = remote.split_once(':')?; + if authority.contains('/') || remote.contains('\\') || path.is_empty() { + return None; + } + Url::parse(&format!("ssh://{authority}/{path}")).ok()? + }; + let project = url + .path() + .trim_start_matches('/') + .trim_end_matches(".git") + .to_string(); + if project.is_empty() { + return None; + } + match url.scheme() { + "http" | "https" => {} + "ssh" => { + // The SSH port is unrelated to the instance's web port. + url.set_port(None).ok()?; + url = Url::parse(&format!("https://{}/", url.host()?)).ok()?; + } + _ => return None, + } + url.host_str()?; + url.set_username("").ok()?; + url.set_password(None).ok()?; + url.set_path("/"); + url.set_query(None); + url.set_fragment(None); + Some((url, project)) + } + + fn remote_projects(repo_path: &str) -> Vec<(Url, String)> { + let Ok(output) = crate::configured_git_command() + .args([ + "-C", + repo_path, + "config", + "--get-regexp", + r"^remote\..*\.url$", + ]) + .output() + else { + return Vec::new(); + }; + if !output.status.success() { + return Vec::new(); + } + let mut bases = Vec::new(); + for line in String::from_utf8_lossy(&output.stdout).lines() { + if let Some((_, remote)) = line.split_once(' ') { + if let Some(base) = Self::remote_project(remote.trim()) { + if !bases.contains(&base) { + bases.push(base); + } + } + } + } + bases + } + + fn fetch_from_instance(&self, email: &str, base: &Url) -> Option { + let mut endpoint = base.join("api/v4/avatar").ok()?; + endpoint + .query_pairs_mut() + .append_pair("email", email) + .append_pair("size", "64"); + let response = self + .client + .get(endpoint) + .send() + .ok()? + .error_for_status() + .ok()?; + let body: serde_json::Value = response.json().ok()?; + self.download_avatar(body.get("avatar_url")?.as_str()?, base) + } + + fn find_author_commit(email: &str, repo_path: &str) -> Option { + let output = crate::configured_git_command() + .args([ + "-C", + repo_path, + "log", + "--all", + "-n", + "1", + "--format=%H%x1f%ae", + "--fixed-strings", + "--regexp-ignore-case", + &format!("--author=<{email}>"), + ]) + .output() + .ok()?; + if !output.status.success() { + return None; + } + let text = String::from_utf8(output.stdout).ok()?; + let (sha, author_email) = text.trim().split_once('\u{1f}')?; + author_email + .eq_ignore_ascii_case(email) + .then(|| sha.to_string()) + } + + fn fetch_commit_author( + &self, + email: &str, + sha: &str, + base: &Url, + project: &str, + ) -> Option { + let response = self.client.get(base.join("api/graphql").ok()?) + .query(&[ + ("query", "query($project: ID!, $sha: String!) { project(fullPath: $project) { repository { commit(ref: $sha) { sha authorEmail author { avatarUrl } } } } }".to_string()), + ("variables", serde_json::json!({"project": project, "sha": sha}).to_string()), + ]) + .send().ok()?.error_for_status().ok()?; + let body: serde_json::Value = response.json().ok()?; + let commit = body.pointer("/data/project/repository/commit")?; + if commit.get("sha")?.as_str()? != sha + || !commit + .get("authorEmail")? + .as_str()? + .eq_ignore_ascii_case(email) + { + return None; + } + self.download_avatar(commit.pointer("/author/avatarUrl")?.as_str()?, base) + } + + fn download_avatar(&self, raw: &str, base: &Url) -> Option { + let raw = raw.trim(); + if raw.is_empty() { + return None; + } + let avatar = base.join(raw).ok()?; + if !matches!(avatar.scheme(), "http" | "https") { + return None; + } + let response = self + .client + .get(avatar) + .send() + .ok()? + .error_for_status() + .ok()?; + let content_type = response + .headers() + .get("content-type")? + .to_str() + .ok()? + .split(';') + .next()? + .trim() + .to_string(); + if !content_type.starts_with("image/") { + return None; + } + let bytes = response.bytes().ok()?; + if bytes.is_empty() { + return None; + } + Some(format!( + "data:{content_type};base64,{}", + STANDARD.encode(&bytes) + )) + } +} + +impl ConditionalProvider for GitLabProvider { + fn applies_to(&self, repo_path: &str) -> bool { + !Self::remote_projects(repo_path).is_empty() + } + + fn fetch(&self, email: &str, repo_path: &str) -> Option { + let projects = Self::remote_projects(repo_path); + // The email endpoint only matches public emails; commits can identify other verified emails. + if let Some(sha) = Self::find_author_commit(email, repo_path) { + if let Some(avatar) = projects + .iter() + .find_map(|(base, project)| self.fetch_commit_author(email, &sha, base, project)) + { + return Some(avatar); + } + } + projects + .iter() + .find_map(|(base, _)| self.fetch_from_instance(email, base)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{BufRead, BufReader, Write}; + use std::net::TcpListener; + + #[test] + fn parses_self_hosted_remotes() { + for (remote, expected) in [ + ( + "https://user:secret@code.example.org:8443/team/sub/repo.git", + "https://code.example.org:8443/", + ), + ( + "http://code.example.org:8080/team/repo.git", + "http://code.example.org:8080/", + ), + ( + "git@code.example.org:team/sub/repo.git", + "https://code.example.org/", + ), + ( + "ssh://git@code.example.org:2222/team/repo.git", + "https://code.example.org/", + ), + ] { + assert_eq!( + GitLabProvider::remote_project(remote).unwrap().0.as_str(), + expected + ); + } + for remote in [ + "/tmp/repo", + "../repo", + "file:///tmp/repo", + "C:\\repo", + "git://code.example.org/repo", + ] { + assert!(GitLabProvider::remote_project(remote).is_none(), "{remote}"); + } + } + + #[test] + fn reads_projects_through_a_git_directory_file() { + let directory = tempfile::tempdir().unwrap(); + let repository = directory.path().join("repository"); + let git_directory = directory.path().join("git-directory"); + let output = crate::configured_git_command() + .arg("init") + .arg("--separate-git-dir") + .arg(&git_directory) + .arg(&repository) + .output() + .unwrap(); + assert!(output.status.success()); + std::fs::write( + git_directory.join("config"), + "[remote \"origin\"]\nurl = https://code.example.org/team/repo.git\n\ + [remote \"upstream\"]\nurl = git@code.example.org:team/upstream.git\n\ + [remote \"mirror\"]\nurl = https://mirror.example.org/team/repo.git\n", + ) + .unwrap(); + assert_eq!( + GitLabProvider::remote_projects(repository.to_str().unwrap()), + vec![ + ( + Url::parse("https://code.example.org/").unwrap(), + "team/repo".to_string() + ), + ( + Url::parse("https://code.example.org/").unwrap(), + "team/upstream".to_string() + ), + ( + Url::parse("https://mirror.example.org/").unwrap(), + "team/repo".to_string() + ), + ] + ); + } + + #[test] + fn prefers_commit_account_avatar_and_falls_back_when_unavailable() { + for outcome in [ + "account", + "no account", + "wrong email", + "wrong sha", + "errors", + "forbidden", + ] { + let repository = tempfile::tempdir().unwrap(); + for args in [ + vec!["init"], + vec![ + "-c", + "user.name=Example Author", + "-c", + "user.email=author+git@example.com", + "-c", + "commit.gpgsign=false", + "commit", + "--allow-empty", + "-m", + "Avatar fixture", + ], + ] { + assert!( + crate::configured_git_command() + .arg("-C") + .arg(repository.path()) + .args(args) + .output() + .unwrap() + .status + .success() + ); + } + let sha = GitLabProvider::find_author_commit( + "author+git@example.com", + repository.path().to_str().unwrap(), + ) + .unwrap(); + assert!( + GitLabProvider::find_author_commit( + "git@example.com", + repository.path().to_str().unwrap() + ) + .is_none() + ); + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + listener.set_nonblocking(true).unwrap(); + let base = format!("http://{}/", listener.local_addr().unwrap()); + assert!( + crate::configured_git_command() + .arg("-C") + .arg(repository.path()) + .args([ + "remote", + "add", + "origin", + &format!("{base}team/subgroup/repo.git") + ]) + .output() + .unwrap() + .status + .success() + ); + let server = std::thread::spawn(move || { + let mut commit = serde_json::json!({ + "sha": sha, "authorEmail": "author+git@example.com", + "author": {"avatarUrl": "/uploads/local.png"} + }); + match outcome { + "no account" => commit["author"] = serde_json::Value::Null, + "wrong email" => commit["authorEmail"] = "other@example.com".into(), + "wrong sha" => commit["sha"] = "different".into(), + _ => {} + } + let body = if outcome == "errors" { + serde_json::json!({"errors": [{"message": "Unavailable"}]}) + } else { + serde_json::json!({"data": {"project": {"repository": {"commit": commit}}}}) + }; + let mut responses = vec![("/api/graphql", "application/json", body.to_string())]; + if outcome != "account" { + responses.push(( + "/api/v4/avatar", + "application/json", + r#"{"avatar_url":"/fallback.png"}"#.to_string(), + )); + } + responses.push(( + if outcome == "account" { + "/uploads/local.png" + } else { + "/fallback.png" + }, + "image/png", + outcome.to_string(), + )); + for (path, content_type, body) in responses { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + let mut stream = loop { + match listener.accept() { + Ok((stream, _)) => break stream, + Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => { + assert!( + std::time::Instant::now() < deadline, + "Missing request for {path}" + ); + std::thread::sleep(std::time::Duration::from_millis(10)); + } + Err(error) => panic!("{error}"), + } + }; + stream + .set_read_timeout(Some(std::time::Duration::from_secs(5))) + .unwrap(); + let mut reader = BufReader::new(stream.try_clone().unwrap()); + let mut request = String::new(); + reader.read_line(&mut request).unwrap(); + let url = Url::parse(&format!( + "http://localhost{}", + request.split_whitespace().nth(1).unwrap() + )) + .unwrap(); + assert_eq!(url.path(), path); + if path == "/api/graphql" { + let variables = url + .query_pairs() + .find(|(key, _)| key == "variables") + .unwrap() + .1 + .into_owned(); + let variables: serde_json::Value = + serde_json::from_str(&variables).unwrap(); + assert_eq!( + variables, + serde_json::json!({"project": "team/subgroup/repo", "sha": sha}) + ); + } + loop { + let mut header = String::new(); + reader.read_line(&mut header).unwrap(); + if header == "\r\n" || header.is_empty() { + break; + } + } + let status = if outcome == "forbidden" && path == "/api/graphql" { + "403 Forbidden" + } else { + "200 OK" + }; + write!(stream, "HTTP/1.1 {status}\r\nContent-Type: {content_type}\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", body.len()).unwrap(); + } + }); + let provider = GitLabProvider { + client: reqwest::blocking::Client::builder() + .no_proxy() + .timeout(std::time::Duration::from_secs(5)) + .build() + .unwrap(), + }; + assert_eq!( + provider.fetch( + "author+git@example.com", + repository.path().to_str().unwrap() + ), + Some(format!( + "data:image/png;base64,{}", + STANDARD.encode(outcome) + )) + ); + server.join().unwrap(); + } + } + + #[test] + fn downloads_uploaded_avatar_and_handles_missing_or_restricted_avatars() { + for (status, body, image_type, expected) in [ + ( + "200 OK", + r#"{"avatar_url":"/uploads/-/system/user/avatar/42/avatar.png"}"#, + "image/png", + Some("data:image/png;base64,YXZhdGFy"), + ), + ("200 OK", r#"{"avatar_url":null}"#, "", None), + ("200 OK", r#"{"avatar_url":""}"#, "", None), + ("403 Forbidden", r#"{"message":"403 Forbidden"}"#, "", None), + ( + "200 OK", + r#"{"avatar_url":"/users/sign_in"}"#, + "text/html", + None, + ), + ] { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let base = Url::parse(&format!("http://{}/", listener.local_addr().unwrap())).unwrap(); + let server = std::thread::spawn(move || { + for (index, (response_status, response_type, response_body)) in + std::iter::once((status, "application/json", body)) + .chain((!image_type.is_empty()).then_some(("200 OK", image_type, "avatar"))) + .enumerate() + { + let (mut stream, _) = listener.accept().unwrap(); + stream + .set_read_timeout(Some(std::time::Duration::from_secs(5))) + .unwrap(); + let mut reader = BufReader::new(stream.try_clone().unwrap()); + let mut request = String::new(); + reader.read_line(&mut request).unwrap(); + if index == 0 { + assert_eq!( + request.trim(), + "GET /api/v4/avatar?email=author%2Bgit%40example.com&size=64 HTTP/1.1" + ); + } else { + assert!( + request.contains("/uploads/") || request.contains("/users/sign_in") + ); + } + loop { + let mut header = String::new(); + reader.read_line(&mut header).unwrap(); + if header == "\r\n" || header.is_empty() { + break; + } + } + write!(stream, "HTTP/1.1 {response_status}\r\nContent-Type: {response_type}\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{response_body}", response_body.len()).unwrap(); + } + }); + let provider = GitLabProvider { + client: reqwest::blocking::Client::builder() + .no_proxy() + .timeout(std::time::Duration::from_secs(5)) + .build() + .unwrap(), + }; + assert_eq!( + provider + .fetch_from_instance("author+git@example.com", &base) + .as_deref(), + expected + ); + server.join().unwrap(); + } + } +} diff --git a/src-tauri/src/avatar/mod.rs b/src-tauri/src/avatar/mod.rs index bb62570..5b6dc82 100644 --- a/src-tauri/src/avatar/mod.rs +++ b/src-tauri/src/avatar/mod.rs @@ -1,6 +1,7 @@ mod conditional; mod forgejo; mod github; +mod gitlab; mod libravatar; mod provider; @@ -10,6 +11,7 @@ pub use provider::AvatarProvider; use crate::git::types::AvatarProviderMode; use forgejo::ForgejoProvider; use github::GitHubProvider; +use gitlab::GitLabProvider; use libravatar::LibravatarProvider; use std::collections::HashMap; use std::sync::{Mutex, MutexGuard}; @@ -48,9 +50,7 @@ impl AvatarService { conditional_providers: vec![ Box::new(GitHubProvider::new()), Box::new(ForgejoProvider::new()), - // Add further platform providers here, e.g.: - // Box::new(GitLabProvider::new()), - // Box::new(BitbucketProvider::new()), + Box::new(GitLabProvider::new()), ], try_platform_first: Mutex::new(try_platform_first), cache: Mutex::new(HashMap::new()), @@ -93,8 +93,8 @@ impl AvatarService { let conditional_result = self .conditional_providers .iter() - .find(|p| p.applies_to(repo_path)) - .and_then(|p| p.fetch(&key_email, repo_path)); + .filter(|p| p.applies_to(repo_path)) + .find_map(|p| p.fetch(&key_email, repo_path)); conditional_result.or_else(|| self.fetch_from_provider(&key_email)) } else { self.fetch_from_provider(&key_email) @@ -111,3 +111,38 @@ impl AvatarService { provider.as_ref().and_then(|p| p.fetch(email)) } } + +#[cfg(test)] +mod tests { + use super::*; + + struct PlatformAvatar(Option<&'static str>); + + impl ConditionalProvider for PlatformAvatar { + fn applies_to(&self, _repo_path: &str) -> bool { + true + } + + fn fetch(&self, _email: &str, _repo_path: &str) -> Option { + self.0.map(str::to_string) + } + } + + #[test] + fn tries_next_platform_when_first_has_no_avatar() { + let mut service = AvatarService::new(AvatarProviderMode::Off, true); + service.conditional_providers = vec![ + Box::new(PlatformAvatar(None)), + Box::new(PlatformAvatar(Some("data:image/png;base64,YXZhdGFy"))), + ]; + assert_eq!( + service.fetch("author@example.com", "/tmp/avatar-repository"), + Some("data:image/png;base64,YXZhdGFy".to_string()) + ); + service.set_try_platform_first(false); + assert_eq!( + service.fetch("author@example.com", "/tmp/avatar-repository"), + None + ); + } +} diff --git a/src-tauri/src/commands/history.rs b/src-tauri/src/commands/history.rs index 56c4e99..e465909 100644 --- a/src-tauri/src/commands/history.rs +++ b/src-tauri/src/commands/history.rs @@ -1,12 +1,14 @@ use crate::AppState; use crate::git::types::{ CherryPickRequest, CherryPickResult, CommitHistoryItem, CommitHistoryRequest, - CommitVerification, FileRequest, MergeRequest, MergeResult, OperationResult, RebaseRequest, - RebaseResult, RepoRequest, ResetRequest, RevertCommitRequest, SignatureStatus, + CommitProgressEvent, CommitVerification, FileRequest, GitHookAttemptResult, MergeRequest, + MergeResult, OperationResult, RebaseRequest, RebaseResult, RepoRequest, ResetRequest, + RevertCommitRequest, SignatureStatus, }; use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::process::Command; +use std::sync::Arc; use std::time::{SystemTime, UNIX_EPOCH}; use tauri::Manager; @@ -696,10 +698,18 @@ mod tests { #[tauri::command] pub async fn merge_branch( request: MergeRequest, + skip_hooks: bool, + on_progress: tauri::ipc::Channel, app: tauri::AppHandle, -) -> Result { +) -> Result, String> { tauri::async_runtime::spawn_blocking(move || { - app.state::().git_service.merge_branch(request) + app.state::() + .git_service + .merge_branch_with_progress( + request, + skip_hooks, + Arc::new(move |event| drop(on_progress.send(event))), + ) }) .await .map_err(|e| e.to_string())? @@ -722,10 +732,16 @@ pub async fn merge_abort( #[tauri::command] pub async fn rebase_start( request: RebaseRequest, + on_progress: tauri::ipc::Channel, app: tauri::AppHandle, -) -> Result { +) -> Result, String> { tauri::async_runtime::spawn_blocking(move || { - app.state::().git_service.rebase_start(request) + app.state::() + .git_service + .rebase_start_with_progress( + request, + Arc::new(move |event| drop(on_progress.send(event))), + ) }) .await .map_err(|e| e.to_string())? @@ -735,10 +751,16 @@ pub async fn rebase_start( #[tauri::command] pub async fn rebase_continue( request: RepoRequest, + on_progress: tauri::ipc::Channel, app: tauri::AppHandle, -) -> Result { +) -> Result, String> { tauri::async_runtime::spawn_blocking(move || { - app.state::().git_service.rebase_continue(request) + app.state::() + .git_service + .rebase_continue_with_progress( + request, + Arc::new(move |event| drop(on_progress.send(event))), + ) }) .await .map_err(|e| e.to_string())? diff --git a/src-tauri/src/commands/repo.rs b/src-tauri/src/commands/repo.rs index edeb0b2..1a613b4 100644 --- a/src-tauri/src/commands/repo.rs +++ b/src-tauri/src/commands/repo.rs @@ -2478,10 +2478,18 @@ pub fn get_repo_diff_tool( #[tauri::command] pub async fn pull_changes( request: RepoRequest, + skip_hooks: bool, + on_progress: tauri::ipc::Channel, app: tauri::AppHandle, -) -> Result { +) -> Result, String> { tauri::async_runtime::spawn_blocking(move || { - app.state::().git_service.pull_changes(request) + app.state::() + .git_service + .pull_changes_with_progress( + request, + skip_hooks, + Arc::new(move |event| drop(on_progress.send(event))), + ) }) .await .map_err(|e| e.to_string())? @@ -2502,12 +2510,18 @@ pub fn analyze_pull( #[tauri::command] pub async fn pull_with_strategy( request: PullStrategyRequest, + skip_hooks: bool, + on_progress: tauri::ipc::Channel, app: tauri::AppHandle, -) -> Result { +) -> Result, String> { tauri::async_runtime::spawn_blocking(move || { app.state::() .git_service - .pull_with_strategy(request) + .pull_with_strategy_with_progress( + request, + skip_hooks, + Arc::new(move |event| drop(on_progress.send(event))), + ) }) .await .map_err(|e| e.to_string())? diff --git a/src-tauri/src/commands/settings.rs b/src-tauri/src/commands/settings.rs index 982b752..67f7704 100644 --- a/src-tauri/src/commands/settings.rs +++ b/src-tauri/src/commands/settings.rs @@ -232,16 +232,6 @@ pub fn set_row_striping(row_striping: RowStriping, state: tauri::State<'_, AppSt state.git_service.set_row_striping(row_striping) } -#[tauri::command] -pub fn set_show_commit_graph_button( - show_commit_graph_button: bool, - state: tauri::State<'_, AppState>, -) -> Settings { - state - .git_service - .set_show_commit_graph_button(show_commit_graph_button) -} - #[tauri::command] pub fn set_enable_local_copy( enable_local_copy: bool, diff --git a/src-tauri/src/config_file.rs b/src-tauri/src/config_file.rs index 165e209..a61ec43 100644 --- a/src-tauri/src/config_file.rs +++ b/src-tauri/src/config_file.rs @@ -554,7 +554,6 @@ mod tests { assert!(toml_text.contains("# Backend used for Git operations")); assert!(toml_text.contains("backendMode = \"Default\"")); assert!(toml_text.contains("uiTextScale = 1.0")); - assert!(toml_text.contains("showCommitGraphButton = false")); assert!(toml_text.contains("errorToastClearDelayMs = 8000")); assert!(toml_text.contains("commitMessageRecommendedLength = 72")); assert!(!toml_text.contains("enableUpdateWithMSStoreFlow")); @@ -632,10 +631,9 @@ mod tests { ); assert!(updated.contains("uiTextScale = 1.0")); assert!( - updated.contains("# Experimental: show the commit graph toolbar button."), + updated.contains("# Experimental: enable Local Copy in the Clone window and CLI."), "missing key gained its template comment" ); - assert!(updated.contains("showCommitGraphButton = false")); assert!(updated.contains("enableLocalCopy = false")); assert!(updated.contains("# Maximum context sent in each AI commit-message request")); assert!(updated.contains("commitContextLimitKib = 24")); @@ -652,13 +650,6 @@ mod tests { assert!(!updated.contains("enableUpdateWithMSStoreFlow")); } - #[test] - fn missing_commit_graph_button_defaults_to_false() { - let settings: Settings = toml::from_str("backendMode = \"Default\"\n").unwrap(); - - assert!(!settings.show_commit_graph_button); - } - #[test] fn missing_local_copy_setting_defaults_to_false() { let settings: Settings = toml::from_str("backendMode = \"Default\"\n").unwrap(); @@ -680,20 +671,6 @@ mod tests { assert!(updated.contains("enableLocalCopy = true")); } - #[test] - fn persist_writes_commit_graph_button() { - let dir = TempDir::new().unwrap(); - let toml_path = dir.path().join("config.toml"); - - let mut settings = Settings::default(); - settings.show_commit_graph_button = true; - - persist(&toml_path, &settings).unwrap(); - - let updated = std::fs::read_to_string(&toml_path).unwrap(); - assert!(updated.contains("showCommitGraphButton = true")); - } - #[test] fn persist_leaves_old_ms_store_update_flow_config_untouched() { let dir = TempDir::new().unwrap(); diff --git a/src-tauri/src/git/cli.rs b/src-tauri/src/git/cli.rs index a082e6d..7e3b31d 100644 --- a/src-tauri/src/git/cli.rs +++ b/src-tauri/src/git/cli.rs @@ -21,17 +21,16 @@ use super::types::{ DiffHunk, DiffLine, DiffLineKind, DiffRequest, ExportCommitPatchRequest, ExportPatchFileSelection, ExportPatchRequest, ExportPatchScope, ExternalDiffRequest, FetchRequest, FileDiff, FileRequest, FileStatusItem, GitHookAttemptResult, GitHookFailure, - GitIdentity, HunkStageRequest, - IdentityRequest, IdentityScope, ImportPatchRequest, LineEndingStyle, MergeRequest, MergeResult, - NumstatRequest, NumstatResult, OperationResult, PruneRemoteRequest, PullAnalysis, - PullRecommendedAction, PullState, PullStrategy, PullStrategyRequest, PushFailureKind, - PushRejectionAnalysis, PushRequest, PushResult, PushTagRequest, RebaseRequest, RebaseResult, - RemoteInfo, RemoveRemoteRequest, RenameBranchRequest, RenameRemoteRequest, RepoRequest, - RepoStatus, ResetMode, ResetRequest, RevertCommitRequest, SetBranchUpstreamRequest, - SetIdentityRequest, SetRemoteUrlRequest, SignatureStatus, SshAllowedSignerReason, - SshAllowedSignerStatus, StageFilesRequest, StashEntry, StashPushRequest, StashRequest, - SubmoduleActionRequest, SubmoduleState, SubmoduleStatus, TagInfo, UnversionedItem, - UnversionedItemKind, UpstreamStatus, + GitIdentity, HunkStageRequest, IdentityRequest, IdentityScope, ImportPatchRequest, + LineEndingStyle, MergeRequest, MergeResult, NumstatRequest, NumstatResult, OperationResult, + PruneRemoteRequest, PullAnalysis, PullRecommendedAction, PullState, PullStrategy, + PullStrategyRequest, PushFailureKind, PushRejectionAnalysis, PushRequest, PushResult, + PushTagRequest, RebaseRequest, RebaseResult, RemoteInfo, RemoveRemoteRequest, + RenameBranchRequest, RenameRemoteRequest, RepoRequest, RepoStatus, ResetMode, ResetRequest, + RevertCommitRequest, SetBranchUpstreamRequest, SetIdentityRequest, SetRemoteUrlRequest, + SignatureStatus, SshAllowedSignerReason, SshAllowedSignerStatus, StageFilesRequest, StashEntry, + StashPushRequest, StashRequest, SubmoduleActionRequest, SubmoduleState, SubmoduleStatus, + TagInfo, UnversionedItem, UnversionedItemKind, UpstreamStatus, }; pub struct CliGitHandler; @@ -43,6 +42,13 @@ struct HookCommandOutput { hooks: Vec<(String, String, Option)>, } +#[derive(Clone, Copy)] +enum PullIntegrationKind { + Auto, + Merge, + Rebase, +} + #[derive(Debug, Clone)] struct ConfiguredSubmodule { name: String, @@ -781,8 +787,14 @@ impl CliGitHandler { let Ok(event) = serde_json::from_str::(line) else { continue; }; - let session_id = event.get("sid").and_then(serde_json::Value::as_str); - if session_id != Some(root_session_id.as_str()) { + let Some(session_id) = event.get("sid").and_then(serde_json::Value::as_str) else { + continue; + }; + if session_id != root_session_id + && !session_id + .strip_prefix(root_session_id.as_str()) + .is_some_and(|suffix| suffix.starts_with('/')) + { continue; } let child_id = event.get("child_id").and_then(serde_json::Value::as_u64); @@ -795,7 +807,7 @@ impl CliGitHandler { child_id, event.get("hook_name").and_then(serde_json::Value::as_str), ) { - hooks.push((child_id, hook_name.to_string())); + hooks.push((format!("{session_id}:{child_id}"), hook_name.to_string())); } } Some("child_exit") => { @@ -803,7 +815,7 @@ impl CliGitHandler { child_id, event.get("code").and_then(serde_json::Value::as_i64), ) { - exit_codes.insert(child_id, code as i32); + exit_codes.insert(format!("{session_id}:{child_id}"), code as i32); } } _ => {} @@ -811,13 +823,7 @@ impl CliGitHandler { } hooks .into_iter() - .map(|(child_id, hook_name)| { - ( - format!("{root_session_id}:{child_id}"), - hook_name, - exit_codes.get(&child_id).copied(), - ) - }) + .map(|(key, hook_name)| (key.clone(), hook_name, exit_codes.get(&key).copied())) .collect() } @@ -1891,6 +1897,107 @@ impl CliGitHandler { } } + fn execute_pull_command_with_progress( + &self, + repo_path: &Path, + mut args: Vec, + success_message: &str, + conflict_message: &str, + integration_kind: PullIntegrationKind, + skip_hooks: bool, + on_progress: Arc, + ) -> GitResult> { + if skip_hooks { + args.push("--no-verify".to_string()); + } + + let outcome = Self::run_git_with_hook_progress(repo_path, &args, on_progress)?; + let is_rebase_pull = matches!(integration_kind, PullIntegrationKind::Rebase) + || matches!(integration_kind, PullIntegrationKind::Auto) + && (outcome + .hooks + .iter() + .any(|(name, _, _)| name == "pre-rebase") + || Self::is_rebase_in_progress(repo_path)); + + if !is_rebase_pull + && let Some(failure) = Self::latest_hook_failure( + &outcome, + &["pre-merge-commit", "prepare-commit-msg", "commit-msg"], + &["pre-merge-commit", "commit-msg"], + ) + { + Self::rollback_merge_after_hook_failure(repo_path, &failure)?; + return Ok(GitHookAttemptResult::HookRejected { + hook_name: failure.hook_name, + exit_status: failure.exit_status, + output: failure.output, + output_truncated: failure.output_truncated, + bypass_supported: failure.bypass_supported, + }); + } + + if is_rebase_pull + && !outcome.status.success() + && let Some(failure) = Self::latest_hook_failure( + &outcome, + &[ + "pre-rebase", + "prepare-commit-msg", + "commit-msg", + "applypatch-msg", + "pre-applypatch", + ], + &[], + ) + { + return Ok(GitHookAttemptResult::HookRejected { + hook_name: failure.hook_name, + exit_status: failure.exit_status, + output: failure.output, + output_truncated: failure.output_truncated, + bypass_supported: false, + }); + } + + let hook_warning = + Self::latest_hook_failure(&outcome, &["post-merge", "post-rewrite"], &[]); + if outcome.status.success() || hook_warning.is_some() { + return Ok(GitHookAttemptResult::Completed { + result: OperationResult { + message: success_message.to_string(), + output: outcome.output, + repo_path: Some(Self::path_to_string(repo_path)), + backend_used: "git-cli".to_string(), + interpreted_error: None, + }, + hook_warning, + output_truncated: outcome.output_truncated, + }); + } + + let status = self.get_repo_status(&Self::repo_request(repo_path))?; + if status.merge_in_progress || status.rebase_in_progress { + return Ok(GitHookAttemptResult::Completed { + result: OperationResult { + message: conflict_message.to_string(), + output: outcome.output, + repo_path: Some(Self::path_to_string(repo_path)), + backend_used: "git-cli".to_string(), + interpreted_error: None, + }, + hook_warning: None, + output_truncated: outcome.output_truncated, + }); + } + + Err(GitError::CommandFailed { + command: format!("git {}", args.join(" ")), + stderr: outcome.output.unwrap_or_default(), + exit_code: outcome.status.code(), + }) + } + fn try_rev_parse(repo_path: &Path, rev: &str) -> Option { Self::run_git(&["rev-parse", rev], Some(repo_path)) .ok() @@ -2684,6 +2791,65 @@ impl CliGitHandler { }) } + fn latest_hook_failure( + outcome: &HookCommandOutput, + hook_names: &[&str], + bypassable_hook_names: &[&str], + ) -> Option { + outcome + .hooks + .iter() + .rev() + .find(|(_, name, status)| { + hook_names.contains(&name.as_str()) && status.is_some_and(|code| code != 0) + }) + .map(|(_, name, status)| GitHookFailure { + hook_name: name.clone(), + exit_status: *status, + output: outcome.output.clone(), + output_truncated: outcome.output_truncated, + bypass_supported: bypassable_hook_names.contains(&name.as_str()), + }) + } + + fn rollback_merge_after_hook_failure( + repo_path: &Path, + failure: &GitHookFailure, + ) -> GitResult<()> { + if !Self::is_merge_in_progress(repo_path) { + return Ok(()); + } + + if let Err(rollback_error) = Self::run_git(&["merge", "--abort"], Some(repo_path)) { + return Err(GitError::CommandFailed { + command: "git merge --abort".to_string(), + stderr: format!( + "GITMUN_MERGE_HOOK_ROLLBACK_FAILED: {}\nHook output:\n{}\nRollback failure:\n{}", + failure.hook_name, + failure + .output + .as_deref() + .unwrap_or("No hook output was captured."), + rollback_error + ), + exit_code: None, + }); + } + + if Self::is_merge_in_progress(repo_path) { + return Err(GitError::CommandFailed { + command: "git merge --abort".to_string(), + stderr: format!( + "GITMUN_MERGE_HOOK_ROLLBACK_INCOMPLETE: {}", + failure.hook_name + ), + exit_code: None, + }); + } + + Ok(()) + } + fn completed_hook_operation( repo_path: &Path, args: Vec, @@ -3011,6 +3177,299 @@ impl CliGitHandler { output_truncated: outcome.output_truncated, }) } + + pub fn pull_changes_with_progress( + &self, + request: &RepoRequest, + skip_hooks: bool, + on_progress: Arc, + ) -> GitResult> { + let repo_path = Self::normalise_repo_path(&request.repo_path)?; + self.execute_pull_command_with_progress( + &repo_path, + vec!["pull".to_string()], + &format!("Pulled latest changes in {}", repo_path.display()), + "Pull started a conflict resolution flow. Resolve the conflicts, then continue or complete the operation.", + PullIntegrationKind::Auto, + skip_hooks, + on_progress, + ) + } + + pub fn pull_with_strategy_with_progress( + &self, + request: &PullStrategyRequest, + skip_hooks: bool, + on_progress: Arc, + ) -> GitResult> { + let repo_path = Self::normalise_repo_path(&request.repo_path)?; + let analysis = self.build_pull_analysis(&repo_path)?; + let (args, success_message, conflict_message, integration_kind) = match request.strategy { + PullStrategy::FfOnly => { + if !matches!(analysis.state, PullState::BehindOnly) { + return Err(GitError::InvalidInput( + "Fast-forward pull is only available when the branch is behind its upstream." + .to_string(), + )); + } + ( + vec!["pull".to_string(), "--ff-only".to_string()], + "Fast-forward pull complete.", + "Pull started a conflict resolution flow. Resolve the conflicts, then continue or complete the operation.", + PullIntegrationKind::Merge, + ) + } + PullStrategy::Rebase => { + if !matches!(analysis.state, PullState::BehindOnly | PullState::Divergent) { + return Err(GitError::InvalidInput( + "Rebase pull is only available when remote changes need to be integrated." + .to_string(), + )); + } + ( + vec!["pull".to_string(), "--rebase".to_string()], + "Rebase pull complete.", + "Rebase started and needs conflict resolution. Resolve the conflicts, then continue the rebase.", + PullIntegrationKind::Rebase, + ) + } + PullStrategy::Merge => { + if !matches!(analysis.state, PullState::BehindOnly | PullState::Divergent) { + return Err(GitError::InvalidInput( + "Merge pull is only available when remote changes need to be integrated." + .to_string(), + )); + } + ( + vec!["pull".to_string(), "--no-rebase".to_string()], + "Merge pull complete.", + "Merge started and needs conflict resolution. Resolve the conflicts, then complete or abort the merge.", + PullIntegrationKind::Merge, + ) + } + }; + self.execute_pull_command_with_progress( + &repo_path, + args, + success_message, + conflict_message, + integration_kind, + skip_hooks, + on_progress, + ) + } + + pub fn merge_branch_with_progress( + &self, + request: &MergeRequest, + skip_hooks: bool, + on_progress: Arc, + ) -> GitResult> { + let repo_path = Self::normalise_repo_path(&request.repo_path)?; + if Self::is_merge_in_progress(&repo_path) { + return Err(GitError::InvalidInput( + "Cannot start merge while another merge is in progress".to_string(), + )); + } + if Self::is_rebase_in_progress(&repo_path) { + return Err(GitError::InvalidInput( + "Cannot start merge while a rebase is in progress".to_string(), + )); + } + if Self::is_cherry_pick_in_progress(&repo_path) { + return Err(GitError::InvalidInput( + "Cannot start merge while a cherry-pick is in progress".to_string(), + )); + } + let mut args = vec!["merge".to_string()]; + if skip_hooks { + args.push("--no-verify".to_string()); + } + if request.no_ff == Some(true) { + args.push("--no-ff".to_string()); + } + if request.ff_only == Some(true) { + args.push("--ff-only".to_string()); + } + if let Some(message) = &request.message { + args.push("-m".to_string()); + args.push(message.clone()); + } + args.push(request.branch_name.clone()); + let outcome = Self::run_git_with_hook_progress(&repo_path, &args, on_progress)?; + if let Some(failure) = Self::latest_hook_failure( + &outcome, + &["pre-merge-commit", "prepare-commit-msg", "commit-msg"], + &["pre-merge-commit", "commit-msg"], + ) { + Self::rollback_merge_after_hook_failure(&repo_path, &failure)?; + return Ok(GitHookAttemptResult::HookRejected { + hook_name: failure.hook_name, + exit_status: failure.exit_status, + output: failure.output, + output_truncated: failure.output_truncated, + bypass_supported: failure.bypass_supported, + }); + } + let hook_warning = Self::hook_failure(&outcome, "post-merge", false); + let has_conflicts = Self::is_merge_in_progress(&repo_path); + if !outcome.status.success() && !has_conflicts && hook_warning.is_none() { + return Err(GitError::CommandFailed { + command: format!("git {}", args.join(" ")), + stderr: outcome.output.unwrap_or_default(), + exit_code: outcome.status.code(), + }); + } + let conflicted_files = if has_conflicts { + Self::get_conflicted_files(&repo_path) + } else { + vec![] + }; + let result = MergeResult { + message: if has_conflicts { + format!( + "Merge conflicts in {} file(s) - resolve and commit", + conflicted_files.len() + ) + } else { + format!("Merged '{}' into current branch", request.branch_name) + }, + output: outcome.output.clone(), + repo_path: Some(Self::path_to_string(&repo_path)), + backend_used: "git-cli".to_string(), + interpreted_error: None, + success: !has_conflicts, + has_conflicts, + conflicted_files, + }; + Ok(GitHookAttemptResult::Completed { + result, + hook_warning, + output_truncated: outcome.output_truncated, + }) + } + + fn rebase_hook_attempt_result( + repo_path: &Path, + args: &[String], + outcome: HookCommandOutput, + complete_message: String, + in_progress_message: &str, + ) -> GitResult> { + if !outcome.status.success() + && let Some(failure) = Self::latest_hook_failure( + &outcome, + &[ + "pre-rebase", + "pre-commit", + "prepare-commit-msg", + "commit-msg", + "applypatch-msg", + "pre-applypatch", + ], + &[], + ) + { + return Ok(GitHookAttemptResult::HookRejected { + hook_name: failure.hook_name, + exit_status: failure.exit_status, + output: failure.output, + output_truncated: failure.output_truncated, + bypass_supported: false, + }); + } + let hook_warning = Self::hook_failure(&outcome, "post-rewrite", false); + let rebase_in_progress = Self::is_rebase_in_progress(repo_path); + if !outcome.status.success() && outcome.status.code() != Some(1) && hook_warning.is_none() { + return Err(GitError::CommandFailed { + command: format!("git {}", args.join(" ")), + stderr: outcome.output.unwrap_or_default(), + exit_code: outcome.status.code(), + }); + } + let conflicted_files = if rebase_in_progress { + Self::get_conflicted_files(repo_path) + } else { + vec![] + }; + let has_conflicts = !conflicted_files.is_empty(); + let result = RebaseResult { + message: if has_conflicts { + format!( + "Rebase conflicts in {} file(s) - resolve and continue", + conflicted_files.len() + ) + } else if rebase_in_progress { + in_progress_message.to_string() + } else { + complete_message + }, + output: outcome.output, + repo_path: Some(Self::path_to_string(repo_path)), + backend_used: "git-cli".to_string(), + interpreted_error: None, + success: !has_conflicts, + has_conflicts, + conflicted_files, + rebase_in_progress, + }; + Ok(GitHookAttemptResult::Completed { + result, + hook_warning, + output_truncated: outcome.output_truncated, + }) + } + + pub fn rebase_start_with_progress( + &self, + request: &RebaseRequest, + on_progress: Arc, + ) -> GitResult> { + let repo_path = Self::normalise_repo_path(&request.repo_path)?; + Self::ensure_no_active_branch_operation(&repo_path, "start a rebase")?; + let onto = request.onto.trim(); + if onto.is_empty() { + return Err(GitError::InvalidInput( + "Rebase target cannot be empty".to_string(), + )); + } + let args = vec!["rebase".to_string(), onto.to_string()]; + let outcome = Self::run_git_with_hook_progress(&repo_path, &args, on_progress)?; + Self::rebase_hook_attempt_result( + &repo_path, + &args, + outcome, + format!("Rebased current branch onto '{onto}'"), + "Rebase continued", + ) + } + + pub fn rebase_continue_with_progress( + &self, + request: &RepoRequest, + on_progress: Arc, + ) -> GitResult> { + let repo_path = Self::normalise_repo_path(&request.repo_path)?; + if !Self::is_rebase_in_progress(&repo_path) { + return Err(GitError::InvalidInput( + "No rebase in progress to continue".to_string(), + )); + } + let args = vec![ + "-c".to_string(), + "core.editor=true".to_string(), + "rebase".to_string(), + "--continue".to_string(), + ]; + let outcome = Self::run_git_with_hook_progress(&repo_path, &args, on_progress)?; + Self::rebase_hook_attempt_result( + &repo_path, + &args, + outcome, + "Rebase complete".to_string(), + "Rebase continued", + ) + } } impl GitOperationHandler for CliGitHandler { diff --git a/src-tauri/src/git/handler.rs b/src-tauri/src/git/handler.rs index 71c8d89..6874224 100644 --- a/src-tauri/src/git/handler.rs +++ b/src-tauri/src/git/handler.rs @@ -271,12 +271,6 @@ impl GitService { }) } - pub fn set_show_commit_graph_button(&self, show_commit_graph_button: bool) -> Settings { - self.update_settings(|settings| { - settings.show_commit_graph_button = show_commit_graph_button; - }) - } - pub fn set_enable_local_copy(&self, enable_local_copy: bool) -> Settings { self.update_settings(|settings| { settings.enable_local_copy = enable_local_copy; @@ -670,6 +664,54 @@ impl GitService { .push_changes_with_progress(&request, skip_hooks, on_progress) } + pub fn pull_changes_with_progress( + &self, + request: RepoRequest, + skip_hooks: bool, + on_progress: Arc, + ) -> GitResult> { + self.cli_handler + .pull_changes_with_progress(&request, skip_hooks, on_progress) + } + + pub fn pull_with_strategy_with_progress( + &self, + request: PullStrategyRequest, + skip_hooks: bool, + on_progress: Arc, + ) -> GitResult> { + self.cli_handler + .pull_with_strategy_with_progress(&request, skip_hooks, on_progress) + } + + pub fn merge_branch_with_progress( + &self, + request: MergeRequest, + skip_hooks: bool, + on_progress: Arc, + ) -> GitResult> { + self.cli_handler + .merge_branch_with_progress(&request, skip_hooks, on_progress) + } + + pub fn rebase_start_with_progress( + &self, + request: RebaseRequest, + on_progress: Arc, + ) -> GitResult> { + self.cli_handler + .rebase_start_with_progress(&request, on_progress) + } + + pub fn rebase_continue_with_progress( + &self, + request: RepoRequest, + on_progress: Arc, + ) -> GitResult> { + self.cli_handler + .rebase_continue_with_progress(&request, on_progress) + } + pub fn switch_branch_with_progress( &self, request: BranchRequest, @@ -819,16 +861,6 @@ impl GitService { mod tests { use super::GitService; - #[test] - fn set_show_commit_graph_button_updates_settings() { - let service = GitService::new(); - - let settings = service.set_show_commit_graph_button(true); - - assert!(settings.show_commit_graph_button); - assert!(service.get_settings().show_commit_graph_button); - } - #[test] fn set_enable_local_copy_updates_settings() { let service = GitService::new(); diff --git a/src-tauri/src/git/types.rs b/src-tauri/src/git/types.rs index 09c6dd9..1bdefbd 100644 --- a/src-tauri/src/git/types.rs +++ b/src-tauri/src/git/types.rs @@ -286,8 +286,6 @@ pub struct Settings { #[serde(default)] pub row_striping: RowStriping, #[serde(default)] - pub show_commit_graph_button: bool, - #[serde(default)] pub enable_local_copy: bool, #[serde(default)] pub persistent_error_toasts: bool, @@ -411,7 +409,6 @@ impl Default for Settings { ui_text_scale: default_ui_text_scale(), wrap_diff_lines: false, row_striping: RowStriping::Off, - show_commit_graph_button: false, enable_local_copy: false, persistent_error_toasts: false, error_toast_clear_delay_ms: DEFAULT_ERROR_TOAST_CLEAR_DELAY_MS, diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 51d83be..a25617e 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1571,7 +1571,6 @@ pub fn run() { commands::settings::set_ui_text_scale, commands::settings::set_wrap_diff_lines, commands::settings::set_row_striping, - commands::settings::set_show_commit_graph_button, commands::settings::set_enable_local_copy, commands::settings::set_persistent_error_toasts, commands::settings::set_error_toast_clear_delay_ms, diff --git a/src-tauri/tests/git.rs b/src-tauri/tests/git.rs index 05ea52c..e5bb924 100644 --- a/src-tauri/tests/git.rs +++ b/src-tauri/tests/git.rs @@ -13,9 +13,10 @@ use gitmun_lib::git::types::{ CommitProgressEvent, CommitRefKind, CommitRequest, CreateBranchRequest, DeleteBranchRequest, ExportCommitPatchRequest, ExportPatchFileSelection, ExportPatchRequest, ExportPatchScope, FileRequest, GitHookAttemptResult, IdentityRequest, IdentityScope, ImportPatchRequest, - PushFailureKind, PushRequest, RepoRequest, RepoStatus, ResetMode, ResetRequest, - SetBranchUpstreamRequest, SetIdentityRequest, SshAllowedSignerReason, StageFilesRequest, - SubmoduleActionRequest, SubmoduleState, UnversionedItemKind, + MergeRequest, PullStrategy, PullStrategyRequest, PushFailureKind, PushRequest, RebaseRequest, + RepoRequest, RepoStatus, ResetMode, ResetRequest, SetBranchUpstreamRequest, SetIdentityRequest, + SshAllowedSignerReason, StageFilesRequest, SubmoduleActionRequest, SubmoduleState, + UnversionedItemKind, }; fn init_repo() -> TempDir { @@ -74,6 +75,19 @@ fn write_file(repo: &Path, name: &str, content: &str) { fs::write(repo.join(name), content).expect("write file"); } +#[cfg(unix)] +fn install_hook(repo: &Path, hook_name: &str, script: &str) { + use std::os::unix::fs::PermissionsExt; + + let hook_path = repo.join(".git/hooks").join(hook_name); + fs::write(&hook_path, script).expect("write hook"); + let mut permissions = fs::metadata(&hook_path) + .expect("hook metadata") + .permissions(); + permissions.set_mode(0o755); + fs::set_permissions(hook_path, permissions).expect("make hook executable"); +} + fn make_index_entry_stale(repo: &Path, file_path: &str) -> (std::path::PathBuf, Vec) { let raw_index_path = git_stdout(repo, &["rev-parse", "--git-path", "index"]); let index_path = { @@ -165,6 +179,22 @@ fn init_remote_with_clone() -> (TempDir, TempDir) { (remote, local) } +fn clone_test_remote(remote: &TempDir) -> TempDir { + let clone = TempDir::new().expect("create clone dir"); + git( + Path::new("."), + &[ + "clone", + remote.path().to_str().unwrap(), + clone.path().to_str().unwrap(), + ], + ); + git(clone.path(), &["config", "user.email", "peer@gitmun.test"]); + git(clone.path(), &["config", "user.name", "Gitmun Test Peer"]); + git(clone.path(), &["config", "commit.gpgsign", "false"]); + clone +} + fn init_submodule_source() -> TempDir { let dir = init_repo(); write_file(dir.path(), "lib.txt", "v1"); @@ -1792,6 +1822,433 @@ fn post_checkout_failure_reports_warning_without_repeating_checkout() { ); } +#[cfg(unix)] +#[test] +fn merge_hook_rejections_roll_back_and_only_supported_hooks_can_be_bypassed() { + for (installed_hook_name, bypass_supported) in [ + ("pre-merge-commit", true), + ("prepare-commit-msg", false), + ("commit-msg", true), + ] { + let dir = init_repo(); + git(dir.path(), &["switch", "-c", "feature/hook-check"]); + write_file(dir.path(), "feature.txt", installed_hook_name); + git(dir.path(), &["add", "feature.txt"]); + git(dir.path(), &["commit", "-m", "feature change"]); + git(dir.path(), &["switch", "main"]); + install_hook( + dir.path(), + installed_hook_name, + "#!/bin/sh\necho merge hook rejected >&2\nexit 1\n", + ); + let head_before = head_hash(dir.path()); + let request = MergeRequest { + repo_path: dir.path().to_string_lossy().into_owned(), + branch_name: "feature/hook-check".to_string(), + no_ff: Some(true), + ff_only: None, + message: None, + }; + + let rejected = handler() + .merge_branch_with_progress(&request, false, Arc::new(|_| {})) + .expect("merge hook rejection"); + assert!(matches!( + rejected, + GitHookAttemptResult::HookRejected { + ref hook_name, + bypass_supported: actual_bypass, + .. + } if hook_name == installed_hook_name && actual_bypass == bypass_supported + )); + assert_eq!(head_hash(dir.path()), head_before); + assert!(!dir.path().join(".git/MERGE_HEAD").exists()); + assert!(git_stdout(dir.path(), &["status", "--porcelain"]).is_empty()); + + if bypass_supported { + let bypassed = handler() + .merge_branch_with_progress(&request, true, Arc::new(|_| {})) + .expect("merge hook bypass"); + assert!(matches!( + bypassed, + GitHookAttemptResult::Completed { ref result, .. } if result.success + )); + assert_ne!(head_hash(dir.path()), head_before); + } + } +} + +#[cfg(unix)] +#[test] +fn clean_merge_emits_merge_message_and_post_merge_hook_progress() { + let dir = init_repo(); + git(dir.path(), &["switch", "-c", "feature/hook-progress"]); + write_file(dir.path(), "feature.txt", "feature change"); + git(dir.path(), &["add", "feature.txt"]); + git(dir.path(), &["commit", "-m", "feature change"]); + git(dir.path(), &["switch", "main"]); + for hook_name in [ + "pre-merge-commit", + "prepare-commit-msg", + "commit-msg", + "post-merge", + ] { + install_hook(dir.path(), hook_name, "#!/bin/sh\nexit 0\n"); + } + let events = Arc::new(Mutex::new(Vec::new())); + let recorded_events = Arc::clone(&events); + + let result = handler() + .merge_branch_with_progress( + &MergeRequest { + repo_path: dir.path().to_string_lossy().into_owned(), + branch_name: "feature/hook-progress".to_string(), + no_ff: Some(true), + ff_only: None, + message: None, + }, + false, + Arc::new(move |event| { + recorded_events.lock().expect("event lock").push(event); + }), + ) + .expect("clean merge"); + + assert!(matches!( + result, + GitHookAttemptResult::Completed { ref result, hook_warning: None, .. } if result.success + )); + let events = events.lock().expect("event lock"); + for expected_hook in [ + "pre-merge-commit", + "prepare-commit-msg", + "commit-msg", + "post-merge", + ] { + assert!(events.iter().any(|event| matches!( + event, + CommitProgressEvent::HookStarted { hook_name } if hook_name == expected_hook + ))); + } +} + +#[cfg(unix)] +#[test] +fn fast_forward_pull_reports_post_merge_failure_after_updating_head() { + let (remote, local) = init_remote_with_clone(); + let peer = clone_test_remote(&remote); + write_file(peer.path(), "remote.txt", "remote change"); + git(peer.path(), &["add", "remote.txt"]); + git(peer.path(), &["commit", "-m", "remote change"]); + git(peer.path(), &["push", "origin", "main"]); + git(local.path(), &["fetch", "origin"]); + install_hook( + local.path(), + "post-merge", + "#!/bin/sh\necho post-merge warning >&2\nexit 1\n", + ); + let events = Arc::new(Mutex::new(Vec::new())); + let recorded_events = Arc::clone(&events); + + let result = handler() + .pull_with_strategy_with_progress( + &PullStrategyRequest { + repo_path: local.path().to_string_lossy().into_owned(), + strategy: PullStrategy::FfOnly, + }, + false, + Arc::new(move |event| { + recorded_events.lock().expect("event lock").push(event); + }), + ) + .expect("fast-forward pull result"); + + assert!(matches!( + result, + GitHookAttemptResult::Completed { hook_warning: Some(ref warning), .. } + if warning.hook_name == "post-merge" + )); + assert_eq!(head_hash(local.path()), head_hash(peer.path())); + assert!( + events + .lock() + .expect("event lock") + .iter() + .any(|event| matches!( + event, + CommitProgressEvent::HookStarted { hook_name } if hook_name == "post-merge" + )) + ); +} + +#[cfg(unix)] +#[test] +fn merge_pull_rejection_rolls_back_before_bypass_retry() { + let (remote, local) = init_remote_with_clone(); + let peer = clone_test_remote(&remote); + write_file(local.path(), "local.txt", "local change"); + git(local.path(), &["add", "local.txt"]); + git(local.path(), &["commit", "-m", "local change"]); + write_file(peer.path(), "remote.txt", "remote change"); + git(peer.path(), &["add", "remote.txt"]); + git(peer.path(), &["commit", "-m", "remote change"]); + git(peer.path(), &["push", "origin", "main"]); + git(local.path(), &["fetch", "origin"]); + install_hook( + local.path(), + "pre-merge-commit", + "#!/bin/sh\necho pull merge rejected >&2\nexit 1\n", + ); + let head_before = head_hash(local.path()); + let request = PullStrategyRequest { + repo_path: local.path().to_string_lossy().into_owned(), + strategy: PullStrategy::Merge, + }; + + let rejected = handler() + .pull_with_strategy_with_progress(&request, false, Arc::new(|_| {})) + .expect("pull merge rejection"); + assert!(matches!( + rejected, + GitHookAttemptResult::HookRejected { + bypass_supported: true, + .. + } + )); + assert_eq!(head_hash(local.path()), head_before); + assert!(!local.path().join(".git/MERGE_HEAD").exists()); + + let bypassed = handler() + .pull_with_strategy_with_progress(&request, true, Arc::new(|_| {})) + .expect("pull merge bypass"); + assert!(matches!(bypassed, GitHookAttemptResult::Completed { .. })); + assert_ne!(head_hash(local.path()), head_before); +} + +#[cfg(unix)] +#[test] +fn merge_hook_rollback_failure_reports_both_failures() { + let dir = init_repo(); + git(dir.path(), &["switch", "-c", "feature/rollback-failure"]); + write_file(dir.path(), "feature.txt", "feature change"); + git(dir.path(), &["add", "feature.txt"]); + git(dir.path(), &["commit", "-m", "feature change"]); + git(dir.path(), &["switch", "main"]); + install_hook( + dir.path(), + "pre-merge-commit", + "#!/bin/sh\ntouch .git/index.lock\necho merge hook rejected >&2\nexit 1\n", + ); + + let error = handler() + .merge_branch_with_progress( + &MergeRequest { + repo_path: dir.path().to_string_lossy().into_owned(), + branch_name: "feature/rollback-failure".to_string(), + no_ff: Some(true), + ff_only: None, + message: None, + }, + false, + Arc::new(|_| {}), + ) + .expect_err("rollback should fail while the index is locked"); + + let error_message = error.to_string(); + assert!(error_message.contains("GITMUN_MERGE_HOOK_ROLLBACK_FAILED")); + assert!(error_message.contains("pre-merge-commit")); + assert!(dir.path().join(".git/MERGE_HEAD").exists()); + fs::remove_file(dir.path().join(".git/index.lock")).expect("remove index lock"); +} + +#[cfg(unix)] +#[test] +fn pre_rebase_rejects_pull_without_changing_head_or_offering_bypass() { + let (remote, local) = init_remote_with_clone(); + let peer = clone_test_remote(&remote); + write_file(local.path(), "local.txt", "local change"); + git(local.path(), &["add", "local.txt"]); + git(local.path(), &["commit", "-m", "local change"]); + write_file(peer.path(), "remote.txt", "remote change"); + git(peer.path(), &["add", "remote.txt"]); + git(peer.path(), &["commit", "-m", "remote change"]); + git(peer.path(), &["push", "origin", "main"]); + git(local.path(), &["fetch", "origin"]); + install_hook( + local.path(), + "pre-rebase", + "#!/bin/sh\necho rebase rejected >&2\nexit 1\n", + ); + let head_before = head_hash(local.path()); + + let rejected = handler() + .pull_with_strategy_with_progress( + &PullStrategyRequest { + repo_path: local.path().to_string_lossy().into_owned(), + strategy: PullStrategy::Rebase, + }, + false, + Arc::new(|_| {}), + ) + .expect("pre-rebase rejection"); + + assert!(matches!( + rejected, + GitHookAttemptResult::HookRejected { + ref hook_name, + bypass_supported: false, + .. + } if hook_name == "pre-rebase" + )); + assert_eq!(head_hash(local.path()), head_before); + assert!(!local.path().join(".git/rebase-merge").exists()); + assert!(!local.path().join(".git/rebase-apply").exists()); +} + +#[cfg(unix)] +#[test] +fn rebase_reports_post_rewrite_failure_after_rewriting_history() { + let dir = init_repo(); + git(dir.path(), &["switch", "-c", "feature/rebase-hook"]); + write_file(dir.path(), "feature.txt", "feature change"); + git(dir.path(), &["add", "feature.txt"]); + git(dir.path(), &["commit", "-m", "feature change"]); + let head_before = head_hash(dir.path()); + git(dir.path(), &["switch", "main"]); + write_file(dir.path(), "main.txt", "main change"); + git(dir.path(), &["add", "main.txt"]); + git(dir.path(), &["commit", "-m", "main change"]); + git(dir.path(), &["switch", "feature/rebase-hook"]); + install_hook( + dir.path(), + "post-rewrite", + "#!/bin/sh\necho post-rewrite warning >&2\nexit 1\n", + ); + + let result = handler() + .rebase_start_with_progress( + &RebaseRequest { + repo_path: dir.path().to_string_lossy().into_owned(), + onto: "main".to_string(), + }, + Arc::new(|_| {}), + ) + .expect("rebase result"); + + assert!(matches!( + result, + GitHookAttemptResult::Completed { hook_warning: Some(ref warning), ref result, .. } + if warning.hook_name == "post-rewrite" && result.success + )); + assert_ne!(head_hash(dir.path()), head_before); +} + +#[cfg(unix)] +#[test] +fn successful_rebase_emits_pre_rebase_and_post_rewrite_progress() { + let dir = init_repo(); + git(dir.path(), &["switch", "-c", "feature/rebase-progress"]); + write_file(dir.path(), "feature.txt", "feature change"); + git(dir.path(), &["add", "feature.txt"]); + git(dir.path(), &["commit", "-m", "feature change"]); + git(dir.path(), &["switch", "main"]); + write_file(dir.path(), "main.txt", "main change"); + git(dir.path(), &["add", "main.txt"]); + git(dir.path(), &["commit", "-m", "main change"]); + git(dir.path(), &["switch", "feature/rebase-progress"]); + install_hook(dir.path(), "pre-rebase", "#!/bin/sh\nexit 0\n"); + install_hook(dir.path(), "post-rewrite", "#!/bin/sh\nexit 0\n"); + let events = Arc::new(Mutex::new(Vec::new())); + let recorded_events = Arc::clone(&events); + + let result = handler() + .rebase_start_with_progress( + &RebaseRequest { + repo_path: dir.path().to_string_lossy().into_owned(), + onto: "main".to_string(), + }, + Arc::new(move |event| { + recorded_events.lock().expect("event lock").push(event); + }), + ) + .expect("successful rebase"); + + assert!(matches!( + result, + GitHookAttemptResult::Completed { ref result, hook_warning: None, .. } if result.success + )); + let events = events.lock().expect("event lock"); + for expected_hook in ["pre-rebase", "post-rewrite"] { + assert!(events.iter().any(|event| matches!( + event, + CommitProgressEvent::HookStarted { hook_name } if hook_name == expected_hook + ))); + } +} + +#[cfg(unix)] +#[test] +fn rebase_continue_preserves_conflict_state_and_reports_final_post_rewrite() { + let dir = init_repo(); + write_file(dir.path(), "shared.txt", "base"); + git(dir.path(), &["add", "shared.txt"]); + git(dir.path(), &["commit", "-m", "shared base"]); + git(dir.path(), &["switch", "-c", "feature/rebase-conflict"]); + write_file(dir.path(), "shared.txt", "feature"); + git(dir.path(), &["add", "shared.txt"]); + git(dir.path(), &["commit", "-m", "feature change"]); + git(dir.path(), &["switch", "main"]); + write_file(dir.path(), "shared.txt", "main"); + git(dir.path(), &["add", "shared.txt"]); + git(dir.path(), &["commit", "-m", "main change"]); + git(dir.path(), &["switch", "feature/rebase-conflict"]); + install_hook(dir.path(), "post-rewrite", "#!/bin/sh\nexit 0\n"); + + let started = handler() + .rebase_start_with_progress( + &RebaseRequest { + repo_path: dir.path().to_string_lossy().into_owned(), + onto: "main".to_string(), + }, + Arc::new(|_| {}), + ) + .expect("start conflicting rebase"); + assert!(matches!( + started, + GitHookAttemptResult::Completed { ref result, .. } + if result.has_conflicts && result.rebase_in_progress + )); + + write_file(dir.path(), "shared.txt", "resolved"); + git(dir.path(), &["add", "shared.txt"]); + let events = Arc::new(Mutex::new(Vec::new())); + let recorded_events = Arc::clone(&events); + let continued = handler() + .rebase_continue_with_progress( + &repo_request(&dir), + Arc::new(move |event| { + recorded_events.lock().expect("event lock").push(event); + }), + ) + .expect("continue rebase"); + + assert!(matches!( + continued, + GitHookAttemptResult::Completed { ref result, hook_warning: None, .. } + if result.success && !result.has_conflicts && !result.rebase_in_progress + )); + assert!( + events + .lock() + .expect("event lock") + .iter() + .any(|event| matches!( + event, + CommitProgressEvent::HookStarted { hook_name } if hook_name == "post-rewrite" + )) + ); +} + #[test] fn commit_message_recovery_reads_commit_editmsg() { let dir = init_repo(); diff --git a/src/api/commands.ts b/src/api/commands.ts index 0acbde7..77e0b65 100644 --- a/src/api/commands.ts +++ b/src/api/commands.ts @@ -310,16 +310,25 @@ export function fetchRemote(repoPath: string, remote?: string): Promise("fetch_remote", {request: {repoPath, remote}}); } -export function pullChanges(repoPath: string): Promise { - return invoke("pull_changes", {request: {repoPath}}); +export function pullChanges( + repoPath: string, + onProgress: Channel, + skipHooks = false, +): Promise> { + return invoke>("pull_changes", {request: {repoPath}, onProgress, skipHooks}); } export function analyzePull(repoPath: string): Promise { return invoke("analyze_pull", {request: {repoPath}}); } -export function pullWithStrategy(repoPath: string, strategy: PullStrategy): Promise { - return invoke("pull_with_strategy", {request: {repoPath, strategy}}); +export function pullWithStrategy( + repoPath: string, + strategy: PullStrategy, + onProgress: Channel, + skipHooks = false, +): Promise> { + return invoke>("pull_with_strategy", {request: {repoPath, strategy}, onProgress, skipHooks}); } export function pushChanges(request: PushRequest, onProgress: Channel, skipHooks = false): Promise> { @@ -358,10 +367,14 @@ export function stashDrop(repoPath: string, stashIndex: number): Promise { - return invoke("merge_branch", { + options: { noFf?: boolean; ffOnly?: boolean; message?: string } | undefined, + onProgress: Channel, + skipHooks = false, +): Promise> { + return invoke>("merge_branch", { request: {repoPath, branchName, ...options}, + onProgress, + skipHooks, }); } @@ -369,12 +382,18 @@ export function mergeAbort(repoPath: string): Promise { return invoke("merge_abort", {request: {repoPath}}); } -export function rebaseStart(request: RebaseRequest): Promise { - return invoke("rebase_start", {request}); +export function rebaseStart( + request: RebaseRequest, + onProgress: Channel, +): Promise> { + return invoke>("rebase_start", {request, onProgress}); } -export function rebaseContinue(repoPath: string): Promise { - return invoke("rebase_continue", {request: {repoPath}}); +export function rebaseContinue( + repoPath: string, + onProgress: Channel, +): Promise> { + return invoke>("rebase_continue", {request: {repoPath}, onProgress}); } export function rebaseAbort(repoPath: string): Promise { @@ -499,10 +518,6 @@ export function setRowStriping(rowStriping: RowStriping): Promise { return invoke("set_row_striping", {rowStriping}); } -export function setShowCommitGraphButton(showCommitGraphButton: boolean): Promise { - return invoke("set_show_commit_graph_button", {showCommitGraphButton}); -} - export function setPersistentErrorToasts(persistentErrorToasts: boolean): Promise { return invoke("set_persistent_error_toasts", {persistentErrorToasts}); } diff --git a/src/components/ProjectView.tsx b/src/components/ProjectView.tsx index 13c007d..21c1acf 100644 --- a/src/components/ProjectView.tsx +++ b/src/components/ProjectView.tsx @@ -60,6 +60,7 @@ import type { GitIdentity, GitHookAttemptResult, GitHookFailure, + GitHookOperation, GitHookProgressEvent, GitHookProgressState, ImportPatchRequest, @@ -143,6 +144,8 @@ const PATCH_EXPORT_ERROR_CODES = [ ] as const; const PATCH_IMPORT_APPLIED = "GITMUN_PATCH_IMPORT_APPLIED"; const PATCH_IMPORT_CONFLICTS = "GITMUN_PATCH_IMPORT_CONFLICTS"; +const MERGE_HOOK_ROLLBACK_FAILED = "GITMUN_MERGE_HOOK_ROLLBACK_FAILED"; +const MERGE_HOOK_ROLLBACK_INCOMPLETE = "GITMUN_MERGE_HOOK_ROLLBACK_INCOMPLETE"; const PATCH_IMPORT_MESSAGE_CODES = [ PATCH_IMPORT_APPLIED, PATCH_IMPORT_CONFLICTS, @@ -170,6 +173,13 @@ function localisePatchImportMessage(message: string, t: TFunction<"projectView"> return code ? t(`patch.import.${code}`) : message; } +function localiseMergeHookRollbackError(error: unknown, t: TFunction<"projectView">): string { + const message = String(error); + if (message.includes(MERGE_HOOK_ROLLBACK_FAILED)) return t("toast.mergeHookRollbackFailed"); + if (message.includes(MERGE_HOOK_ROLLBACK_INCOMPLETE)) return t("toast.mergeHookRollbackIncomplete"); + return message; +} + export function buildStashDropPrompt( stash: Pick, t: TFunction<"projectView">, @@ -428,7 +438,7 @@ export function ProjectView({ const operationLockRef = useRef(null); const nextOperationIdRef = useRef(1); const [hookProgress, setHookProgress] = useState(null); - const [hookRejection, setHookRejection] = useState<(GitHookFailure & {operation: "commit" | "push"}) | null>(null); + const [hookRejection, setHookRejection] = useState<(GitHookFailure & {operation: GitHookOperation}) | null>(null); const hookDecisionRef = useRef<((skipHooks: boolean) => void) | null>(null); useEffect(() => () => { @@ -447,7 +457,6 @@ export function ProjectView({ const [rebasedBranchAwaitingPush, setRebasedBranchAwaitingPush] = useState(null); const [wrapDiffLines, setWrapDiffLines] = useState(false); const [rowStriping, setRowStriping] = useState("Off"); - const [showCommitGraphButton, setShowCommitGraphButton] = useState(false); const [showCommitGraph, setShowCommitGraph] = useState(readShowCommitGraphPreference); const [showAiWriting, setShowAiWriting] = useState(false); const [searchQuery, setSearchQuery] = useState(""); @@ -509,7 +518,7 @@ export function ProjectView({ loadMoreError: logLoadMoreError, pageSize: logPageSize, refresh: refreshLog, - } = useGitLog(repoPath, logScope, windowFocused, showCommitGraphButton && showCommitGraph); + } = useGitLog(repoPath, logScope, windowFocused, showCommitGraph); const searching = deferredSearchQuery.length > 0; const visibleCommits = useMemo(() => { if (!searching) return commits; @@ -694,7 +703,6 @@ export function ProjectView({ setPushFollowTags(settings.pushFollowTags ?? false); setWrapDiffLines(settings.wrapDiffLines ?? false); setRowStriping(settings.rowStriping ?? "Off"); - setShowCommitGraphButton(settings.showCommitGraphButton ?? false); } }) .catch(() => { @@ -704,7 +712,6 @@ export function ProjectView({ setPushFollowTags(false); setWrapDiffLines(false); setRowStriping("Off"); - setShowCommitGraphButton(false); } }); @@ -731,7 +738,7 @@ export function ProjectView({ await Promise.all([refreshStatus(), refreshBranches(), refreshTags(), refreshRemotes(), refreshLog(), refreshStashes()]); }, [refreshStatus, refreshBranches, refreshTags, refreshRemotes, refreshLog, refreshStashes]); - const createHookProgressChannel = useCallback((operation: "commit" | "push" | "checkout") => { + const createHookProgressChannel = useCallback((operation: GitHookOperation) => { const progress = new Channel(); setHookProgress({operation, startedAt: Date.now(), phase: "running", hookName: null, output: "", outputTruncated: false, expanded: false}); progress.onmessage = event => { @@ -747,33 +754,54 @@ export function ProjectView({ return progress; }, []); - const runPushHookOperation = useCallback(async ( + const runBlockingHookOperation = useCallback(async ,>( + hookOperation: Exclude, operation: (progress: Channel, skipHooks: boolean) => Promise>, ): Promise => { let skipHooks = false; for (;;) { let attempt: GitHookAttemptResult; try { - attempt = await operation(createHookProgressChannel("push"), skipHooks); + attempt = await operation(createHookProgressChannel(hookOperation), skipHooks); } catch (error) { setHookProgress(null); throw error; } if (attempt.status === "completed") { - setHookProgress(null); - if (skipHooks) appendResultLog("info", t("log.pushHooksSkipped"), "git-cli"); + await refreshAll().catch(() => undefined); + if (attempt.hookWarning) { + setHookProgress(current => ({ + operation: hookOperation, + startedAt: current?.startedAt ?? Date.now(), + phase: "warning", + hookName: attempt.hookWarning?.hookName ?? null, + output: attempt.hookWarning?.output ?? current?.output ?? "", + outputTruncated: attempt.hookWarning?.outputTruncated ?? false, + expanded: true, + })); + showToast(t("toast.hookWarning", {operation: t(`hookOperationNames.${hookOperation}`)}), "info"); + appendResultLog("error", t("log.hookWarning", {operation: t(`hookOperationNames.${hookOperation}`), hook: attempt.hookWarning.hookName}), attempt.result.backendUsed, undefined, attempt.hookWarning.output ?? undefined); + } else { + setHookProgress(null); + } + if (skipHooks) appendResultLog("info", t("log.hooksSkipped", {operation: t(`hookOperationNames.${hookOperation}`)}), "git-cli"); return attempt.result; } setHookProgress(current => current ? {...current, phase: "awaitingDecision", hookName: attempt.hookName, output: attempt.output ?? current.output, outputTruncated: attempt.outputTruncated, expanded: true} : current); - setHookRejection({...attempt, operation: "push"}); + await refreshAll().catch(() => undefined); + setHookRejection({...attempt, operation: hookOperation}); skipHooks = await new Promise(resolve => { hookDecisionRef.current = resolve; }); if (!skipHooks) { - appendResultLog("error", t("log.pushHookRejected", {hook: attempt.hookName}), "git-cli", undefined, attempt.output ?? undefined); + appendResultLog("error", t("log.hookRejected", {operation: t(`hookOperationNames.${hookOperation}`), hook: attempt.hookName}), "git-cli", undefined, attempt.output ?? undefined); setHookProgress(null); return null; } } - }, [createHookProgressChannel, t]); + }, [createHookProgressChannel, refreshAll, showToast, t]); + + const runPushHookOperation = useCallback(>( + operation: (progress: Channel, skipHooks: boolean) => Promise>, + ) => runBlockingHookOperation("push", operation), [runBlockingHookOperation]); const runCheckoutHookOperation = useCallback(async ( operation: (progress: Channel) => Promise>, @@ -840,7 +868,8 @@ export function ProjectView({ showToast, onForcePushComplete: handleForcePushComplete, onFetchAttemptComplete, - pushChanges: request => runPushHookOperation((progress, skipHooks) => api.pushChanges(request, progress, skipHooks)), + pushChanges: request => runBlockingHookOperation("push", (progress, skipHooks) => api.pushChanges(request, progress, skipHooks)), + pullWithStrategy: strategy => runBlockingHookOperation("pull", (progress, skipHooks) => api.pullWithStrategy(repoPath!, strategy, progress, skipHooks)), }); useEffect(() => { @@ -1907,7 +1936,8 @@ export function ProjectView({ setIsRebaseActionRunning(true); try { - const result = await api.rebaseStart({ repoPath, onto: ontoBranch }); + const result = await runBlockingHookOperation("rebase", progress => api.rebaseStart({ repoPath, onto: ontoBranch }, progress)); + if (!result) return; if (result.hasConflicts) { showToast(t("toast.rebaseConflicts", { count: result.conflictedFiles.length }), "error"); appendResultLog("error", result.message, result.backendUsed); @@ -1919,18 +1949,20 @@ export function ProjectView({ } await refreshAll(); } catch (e) { + await refreshAll().catch(() => undefined); showToast(String(e), "error"); appendResultLog("error", t("log.rebaseFailed", { message: String(e) }), "unknown"); } finally { setIsRebaseActionRunning(false); } - }, [repoPath, cherryPickInProgress, mergeInProgress, rebaseInProgress, currentBranch, hasWorkingTreeChanges, refreshAll, showToast, t]); + }, [repoPath, cherryPickInProgress, mergeInProgress, rebaseInProgress, currentBranch, hasWorkingTreeChanges, refreshAll, runBlockingHookOperation, showToast, t]); const handleRebaseContinue = useCallback(async () => { if (!repoPath || !rebaseInProgress) return; setIsRebaseActionRunning(true); try { - const result = await api.rebaseContinue(repoPath); + const result = await runBlockingHookOperation("rebase", progress => api.rebaseContinue(repoPath, progress)); + if (!result) return; if (result.hasConflicts) { showToast(t("toast.rebaseConflicts", { count: result.conflictedFiles.length }), "error"); appendResultLog("error", result.message, result.backendUsed); @@ -1944,12 +1976,13 @@ export function ProjectView({ } await refreshAll(); } catch (e) { + await refreshAll().catch(() => undefined); showToast(String(e), "error"); appendResultLog("error", t("log.rebaseContinueFailed", { message: String(e) }), "unknown"); } finally { setIsRebaseActionRunning(false); } - }, [repoPath, rebaseInProgress, currentBranch, refreshAll, showToast, t]); + }, [repoPath, rebaseInProgress, currentBranch, refreshAll, runBlockingHookOperation, showToast, t]); const handleRebaseAbort = useCallback(async () => { if (!repoPath || !rebaseInProgress) return; @@ -2163,7 +2196,8 @@ export function ProjectView({ noFf: strategy === "no-ff", ffOnly: strategy === "ff-only", }; - const result = await api.mergeBranch(repoPath, mergePendingBranch, options); + const result = await runBlockingHookOperation("merge", (progress, skipHooks) => api.mergeBranch(repoPath, mergePendingBranch, options, progress, skipHooks)); + if (!result) return; if (result.hasConflicts) { showToast(t("toast.mergeConflicts", { count: result.conflictedFiles.length }), "error"); appendResultLog("error", result.message, result.backendUsed); @@ -2174,10 +2208,11 @@ export function ProjectView({ await refreshAll(); setCentreTab("changes"); } catch (e) { - showToast(String(e), "error"); + await refreshAll().catch(() => undefined); + showToast(localiseMergeHookRollbackError(e, t), "error"); appendResultLog("error", t("log.mergeFailed", { message: String(e) }), "unknown"); } - }, [repoPath, mergePendingBranch, refreshAll, showToast, t]); + }, [repoPath, mergePendingBranch, refreshAll, runBlockingHookOperation, showToast, t]); const handleMergeAbort = useCallback(async () => { if (!repoPath) return; @@ -2545,7 +2580,6 @@ export function ProjectView({ commitMarkers={commitMarkers} logScope={logScope} rowStriping={rowStriping} - showCommitGraphButton={showCommitGraphButton} onCommitGraphVisibilityChange={handleCommitGraphVisibilityChange} onLogScopeChange={setLogScope} detachedHead={status?.detachedHead ?? false} diff --git a/src/components/centre/CentrePanel.test.tsx b/src/components/centre/CentrePanel.test.tsx index 1c9b17c..3791684 100644 --- a/src/components/centre/CentrePanel.test.tsx +++ b/src/components/centre/CentrePanel.test.tsx @@ -88,7 +88,6 @@ function renderCentrePanel(overrides: Partial { expect(screen.getByLabelText("Hide commit graph")).toBeInTheDocument(); }); - it("hides the graph button and forces the graph hidden when the setting is off", () => { - localStorage.setItem("gitmun.showCommitGraph", "true"); - - const { container } = renderCentrePanel({ showCommitGraphButton: false }); - - expect(screen.queryByLabelText("Hide commit graph")).not.toBeInTheDocument(); - expect(screen.queryByLabelText("Show commit graph")).not.toBeInTheDocument(); - expect(container.querySelector(".log-view__graph")).toBeNull(); - expect(localStorage.getItem("gitmun.showCommitGraph")).toBe("true"); - }); - it("disables the commit graph while searching without changing the saved preference", () => { localStorage.setItem("gitmun.showCommitGraph", "true"); const onCommitGraphVisibilityChange = vi.fn(); @@ -433,6 +421,52 @@ describe("CentrePanel hook feedback", () => { expect(onBypass).toHaveBeenCalledOnce(); }); + it("offers the pull bypass action when the backend permits it", () => { + renderCentrePanel({ + hookRejection: { + operation: "pull", + hookName: "pre-merge-commit", + exitStatus: 1, + output: null, + outputTruncated: false, + bypassSupported: true, + }, + }); + + expect(screen.getByRole("button", {name: "Pull without hooks"})).toBeInTheDocument(); + }); + + it("offers the merge bypass action when the backend permits it", () => { + renderCentrePanel({ + hookRejection: { + operation: "merge", + hookName: "commit-msg", + exitStatus: 1, + output: null, + outputTruncated: false, + bypassSupported: true, + }, + }); + + expect(screen.getByRole("button", {name: "Merge without hooks"})).toBeInTheDocument(); + }); + + it("does not offer a bypass for a rejected rebase hook", () => { + renderCentrePanel({ + hookRejection: { + operation: "rebase", + hookName: "pre-rebase", + exitStatus: 1, + output: null, + outputTruncated: false, + bypassSupported: false, + }, + }); + + expect(screen.getByRole("button", {name: "Close"})).toBeInTheDocument(); + expect(screen.queryByRole("button", {name: /without hooks/i})).not.toBeInTheDocument(); + }); + it("reports checkout completion with a dismissible warning", () => { const onDismiss = vi.fn(); renderCentrePanel({ diff --git a/src/components/centre/CentrePanel.tsx b/src/components/centre/CentrePanel.tsx index 01d329d..5c836c9 100644 --- a/src/components/centre/CentrePanel.tsx +++ b/src/components/centre/CentrePanel.tsx @@ -70,7 +70,6 @@ type CentrePanelProps = { commitMarkers: CommitMarkers; logScope: CommitLogScope; rowStriping: RowStriping; - showCommitGraphButton: boolean; onCommitGraphVisibilityChange?: (visible: boolean) => void; onLogScopeChange: (scope: CommitLogScope) => void; detachedHead: boolean; @@ -129,7 +128,7 @@ type CentrePanelProps = { stagingOperation: StagingOperation | null; operationLock: LongRunningOperation | null; hookProgress?: GitHookProgressState | null; - hookRejection?: (GitHookFailure & {operation: "commit" | "push"}) | null; + hookRejection?: (GitHookFailure & {operation: GitHookProgressState["operation"]}) | null; onHookRejectionClose?: () => void; onHookRejectionBypass?: () => void; isCommitting: boolean; @@ -159,7 +158,7 @@ function HookProgressBanner({progress, onDismiss}: {progress: GitHookProgressSta return () => window.clearInterval(timer); }, [progress.startedAt]); const title = progress.phase === "warning" - ? t("gitHooks.checkoutWarningTitle") + ? t("gitHooks.warningTitle", {operation: t(`gitHooks.operations.${progress.operation}`)}) : progress.phase === "awaitingDecision" ? t("gitHooks.failedTitle", {operation: t(`gitHooks.operations.${progress.operation}`)}) : progress.hookName @@ -170,7 +169,7 @@ function HookProgressBanner({progress, onDismiss}: {progress: GitHookProgressSta {progress.phase === "running" ? ; } -function HookFailureDialog({failure, onClose, onBypass}: {failure: GitHookFailure & {operation: "commit" | "push"}; onClose: () => void; onBypass: () => void}) { +function HookFailureDialog({failure, onClose, onBypass}: {failure: GitHookFailure & {operation: GitHookProgressState["operation"]}; onClose: () => void; onBypass: () => void}) { const {t} = useTranslation("centre"); const closeButtonRef = React.useRef(null); React.useEffect(() => { closeButtonRef.current?.focus(); }, []); @@ -273,8 +272,7 @@ function getOperationContent( export function CentrePanel(props: CentrePanelProps) { const { t } = useTranslation("centre"); const [showCommitGraph, setShowCommitGraph] = React.useState(readShowCommitGraphPreference); - const preferredShowCommitGraph = props.showCommitGraphButton && showCommitGraph; - const effectiveShowCommitGraph = preferredShowCommitGraph && !props.searching; + const effectiveShowCommitGraph = showCommitGraph && !props.searching; const tab = props.activeTab; const operationContent = getOperationContent(props.operationLock, t); const operationFeedback = useDelayedOperationFeedback(props.operationLock); @@ -286,8 +284,8 @@ export function CentrePanel(props: CentrePanelProps) { const totalChanges = props.stagedFiles.length + props.unstagedFiles.length + props.unversionedFiles.length + submoduleChanges; React.useEffect(() => { - props.onCommitGraphVisibilityChange?.(preferredShowCommitGraph); - }, [preferredShowCommitGraph, props.onCommitGraphVisibilityChange]); + props.onCommitGraphVisibilityChange?.(showCommitGraph); + }, [showCommitGraph, props.onCommitGraphVisibilityChange]); const handleToggleCommitGraph = () => { setShowCommitGraph(previous => { @@ -370,19 +368,17 @@ export function CentrePanel(props: CentrePanelProps) {
{tab === "log" && (
- {props.showCommitGraphButton && ( - - )} +