diff --git a/rust/src/cli/tty_runner.rs b/rust/src/cli/tty_runner.rs index 1ff9cc12c8..bd547f8a29 100755 --- a/rust/src/cli/tty_runner.rs +++ b/rust/src/cli/tty_runner.rs @@ -191,7 +191,7 @@ impl TtyCommandRunner { pub fn which(tool: &str) -> Option { // Check for specific tool overrides if tool == "codex" - && let Some(path) = Self::locate_codex_binary() + && let Some(path) = crate::codex_cli::locate_codex_binary() { return Some(path); } @@ -205,40 +205,6 @@ impl TtyCommandRunner { Self::run_where(tool) } - /// Locate the Codex binary - fn locate_codex_binary() -> Option { - // Check environment override - if let Ok(path) = std::env::var("CODEX_BINARY") { - let path = PathBuf::from(path); - if path.exists() { - return Some(path); - } - } - - // Check common Windows locations - let candidates = [ - // npm global install locations - dirs::data_local_dir().map(|d| d.join("npm").join("codex.cmd")), - dirs::home_dir().map(|h| { - h.join("AppData") - .join("Roaming") - .join("npm") - .join("codex.cmd") - }), - // Bun install - dirs::home_dir().map(|h| h.join(".bun").join("bin").join("codex.exe")), - ]; - - for candidate in candidates.into_iter().flatten() { - if candidate.exists() { - return Some(candidate); - } - } - - // Fall back to PATH search - Self::run_where("codex") - } - /// Locate the Claude binary fn locate_claude_binary() -> Option { // Check environment override diff --git a/rust/src/codex_accounts/account_manager.rs b/rust/src/codex_accounts/account_manager.rs index 4c28129c73..bf25578fbd 100644 --- a/rust/src/codex_accounts/account_manager.rs +++ b/rust/src/codex_accounts/account_manager.rs @@ -422,7 +422,7 @@ impl CodexAccountManager { } CodexLoginOutcome::MissingBinary => { return Err(CodexAccountManagerError::Message( - "The `codex` command could not be found.".to_string(), + "Codex CLI could not be found. Install Codex Desktop or the Codex CLI, then restart CodexBar.".to_string(), )); } CodexLoginOutcome::TimedOut(_) => { diff --git a/rust/src/codex_accounts/login_runner.rs b/rust/src/codex_accounts/login_runner.rs index 0f17b8288c..d4b32c725d 100644 --- a/rust/src/codex_accounts/login_runner.rs +++ b/rust/src/codex_accounts/login_runner.rs @@ -2,7 +2,7 @@ //! timeouts, and combined output capture. Split out of `account_manager.rs` //! (port of the login-running slice of `windows/.../account_manager.py`, MIT). -use std::path::{Path, PathBuf}; +use std::path::Path; use std::process::{Child, Command, Stdio}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; @@ -85,14 +85,9 @@ impl ManagedLoginProcess { pub struct CodexLoginRunner; impl CodexLoginRunner { - /// Resolve the `codex` executable, falling back to known install paths. - pub fn locate_codex_binary() -> Option { - if let Ok(found) = which::which("codex") { - return Some(found); - } - path_candidates() - .into_iter() - .find(|candidate| candidate.is_file()) + /// Resolve the `codex` executable through the crate-wide canonical locator. + pub fn locate_codex_binary() -> Option { + crate::codex_cli::locate_codex_binary() } pub fn run( @@ -108,7 +103,13 @@ impl CodexLoginRunner { }; let mut command = Command::new(binary); + #[cfg(windows)] + { + use std::os::windows::process::CommandExt; + command.creation_flags(0x0800_0000); // CREATE_NO_WINDOW + } command + .args(["-c", "cli_auth_credentials_store=\"file\""]) .arg("login") .env("CODEX_HOME", home_path) .stdout(Stdio::piped()) @@ -152,30 +153,6 @@ impl CodexLoginRunner { } } -fn path_candidates() -> Vec { - let local_app_data = std::env::var("LOCALAPPDATA") - .map(PathBuf::from) - .unwrap_or_else(|_| { - dirs::home_dir() - .unwrap_or_else(|| PathBuf::from(".")) - .join("AppData") - .join("Local") - }); - let home = dirs::home_dir().unwrap_or_else(|| PathBuf::from(".")); - vec![ - local_app_data - .join("OpenAI") - .join("Codex") - .join("bin") - .join("codex.exe"), - home.join(".bun").join("bin").join("codex.exe"), - local_app_data - .join("Microsoft") - .join("WindowsApps") - .join("codex.exe"), - ] -} - fn wait_for_child(handle: &ManagedLoginProcess, timeout: Duration) -> Option { let deadline = Instant::now() + timeout; loop { @@ -183,16 +160,16 @@ fn wait_for_child(handle: &ManagedLoginProcess, timeout: Duration) -> Option take_child(handle)?.wait_with_output().ok(), - Some(Err(_)) => take_child(handle)?.wait_with_output().ok(), - _ => None, - } + matches!( + guard.as_mut().map(|child| child.try_wait()), + Some(Ok(Some(_))) | Some(Err(_)) + ) }; - if polled.is_some() { - return polled; + // Drop the polling lock before taking ownership of the child. + if finished { + return take_child(handle)?.wait_with_output().ok(); } if Instant::now() >= deadline { return None; @@ -235,3 +212,40 @@ fn combine_output(output: &std::process::Output) -> String { merged.chars().take(4000).collect() } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn completed_child_is_collected_without_locking_twice() { + let (sender, receiver) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + #[cfg(windows)] + let child = Command::new("cmd.exe") + .args(["/d", "/c", "echo login-complete"]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + #[cfg(not(windows))] + let child = Command::new("sh") + .args(["-c", "echo login-complete"]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let handle = ManagedLoginProcess::default(); + handle.bind(child); + sender + .send(wait_for_child(&handle, Duration::from_secs(2))) + .unwrap(); + }); + let output = receiver + .recv_timeout(Duration::from_secs(5)) + .expect("completed login must not deadlock") + .expect("child output"); + assert!(output.status.success()); + assert!(String::from_utf8_lossy(&output.stdout).contains("login-complete")); + } +} diff --git a/rust/src/codex_cli.rs b/rust/src/codex_cli.rs new file mode 100644 index 0000000000..431ad66f71 --- /dev/null +++ b/rust/src/codex_cli.rs @@ -0,0 +1,127 @@ +//! Canonical Codex CLI executable discovery. +//! +//! Keep install-layout knowledge here so login, provider version detection, and +//! TTY launching do not drift into separate path lists. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +/// Locate the Codex CLI using the explicit override, PATH, and known installs. +pub(crate) fn locate_codex_binary() -> Option { + if let Some(path) = std::env::var_os("CODEX_BINARY") + .map(PathBuf::from) + .filter(|path| path.is_file()) + { + return Some(path); + } + if let Ok(path) = which::which("codex") { + return Some(path); + } + + known_candidates() + .into_iter() + .find(|candidate| candidate.is_file()) + .or_else(desktop_package_binary) +} + +fn known_candidates() -> Vec { + let local_app_data = std::env::var_os("LOCALAPPDATA") + .map(PathBuf::from) + .or_else(dirs::data_local_dir) + .unwrap_or_else(|| { + dirs::home_dir() + .unwrap_or_else(|| PathBuf::from(".")) + .join("AppData") + .join("Local") + }); + let home = dirs::home_dir().unwrap_or_else(|| PathBuf::from(".")); + let desktop_bin = local_app_data.join("OpenAI").join("Codex").join("bin"); + + let mut candidates = vec![ + desktop_bin.join("codex.exe"), + home.join(".bun").join("bin").join("codex.exe"), + local_app_data + .join("Microsoft") + .join("WindowsApps") + .join("codex.exe"), + local_app_data + .join("Programs") + .join("codex") + .join("codex.exe"), + ]; + candidates.extend(versioned_binaries(&desktop_bin)); + + if let Some(roaming) = dirs::config_dir().or_else(dirs::data_dir) { + candidates.push(roaming.join("npm").join("codex.cmd")); + candidates.push( + roaming + .join("fnm") + .join("aliases") + .join("default") + .join("codex.cmd"), + ); + } + candidates +} + +fn versioned_binaries(root: &Path) -> Vec { + let mut binaries: Vec<_> = std::fs::read_dir(root) + .into_iter() + .flatten() + .filter_map(Result::ok) + .map(|entry| entry.path().join("codex.exe")) + .filter(|path| path.is_file()) + .collect(); + binaries.sort_by_key(|path| { + std::cmp::Reverse(path.metadata().and_then(|meta| meta.modified()).ok()) + }); + binaries +} + +#[cfg(windows)] +fn desktop_package_binary() -> Option { + use std::os::windows::process::CommandExt; + + let powershell = PathBuf::from(std::env::var_os("WINDIR")?) + .join("System32/WindowsPowerShell/v1.0/powershell.exe"); + let output = Command::new(powershell) + .args([ + "-NoProfile", + "-NonInteractive", + "-Command", + "Get-AppxPackage -Name OpenAI.Codex | Sort-Object Version -Descending | ForEach-Object { Join-Path $_.InstallLocation 'app\\resources\\codex.exe' } | Where-Object { Test-Path -LiteralPath $_ -PathType Leaf } | Select-Object -First 1", + ]) + .creation_flags(0x0800_0000) + .output() + .ok()?; + if !output.status.success() { + return None; + } + let path = PathBuf::from(String::from_utf8(output.stdout).ok()?.trim()); + path.is_file().then_some(path) +} + +#[cfg(not(windows))] +fn desktop_package_binary() -> Option { + None +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn discovers_versioned_desktop_cli_and_ignores_incomplete_updates() { + let root = tempfile::tempdir().unwrap(); + let installed = root.path().join("hash with spaces"); + std::fs::create_dir(&installed).unwrap(); + std::fs::write(installed.join("codex.exe"), b"fixture").unwrap(); + std::fs::create_dir(root.path().join("incomplete")).unwrap(); + std::fs::write(root.path().join("unrelated"), b"fixture").unwrap(); + assert_eq!( + versioned_binaries(root.path()), + vec![installed.join("codex.exe")] + ); + assert!(versioned_binaries(&root.path().join("missing")).is_empty()); + } +} diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 08ca714a87..3fda0076e7 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -8,6 +8,7 @@ pub mod atomic_file; pub mod browser; pub mod cli; pub mod codex_accounts; +pub(crate) mod codex_cli; pub mod codex_workspaces; pub mod core; pub mod cost_scanner; diff --git a/rust/src/providers/codex/mod.rs b/rust/src/providers/codex/mod.rs index 63c16a6a64..bb4695ff7d 100755 --- a/rust/src/providers/codex/mod.rs +++ b/rust/src/providers/codex/mod.rs @@ -150,24 +150,9 @@ impl Provider for CodexProvider { } } -/// Try to find the codex CLI binary -fn which_codex() -> Option { - // Check common locations on Windows - let possible_paths = [ - // In PATH - which::which("codex").ok(), - // npm global install - dirs::data_dir().map(|p| p.join("npm").join("codex.cmd")), - // AppData locations - dirs::data_local_dir().map(|p| p.join("Programs").join("codex").join("codex.exe")), - ]; - - possible_paths.into_iter().flatten().find(|p| p.exists()) -} - /// Detect the version of the codex CLI fn detect_codex_version() -> Option { - let codex_path = which_codex()?; + let codex_path = crate::codex_cli::locate_codex_binary()?; #[cfg(windows)] const CREATE_NO_WINDOW: u32 = 0x08000000;