From 7cddb14fb15435d10ffad06c4ee42c97265b5aff Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 9 Mar 2026 15:59:59 -0400 Subject: [PATCH] Skip push when local branch is already in sync with origin Before worktree creation, compare local and remote ref SHAs to avoid unnecessary pushes. When already in sync, log at INFO and print nothing. Co-Authored-By: Claude Opus 4.6 --- lib/crates/arc-workflows/src/cli/run.rs | 50 +++++---- lib/crates/arc-workflows/src/git.rs | 138 ++++++++++++++++++++++++ 2 files changed, 169 insertions(+), 19 deletions(-) diff --git a/lib/crates/arc-workflows/src/cli/run.rs b/lib/crates/arc-workflows/src/cli/run.rs index cddc21494..84d2e34b1 100644 --- a/lib/crates/arc-workflows/src/cli/run.rs +++ b/lib/crates/arc-workflows/src/cli/run.rs @@ -531,27 +531,39 @@ pub async fn run_command( if should_create_worktree { if let Some(ref branch) = detected_base_branch { - let repo_path = original_cwd.clone(); - let branch_owned = branch.clone(); - let result = crate::git::blocking_push_with_timeout(60, move || { - crate::git::push_branch(&repo_path, "origin", &branch_owned) + let check_repo = original_cwd.clone(); + let check_branch = branch.clone(); + let needs_push = tokio::task::spawn_blocking(move || { + crate::git::branch_needs_push(&check_repo, "origin", &check_branch) }) - .await; - match result { - Ok(()) => { - tracing::info!(%branch, "Pushed current branch to origin"); - eprintln!( - "{} {branch} (synced local commits to remote)", - styles.bold.apply_to("Pushed branch:") - ); - } - Err(e) => { - tracing::warn!(error = %e, %branch, "Failed to push current branch"); - eprintln!( - "{} Failed to push {branch} to origin: {e}", - styles.yellow.apply_to("Warning:") - ); + .await + .unwrap_or(true); + + if needs_push { + let repo_path = original_cwd.clone(); + let branch_owned = branch.clone(); + let result = crate::git::blocking_push_with_timeout(60, move || { + crate::git::push_branch(&repo_path, "origin", &branch_owned) + }) + .await; + match result { + Ok(()) => { + tracing::info!(%branch, "Pushed current branch to origin"); + eprintln!( + "{} {branch} (synced local commits to remote)", + styles.bold.apply_to("Pushed branch:") + ); + } + Err(e) => { + tracing::warn!(error = %e, %branch, "Failed to push current branch"); + eprintln!( + "{} Failed to push {branch} to origin: {e}", + styles.yellow.apply_to("Warning:") + ); + } } + } else { + tracing::info!(%branch, "Branch already in sync with origin, skipping push"); } } } diff --git a/lib/crates/arc-workflows/src/git.rs b/lib/crates/arc-workflows/src/git.rs index 660090f9f..a18d366c5 100644 --- a/lib/crates/arc-workflows/src/git.rs +++ b/lib/crates/arc-workflows/src/git.rs @@ -358,6 +358,22 @@ where } } +/// Returns true if the local branch has commits not yet on the remote. +/// On any git error (no remote ref, detached HEAD, etc.), returns true +/// so the caller falls back to pushing. +pub fn branch_needs_push(repo: &Path, remote: &str, branch: &str) -> bool { + let local = git_cmd(repo) + .args(["rev-parse", &format!("refs/heads/{branch}")]) + .output(); + let remote_ref = git_cmd(repo) + .args(["rev-parse", &format!("refs/remotes/{remote}/{branch}")]) + .output(); + match (local, remote_ref) { + (Ok(l), Ok(r)) if l.status.success() && r.status.success() => l.stdout != r.stdout, + _ => true, + } +} + /// Sanitize a string for use as a git ref component. /// Lowercases, replaces non-alphanumeric chars with dashes, collapses runs. pub fn sanitize_ref_component(s: &str) -> String { @@ -1318,4 +1334,126 @@ mod tests { let result = push_branch(dir.path(), "nonexistent", "main"); assert!(result.is_err()); } + + #[test] + fn branch_needs_push_when_ahead() { + let dir = tempfile::tempdir().unwrap(); + let repo_dir = dir.path().join("repo"); + let remote_dir = dir.path().join("remote.git"); + + Command::new("git") + .args(["init", "--bare"]) + .arg(&remote_dir) + .output() + .unwrap(); + Command::new("git") + .args(["init"]) + .arg(&repo_dir) + .output() + .unwrap(); + Command::new("git") + .args(["remote", "add", "origin"]) + .arg(&remote_dir) + .current_dir(&repo_dir) + .output() + .unwrap(); + Command::new("git") + .args([ + "-c", + "user.name=test", + "-c", + "user.email=test@test", + "commit", + "--allow-empty", + "-m", + "init", + ]) + .current_dir(&repo_dir) + .output() + .unwrap(); + Command::new("git") + .args(["branch", "-M", "main"]) + .current_dir(&repo_dir) + .output() + .unwrap(); + + // Push once to establish remote tracking + push_branch(&repo_dir, "origin", "main").unwrap(); + + // Make another commit locally (now ahead of remote) + Command::new("git") + .args([ + "-c", + "user.name=test", + "-c", + "user.email=test@test", + "commit", + "--allow-empty", + "-m", + "second", + ]) + .current_dir(&repo_dir) + .output() + .unwrap(); + + assert!(branch_needs_push(&repo_dir, "origin", "main")); + } + + #[test] + fn branch_needs_push_when_in_sync() { + let dir = tempfile::tempdir().unwrap(); + let repo_dir = dir.path().join("repo"); + let remote_dir = dir.path().join("remote.git"); + + Command::new("git") + .args(["init", "--bare"]) + .arg(&remote_dir) + .output() + .unwrap(); + Command::new("git") + .args(["init"]) + .arg(&repo_dir) + .output() + .unwrap(); + Command::new("git") + .args(["remote", "add", "origin"]) + .arg(&remote_dir) + .current_dir(&repo_dir) + .output() + .unwrap(); + Command::new("git") + .args([ + "-c", + "user.name=test", + "-c", + "user.email=test@test", + "commit", + "--allow-empty", + "-m", + "init", + ]) + .current_dir(&repo_dir) + .output() + .unwrap(); + Command::new("git") + .args(["branch", "-M", "main"]) + .current_dir(&repo_dir) + .output() + .unwrap(); + + push_branch(&repo_dir, "origin", "main").unwrap(); + + assert!(!branch_needs_push(&repo_dir, "origin", "main")); + } + + #[test] + fn branch_needs_push_when_no_remote_ref() { + let dir = tempfile::tempdir().unwrap(); + let repo_dir = dir.path(); + + init_repo(repo_dir); + + // No remote at all — should return true (safe default) + assert!(branch_needs_push(repo_dir, "origin", "main")); + } }