diff --git a/src-tauri/src/git.rs b/src-tauri/src/git.rs index ab9f1f7..b3962ae 100644 --- a/src-tauri/src/git.rs +++ b/src-tauri/src/git.rs @@ -1,4 +1,5 @@ pub mod submodules; +pub(crate) mod review_cleanup; use serde::{Deserialize, Serialize}; use std::{ collections::{BTreeMap, BTreeSet}, diff --git a/src-tauri/src/git/review_cleanup.rs b/src-tauri/src/git/review_cleanup.rs new file mode 100644 index 0000000..e692cfa --- /dev/null +++ b/src-tauri/src/git/review_cleanup.rs @@ -0,0 +1,455 @@ +use super::*; + +#[derive(Debug)] +pub(crate) struct CleanupPlan { + pub path: String, + remote: String, + remote_url: String, + source: String, + target: String, + source_sha: String, + local_sha: Option, +} + +fn clean_worktree(repo: &Path) -> Result<(), String> { + let status = status_for_repo(repo)?; + if !status.clean + || status.merge_in_progress + || status.rebase_in_progress + || status.cherry_pick_in_progress + { + return Err("Commit or stash local changes and finish pending Git operations before merging with branch cleanup.".into()); + } + Ok(()) +} + +fn validate_branch(repo: &Path, branch: &str) -> Result<(), String> { + if branch.is_empty() || branch.starts_with('-') { + return Err("Invalid review branch.".into()); + } + run_git(repo, ["check-ref-format", &format!("refs/heads/{branch}")])?; + Ok(()) +} + +fn branch_sha(repo: &Path, branch: &str) -> Result, String> { + let refs = run_git( + repo, + [ + "for-each-ref", + "--format=%(refname) %(objectname)", + &format!("refs/heads/{branch}"), + ], + )?; + Ok(String::from_utf8_lossy(&refs).lines().find_map(|line| { + let (reference, sha) = line.split_once(' ')?; + (reference == format!("refs/heads/{branch}")).then(|| sha.to_string()) + })) +} + +fn unused_in_other_worktrees(repo: &Path, branch: &str, allow_current: bool) -> Result<(), String> { + let output = run_git(repo, ["worktree", "list", "--porcelain"])?; + let current = fs::canonicalize(repo).map_err(|err| err.to_string())?; + for entry in String::from_utf8_lossy(&output).split("\n\n") { + if entry + .lines() + .any(|line| line == format!("branch refs/heads/{branch}")) + { + let path = entry + .lines() + .find_map(|line| line.strip_prefix("worktree ")); + if !allow_current + || path.and_then(|path| fs::canonicalize(path).ok()).as_ref() != Some(¤t) + { + return Err(format!( + "Branch '{branch}' is checked out in another worktree. No branch was deleted." + )); + } + } + } + Ok(()) +} + +// Compare server-provided clone URLs exactly apart from trailing slash/.git. +// Never infer a destructive target from just a repository or branch name. +fn same_url(left: &str, right: &str) -> bool { + fn clean(value: &str) -> &str { + value.trim().trim_end_matches('/').trim_end_matches(".git") + } + clean(left) == clean(right) +} + +fn check_remote(repo: &Path, remote: &str, urls: &[String]) -> Result { + let fetch = run_git(repo, ["remote", "get-url", "--all", remote])?; + let push = run_git(repo, ["remote", "get-url", "--push", "--all", remote])?; + let fetch = String::from_utf8_lossy(&fetch); + let push = String::from_utf8_lossy(&push); + if fetch.lines().count() != 1 + || push.lines().count() != 1 + || !fetch + .lines() + .chain(push.lines()) + .all(|url| urls.iter().any(|expected| same_url(url, expected))) + { + return Err( + "The local remote's fetch and push URLs must both match the PR repository.".into(), + ); + } + Ok(fetch.trim().to_string()) +} + +fn ancestor(repo: &Path, older: &str, newer: &str) -> bool { + run_git(repo, ["merge-base", "--is-ancestor", older, newer]).is_ok() +} + +pub(crate) fn prepare( + path: &str, + source: &str, + target: &str, + source_sha: &str, + urls: &[String], + username: &str, + token: &str, +) -> Result { + let repo = resolve_repo(path)?; + validate_branch(&repo, source)?; + validate_branch(&repo, target)?; + if source == target { + return Err("Source and target branch must differ.".into()); + } + if !matches!(source_sha.len(), 40 | 64) + || !source_sha.bytes().all(|byte| byte.is_ascii_hexdigit()) + { + return Err("The provider did not return a valid PR source commit.".into()); + } + clean_worktree(&repo)?; + unused_in_other_worktrees(&repo, source, true)?; + unused_in_other_worktrees(&repo, target, true)?; + let remotes = run_git(&repo, ["remote"])?; + let (remote, remote_url) = String::from_utf8_lossy(&remotes).lines() + .find_map(|remote| check_remote(&repo, remote, urls).ok().map(|url| (remote.to_string(), url))) + .ok_or("Open the local repository matching this PR, with a matching fetch and push remote, before enabling branch cleanup.")?; + run_git_authenticated( + &repo, + [ + "fetch", + "--no-tags", + &remote_url, + &format!("+refs/heads/{source}:refs/remotes/{remote}/{source}"), + &format!("+refs/heads/{target}:refs/remotes/{remote}/{target}"), + ], + username, + token, + )?; + if verify_commit(&repo, &format!("refs/remotes/{remote}/{source}"))? != source_sha { + return Err("The PR source branch changed. Refresh the PR before merging.".into()); + } + let local_sha = branch_sha(&repo, source)?; + if local_sha + .as_ref() + .is_some_and(|sha| !ancestor(&repo, sha, source_sha)) + { + return Err("The local source branch contains commits outside this PR. Push or preserve them before enabling cleanup.".into()); + } + if branch_sha(&repo, target)? + .as_ref() + .is_some_and(|sha| !ancestor(&repo, sha, &format!("refs/remotes/{remote}/{target}"))) + { + return Err("The local target branch has diverged or contains unpushed commits. Synchronize it before enabling cleanup.".into()); + } + Ok(CleanupPlan { + path: repo.to_string_lossy().into_owned(), + remote, + remote_url, + source: source.into(), + target: target.into(), + source_sha: source_sha.into(), + local_sha, + }) +} + +pub(crate) fn finish(plan: &CleanupPlan, username: &str, token: &str) -> Result<(), String> { + let repo = resolve_repo(&plan.path)?; + clean_worktree(&repo)?; + // A changed remote config must not redirect cleanup after the merge. + let current_fetch = remote_url_for(&repo, &plan.remote).unwrap_or_default(); + if !same_url(¤t_fetch, &plan.remote_url) { + return Err("The local remote changed; branch cleanup was stopped.".into()); + } + if branch_sha(&repo, &plan.source)? != plan.local_sha { + return Err("The local source branch changed during the merge; it was preserved.".into()); + } + unused_in_other_worktrees(&repo, &plan.source, true)?; + unused_in_other_worktrees(&repo, &plan.target, true)?; + let target_ref = format!("refs/remotes/{}/{}", plan.remote, plan.target); + run_git_authenticated( + &repo, + [ + "fetch", + "--no-tags", + &plan.remote_url, + &format!("+refs/heads/{}:{target_ref}", plan.target), + ], + username, + token, + )?; + if let Some(local_target) = branch_sha(&repo, &plan.target)? { + if !ancestor(&repo, &local_target, &target_ref) { + return Err("The local target branch has diverged; branches were preserved.".into()); + } + run_git(&repo, ["checkout", &plan.target])?; + run_git(&repo, ["merge", "--ff-only", &target_ref])?; + } else { + run_git( + &repo, + ["checkout", "--track", "-b", &plan.target, &target_ref], + )?; + } + clean_worktree(&repo)?; + let source_ref = format!("refs/heads/{}", plan.source); + let remote_heads = run_git_authenticated( + &repo, + ["ls-remote", "--heads", &plan.remote_url, &source_ref], + username, + token, + )?; + if let Some(sha) = String::from_utf8_lossy(&remote_heads) + .lines() + .find_map(|line| { + let (sha, reference) = line.split_once('\t')?; + (reference == source_ref).then_some(sha) + }) + { + if sha != plan.source_sha { + return Err("The remote source branch has new commits; it was preserved.".into()); + } + run_git_authenticated( + &repo, + [ + "push", + &format!("--force-with-lease={source_ref}:{}", plan.source_sha), + &plan.remote_url, + &format!(":{source_ref}"), + ], + username, + token, + )?; + } + if let Some(local_sha) = &plan.local_sha { + unused_in_other_worktrees(&repo, &plan.source, false)?; + // Expected-old-value deletion is safe even after squash/rebase, and rejects concurrent updates. + run_git(&repo, ["update-ref", "-d", &source_ref, local_sha])?; + if git_config_value(&repo, &format!("branch.{}.remote", plan.source)).is_some() { + run_git( + &repo, + [ + "config", + "--remove-section", + &format!("branch.{}", plan.source), + ], + )?; + } + } + let tracking_ref = format!("refs/remotes/{}/{}", plan.remote, plan.source); + // Do not remove a tracking ref that was advanced concurrently. + run_git(&repo, ["update-ref", "-d", &tracking_ref, &plan.source_sha])?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + struct Fixture { + root: PathBuf, + local: PathBuf, + server: PathBuf, + remote: String, + } + impl Drop for Fixture { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.root); + } + } + impl Fixture { + fn new() -> Self { + static NEXT: AtomicU64 = AtomicU64::new(0); + let root = env::temp_dir().join(format!( + "gitty-review-cleanup-{}-{}", + std::process::id(), + NEXT.fetch_add(1, Ordering::Relaxed) + )); + fs::create_dir_all(&root).unwrap(); + let local = root.join("local"); + let server = root.join("server"); + let remote = root.join("remote.git").to_string_lossy().into_owned(); + run_git(&root, ["init", "--bare", &remote]).unwrap(); + run_git(&root, ["init", "-b", "release", local.to_str().unwrap()]).unwrap(); + run_git(&local, ["config", "user.name", "QA"]).unwrap(); + run_git(&local, ["config", "user.email", "qa@example.test"]).unwrap(); + run_git(&local, ["config", "commit.gpgsign", "false"]).unwrap(); + run_git(&local, ["commit", "--allow-empty", "-m", "base"]).unwrap(); + run_git(&local, ["remote", "add", "origin", &remote]).unwrap(); + run_git(&local, ["push", "-u", "origin", "release"]).unwrap(); + run_git(&local, ["checkout", "-b", "feature"]).unwrap(); + fs::write(local.join("feature.txt"), "feature").unwrap(); + run_git(&local, ["add", "."]).unwrap(); + run_git(&local, ["commit", "-m", "feature"]).unwrap(); + run_git(&local, ["push", "-u", "origin", "feature"]).unwrap(); + run_git( + &root, + ["clone", "-b", "release", &remote, server.to_str().unwrap()], + ) + .unwrap(); + run_git(&server, ["config", "user.name", "QA"]).unwrap(); + run_git(&server, ["config", "user.email", "qa@example.test"]).unwrap(); + run_git(&server, ["config", "commit.gpgsign", "false"]).unwrap(); + Self { + root, + local, + server, + remote, + } + } + fn plan(&self) -> Result { + let sha = verify_commit(&self.local, "refs/remotes/origin/feature")?; + prepare( + self.local.to_str().unwrap(), + "feature", + "release", + &sha, + &[self.remote.clone()], + "", + "", + ) + } + fn merge(&self, squash: bool) { + if squash { + run_git(&self.server, ["merge", "--squash", "origin/feature"]).unwrap(); + run_git(&self.server, ["commit", "-m", "squashed PR"]).unwrap(); + } else { + run_git( + &self.server, + ["merge", "--no-ff", "-m", "merged PR", "origin/feature"], + ) + .unwrap(); + } + run_git(&self.server, ["push", "origin", "release"]).unwrap(); + } + fn remote_source_exists(&self) -> bool { + !run_git( + &self.local, + ["ls-remote", "--heads", "origin", "refs/heads/feature"], + ) + .unwrap() + .is_empty() + } + } + #[test] + fn review_cleanup_switches_to_actual_target_and_deletes_after_merge_and_squash() { + for squash in [false, true] { + let fixture = Fixture::new(); + let plan = fixture.plan().unwrap(); + fixture.merge(squash); + finish(&plan, "", "").unwrap(); + assert_eq!( + status_for_repo(&fixture.local) + .unwrap() + .current_branch + .as_deref(), + Some("release") + ); + assert_eq!( + verify_commit(&fixture.local, "HEAD").unwrap(), + verify_commit(&fixture.server, "HEAD").unwrap() + ); + assert!(branch_sha(&fixture.local, "feature").unwrap().is_none()); + assert!(!fixture.remote_source_exists()); + assert!(git_config_value(&fixture.local, "branch.feature.remote").is_none()); + } + } + #[test] + fn review_cleanup_accepts_server_auto_deletion_and_missing_local_target() { + let fixture = Fixture::new(); + run_git(&fixture.local, ["branch", "-D", "release"]).unwrap(); + let plan = fixture.plan().unwrap(); + fixture.merge(true); + run_git(&fixture.server, ["push", "origin", "--delete", "feature"]).unwrap(); + finish(&plan, "", "").unwrap(); + assert_eq!( + status_for_repo(&fixture.local) + .unwrap() + .current_branch + .as_deref(), + Some("release") + ); + assert!(branch_sha(&fixture.local, "feature").unwrap().is_none()); + } + #[test] + fn review_cleanup_preserves_dirty_and_unpushed_local_work() { + let fixture = Fixture::new(); + fs::write(fixture.local.join("untracked.txt"), "keep").unwrap(); + assert!(fixture.plan().unwrap_err().contains("Commit or stash")); + fs::remove_file(fixture.local.join("untracked.txt")).unwrap(); + run_git( + &fixture.local, + ["commit", "--allow-empty", "-m", "unpushed"], + ) + .unwrap(); + assert!(fixture.plan().unwrap_err().contains("outside this PR")); + assert!(fixture.remote_source_exists()); + } + #[test] + fn review_cleanup_preserves_concurrent_local_or_remote_commits() { + for local in [false, true] { + let fixture = Fixture::new(); + let plan = fixture.plan().unwrap(); + fixture.merge(true); + if local { + run_git( + &fixture.local, + ["commit", "--allow-empty", "-m", "new local work"], + ) + .unwrap(); + } else { + run_git(&fixture.server, ["checkout", "feature"]).unwrap(); + run_git( + &fixture.server, + ["commit", "--allow-empty", "-m", "new remote work"], + ) + .unwrap(); + run_git(&fixture.server, ["push", "origin", "feature"]).unwrap(); + } + assert!(finish(&plan, "", "").is_err()); + assert!(branch_sha(&fixture.local, "feature").unwrap().is_some()); + assert!(fixture.remote_source_exists()); + } + } + #[test] + fn review_cleanup_rejects_other_push_repository_and_busy_worktrees() { + let fixture = Fixture::new(); + run_git( + &fixture.local, + [ + "remote", + "set-url", + "--push", + "origin", + "/tmp/different-repository.git", + ], + ) + .unwrap(); + assert!(fixture.plan().is_err()); + run_git( + &fixture.local, + ["remote", "set-url", "--push", "origin", &fixture.remote], + ) + .unwrap(); + let linked = fixture.root.join("linked"); + run_git( + &fixture.local, + ["worktree", "add", linked.to_str().unwrap(), "release"], + ) + .unwrap(); + assert!(fixture.plan().unwrap_err().contains("another worktree")); + assert!(fixture.remote_source_exists()); + } +} diff --git a/src-tauri/src/integrations.rs b/src-tauri/src/integrations.rs index 5f1e654..f086f4a 100644 --- a/src-tauri/src/integrations.rs +++ b/src-tauri/src/integrations.rs @@ -1,3 +1,4 @@ +mod cleanup; mod merge; pub use merge::get_integration_review_merge_options; use merge::merge_payload; @@ -1197,26 +1198,31 @@ pub async fn run_integration_review_action( number: u64, action: String, merge_method: Option, + cleanup_path: Option, ) -> Result<(), String> { - tokio::time::timeout( - REVIEW_REQUEST_TIMEOUT, - tauri::async_runtime::spawn_blocking(move || { + tauri::async_runtime::spawn_blocking(move || { if token.trim().is_empty() { return Err("No token is stored for this integration.".to_string()); } if !matches!(action.as_str(), "merge" | "approve" | "close" | "reopen") { return Err("Unsupported review action.".to_string()); } if action == "merge" { merge_payload(&provider, merge_method.as_deref())?; } let base_url = normalized_base_url(&base_url)?; let client = client()?; + let cleanup = if action == "merge" { + cleanup_path.as_deref().map(|path| cleanup::prepare(&client, &base_url, &username, &token, &provider, &repository_id, &repository_name, number, path)).transpose()? + } else { None }; match provider.as_str() { "github" => github_review_action(&client, &base_url, &token, &repository_name, number, &action, merge_method.as_deref()), "gitlab" | "gitlab-self-hosted" => gitlab_review_action(&client, &base_url, &token, &repository_id, number, &action, merge_method.as_deref()), "gitea" => gitea_review_action(&client, &base_url, &token, &repository_name, number, &action, merge_method.as_deref()), "azure-devops" => azure_review_action(&client, &base_url, &username, &token, &repository_name, &repository_id, number, &action, merge_method.as_deref()), _ => Err("Unsupported integration provider.".to_string()), + }?; + if let Some(cleanup) = cleanup { + cleanup::finish(&cleanup, &client, &base_url, &username, &token, &provider, &repository_id, &repository_name, number) + .map_err(|err| format!("MERGE_ACCEPTED_CLEANUP_FAILED: {err}"))?; } - }), - ) + Ok(()) + }) .await - .map_err(|_| "The integration API did not respond within 35 seconds.".to_string())? .map_err(|err| format!("Could not update review request: {err}"))? } diff --git a/src-tauri/src/integrations/cleanup.rs b/src-tauri/src/integrations/cleanup.rs new file mode 100644 index 0000000..f72b8d8 --- /dev/null +++ b/src-tauri/src/integrations/cleanup.rs @@ -0,0 +1,279 @@ +use super::*; +use crate::git::review_cleanup::{self, CleanupPlan}; + +struct ReviewHead { + source: String, + target: String, + sha: String, + merged: bool, + urls: Vec, +} + +fn read_payload( + client: &Client, + base: &str, + username: &str, + token: &str, + provider: &str, + repository_id: &str, + repository_name: &str, + number: u64, +) -> Result { + let request = match provider { + "github" => client + .get(format!( + "{}/repos/{repository_name}/pulls/{number}", + github_api_base_url(base)? + )) + .bearer_auth(token) + .header(ACCEPT, "application/vnd.github+json"), + "gitea" => client + .get(format!( + "{base}/api/v1/repos/{repository_name}/pulls/{number}" + )) + .header("Authorization", format!("token {token}")), + "gitlab" | "gitlab-self-hosted" => client + .get(format!( + "{base}/api/v4/projects/{repository_id}/merge_requests/{number}" + )) + .header("PRIVATE-TOKEN", token), + "azure-devops" => client + .get(azure_review_endpoint( + base, + repository_name, + repository_id, + number, + )?) + .basic_auth( + if username.is_empty() { + "gitty" + } else { + username + }, + Some(token), + ), + _ => return Err("Unsupported integration provider.".into()), + }; + let response = request + .header(USER_AGENT, "Gitty") + .send() + .map_err(|err| format!("Could not verify PR cleanup: {err}"))?; + if !response.status().is_success() { + return Err(response_error(response, provider)); + } + response + .json() + .map_err(|err| format!("Could not read PR cleanup details: {err}")) +} + +fn review_head(provider: &str, payload: &serde_json::Value) -> Result { + let (source, target, sha, merged, repository) = match provider { + "github" | "gitea" => { + let head_id = payload.pointer("/head/repo/id").filter(|id| !id.is_null()); + let base_id = payload.pointer("/base/repo/id").filter(|id| !id.is_null()); + if head_id.is_none() || head_id != base_id { + return Err("Automatic local and remote cleanup is only supported for PRs within the same repository. Disable cleanup for this fork PR.".into()); + } + ( + value_string(payload, &["head", "ref"]), + value_string(payload, &["base", "ref"]), + value_string(payload, &["head", "sha"]), + payload.get("merged").and_then(serde_json::Value::as_bool) == Some(true), + &payload["base"]["repo"], + ) + } + "gitlab" | "gitlab-self-hosted" => { + let source_id = payload.get("source_project_id").filter(|id| !id.is_null()); + if source_id.is_none() || source_id != payload.get("target_project_id") { + return Err("Automatic cleanup is only supported for merge requests within the same project. Disable cleanup for this fork MR.".into()); + } + ( + value_string(payload, &["source_branch"]), + value_string(payload, &["target_branch"]), + value_string(payload, &["sha"]), + value_string(payload, &["state"]) == "merged", + &payload["gitty_repository"], + ) + } + "azure-devops" => { + if payload + .get("forkSource") + .is_some_and(|fork| !fork.is_null()) + { + return Err( + "Automatic cleanup is not supported for fork PRs. Disable cleanup for this PR." + .into(), + ); + } + ( + value_string(payload, &["sourceRefName"]) + .trim_start_matches("refs/heads/") + .into(), + value_string(payload, &["targetRefName"]) + .trim_start_matches("refs/heads/") + .into(), + value_string(payload, &["lastMergeSourceCommit", "commitId"]), + value_string(payload, &["status"]) == "completed", + &payload["repository"], + ) + } + _ => return Err("Unsupported integration provider.".into()), + }; + let default_branch = value_string( + repository, + &[if provider == "azure-devops" { + "defaultBranch" + } else { + "default_branch" + }], + ); + if !source.is_empty() && source == default_branch.trim_start_matches("refs/heads/") { + return Err("The repository's default branch cannot be deleted by PR cleanup.".into()); + } + let urls = [ + "clone_url", + "ssh_url", + "http_url_to_repo", + "ssh_url_to_repo", + "remoteUrl", + "sshUrl", + ] + .iter() + .filter_map(|key| { + repository + .get(key) + .and_then(serde_json::Value::as_str) + .filter(|url| !url.is_empty()) + .map(str::to_string) + }) + .collect(); + Ok(ReviewHead { + source, + target, + sha, + merged, + urls, + }) +} + +pub(super) struct PreparedCleanup { + plan: CleanupPlan, + source: String, + target: String, + sha: String, +} + +pub(super) fn prepare( + client: &Client, + base: &str, + username: &str, + token: &str, + provider: &str, + repository_id: &str, + repository_name: &str, + number: u64, + path: &str, +) -> Result { + let mut payload = read_payload( + client, + base, + username, + token, + provider, + repository_id, + repository_name, + number, + )?; + if provider.starts_with("gitlab") { + let response = client + .get(format!("{base}/api/v4/projects/{repository_id}")) + .header(USER_AGENT, "Gitty") + .header("PRIVATE-TOKEN", token) + .send() + .map_err(|err| err.to_string())?; + if !response.status().is_success() { + return Err(response_error(response, "GitLab")); + } + payload["gitty_repository"] = response.json().map_err(|err| err.to_string())?; + } + let head = review_head(provider, &payload)?; + if head.merged { + return Err("This PR is already merged. Refresh the review list.".into()); + } + let plan = review_cleanup::prepare( + path, + &head.source, + &head.target, + &head.sha, + &head.urls, + username, + token, + )?; + Ok(PreparedCleanup { + plan, + source: head.source, + target: head.target, + sha: head.sha, + }) +} + +pub(super) fn finish( + prepared: &PreparedCleanup, + client: &Client, + base: &str, + username: &str, + token: &str, + provider: &str, + repository_id: &str, + repository_name: &str, + number: u64, +) -> Result<(), String> { + // Providers may accept a merge asynchronously. Never delete a branch on acceptance alone. + for attempt in 0..10 { + let payload = read_payload( + client, + base, + username, + token, + provider, + repository_id, + repository_name, + number, + )?; + let head = review_head(provider, &payload)?; + if head.source != prepared.source + || head.target != prepared.target + || head.sha != prepared.sha + { + return Err( + "The PR branches or source commit changed during merging. Branches were preserved." + .into(), + ); + } + if head.merged { + return review_cleanup::finish(&prepared.plan, username, token); + } + if attempt < 9 { + std::thread::sleep(Duration::from_millis(500)); + } + } + Err("The provider has not confirmed that the merge completed. No branches were deleted.".into()) +} + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn cleanup_rejects_forks_and_default_branch_and_reads_merge_confirmation() { + let mut payload = serde_json::json!({"head":{"ref":"feature","sha":"abc","repo":{"id":1}},"base":{"ref":"release","repo":{"id":1,"default_branch":"main","clone_url":"https://git.test/team/app.git"}},"merged":true}); + let head = review_head("gitea", &payload).unwrap(); + assert!(head.merged); + assert_eq!(head.target, "release"); + assert_eq!(head.urls, ["https://git.test/team/app.git"]); + payload["head"]["repo"]["id"] = serde_json::json!(2); + assert!(review_head("gitea", &payload).is_err()); + payload["head"]["repo"]["id"] = serde_json::json!(1); + payload["head"]["ref"] = serde_json::json!("main"); + assert!(review_head("github", &payload).is_err()); + } +} diff --git a/src/App.svelte b/src/App.svelte index 41d45b2..1c6af8b 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -6117,6 +6117,14 @@ localResolutionMessage={reviewConflictMessage} loadCredential={loadStoredCredential} onOpenSettings={() => { appSettingsOpen = true; }} + onCleanupStateChange={async (busy, path) => { + if (busy) { + operation = appLanguage === "de" ? "PR zusammenführen und Branch aufräumen" : "Merging PR and cleaning up branch"; + } else { + try { if (sameRepoPath(activeRepoPath, path)) await refreshRepositorySnapshot(path); } + finally { operation = ""; } + } + }} onStartLocalResolution={startReviewConflictResolution} onOpenLocalResolver={reopenReviewConflictResolver} onContinueLocalResolution={continueReviewConflictMerge} diff --git a/src/lib/components/ReviewCenter.svelte b/src/lib/components/ReviewCenter.svelte index 7b7b899..63e9605 100644 --- a/src/lib/components/ReviewCenter.svelte +++ b/src/lib/components/ReviewCenter.svelte @@ -31,6 +31,7 @@ localResolutionMessage?: string; loadCredential: (key: string) => Promise; onOpenSettings: () => void; + onCleanupStateChange?: (busy: boolean, path: string) => void | Promise; onStartLocalResolution?: (request: IntegrationReviewRequest, sourceId: string) => void | Promise; onOpenLocalResolver?: () => void | Promise; onContinueLocalResolution?: () => void | Promise; @@ -38,7 +39,7 @@ onPushLocalResolution?: () => void | Promise; } - let { aiSettings, localRepositoryPath = "", language = "en", integrations, initialQuery = "", initialSourceId = "", localResolutionRequestId = "", localResolutionPhase = "idle", localResolutionMessage = "", loadCredential, onOpenSettings = () => {}, onStartLocalResolution = () => {}, onOpenLocalResolver = () => {}, onContinueLocalResolution = () => {}, onAbortLocalResolution = () => {}, onPushLocalResolution = () => {} }: Props = $props(); + let { aiSettings, localRepositoryPath = "", language = "en", integrations, initialQuery = "", initialSourceId = "", localResolutionRequestId = "", localResolutionPhase = "idle", localResolutionMessage = "", loadCredential, onOpenSettings = () => {}, onCleanupStateChange = () => {}, onStartLocalResolution = () => {}, onOpenLocalResolver = () => {}, onContinueLocalResolution = () => {}, onAbortLocalResolution = () => {}, onPushLocalResolution = () => {} }: Props = $props(); let createOpen = $state(false); let requests = $state([]); let loading = $state(false); @@ -400,27 +401,29 @@ } let reviewConfirmRequest = $state(null); - let reviewConfirmResolve: ((result: { confirmed: boolean; method?: IntegrationMergeMethod }) => void) | null = null; + let reviewConfirmResolve: ((result: { confirmed: boolean; method?: IntegrationMergeMethod; deleteBranch?: boolean }) => void) | null = null; - function askReviewConfirmation(request: IntegrationReviewRequest, action: IntegrationReviewAction, mergeOptions?: IntegrationMergeOptions): Promise<{ confirmed: boolean; method?: IntegrationMergeMethod }> { + function askReviewConfirmation(request: IntegrationReviewRequest, action: IntegrationReviewAction, mergeOptions?: IntegrationMergeOptions): Promise<{ confirmed: boolean; method?: IntegrationMergeMethod; deleteBranch?: boolean }> { const values = { number: request.number }; reviewConfirmRequest = action === "merge" - ? { title: t("confirm.review.mergeTitle", values), message: t("confirm.review.mergeMessage"), items: [request.title], confirmLabel: t("confirm.review.mergeAction"), danger: false, select: mergeOptions ? { label: de ? "Merge-Methode" : "Merge method", value: mergeOptions.defaultMethod, options: mergeOptions.methods.map(method => ({ value: method, label: mergeMethodLabel(method, request) })) } : undefined } + ? { title: t("confirm.review.mergeTitle", values), message: t("confirm.review.mergeMessage"), items: [request.title], confirmLabel: t("confirm.review.mergeAction"), danger: false, + checkbox: { label: de ? "Quellbranch remote und lokal löschen" : "Delete source branch remotely and locally", note: de ? `Nach dem Merge zu „${request.targetBranch}“ wechseln und „${request.sourceBranch}“ löschen. Ein passendes lokales Repository ohne ungesicherte Änderungen muss geöffnet sein.` : `After merging, switch to “${request.targetBranch}” and delete “${request.sourceBranch}”. Open the matching local repository with a clean working tree first.`, defaultChecked: false }, + select: mergeOptions ? { label: de ? "Merge-Methode" : "Merge method", value: mergeOptions.defaultMethod, options: mergeOptions.methods.map(method => ({ value: method, label: mergeMethodLabel(method, request) })) } : undefined } : action === "close" ? { title: t("confirm.review.closeTitle", values), message: t("confirm.review.closeMessage"), items: [request.title], confirmLabel: t("confirm.review.closeAction") } : { title: t("confirm.review.reopenTitle", values), message: t("confirm.review.reopenMessage"), items: [request.title], confirmLabel: t("confirm.review.reopenAction"), danger: false }; - return new Promise<{ confirmed: boolean; method?: IntegrationMergeMethod }>((resolve) => { + return new Promise<{ confirmed: boolean; method?: IntegrationMergeMethod; deleteBranch?: boolean }>((resolve) => { reviewConfirmResolve = resolve; }); } - function answerReviewConfirmation(confirmed: boolean, value?: string) { + function answerReviewConfirmation(confirmed: boolean, value?: string, checked = false) { const method = reviewConfirmRequest?.select?.options.find(option => option.value === value)?.value as IntegrationMergeMethod | undefined; const resolve = reviewConfirmResolve; reviewConfirmRequest = null; reviewConfirmResolve = null; - resolve?.({ confirmed, method }); + resolve?.({ confirmed, method, deleteBranch: checked }); } async function performReviewAction(request: IntegrationReviewRequest, action: IntegrationReviewAction) { @@ -430,6 +433,7 @@ actionNotice = ""; actionBusyId = request.id; errors = []; + let cleanupPath: string | undefined; try { const credential = await withTimeout(loadCredential(integrationCredentialKey(source.provider, source.accountId)), `${source.label} keychain`, 15_000); if (!credential?.password) throw new Error(de ? "Kein Token für diese Integration gespeichert." : "No token is stored for this integration."); @@ -440,15 +444,35 @@ const result = await askReviewConfirmation(request, action, options); if (!result.confirmed) return; mergeMethod = result.method; + if (action === "merge" && result.deleteBranch) { + if (!localRepositoryPath) throw new Error(de ? "Öffne zuerst das passende lokale Repository, um den Quellbranch remote und lokal zu löschen." : "Open the matching local repository before deleting the source branch remotely and locally."); + cleanupPath = localRepositoryPath; + await onCleanupStateChange(true, cleanupPath); + } + } + let cleanupError = ""; + try { + // Cleanup can include fetch, checkout and push; do not time out a still-running mutation. + await runIntegrationReviewAction(source.provider, source.baseUrl, credential.username, credential.password, request, action, mergeMethod, cleanupPath); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + if (!message.startsWith("MERGE_ACCEPTED_CLEANUP_FAILED:")) throw error; + cleanupError = (de ? "Merge vom Anbieter angenommen, Branch-Aufräumen nicht abgeschlossen: " : "Merge accepted by the provider, branch cleanup incomplete: ") + message.replace("MERGE_ACCEPTED_CLEANUP_FAILED:", "").trim(); } - await withTimeout(runIntegrationReviewAction(source.provider, source.baseUrl, credential.username, credential.password, request, action, mergeMethod), source.label); - actionNotice = de ? `${reviewActionLabel(action)} erfolgreich.` : `${reviewActionLabel(action)} succeeded.`; requests = []; loadedStates = new Set(); await loadRequests(stateFilter === "draft" ? "open" : stateFilter); + if (cleanupError) errors = [...errors, { source: source.label, message: cleanupError }]; + else actionNotice = cleanupPath + ? (de ? `PR zusammengeführt. Zu „${request.targetBranch}“ gewechselt und „${request.sourceBranch}“ remote und lokal gelöscht.` : `PR merged. Switched to “${request.targetBranch}” and deleted “${request.sourceBranch}” remotely and locally.`) + : (de ? `${reviewActionLabel(action)} erfolgreich.` : `${reviewActionLabel(action)} succeeded.`); } catch (error) { errors = [{ source: source.label, message: error instanceof Error ? error.message : String(error) }]; } finally { + if (cleanupPath) { + try { await onCleanupStateChange(false, cleanupPath); } + catch (error) { errors = [...errors, { source: source.label, message: String(error) }]; } + } actionBusyId = ""; } } @@ -743,7 +767,7 @@ {#if reviewConfirmRequest} answerReviewConfirmation(true, value)} + onConfirm={({ value, checked }) => answerReviewConfirmation(true, value, checked)} onCancel={() => answerReviewConfirmation(false)} /> {/if} diff --git a/src/lib/git.ts b/src/lib/git.ts index 62a35aa..9864203 100644 --- a/src/lib/git.ts +++ b/src/lib/git.ts @@ -72,8 +72,8 @@ export function getIntegrationReviewMergeOptions(provider: GitIntegrationProvide return invoke("get_integration_review_merge_options", { provider, baseUrl, token, repositoryId: request.repositoryId, repositoryName: request.repositoryName }); } -export function runIntegrationReviewAction(provider: GitIntegrationProvider, baseUrl: string, username: string, token: string, request: IntegrationReviewRequest, action: IntegrationReviewAction, mergeMethod?: IntegrationMergeMethod): Promise { - return invoke("run_integration_review_action", { provider, baseUrl, username, token, repositoryId: request.repositoryId, repositoryName: request.repositoryName, number: request.number, action, mergeMethod }); +export function runIntegrationReviewAction(provider: GitIntegrationProvider, baseUrl: string, username: string, token: string, request: IntegrationReviewRequest, action: IntegrationReviewAction, mergeMethod?: IntegrationMergeMethod, cleanupPath?: string): Promise { + return invoke("run_integration_review_action", { provider, baseUrl, username, token, repositoryId: request.repositoryId, repositoryName: request.repositoryName, number: request.number, action, mergeMethod, cleanupPath }); } export function getIntegrationReviewDetails(provider: GitIntegrationProvider, baseUrl: string, username: string, token: string, request: IntegrationReviewRequest): Promise {