CI validation mirror for #449

rust/src/cli/tty_runner.rs

@@ -191,7 +191,7 @@ impl TtyCommandRunner {
     pub fn which(tool: &str) -> Option<PathBuf> {
         // 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<PathBuf> {
-        // 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<PathBuf> {
         // Check environment override

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(_) => {

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<PathBuf> {
-        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<std::path::PathBuf> {
+        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,47 +153,23 @@ impl CodexLoginRunner {
     }
 }
 
-fn path_candidates() -> Vec<PathBuf> {
-    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<std::process::Output> {
     let deadline = Instant::now() + timeout;
     loop {
         if handle.is_cancelled() {
             let output = take_child(handle)?.wait_with_output().ok();
             return output;
         }
-        let polled = {
+        let finished = {
             let mut guard = handle.inner.lock().expect("login process lock");
-            match guard.as_mut().map(|child| child.try_wait()) {
-                Some(Ok(Some(_status))) => 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"));
+    }
+}

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<PathBuf> {
+    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<PathBuf> {
+    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<PathBuf> {
+    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<PathBuf> {
+    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<PathBuf> {
+    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());
+    }
+}

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;

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<std::path::PathBuf> {
-    // 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<String> {
-    let codex_path = which_codex()?;
+    let codex_path = crate::codex_cli::locate_codex_binary()?;
 
     #[cfg(windows)]
     const CREATE_NO_WINDOW: u32 = 0x08000000;