From eb45c01ea9de949eb45288d7a65478c16afe4ccb Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Wed, 17 Jun 2026 10:47:54 -0700 Subject: [PATCH] Keep default-branch discovery local --- codex-rs/core/src/git_info_tests.rs | 71 ++++++++++++++++++++++++++ codex-rs/git-utils/src/info.rs | 20 +------- codex-rs/tui/src/branch_summary.rs | 78 ++++++++++++++--------------- 3 files changed, 111 insertions(+), 58 deletions(-) diff --git a/codex-rs/core/src/git_info_tests.rs b/codex-rs/core/src/git_info_tests.rs index e172cd5f20..379feea64d 100644 --- a/codex-rs/core/src/git_info_tests.rs +++ b/codex-rs/core/src/git_info_tests.rs @@ -2,6 +2,7 @@ use codex_exec_server::LOCAL_FS; use codex_git_utils::GitInfo; use codex_git_utils::GitSha; use codex_git_utils::collect_git_info; +use codex_git_utils::default_branch_name; use codex_git_utils::get_git_repo_root_with_fs; use codex_git_utils::get_has_changes; use codex_git_utils::git_diff_to_remote; @@ -186,6 +187,76 @@ async fn create_test_git_repo_with_remote(temp_dir: &TempDir) -> (PathBuf, Strin (repo_path, branch) } +#[cfg(unix)] +#[tokio::test] +async fn test_default_branch_discovery_does_not_invoke_remote_transport() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let repo_path = create_test_git_repo(&temp_dir).await; + let helper_path = temp_dir.path().join("ssh-helper.sh"); + let marker_path = temp_dir.path().join("transport-ran"); + fs::write( + &helper_path, + format!( + "#!/bin/sh\nprintf ran > \"{}\"\nexit 1\n", + marker_path.to_string_lossy() + ), + ) + .expect("write transport helper"); + let mut permissions = fs::metadata(&helper_path) + .expect("read helper metadata") + .permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&helper_path, permissions).expect("mark helper executable"); + + let add_remote = Command::new("git") + .args(["remote", "add", "origin", "ssh://example.invalid/repo"]) + .current_dir(&repo_path) + .output() + .await + .expect("add remote"); + assert!( + add_remote.status.success(), + "add remote: {}", + String::from_utf8_lossy(&add_remote.stderr) + ); + let configure_helper = Command::new("git") + .args([ + "config", + "core.sshCommand", + helper_path.to_string_lossy().as_ref(), + ]) + .current_dir(&repo_path) + .output() + .await + .expect("configure transport helper"); + assert!( + configure_helper.status.success(), + "configure transport helper: {}", + String::from_utf8_lossy(&configure_helper.stderr) + ); + let branch_output = Command::new("git") + .args(["branch", "--show-current"]) + .current_dir(&repo_path) + .output() + .await + .expect("read branch"); + assert!( + branch_output.status.success(), + "read branch: {}", + String::from_utf8_lossy(&branch_output.stderr) + ); + let branch = String::from_utf8(branch_output.stdout) + .expect("branch utf8") + .trim() + .to_string(); + + assert_eq!(default_branch_name(&repo_path).await, Some(branch)); + assert!( + !marker_path.exists(), + "default branch discovery must stay on local refs" + ); +} + #[tokio::test] async fn test_collect_git_info_non_git_directory() { let temp_dir = TempDir::new().expect("Failed to create temp dir"); diff --git a/codex-rs/git-utils/src/info.rs b/codex-rs/git-utils/src/info.rs index 48d229df8d..23afbd145f 100644 --- a/codex-rs/git-utils/src/info.rs +++ b/codex-rs/git-utils/src/info.rs @@ -474,8 +474,7 @@ async fn get_git_remotes(cwd: &Path) -> Option> { /// /// Preference order: /// 1) The symbolic ref at `refs/remotes//HEAD` for the first remote (origin prioritized) -/// 2) `git remote show ` parsed for "HEAD branch: " -/// 3) Local fallback to existing `main` or `master` if present +/// 2) Local fallback to existing `main` or `master` if present async fn get_default_branch(cwd: &Path) -> Option { // Prefer the first remote (with origin prioritized) let remotes = get_git_remotes(cwd).await.unwrap_or_default(); @@ -498,23 +497,6 @@ async fn get_default_branch(cwd: &Path) -> Option { return Some(name.to_string()); } } - - // Fall back to parsing `git remote show ` output - if let Some(show_output) = - run_git_command_with_timeout(&["remote", "show", &remote], cwd).await - && show_output.status.success() - && let Ok(text) = String::from_utf8(show_output.stdout) - { - for line in text.lines() { - let line = line.trim(); - if let Some(rest) = line.strip_prefix("HEAD branch:") { - let name = rest.trim(); - if !name.is_empty() { - return Some(name.to_string()); - } - } - } - } } // No remote-derived default; try common local defaults if they exist diff --git a/codex-rs/tui/src/branch_summary.rs b/codex-rs/tui/src/branch_summary.rs index 4698dc96e5..8b90be5d3a 100644 --- a/codex-rs/tui/src/branch_summary.rs +++ b/codex-rs/tui/src/branch_summary.rs @@ -225,11 +225,6 @@ async fn get_default_branch( { return Some(branch); } - - if let Some(branch) = get_remote_default_branch_from_remote_show(runner, cwd, &remote).await - { - return Some(branch); - } } get_default_branch_local(runner, cwd).await @@ -265,40 +260,6 @@ async fn get_remote_default_branch_from_symbolic_ref( }) } -/// Parses `git remote show` output to discover a remote's default branch ref. -/// -/// This is a fallback for repositories where `refs/remotes//HEAD` is not configured but -/// `git remote show` can still report the upstream HEAD branch. The concrete remote-tracking ref -/// must already exist locally before it is accepted. -async fn get_remote_default_branch_from_remote_show( - runner: &dyn WorkspaceCommandExecutor, - cwd: &Path, - remote: &str, -) -> Option { - let output = run_git_command(runner, cwd, &["remote", "show", remote]) - .await - .ok()?; - if !output.success() { - return None; - } - - for line in output.stdout.lines() { - let line = line.trim(); - let Some(rest) = line.strip_prefix("HEAD branch:") else { - continue; - }; - let name = rest.trim(); - let remote_ref = format!("refs/remotes/{remote}/{name}"); - if !name.is_empty() && git_ref_exists(runner, cwd, &remote_ref).await { - return Some(DefaultBranch { - merge_ref: remote_ref, - }); - } - } - - None -} - /// Falls back to local `main` or `master` when no remote default branch can be found. async fn get_default_branch_local( runner: &dyn WorkspaceCommandExecutor, @@ -568,6 +529,45 @@ mod tests { assert!(runner.saw(&["git", "merge-base", "HEAD", "refs/remotes/origin/main"])); } + #[tokio::test] + async fn branch_diff_stats_uses_local_fallback_without_remote_transport() { + let runner = FakeRunner::new(vec![ + response( + &["git", "rev-parse", "--git-dir"], + /*exit_code*/ 0, + ".git\n", + ), + response(&["git", "remote"], /*exit_code*/ 0, "origin\n"), + response( + &["git", "symbolic-ref", "--quiet", "refs/remotes/origin/HEAD"], + /*exit_code*/ 1, + "", + ), + response( + &["git", "rev-parse", "--verify", "--quiet", "refs/heads/main"], + /*exit_code*/ 0, + "main-sha\n", + ), + response( + &["git", "merge-base", "HEAD", "refs/heads/main"], + /*exit_code*/ 0, + "base-sha\n", + ), + response( + &["git", "diff", "--numstat", "base-sha..HEAD"], + /*exit_code*/ 0, + "1\t0\tfile\n", + ), + ]); + + let stats = branch_diff_stats_to_default_branch(&runner, Path::new("/repo")) + .await + .expect("branch diff stats"); + + assert_eq!(stats.additions, 1); + assert!(!runner.saw(&["git", "remote", "show", "origin"])); + } + #[tokio::test] async fn open_pull_request_uses_current_branch_view_first() { let runner = FakeRunner::new(vec![response(