From aa40adc090f3a0db50301a159ca7d1ffbe400361 Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Thu, 2 Jul 2026 15:41:39 -0700 Subject: [PATCH] codex: ignore symbolic refs in branch ancestry --- codex-rs/core/src/git_info_tests.rs | 66 +++++++++++++++++++++++++++++ codex-rs/git-utils/src/info.rs | 12 +++++- 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/git_info_tests.rs b/codex-rs/core/src/git_info_tests.rs index 24413a18d6..3c732ca064 100644 --- a/codex-rs/core/src/git_info_tests.rs +++ b/codex-rs/core/src/git_info_tests.rs @@ -666,6 +666,72 @@ async fn test_git_diff_to_remote_preserves_expanded_remote_ref_identity() { assert_ne!(state.sha, GitSha::new(&origin_sha)); } +#[tokio::test] +async fn test_git_diff_to_remote_ignores_symbolic_refs_during_expansion() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let repo_path = create_test_git_repo(&temp_dir).await; + assert_test_git(&repo_path, &["branch", "-M", "base"]).await; + assert_test_git( + &repo_path, + &["remote", "add", "origin", "/nonexistent/origin.git"], + ) + .await; + assert_test_git( + &repo_path, + &["remote", "add", "upstream", "/nonexistent/upstream.git"], + ) + .await; + + assert_test_git(&repo_path, &["checkout", "-b", "feature", "base"]).await; + fs::write(repo_path.join("feature.txt"), "feature\n").expect("write feature file"); + assert_test_git(&repo_path, &["add", "feature.txt"]).await; + assert_test_git(&repo_path, &["commit", "-m", "feature work"]).await; + let poisoned_sha = test_git_stdout(&repo_path, &["rev-parse", "HEAD"]).await; + assert_test_git( + &repo_path, + &[ + "update-ref", + "refs/remotes/upstream/poisoned", + &poisoned_sha, + ], + ) + .await; + + assert_test_git(&repo_path, &["checkout", "-b", "origin-release", "feature"]).await; + fs::write(repo_path.join("remote-only.txt"), "remote\n").expect("write remote-only file"); + assert_test_git(&repo_path, &["add", "remote-only.txt"]).await; + assert_test_git(&repo_path, &["commit", "-m", "origin release"]).await; + let expected_sha = test_git_stdout(&repo_path, &["rev-parse", "HEAD"]).await; + assert_test_git( + &repo_path, + &[ + "update-ref", + "refs/remotes/origin/release/v1", + &expected_sha, + ], + ) + .await; + + assert_test_git(&repo_path, &["checkout", "feature"]).await; + assert_test_git( + &repo_path, + &[ + "symbolic-ref", + "refs/remotes/origin/HEAD", + "refs/remotes/upstream/poisoned", + ], + ) + .await; + + assert_eq!(default_branch_name(&repo_path).await, None); + let state = git_diff_to_remote(&repo_path) + .await + .expect("Should collect working tree state"); + assert_eq!(state.sha, GitSha::new(&expected_sha)); + assert_ne!(state.sha, GitSha::new(&poisoned_sha)); + assert!(state.diff.contains("remote-only.txt")); +} + #[cfg(unix)] #[tokio::test] async fn test_default_branch_discovery_does_not_invoke_remote_transport() { diff --git a/codex-rs/git-utils/src/info.rs b/codex-rs/git-utils/src/info.rs index 61f5788a04..67ca66246d 100644 --- a/codex-rs/git-utils/src/info.rs +++ b/codex-rs/git-utils/src/info.rs @@ -632,7 +632,7 @@ async fn branch_ancestry(cwd: &Path) -> Option> { if let Some(output) = run_git_command_with_timeout( &[ "for-each-ref", - "--format=%(refname)", + "--format=%(refname)%00%(symref)", "--contains=HEAD", &format!("refs/remotes/{remote}"), ], @@ -643,7 +643,15 @@ async fn branch_ancestry(cwd: &Path) -> Option> { && let Ok(text) = String::from_utf8(output.stdout) { for line in text.lines() { - let remote_ref = line.trim(); + let Some((remote_ref, symbolic_target)) = line.split_once('\0') else { + continue; + }; + // The concrete target will be listed separately when it belongs to + // this remote. Excluding aliases also prevents a cross-remote or + // stale symbolic ref from bypassing default-branch validation. + if !symbolic_target.is_empty() { + continue; + } let remote_ref_prefix = format!("refs/remotes/{remote}/"); if let Some(stripped) = remote_ref.strip_prefix(&remote_ref_prefix) && !stripped.is_empty()