Christoph/GitLite
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects 1 Releases 46 Wiki Activity

feat(integrations): support automatic branch cleanup after merge

Browse Source 
Add a new git::review_cleanup module that implements a CleanupPlan with
prepare() and finish() routines to safely remove/clean tracking and local
branches after a PR/MR is merged. The cleanup logic validates branch names,
ensures a clean worktree, checks remotes/URLs, verifies commits/ancestry,
protects against concurrent worktrees or divergent local/remote commits, and
performs authenticated fetch/push and ref updates. Unit tests for the cleanup
behavior are included.

Wire provider-side cleanup into integrations:
- add an integrations/cleanup module to read provider PR payloads and derive
  cleanup inputs
- run cleanup::prepare(...) before performing a merge when an optional
  cleanup_path is provided
- after a successful provider merge, run cleanup::finish(...); any failure is
  reported as MERGE_ACCEPTED_CLEANUP_FAILED

Also:
- export the new git review_cleanup module (src-tauri/src/git.rs)
- accept an optional cleanup_path parameter in run_integration_review_action
- remove the previous REVIEW_REQUEST_TIMEOUT wrapper around the spawned
  blocking task (the integration action is no longer wrapped with the 35s timeout)
This commit is contained in:
Christoph
2026-09-18 15:22:57 +02:00
parent 6a40159f9f
commit 5db4f36abf
7 changed files with 791 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src-tauri/src/git/review_cleanup.rs
+455
View File
@@ -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<String>,
}
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<Option<String>, 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(&current)
{
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<String, String> {
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<CleanupPlan, String> {
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(&current_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<CleanupPlan, String> {
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());
}
}