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..e85d16a126bd 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -50,6 +50,9 @@ tracing = { workspace = true } url = { 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 eb7813f35e0f..0dd437af8a00 100644 --- a/codex-rs/core-plugins/src/startup_sync.rs +++ b/codex-rs/core-plugins/src/startup_sync.rs @@ -93,9 +93,17 @@ 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_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_binary = Some(PathBuf::from("git")); + sync_openai_plugins_repo_with_transport_overrides( codex_home, - "git", + git_binary.as_deref(), GITHUB_API_BASE_URL, CURATED_PLUGINS_BACKUP_ARCHIVE_API_URL, ) @@ -103,13 +111,18 @@ 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_binary: Option<&Path>, 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_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 { Ok(remote_sha) => { emit_curated_plugins_startup_sync_metric("git", "success"); emit_curated_plugins_startup_sync_final_metric("git", "success"); @@ -119,7 +132,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 +194,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 +244,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 +259,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 +274,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 +289,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 +306,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 +598,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 +609,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 +637,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 +662,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 +671,29 @@ fn git_command(git_binary: &str) -> Command { command } +#[cfg(any(target_os = "macos", test))] +fn macos_git_binary_from_path( + git_path: PathBuf, + apple_developer_tools_available: bool, +) -> Option { + if git_path == Path::new("/usr/bin/git") && !apple_developer_tools_available { + None + } else { + Some(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..a4ea77844302 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_binary = PathBuf::from(git_binary); sync_openai_plugins_repo_with_transport_overrides( codex_home.as_path(), - &git_binary, + Some(git_binary.as_path()), + &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(), + /*git_binary*/ None, &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"), + Some(git_path.as_path()), "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_binary_from_path( + PathBuf::from("/usr/bin/git"), + /*apple_developer_tools_available*/ false, + ), + None + ); + assert_eq!( + macos_git_binary_from_path( + PathBuf::from("/usr/bin/git"), + /*apple_developer_tools_available*/ true, + ), + Some(PathBuf::from("/usr/bin/git")) + ); + assert_eq!( + macos_git_binary_from_path( + PathBuf::from("/opt/homebrew/bin/git"), + /*apple_developer_tools_available*/ false, + ), + Some(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"));