From f3f547c701663c46edd02eded1c082f7f17b95b7 Mon Sep 17 00:00:00 2001 From: Francis Chalissery Date: Tue, 7 Jul 2026 16:07:11 -0700 Subject: [PATCH 1/4] Skip unavailable Apple Git for plugin sync --- codex-rs/Cargo.lock | 1 + codex-rs/core-plugins/Cargo.toml | 1 + codex-rs/core-plugins/src/startup_sync.rs | 82 +++++++++++++--- .../core-plugins/src/startup_sync_tests.rs | 93 ++++++++++++++++--- 4 files changed, 148 insertions(+), 29 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index f6879c58e6e5..52ea1a79767c 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2803,6 +2803,7 @@ dependencies = [ "tracing-subscriber", "tracing-test", "url", + "which 8.0.0", "wiremock", "zip 2.4.2", ] diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index 81cfe2f89cc9..ee6301e403f4 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -48,6 +48,7 @@ tokio = { workspace = true, features = ["fs", "macros", "rt", "time"] } toml = { workspace = true } tracing = { workspace = true } url = { workspace = true } +which = { workspace = true } zip = { workspace = true } [dev-dependencies] diff --git a/codex-rs/core-plugins/src/startup_sync.rs b/codex-rs/core-plugins/src/startup_sync.rs index eb7813f35e0f..3b06cd3f9fdc 100644 --- a/codex-rs/core-plugins/src/startup_sync.rs +++ b/codex-rs/core-plugins/src/startup_sync.rs @@ -76,6 +76,12 @@ struct CuratedPluginsBackupArchiveResponse { download_url: String, } +#[derive(Debug, Eq, PartialEq)] +enum GitTransport { + Available(PathBuf), + Unavailable, +} + pub fn curated_plugins_repo_path(codex_home: &Path) -> PathBuf { codex_home.join(CURATED_PLUGINS_RELATIVE_DIR) } @@ -93,9 +99,10 @@ fn curated_plugins_sha_path(codex_home: &Path) -> PathBuf { } pub fn sync_openai_plugins_repo(codex_home: &Path) -> Result { + let git_transport = resolve_git_transport("git"); sync_openai_plugins_repo_with_transport_overrides( codex_home, - "git", + &git_transport, GITHUB_API_BASE_URL, CURATED_PLUGINS_BACKUP_ARCHIVE_API_URL, ) @@ -103,13 +110,20 @@ pub fn sync_openai_plugins_repo(codex_home: &Path) -> Result { fn sync_openai_plugins_repo_with_transport_overrides( codex_home: &Path, - git_binary: &str, + git_transport: &GitTransport, api_base_url: &str, backup_archive_api_url: &str, ) -> Result { let _file_guard = lock_curated_plugins_startup_sync(codex_home)?; - match sync_openai_plugins_repo_via_git(codex_home, git_binary) { + let git_sync_result = match git_transport { + GitTransport::Available(git_binary) => { + sync_openai_plugins_repo_via_git(codex_home, git_binary) + } + GitTransport::Unavailable => Err("git executable is unavailable".to_string()), + }; + + match git_sync_result { Ok(remote_sha) => { emit_curated_plugins_startup_sync_metric("git", "success"); emit_curated_plugins_startup_sync_final_metric("git", "success"); @@ -119,7 +133,6 @@ fn sync_openai_plugins_repo_with_transport_overrides( emit_curated_plugins_startup_sync_metric("git", "failure"); warn!( error = %err, - git_binary, "git sync failed for curated plugin sync; falling back to GitHub HTTP" ); match sync_openai_plugins_repo_via_http(codex_home, api_base_url) { @@ -182,7 +195,10 @@ fn lock_curated_plugins_startup_sync(codex_home: &Path) -> Result Ok(lock_file) } -fn sync_openai_plugins_repo_via_git(codex_home: &Path, git_binary: &str) -> Result { +fn sync_openai_plugins_repo_via_git( + codex_home: &Path, + git_binary: &Path, +) -> Result { let repo_path = curated_plugins_repo_path(codex_home); let sha_path = codex_home.join(CURATED_PLUGINS_SHA_FILE); let remote_sha = git_ls_remote_head_sha(git_binary)?; @@ -229,7 +245,7 @@ fn sync_openai_plugins_repo_via_git(codex_home: &Path, git_binary: &str) -> Resu fn fetch_curated_plugins_commit( repo_path: &Path, remote_sha: &str, - git_binary: &str, + git_binary: &Path, ) -> Result<(), String> { fetch_curated_plugins_commit_from( repo_path, @@ -244,7 +260,7 @@ fn fetch_curated_plugins_commit_from_source( repo_path: &Path, source_repo_path: &Path, remote_sha: &str, - git_binary: &str, + git_binary: &Path, ) -> Result<(), String> { fetch_curated_plugins_commit_from( repo_path, @@ -259,7 +275,7 @@ fn fetch_curated_plugins_commit_from( repo_path: &Path, source: &Path, source_revision: &str, - git_binary: &str, + git_binary: &Path, context: &str, ) -> Result<(), String> { let fetch_refspec = format!("+{source_revision}:{CURATED_PLUGINS_FETCH_REF}"); @@ -274,7 +290,7 @@ fn fetch_curated_plugins_commit_from( ensure_git_success(&output, context) } -fn reset_curated_plugins_checkout(repo_path: &Path, git_binary: &str) -> Result<(), String> { +fn reset_curated_plugins_checkout(repo_path: &Path, git_binary: &Path) -> Result<(), String> { run_git_in_repo( repo_path, git_binary, @@ -291,7 +307,7 @@ fn reset_curated_plugins_checkout(repo_path: &Path, git_binary: &str) -> Result< fn run_git_in_repo( repo_path: &Path, - git_binary: &str, + git_binary: &Path, args: &[&str], context: &str, ) -> Result<(), String> { @@ -583,7 +599,7 @@ fn write_curated_plugins_sha(sha_path: &Path, remote_sha: &str) -> Result<(), St fn read_local_git_or_sha_file( repo_path: &Path, sha_path: &Path, - git_binary: &str, + git_binary: &Path, ) -> Option { if repo_path.join(".git").is_dir() && let Ok(sha) = git_head_sha(repo_path, git_binary) @@ -594,7 +610,7 @@ fn read_local_git_or_sha_file( read_sha_file(sha_path) } -fn git_ls_remote_head_sha(git_binary: &str) -> Result { +fn git_ls_remote_head_sha(git_binary: &Path) -> Result { let mut command = git_command(git_binary); command .arg("ls-remote") @@ -622,7 +638,7 @@ fn git_ls_remote_head_sha(git_binary: &str) -> Result { Ok(sha.to_string()) } -fn git_head_sha(repo_path: &Path, git_binary: &str) -> Result { +fn git_head_sha(repo_path: &Path, git_binary: &Path) -> Result { let output = git_command(git_binary) .arg("-C") .arg(repo_path) @@ -647,7 +663,7 @@ fn git_head_sha(repo_path: &Path, git_binary: &str) -> Result { Ok(sha) } -fn git_command(git_binary: &str) -> Command { +fn git_command(git_binary: &Path) -> Command { let mut command = Command::new(git_binary); command.env("GIT_OPTIONAL_LOCKS", "0"); for name in REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES { @@ -656,6 +672,44 @@ fn git_command(git_binary: &str) -> Command { command } +fn resolve_git_transport(git_binary: &str) -> GitTransport { + let Ok(git_path) = which::which(git_binary) else { + return GitTransport::Unavailable; + }; + + #[cfg(target_os = "macos")] + { + macos_git_transport_from_path(git_path, apple_developer_tools_available()) + } + #[cfg(not(target_os = "macos"))] + { + GitTransport::Available(git_path) + } +} + +#[cfg(any(target_os = "macos", test))] +fn macos_git_transport_from_path( + git_path: PathBuf, + apple_developer_tools_available: bool, +) -> GitTransport { + if git_path == Path::new("/usr/bin/git") && !apple_developer_tools_available { + GitTransport::Unavailable + } else { + GitTransport::Available(git_path) + } +} + +#[cfg(target_os = "macos")] +fn apple_developer_tools_available() -> bool { + Command::new("/usr/bin/xcode-select") + .arg("-p") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .is_ok_and(|status| status.success()) +} + fn run_git_command_with_timeout( command: &mut Command, context: &str, diff --git a/codex-rs/core-plugins/src/startup_sync_tests.rs b/codex-rs/core-plugins/src/startup_sync_tests.rs index fcedf3084222..4e33046f3c17 100644 --- a/codex-rs/core-plugins/src/startup_sync_tests.rs +++ b/codex-rs/core-plugins/src/startup_sync_tests.rs @@ -19,7 +19,7 @@ const TEST_CURATED_PLUGIN_SHA: &str = "0123456789abcdef0123456789abcdef01234567" #[test] fn git_command_sanitizes_ambient_repository_environment() { - let command = git_command("git"); + let command = git_command(Path::new("git")); for name in REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES { assert_eq!( @@ -190,9 +190,29 @@ async fn run_sync_with_transport_overrides( let api_base_url = api_base_url.into(); let backup_archive_api_url = backup_archive_api_url.into(); tokio::task::spawn_blocking(move || { + let git_transport = GitTransport::Available(PathBuf::from(git_binary)); sync_openai_plugins_repo_with_transport_overrides( codex_home.as_path(), - &git_binary, + &git_transport, + &api_base_url, + &backup_archive_api_url, + ) + }) + .await + .expect("sync task should join") +} + +async fn run_sync_without_git( + codex_home: PathBuf, + api_base_url: impl Into, + backup_archive_api_url: impl Into, +) -> Result { + let api_base_url = api_base_url.into(); + let backup_archive_api_url = backup_archive_api_url.into(); + tokio::task::spawn_blocking(move || { + sync_openai_plugins_repo_with_transport_overrides( + codex_home.as_path(), + &GitTransport::Unavailable, &api_base_url, &backup_archive_api_url, ) @@ -343,7 +363,7 @@ exit 1 barrier.wait(); sync_openai_plugins_repo_with_transport_overrides( tmp.path(), - git_path.to_str().expect("utf8 path"), + &GitTransport::Available(git_path.clone()), "http://127.0.0.1:9", "http://127.0.0.1:9/backend-api/plugins/export/curated", ) @@ -463,9 +483,8 @@ fn sync_openai_plugins_repo_via_git_succeeds_with_local_rewritten_remote() { ), ); - let synced_sha = - sync_openai_plugins_repo_via_git(tmp.path(), git_wrapper.to_str().expect("utf8 path")) - .expect("git sync should succeed"); + let synced_sha = sync_openai_plugins_repo_via_git(tmp.path(), &git_wrapper) + .expect("git sync should succeed"); assert_eq!(synced_sha, sha); assert_curated_gmail_repo(&curated_plugins_repo_path(tmp.path())); @@ -517,9 +536,8 @@ fn sync_openai_plugins_repo_via_git_succeeds_with_local_rewritten_remote() { .trim() .to_string(); - let synced_sha = - sync_openai_plugins_repo_via_git(tmp.path(), git_wrapper.to_str().expect("utf8 path")) - .expect("incremental git sync should succeed"); + let synced_sha = sync_openai_plugins_repo_via_git(tmp.path(), &git_wrapper) + .expect("incremental git sync should succeed"); assert_eq!(synced_sha, updated_sha); assert!( @@ -573,9 +591,8 @@ fn sync_openai_plugins_repo_via_git_succeeds_with_local_rewritten_remote() { assert!(!has_plugins_clone_dirs(tmp.path())); let unchanged_sync_invocation_count = invocation_log_contents.lines().count(); - let synced_sha = - sync_openai_plugins_repo_via_git(tmp.path(), git_wrapper.to_str().expect("utf8 path")) - .expect("unchanged git sync should succeed"); + let synced_sha = sync_openai_plugins_repo_via_git(tmp.path(), &git_wrapper) + .expect("unchanged git sync should succeed"); assert_eq!(synced_sha, updated_sha); let invocation_log = std::fs::read_to_string(&invocation_log).expect("read sync invocations"); @@ -619,6 +636,52 @@ async fn sync_openai_plugins_repo_falls_back_to_http_when_git_is_unavailable() { assert_eq!(read_curated_plugins_sha(tmp.path()).as_deref(), Some(sha)); } +#[test] +fn apple_git_without_developer_tools_is_unavailable() { + assert_eq!( + macos_git_transport_from_path( + PathBuf::from("/usr/bin/git"), + /*apple_developer_tools_available*/ false, + ), + GitTransport::Unavailable + ); + assert_eq!( + macos_git_transport_from_path( + PathBuf::from("/usr/bin/git"), + /*apple_developer_tools_available*/ true, + ), + GitTransport::Available(PathBuf::from("/usr/bin/git")) + ); + assert_eq!( + macos_git_transport_from_path( + PathBuf::from("/opt/homebrew/bin/git"), + /*apple_developer_tools_available*/ false, + ), + GitTransport::Available(PathBuf::from("/opt/homebrew/bin/git")) + ); +} + +#[tokio::test] +async fn sync_openai_plugins_repo_uses_http_without_git_transport() { + let tmp = tempdir().expect("tempdir"); + let server = MockServer::start().await; + let sha = "0123456789abcdef0123456789abcdef01234567"; + + mount_github_repo_and_ref(&server, sha).await; + mount_github_zipball(&server, sha, curated_repo_zipball_bytes(sha)).await; + + let synced_sha = run_sync_without_git( + tmp.path().to_path_buf(), + server.uri(), + "http://127.0.0.1:9/backend-api/plugins/export/curated", + ) + .await + .expect("HTTP sync should succeed"); + + assert_eq!(synced_sha, sha); + assert_curated_gmail_repo(&curated_plugins_repo_path(tmp.path())); +} + #[cfg(unix)] #[tokio::test] async fn sync_openai_plugins_repo_falls_back_to_http_when_git_sync_fails() { @@ -690,8 +753,8 @@ exit 1 ), ); - let err = sync_openai_plugins_repo_via_git(tmp.path(), git_path.to_str().expect("utf8 path")) - .expect_err("git sync should fail"); + let err = + sync_openai_plugins_repo_via_git(tmp.path(), &git_path).expect_err("git sync should fail"); assert!(err.contains("fatal: early EOF")); assert!(!has_plugins_clone_dirs(tmp.path())); @@ -755,7 +818,7 @@ exit 1 ), ); - let err = sync_openai_plugins_repo_via_git(tmp.path(), git_path.to_str().expect("utf8 path")) + let err = sync_openai_plugins_repo_via_git(tmp.path(), &git_path) .expect_err("invalid staged checkout should fail"); assert!(err.contains("curated plugins archive missing marketplace manifest")); From f358df36592854b245398cc27be614892f0c6788 Mon Sep 17 00:00:00 2001 From: Francis Chalissery Date: Wed, 8 Jul 2026 09:34:32 -0700 Subject: [PATCH 2/4] Scope Git availability detection to macOS --- codex-rs/core-plugins/Cargo.toml | 4 +++- codex-rs/core-plugins/src/startup_sync.rs | 24 ++++++++--------------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index ee6301e403f4..e85d16a126bd 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -48,9 +48,11 @@ tokio = { workspace = true, features = ["fs", "macros", "rt", "time"] } toml = { workspace = true } tracing = { workspace = true } url = { workspace = true } -which = { workspace = true } zip = { workspace = true } +[target.'cfg(target_os = "macos")'.dependencies] +which = { workspace = true } + [dev-dependencies] libc = { workspace = true } pretty_assertions = { workspace = true } diff --git a/codex-rs/core-plugins/src/startup_sync.rs b/codex-rs/core-plugins/src/startup_sync.rs index 3b06cd3f9fdc..a9f110237eba 100644 --- a/codex-rs/core-plugins/src/startup_sync.rs +++ b/codex-rs/core-plugins/src/startup_sync.rs @@ -99,7 +99,14 @@ fn curated_plugins_sha_path(codex_home: &Path) -> PathBuf { } pub fn sync_openai_plugins_repo(codex_home: &Path) -> Result { - let git_transport = resolve_git_transport("git"); + #[cfg(target_os = "macos")] + let git_transport = match which::which("git") { + Ok(git_path) => macos_git_transport_from_path(git_path, apple_developer_tools_available()), + Err(_) => GitTransport::Unavailable, + }; + #[cfg(not(target_os = "macos"))] + let git_transport = GitTransport::Available(PathBuf::from("git")); + sync_openai_plugins_repo_with_transport_overrides( codex_home, &git_transport, @@ -672,21 +679,6 @@ fn git_command(git_binary: &Path) -> Command { command } -fn resolve_git_transport(git_binary: &str) -> GitTransport { - let Ok(git_path) = which::which(git_binary) else { - return GitTransport::Unavailable; - }; - - #[cfg(target_os = "macos")] - { - macos_git_transport_from_path(git_path, apple_developer_tools_available()) - } - #[cfg(not(target_os = "macos"))] - { - GitTransport::Available(git_path) - } -} - #[cfg(any(target_os = "macos", test))] fn macos_git_transport_from_path( git_path: PathBuf, From ff310019fc17f450031121e70f722250e65577a9 Mon Sep 17 00:00:00 2001 From: Francis Chalissery Date: Wed, 8 Jul 2026 11:16:48 -0700 Subject: [PATCH 3/4] Avoid platform-specific dead code in plugin sync --- codex-rs/core-plugins/src/startup_sync.rs | 34 +++++++------------ .../core-plugins/src/startup_sync_tests.rs | 20 +++++------ 2 files changed, 23 insertions(+), 31 deletions(-) diff --git a/codex-rs/core-plugins/src/startup_sync.rs b/codex-rs/core-plugins/src/startup_sync.rs index a9f110237eba..0dd437af8a00 100644 --- a/codex-rs/core-plugins/src/startup_sync.rs +++ b/codex-rs/core-plugins/src/startup_sync.rs @@ -76,12 +76,6 @@ struct CuratedPluginsBackupArchiveResponse { download_url: String, } -#[derive(Debug, Eq, PartialEq)] -enum GitTransport { - Available(PathBuf), - Unavailable, -} - pub fn curated_plugins_repo_path(codex_home: &Path) -> PathBuf { codex_home.join(CURATED_PLUGINS_RELATIVE_DIR) } @@ -100,16 +94,16 @@ fn curated_plugins_sha_path(codex_home: &Path) -> PathBuf { pub fn sync_openai_plugins_repo(codex_home: &Path) -> Result { #[cfg(target_os = "macos")] - let git_transport = match which::which("git") { - Ok(git_path) => macos_git_transport_from_path(git_path, apple_developer_tools_available()), - Err(_) => GitTransport::Unavailable, + let git_binary = match which::which("git") { + Ok(git_path) => macos_git_binary_from_path(git_path, apple_developer_tools_available()), + Err(_) => None, }; #[cfg(not(target_os = "macos"))] - let git_transport = GitTransport::Available(PathBuf::from("git")); + let git_binary = Some(PathBuf::from("git")); sync_openai_plugins_repo_with_transport_overrides( codex_home, - &git_transport, + git_binary.as_deref(), GITHUB_API_BASE_URL, CURATED_PLUGINS_BACKUP_ARCHIVE_API_URL, ) @@ -117,17 +111,15 @@ pub fn sync_openai_plugins_repo(codex_home: &Path) -> Result { fn sync_openai_plugins_repo_with_transport_overrides( codex_home: &Path, - git_transport: &GitTransport, + git_binary: Option<&Path>, api_base_url: &str, backup_archive_api_url: &str, ) -> Result { let _file_guard = lock_curated_plugins_startup_sync(codex_home)?; - let git_sync_result = match git_transport { - GitTransport::Available(git_binary) => { - sync_openai_plugins_repo_via_git(codex_home, git_binary) - } - GitTransport::Unavailable => Err("git executable is unavailable".to_string()), + let git_sync_result = match git_binary { + Some(git_binary) => sync_openai_plugins_repo_via_git(codex_home, git_binary), + None => Err("git executable is unavailable".to_string()), }; match git_sync_result { @@ -680,14 +672,14 @@ fn git_command(git_binary: &Path) -> Command { } #[cfg(any(target_os = "macos", test))] -fn macos_git_transport_from_path( +fn macos_git_binary_from_path( git_path: PathBuf, apple_developer_tools_available: bool, -) -> GitTransport { +) -> Option { if git_path == Path::new("/usr/bin/git") && !apple_developer_tools_available { - GitTransport::Unavailable + None } else { - GitTransport::Available(git_path) + Some(git_path) } } diff --git a/codex-rs/core-plugins/src/startup_sync_tests.rs b/codex-rs/core-plugins/src/startup_sync_tests.rs index 4e33046f3c17..20b87c1c331f 100644 --- a/codex-rs/core-plugins/src/startup_sync_tests.rs +++ b/codex-rs/core-plugins/src/startup_sync_tests.rs @@ -190,10 +190,10 @@ async fn run_sync_with_transport_overrides( let api_base_url = api_base_url.into(); let backup_archive_api_url = backup_archive_api_url.into(); tokio::task::spawn_blocking(move || { - let git_transport = GitTransport::Available(PathBuf::from(git_binary)); + let git_binary = PathBuf::from(git_binary); sync_openai_plugins_repo_with_transport_overrides( codex_home.as_path(), - &git_transport, + Some(git_binary.as_path()), &api_base_url, &backup_archive_api_url, ) @@ -212,7 +212,7 @@ async fn run_sync_without_git( tokio::task::spawn_blocking(move || { sync_openai_plugins_repo_with_transport_overrides( codex_home.as_path(), - &GitTransport::Unavailable, + None, &api_base_url, &backup_archive_api_url, ) @@ -363,7 +363,7 @@ exit 1 barrier.wait(); sync_openai_plugins_repo_with_transport_overrides( tmp.path(), - &GitTransport::Available(git_path.clone()), + Some(git_path.as_path()), "http://127.0.0.1:9", "http://127.0.0.1:9/backend-api/plugins/export/curated", ) @@ -639,25 +639,25 @@ async fn sync_openai_plugins_repo_falls_back_to_http_when_git_is_unavailable() { #[test] fn apple_git_without_developer_tools_is_unavailable() { assert_eq!( - macos_git_transport_from_path( + macos_git_binary_from_path( PathBuf::from("/usr/bin/git"), /*apple_developer_tools_available*/ false, ), - GitTransport::Unavailable + None ); assert_eq!( - macos_git_transport_from_path( + macos_git_binary_from_path( PathBuf::from("/usr/bin/git"), /*apple_developer_tools_available*/ true, ), - GitTransport::Available(PathBuf::from("/usr/bin/git")) + Some(PathBuf::from("/usr/bin/git")) ); assert_eq!( - macos_git_transport_from_path( + macos_git_binary_from_path( PathBuf::from("/opt/homebrew/bin/git"), /*apple_developer_tools_available*/ false, ), - GitTransport::Available(PathBuf::from("/opt/homebrew/bin/git")) + Some(PathBuf::from("/opt/homebrew/bin/git")) ); } From 80a7c9d027486b915443c9c1e17ab899ec04cf27 Mon Sep 17 00:00:00 2001 From: Francis Chalissery Date: Wed, 8 Jul 2026 11:27:23 -0700 Subject: [PATCH 4/4] Annotate unavailable Git test argument --- codex-rs/core-plugins/src/startup_sync_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/core-plugins/src/startup_sync_tests.rs b/codex-rs/core-plugins/src/startup_sync_tests.rs index 20b87c1c331f..a4ea77844302 100644 --- a/codex-rs/core-plugins/src/startup_sync_tests.rs +++ b/codex-rs/core-plugins/src/startup_sync_tests.rs @@ -212,7 +212,7 @@ async fn run_sync_without_git( tokio::task::spawn_blocking(move || { sync_openai_plugins_repo_with_transport_overrides( codex_home.as_path(), - None, + /*git_binary*/ None, &api_base_url, &backup_archive_api_url, )