From ab3c282791becff47de63088d1c28aa643214acb Mon Sep 17 00:00:00 2001 From: Cmochance <3216202644@qq.com> Date: Fri, 29 May 2026 11:23:05 +0800 Subject: [PATCH 1/4] feat(settings): add auto-detect button for Codex CLI path The app must locate the real `codex` binary; auto-discovery sometimes lands on a wrong/stale path and some users don't know how to set it by hand. Add an "Auto-detect" button to Settings -> Codex CLI path that force-rescans (ignoring the cached/override path) every common location plus PATH and verifies each candidate is actually runnable via `codex --version` (timeout-guarded, off the UI thread). A lone runnable hit is applied immediately; several open the picker dialog with the verified candidates; none falls back to manual entry -- and the empty case distinguishes "no codex installed" from "codex present but won't run". Backend: new `redetect_codex_cli_path` command (async/spawn_blocking) + `CodexPathResolver::redetect_runnable_paths` + shared `wait_child_with_timeout`; mac/win symmetric `probe_codex_runnable` + `redetect_runnable_codex_cli_paths`. Frontend (shared, serves mac+win): button, handler, en/zh strings. README (x2) + CHANGELOG updated. Hardening from local review: diagnostic logs on probe/wait failure, lone-hit set-rejection fallback to the dialog, candidate cap, and tests for wait_child_with_timeout (shared) + probe_codex_runnable (win, runs on the Linux CI job). --- CHANGELOG.md | 4 + README.md | 4 +- README.zh-CN.md | 4 +- src-tauri/mac/front/index.html | 1 + src-tauri/mac/runtime/process.rs | 80 +++++++++ src-tauri/shared/commands/actions.rs | 28 ++- src-tauri/shared/front/actions.ts | 90 +++++++++- src-tauri/shared/front/base.css | 8 + src-tauri/shared/front/i18n.ts | 18 ++ src-tauri/shared/front/render.ts | 2 + src-tauri/shared/front/tauri.ts | 14 ++ src-tauri/shared/front/types.ts | 7 + src-tauri/shared/runtime/codex_cli_path.rs | 188 ++++++++++++++++++++- src-tauri/shared/runtime/models.rs | 15 ++ src-tauri/src/lib.rs | 1 + src-tauri/win/front/index.html | 1 + src-tauri/win/runtime/process.rs | 112 +++++++++++- 17 files changed, 558 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 902c0b4..2ae238c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 1.5.12 - 2026-05-29 + +- Settings → Codex CLI path gains an **Auto-detect** button next to "Change". Unlike the existing path self-check (which trusts the cached / override path), it force-rescans every common install location plus PATH and verifies each candidate is actually runnable via `codex --version`. A lone runnable hit is applied immediately; several open the dialog with the verified candidates to pick from; none falls back to the manual dialog. Targets the two cases the self-check can't: auto-detection landed on a wrong / stale path, or the user doesn't know where to point it. Backed by a new `redetect_codex_cli_path` command that runs on the blocking pool (each candidate probe spawns a child) with a per-candidate timeout so a hung binary can't wedge the scan. macOS + Windows symmetric. + ## 1.5.11 - 2026-05-16 - Added experimental Linux x86_64 build to the release pipeline. Tagged releases now publish `.deb` (Debian/Ubuntu) and `.AppImage` (generic portable) artifacts alongside the existing macOS / Windows ones. Built on `ubuntu-22.04` (glibc 2.35) so binaries run on Ubuntu 22.04+ / Debian 12+ and equivalent distros. UI, profile switching, and plan / quota readout work as on the other platforms; Linux-native paths for Codex CLI discovery and `codex login` spawning are not separately adapted yet (the non-macOS code branch is currently reused), so feedback issues are welcome. diff --git a/README.md b/README.md index c81808c..6f7b99f 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ - **登录可取消**:进行中的 `codex login` OAuth 流程支持点击同一按钮取消(向子进程 SIGTERM / taskkill),解决浏览器关闭后应用卡在等待回调的场景。 - **plan / quota 智能缓存**:bulk plan refresh 在 6 小时窗口内跳过已确认账号,per-card 刷新按钮也共享同一缓存;切换 / 登录 / 刷新后直接复用 backend 写回的 snapshot,不重复发 IPC。 - **Custom Base URL**:每个账号可独立配置 `OPENAI_BASE_URL`;配置后按钮变红警示(自定义 Base 与 ChatGPT OAuth 账号互斥)。 -- **Codex CLI 路径自检**:自动定位 `codex` 可执行(PATH / `~/.codex/bin` / Homebrew / nvm),找不到或路径错误时设置页可手动指定,结果写入 `install_state.json` 优先生效。 +- **Codex CLI 路径自检**:自动定位 `codex` 可执行(PATH / `~/.codex/bin` / Homebrew / nvm),找不到或路径错误时设置页可手动指定,结果写入 `install_state.json` 优先生效。设置页还提供「自动检测」按钮:忽略可能出错的缓存重新扫描所有常见位置,并用 `codex --version` 验证候选确实可运行——唯一命中直接应用,多个命中时让你选。 - **跨平台原生 Tauri**:macOS arm64 / x64 与 Windows x64 提供原生窗口、原生标题栏 / 关闭按钮,配套 5 套浅色 / 深色主题与中英文界面。 - **本地预览模式**:没有 Tauri 运行时(直接 `vite` 跑前端)时自动使用 mock snapshot,方便单纯调样式。 @@ -143,7 +143,7 @@ npm run version:check # CI 用:拒绝把 semver 字面量写回 *. ### 没有 Codex CLI 怎么办 -应用启动后会探测 `codex` 路径;探测失败时设置页会标红「Codex CLI 路径未找到」,可手动指定(写入 `install_state.json` 的 `user_codex_path` 优先级最高)。未装 Codex CLI 也能用 plan / quota 查看(走 ChatGPT OAuth token 直接拉 rate-limits),但「登录」按钮和切换后启动 Codex 等动作需要 CLI 存在。 +应用启动后会探测 `codex` 路径;探测失败时设置页会标红「Codex CLI 路径未找到」,可手动指定(写入 `install_state.json` 的 `user_codex_path` 优先级最高)。也可以点设置页「Codex CLI 路径」行的「自动检测」按钮强制重新扫描,并用 `codex --version` 验证候选——比启动时的自检更主动,适合自动定位出错或不清楚 `codex` 装在哪的情况。未装 Codex CLI 也能用 plan / quota 查看(走 ChatGPT OAuth token 直接拉 rate-limits),但「登录」按钮和切换后启动 Codex 等动作需要 CLI 存在。 ### 切换账号会丢失原账号的 sessions / 历史吗 diff --git a/README.zh-CN.md b/README.zh-CN.md index c81808c..6f7b99f 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -28,7 +28,7 @@ - **登录可取消**:进行中的 `codex login` OAuth 流程支持点击同一按钮取消(向子进程 SIGTERM / taskkill),解决浏览器关闭后应用卡在等待回调的场景。 - **plan / quota 智能缓存**:bulk plan refresh 在 6 小时窗口内跳过已确认账号,per-card 刷新按钮也共享同一缓存;切换 / 登录 / 刷新后直接复用 backend 写回的 snapshot,不重复发 IPC。 - **Custom Base URL**:每个账号可独立配置 `OPENAI_BASE_URL`;配置后按钮变红警示(自定义 Base 与 ChatGPT OAuth 账号互斥)。 -- **Codex CLI 路径自检**:自动定位 `codex` 可执行(PATH / `~/.codex/bin` / Homebrew / nvm),找不到或路径错误时设置页可手动指定,结果写入 `install_state.json` 优先生效。 +- **Codex CLI 路径自检**:自动定位 `codex` 可执行(PATH / `~/.codex/bin` / Homebrew / nvm),找不到或路径错误时设置页可手动指定,结果写入 `install_state.json` 优先生效。设置页还提供「自动检测」按钮:忽略可能出错的缓存重新扫描所有常见位置,并用 `codex --version` 验证候选确实可运行——唯一命中直接应用,多个命中时让你选。 - **跨平台原生 Tauri**:macOS arm64 / x64 与 Windows x64 提供原生窗口、原生标题栏 / 关闭按钮,配套 5 套浅色 / 深色主题与中英文界面。 - **本地预览模式**:没有 Tauri 运行时(直接 `vite` 跑前端)时自动使用 mock snapshot,方便单纯调样式。 @@ -143,7 +143,7 @@ npm run version:check # CI 用:拒绝把 semver 字面量写回 *. ### 没有 Codex CLI 怎么办 -应用启动后会探测 `codex` 路径;探测失败时设置页会标红「Codex CLI 路径未找到」,可手动指定(写入 `install_state.json` 的 `user_codex_path` 优先级最高)。未装 Codex CLI 也能用 plan / quota 查看(走 ChatGPT OAuth token 直接拉 rate-limits),但「登录」按钮和切换后启动 Codex 等动作需要 CLI 存在。 +应用启动后会探测 `codex` 路径;探测失败时设置页会标红「Codex CLI 路径未找到」,可手动指定(写入 `install_state.json` 的 `user_codex_path` 优先级最高)。也可以点设置页「Codex CLI 路径」行的「自动检测」按钮强制重新扫描,并用 `codex --version` 验证候选——比启动时的自检更主动,适合自动定位出错或不清楚 `codex` 装在哪的情况。未装 Codex CLI 也能用 plan / quota 查看(走 ChatGPT OAuth token 直接拉 rate-limits),但「登录」按钮和切换后启动 Codex 等动作需要 CLI 存在。 ### 切换账号会丢失原账号的 sessions / 历史吗 diff --git a/src-tauri/mac/front/index.html b/src-tauri/mac/front/index.html index 371656b..4c8502f 100644 --- a/src-tauri/mac/front/index.html +++ b/src-tauri/mac/front/index.html @@ -151,6 +151,7 @@

Profiles

Codex CLI path

--

+
diff --git a/src-tauri/mac/runtime/process.rs b/src-tauri/mac/runtime/process.rs index 2d28aa5..3b09354 100644 --- a/src-tauri/mac/runtime/process.rs +++ b/src-tauri/mac/runtime/process.rs @@ -252,6 +252,10 @@ impl CodexPathResolver for MacosCodexPathResolver { fn suggested_paths(&self, codex_home: &Path) -> Vec { suggested_codex_cli_paths(Some(codex_home)) } + + fn redetect_runnable_paths(&self, codex_home: &Path) -> Vec { + redetect_runnable_codex_cli_paths(Some(codex_home)) + } } /// Soft cap on how long the PATH walk can spend stat'ing entries @@ -301,6 +305,82 @@ pub fn suggested_codex_cli_paths(codex_home: Option<&Path>) -> Vec { suggestions } +/// How long a single `codex --version` probe may run before we kill it +/// and treat the candidate as unusable. Keeps a hung or input-waiting +/// binary from wedging the auto-detect scan; a healthy codex answers +/// well under this. +const RUNNABLE_PROBE_TIMEOUT: Duration = Duration::from_secs(3); + +/// Upper bound on how many candidates the auto-detect scan will probe. +/// Each probe spawns a child (up to `RUNNABLE_PROBE_TIMEOUT`), so without +/// a cap a pathological PATH with many `codex` entries could stall the +/// scan. Realistic machines have 1-3 candidates. +const MAX_PROBE_CANDIDATES: usize = 12; + +/// Whether `path` is an actually-runnable codex CLI: it must be a file +/// and `codex --version` must exit successfully within the probe +/// timeout. This is what lets auto-detect reject a stale or broken path +/// that a mere existence check would accept. The "ran but failed" and +/// "couldn't spawn" cases are logged so a broken install leaves a +/// diagnostic trail instead of looking identical to "not found". +fn probe_codex_runnable(path: &Path) -> bool { + if !path.is_file() { + return false; + } + let mut command = Command::new(path); + command + .arg("--version") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + match command.spawn() { + Ok(child) => match crate::shared::codex_cli_path::wait_child_with_timeout( + child, + RUNNABLE_PROBE_TIMEOUT, + ) { + Some(status) if status.success() => true, + Some(status) => { + eprintln!( + "codex probe: {} ran but `--version` exited unsuccessfully ({status})", + path.display() + ); + false + } + None => false, + }, + Err(error) => { + eprintln!("codex probe: failed to spawn {}: {error}", path.display()); + false + } + } +} + +/// Force a fresh scan for the Settings auto-detect button: gather every +/// candidate the discovery + suggestion paths know about (login-shell +/// resolver, fixed install locations, PATH), then keep only those that +/// pass the runnable probe. Ignores the cached/override path entirely so +/// a wrong saved path can be corrected. +pub fn redetect_runnable_codex_cli_paths(codex_home: Option<&Path>) -> Vec { + let managed_shim = codex_home.map(managed_shim_path); + let mut candidates: Vec = Vec::new(); + + // The login-shell resolver catches version-manager installs (nvm / + // asdf) that aren't on the GUI app's PATH; suggestions cover the + // fixed locations plus a bounded PATH walk. + if let Some(shell_path) = discover_real_codex_cli_from_shell(managed_shim.as_deref()) { + push_candidate(&mut candidates, shell_path); + } + for path in suggested_codex_cli_paths(codex_home) { + push_candidate(&mut candidates, path); + } + + candidates + .into_iter() + .take(MAX_PROBE_CANDIDATES) + .filter(|path| probe_codex_runnable(path)) + .collect() +} + fn codex_app_candidates() -> Vec { let mut candidates = vec![PathBuf::from("/Applications/Codex.app")]; if let Some(home) = env::var_os("HOME") { diff --git a/src-tauri/shared/commands/actions.rs b/src-tauri/shared/commands/actions.rs index 581a023..1b86cd6 100644 --- a/src-tauri/shared/commands/actions.rs +++ b/src-tauri/shared/commands/actions.rs @@ -1,8 +1,8 @@ use crate::errors::CommandError; use crate::models::{ - ActionResponse, AddProfilePayload, CodexCliStatus, OpenUrlPayload, ProfilePayload, - RenameProfilePayload, SetCodexCliPathPayload, UpdateCheckPayload, UpdateCheckResponse, - UpdateProfileBaseUrlPayload, + ActionResponse, AddProfilePayload, CodexCliRedetectResult, CodexCliStatus, OpenUrlPayload, + ProfilePayload, RenameProfilePayload, SetCodexCliPathPayload, UpdateCheckPayload, + UpdateCheckResponse, UpdateProfileBaseUrlPayload, }; #[cfg(target_os = "macos")] @@ -222,6 +222,28 @@ pub fn clear_codex_cli_path() -> Result { )) } +/// Force a fresh codex CLI detection scan for the Settings auto-detect +/// button. Runs on the blocking pool because it probes each candidate +/// with `codex --version`, which can take a second or two per path and +/// would otherwise stall the UI thread. +#[tauri::command] +pub async fn redetect_codex_cli_path() -> Result { + tauri::async_runtime::spawn_blocking(|| { + let codex_home = platform_runtime::paths::get_codex_home(); + crate::shared::codex_cli_path::redetect_codex_cli_path( + platform_runtime::codex_cli_resolver(), + &codex_home, + ) + }) + .await + .map_err(|error| { + CommandError::new( + "CODEX_CLI_REDETECT_FAILED", + format!("Redetect task failed: {error}"), + ) + }) +} + #[tauri::command] pub fn cancel_codex_login() -> Result { Ok(crate::shared::login_cancel::cancel_login_in_progress()) diff --git a/src-tauri/shared/front/actions.ts b/src-tauri/shared/front/actions.ts index eebba20..ea89041 100644 --- a/src-tauri/shared/front/actions.ts +++ b/src-tauri/shared/front/actions.ts @@ -34,12 +34,13 @@ import { refreshActiveProfileQuotaSilent, refreshAllOauthProfilePlansSilent, refreshProfile, + redetectCodexCliPath, renameProfile, setCodexCliPath, switchProfile, updateProfileBaseUrl, } from "@front-shared/tauri"; -import type { CodexCliStatus } from "@front-shared/types"; +import type { CodexCliRedetectResult, CodexCliStatus } from "@front-shared/types"; import { applyLocale, elements, @@ -678,7 +679,7 @@ function codexCliSourceLabel(source: CodexCliStatus["source"]): string { } } -function renderCodexCliStatus(status: CodexCliStatus): void { +function renderCodexCliStatus(status: CodexCliStatus, detected?: string[]): void { if (status.resolved_path) { elements.codexCliCurrentValue.textContent = status.resolved_path; elements.codexCliCurrentSource.textContent = ` (${codexCliSourceLabel(status.source)})`; @@ -691,8 +692,17 @@ function renderCodexCliStatus(status: CodexCliStatus): void { elements.clearCodexCliButton.hidden = true; } + // When auto-detect routes here with verified-runnable candidates, show + // those (labelled accordingly) instead of the raw common-location + // hints, so the user only picks from paths that actually ran. + const showingDetected = detected !== undefined && detected.length > 0; + const chips = showingDetected ? detected : status.suggested_paths; + elements.codexCliSuggestionsHeading.textContent = showingDetected + ? t(state.locale, "codexCliDetectedHeading") + : t(state.locale, "codexCliSuggestionsHeading"); + elements.codexCliSuggestions.replaceChildren(); - if (status.suggested_paths.length === 0) { + if (chips.length === 0) { const empty = document.createElement("p"); empty.className = "codex-cli-suggestions-empty"; empty.textContent = t(state.locale, "codexCliSuggestionsEmpty"); @@ -700,7 +710,7 @@ function renderCodexCliStatus(status: CodexCliStatus): void { return; } - for (const path of status.suggested_paths) { + for (const path of chips) { const button = document.createElement("button"); button.type = "button"; button.className = "codex-cli-suggestion"; @@ -715,7 +725,10 @@ function renderCodexCliStatus(status: CodexCliStatus): void { } } -async function openCodexCliDialog(onSavedRetry?: () => Promise): Promise { +async function openCodexCliDialog( + onSavedRetry?: () => Promise, + detectedCandidates?: string[], +): Promise { pendingLoginRetry = onSavedRetry ?? null; clearDialogError(elements.codexCliDialogError); elements.codexCliInput.value = ""; @@ -734,19 +747,79 @@ async function openCodexCliDialog(onSavedRetry?: () => Promise): Promise 0) { + elements.codexCliInput.value = detectedCandidates[0]; + } else if (status.resolved_path) { elements.codexCliInput.value = status.resolved_path; } elements.submitCodexCliButton.textContent = onSavedRetry ? t(state.locale, "codexCliRetryLogin") : t(state.locale, "save"); - renderCodexCliStatus(status); + renderCodexCliStatus(status, detectedCandidates); elements.codexCliDialog.showModal(); elements.codexCliInput.focus(); elements.codexCliInput.select(); } +/// Settings "auto-detect" button: force a fresh runnable scan and act on +/// the result — apply a lone hit, let the user pick when several survive, +/// or fall back to the manual dialog when none do. +async function handleDetectCodexCli(): Promise { + const button = elements.settingsCodexCliDetectButton; + if (button.disabled) { + return; + } + button.disabled = true; + button.textContent = t(state.locale, "settingsCodexCliDetecting"); + try { + const result: CodexCliRedetectResult = await redetectCodexCliPath(); + if (result.candidates.length === 1) { + // Lone runnable hit → apply it straight away. If the backend's + // set/validate then rejects it (managed shim, or the file vanished + // between probe and set), don't dump the raw error — fall back to + // the dialog prefilled with the candidate so the user can adjust. + const only = result.candidates[0]; + try { + const status = await setCodexCliPath(only); + applyCodexCliSettingsDisplay(status); + showToast(t(state.locale, "codexCliDetectApplied", { path: only })); + } catch { + applyCodexCliSettingsDisplay(result.status); + void openCodexCliDialog(undefined, [only]); + } + } else if (result.candidates.length === 0) { + // Nothing runnable. Distinguish "no codex anywhere" from "codex + // exists on disk but none would run" (a broken install, not a + // missing one) via the on-disk suggestions in the refreshed status. + applyCodexCliSettingsDisplay(result.status); + const foundButBroken = result.status.suggested_paths.length > 0; + showToast( + t(state.locale, foundButBroken ? "codexCliDetectFoundButBroken" : "codexCliDetectNone"), + true, + ); + void openCodexCliDialog(); + } else { + // Several runnable hits → let the user choose in the dialog. + applyCodexCliSettingsDisplay(result.status); + showToast( + t(state.locale, "codexCliDetectMultiple", { count: String(result.candidates.length) }), + ); + void openCodexCliDialog(undefined, result.candidates); + } + } catch (error) { + showToast( + error instanceof Error ? error.message : t(state.locale, "codexCliDetectFailed"), + true, + ); + } finally { + button.disabled = false; + button.textContent = t(state.locale, "settingsCodexCliDetect"); + } +} + function closeCodexCliDialog(): void { pendingLoginRetry = null; elements.codexCliDialog.close(); @@ -933,6 +1006,9 @@ export function bootstrap(): void { elements.codexCliForm.addEventListener("submit", (event) => { void handleSubmitCodexCliPath(event as SubmitEvent); }); + elements.settingsCodexCliDetectButton.addEventListener("click", () => { + void handleDetectCodexCli(); + }); elements.settingsCodexCliButton.addEventListener("click", () => { void openCodexCliDialog(); }); diff --git a/src-tauri/shared/front/base.css b/src-tauri/shared/front/base.css index ad69c09..e734a19 100644 --- a/src-tauri/shared/front/base.css +++ b/src-tauri/shared/front/base.css @@ -988,6 +988,14 @@ p { min-width: 0; } +/* Two action buttons now share this row with the path value (auto-detect + + change). Drop the fixed min-width so they size to their label and + leave the flexible remainder to the (ellipsised) path value. */ +.settings-cli-inline .settings-action-button { + min-width: auto; + flex: none; +} + .settings-value--inline { flex: 1; min-width: 0; diff --git a/src-tauri/shared/front/i18n.ts b/src-tauri/shared/front/i18n.ts index 48e8f70..0eaebc1 100644 --- a/src-tauri/shared/front/i18n.ts +++ b/src-tauri/shared/front/i18n.ts @@ -242,6 +242,15 @@ const enMessages = { settingsCodexCli: "Codex CLI path", settingsCodexCliChange: "Change", settingsCodexCliEmpty: "Not detected", + settingsCodexCliDetect: "Auto-detect", + settingsCodexCliDetecting: "Detecting…", + codexCliDetectedHeading: "Detected (verified runnable)", + codexCliDetectApplied: "Detected and set: {path}", + codexCliDetectNone: "Couldn't auto-detect codex. Set the path manually below.", + codexCliDetectFoundButBroken: + "Found codex on disk, but none of them would run. Reinstall codex or set a working path below.", + codexCliDetectMultiple: "Found {count} runnable codex binaries — pick one.", + codexCliDetectFailed: "Auto-detect failed.", } as const; export type MessageKey = keyof typeof enMessages; @@ -488,6 +497,15 @@ const messages: Record = { settingsCodexCli: "Codex CLI 路径", settingsCodexCliChange: "更改", settingsCodexCliEmpty: "未检测到", + settingsCodexCliDetect: "自动检测", + settingsCodexCliDetecting: "检测中…", + codexCliDetectedHeading: "已检测到(已验证可运行)", + codexCliDetectApplied: "已检测并设置:{path}", + codexCliDetectNone: "未能自动检测到 codex,请在下方手动设置路径。", + codexCliDetectFoundButBroken: + "找到了 codex,但都无法运行。请重装 codex 或在下方指定一个可用路径。", + codexCliDetectMultiple: "检测到 {count} 个可运行的 codex,请选择一个。", + codexCliDetectFailed: "自动检测失败。", }, }; diff --git a/src-tauri/shared/front/render.ts b/src-tauri/shared/front/render.ts index 86dd902..6669e6e 100644 --- a/src-tauri/shared/front/render.ts +++ b/src-tauri/shared/front/render.ts @@ -116,6 +116,7 @@ export const elements = { settingsCodexCliLabel: requiredElement("settings-codex-cli-label"), settingsCodexCliValue: requiredElement("settings-codex-cli-value"), settingsCodexCliButton: requiredElement("settings-codex-cli-button"), + settingsCodexCliDetectButton: requiredElement("settings-codex-cli-detect-button"), codexCliDialog: requiredElement("codex-cli-dialog"), codexCliForm: requiredElement("codex-cli-form"), codexCliDialogTitle: requiredElement("codex-cli-dialog-title"), @@ -758,6 +759,7 @@ export function applyLocale(): void { elements.submitCodexCliButton.textContent = t(state.locale, "save"); elements.settingsCodexCliLabel.textContent = t(state.locale, "settingsCodexCli"); elements.settingsCodexCliButton.textContent = t(state.locale, "settingsCodexCliChange"); + elements.settingsCodexCliDetectButton.textContent = t(state.locale, "settingsCodexCliDetect"); // Version label is locale-independent but lives next to the i18n // settings rows; set it here so a single render pass paints both. // `__CODEX_APP_VERSION__` is injected by Vite from `package.json` so diff --git a/src-tauri/shared/front/tauri.ts b/src-tauri/shared/front/tauri.ts index d62675e..0b05e36 100644 --- a/src-tauri/shared/front/tauri.ts +++ b/src-tauri/shared/front/tauri.ts @@ -2,6 +2,7 @@ import { invoke } from "@tauri-apps/api/core"; import type { ActionResponse, + CodexCliRedetectResult, CodexCliStatus, CommandError, CurrentCard, @@ -334,6 +335,15 @@ async function invokeCommand(command: string, args?: Record) source: command === "set_codex_cli_path" ? "user_override" : "discovery", suggested_paths: ["/preview/codex", "/preview/usr/local/bin/codex"], }) as Promise; + case "redetect_codex_cli_path": + return Promise.resolve({ + candidates: ["/preview/codex"], + status: { + resolved_path: "/preview/codex", + source: "user_override", + suggested_paths: ["/preview/codex", "/preview/usr/local/bin/codex"], + }, + }) as Promise; case "cancel_codex_login": return Promise.resolve(true) as Promise; case "open_profile_folder": @@ -464,6 +474,10 @@ export function clearCodexCliPath(): Promise { return invokeCommand("clear_codex_cli_path"); } +export function redetectCodexCliPath(): Promise { + return invokeCommand("redetect_codex_cli_path"); +} + export function cancelCodexLogin(): Promise { return invokeCommand("cancel_codex_login"); } diff --git a/src-tauri/shared/front/types.ts b/src-tauri/shared/front/types.ts index 17c52e0..9187073 100644 --- a/src-tauri/shared/front/types.ts +++ b/src-tauri/shared/front/types.ts @@ -98,4 +98,11 @@ export interface CodexCliStatus { suggested_paths: string[]; } +export interface CodexCliRedetectResult { + /** Paths verified runnable by the forced scan, deduped, best-first. */ + candidates: string[]; + /** Refreshed status snapshot so the Settings row can update in step. */ + status: CodexCliStatus; +} + export type ShellRoute = "dashboard" | "profiles" | "settings" | "guide"; diff --git a/src-tauri/shared/runtime/codex_cli_path.rs b/src-tauri/shared/runtime/codex_cli_path.rs index 91a697c..0d4538e 100644 --- a/src-tauri/shared/runtime/codex_cli_path.rs +++ b/src-tauri/shared/runtime/codex_cli_path.rs @@ -1,6 +1,7 @@ -//! Shared `InstallState` schema + `CodexPathResolver` trait + the four +//! Shared `InstallState` schema + `CodexPathResolver` trait + the //! Tauri-command helpers (`get_codex_cli_status` / `set_codex_cli_path` -//! / `clear_codex_cli_path` / `build_codex_cli_status`). +//! / `clear_codex_cli_path` / `redetect_codex_cli_path` / +//! `build_codex_cli_status`). //! //! Before this module each platform (`mac/runtime/process.rs` + //! `mac/runtime/profile_actions.rs` and the Windows mirrors) carried @@ -13,11 +14,14 @@ //! the `CodexPathResolver` trait so this shared layer is OS-agnostic. use std::path::{Path, PathBuf}; +use std::process::{Child, ExitStatus}; +use std::thread; +use std::time::{Duration, Instant}; use serde::{Deserialize, Serialize}; use crate::errors::AppResult; -use crate::models::CodexCliStatus; +use crate::models::{CodexCliRedetectResult, CodexCliStatus}; /// Persistent install metadata. Both mac and Windows used to declare /// this struct independently; consolidating here keeps the on-disk @@ -77,6 +81,13 @@ pub trait CodexPathResolver { /// Common install locations that exist on disk right now. Frontend /// renders these as click-to-fill chips in the dialog. fn suggested_paths(&self, codex_home: &Path) -> Vec; + + /// Force a fresh scan that ignores the cached/override path and + /// returns only candidates verified runnable via `codex --version` + /// (deduped, best-first). Backs the Settings "auto-detect" button: + /// `resolve_with_source` trusts a previously-saved path, so when + /// that path is wrong the user needs this to rescan from scratch. + fn redetect_runnable_paths(&self, codex_home: &Path) -> Vec; } /// Build the snapshot the front-end consumes. Used by both @@ -129,6 +140,57 @@ pub fn clear_codex_cli_path( build_codex_cli_status(resolver, codex_home) } +/// Force a fresh detection scan and report which candidates are +/// runnable, alongside a refreshed status snapshot. Backs the Settings +/// "auto-detect" button — the front-end auto-applies a lone candidate +/// and lets the user pick when several survive the probe. +pub fn redetect_codex_cli_path( + resolver: &dyn CodexPathResolver, + codex_home: &Path, +) -> CodexCliRedetectResult { + let candidates = resolver + .redetect_runnable_paths(codex_home) + .into_iter() + .map(|path| path.to_string_lossy().into_owned()) + .collect(); + CodexCliRedetectResult { + candidates, + status: build_codex_cli_status(resolver, codex_home), + } +} + +/// Poll-wait for a child process, killing it if it outlives `timeout`. +/// Shared by the per-platform `codex --version` runnable probes so a +/// hung or input-waiting candidate can't wedge the re-detection scan. +/// The platforms own `Command` construction (console hiding, Windows +/// extension resolution); only this bounded wait is common. +pub fn wait_child_with_timeout(mut child: Child, timeout: Duration) -> Option { + let deadline = Instant::now() + timeout; + loop { + match child.try_wait() { + Ok(Some(status)) => return Some(status), + Ok(None) => { + if Instant::now() >= deadline { + let _ = child.kill(); + let _ = child.wait(); + return None; + } + thread::sleep(Duration::from_millis(20)); + } + Err(error) => { + // A real try_wait failure (EINTR, ECHILD, a Windows handle + // error) is indistinguishable from a clean timeout to the + // caller — both return None — so leave a diagnostic trail + // instead of silently demoting the candidate. + eprintln!("wait_child_with_timeout: try_wait failed: {error}"); + let _ = child.kill(); + let _ = child.wait(); + return None; + } + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -148,6 +210,10 @@ mod tests { // return Err(that AppError) to test ? propagation. set_error: RefCell>, suggestions: Vec, + // What `redetect_runnable_paths` returns — the "verified + // runnable" subset, independent of `suggestions`. RefCell so a + // test can vary it (empty / multiple) per case. + runnable: RefCell>, clear_calls: RefCell, } @@ -157,6 +223,7 @@ mod tests { state: RefCell::new(None), set_error: RefCell::new(None), suggestions: vec![PathBuf::from("/fake/suggested/codex")], + runnable: RefCell::new(vec![PathBuf::from("/fake/runnable/codex")]), clear_calls: RefCell::new(0), } } @@ -188,6 +255,10 @@ mod tests { fn suggested_paths(&self, _codex_home: &Path) -> Vec { self.suggestions.clone() } + + fn redetect_runnable_paths(&self, _codex_home: &Path) -> Vec { + self.runnable.borrow().clone() + } } #[test] @@ -256,4 +327,115 @@ mod tests { ); assert_eq!(status.source, "discovery"); } + + #[test] + fn redetect_returns_runnable_candidates_plus_refreshed_status() { + let resolver = FakeResolver::new(); + let codex_home = PathBuf::from("/fake/home"); + // Seed auto-discovery so the bundled status snapshot is non-empty. + *resolver.state.borrow_mut() = Some(( + PathBuf::from("/fake/discovered/codex"), + RealCodexPathSource::Discovery, + )); + + let result = redetect_codex_cli_path(&resolver, &codex_home); + + // Candidates come straight from the resolver's runnable probe, + // not from the suggestion list. + assert_eq!(result.candidates, vec!["/fake/runnable/codex".to_string()]); + // The bundled status is rebuilt live, so the Settings row can + // refresh from the same call. + assert_eq!( + result.status.resolved_path.as_deref(), + Some("/fake/discovered/codex") + ); + assert_eq!(result.status.source, "discovery"); + } + + #[test] + fn redetect_with_no_runnable_candidates_returns_empty_plus_status() { + let resolver = FakeResolver::new(); + let codex_home = PathBuf::from("/fake/home"); + // Nothing survives the runnable probe... + *resolver.runnable.borrow_mut() = vec![]; + // ...but auto-discovery still resolves a (stale) path, so the + // bundled status must stay populated for the Settings row. + *resolver.state.borrow_mut() = Some(( + PathBuf::from("/fake/stale/codex"), + RealCodexPathSource::Discovery, + )); + + let result = redetect_codex_cli_path(&resolver, &codex_home); + + assert!(result.candidates.is_empty()); + assert_eq!( + result.status.resolved_path.as_deref(), + Some("/fake/stale/codex") + ); + } + + #[test] + fn redetect_preserves_multiple_candidates_in_order() { + let resolver = FakeResolver::new(); + let codex_home = PathBuf::from("/fake/home"); + *resolver.runnable.borrow_mut() = vec![ + PathBuf::from("/fake/first/codex"), + PathBuf::from("/fake/second/codex"), + ]; + + let result = redetect_codex_cli_path(&resolver, &codex_home); + + // Order is the contract the front-end relies on (it prefills [0]). + assert_eq!( + result.candidates, + vec![ + "/fake/first/codex".to_string(), + "/fake/second/codex".to_string(), + ] + ); + } + + #[cfg(unix)] + #[test] + fn wait_child_with_timeout_returns_status_for_fast_exit() { + use std::process::Command; + + // Absolute path + shell builtin so a sibling test that mutates + // PATH (the mac/win `discover_*` tests do) can't make these spawns + // fail with NotFound. `exit N` is a builtin, so no PATH lookup. + let zero = Command::new("/bin/sh") + .args(["-c", "exit 0"]) + .spawn() + .expect("spawn /bin/sh"); + assert!(wait_child_with_timeout(zero, Duration::from_secs(5)) + .is_some_and(|status| status.success())); + + // A clean non-zero exit must be observed as Some(!success), never + // collapsed into the timeout/error None — `probe_codex_runnable` + // relies on telling "ran but failed" from "didn't run". + let nonzero = Command::new("/bin/sh") + .args(["-c", "exit 3"]) + .spawn() + .expect("spawn /bin/sh"); + assert!(wait_child_with_timeout(nonzero, Duration::from_secs(5)) + .is_some_and(|status| !status.success())); + } + + #[cfg(unix)] + #[test] + fn wait_child_with_timeout_kills_and_returns_none_on_overrun() { + use std::process::Command; + + let start = Instant::now(); + // Absolute path (present on macOS + Ubuntu) so a PATH-mutating + // sibling test can't break the spawn. + let sleeper = Command::new("/bin/sleep") + .arg("30") + .spawn() + .expect("spawn /bin/sleep"); + // 200ms budget against a 30s sleep: it must be killed and reported + // as None, and we must not actually block anywhere near 30s. + assert!(wait_child_with_timeout(sleeper, Duration::from_millis(200)).is_none()); + assert!(start.elapsed() < Duration::from_secs(5)); + } } diff --git a/src-tauri/shared/runtime/models.rs b/src-tauri/shared/runtime/models.rs index 0243193..3bb30db 100644 --- a/src-tauri/shared/runtime/models.rs +++ b/src-tauri/shared/runtime/models.rs @@ -214,6 +214,21 @@ pub struct CodexCliStatus { pub suggested_paths: Vec, } +/// Result of a forced re-detection scan triggered by the Settings +/// "auto-detect" button. Unlike `get_codex_cli_status` (which honours +/// the cached/override path), this rescans from scratch and keeps only +/// the candidates that pass a `codex --version` runnable probe. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct CodexCliRedetectResult { + /// Paths that were verified runnable, deduped and best-first. The + /// front-end auto-applies a lone hit and lets the user pick when + /// there are several. + pub candidates: Vec, + /// Refreshed status snapshot so the Settings row and the dialog can + /// update in lock-step after the scan. + pub status: CodexCliStatus, +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SetCodexCliPathPayload { pub path: String, diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 71d3d36..579546b 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -59,6 +59,7 @@ pub fn run() { commands::actions::get_codex_cli_status, commands::actions::set_codex_cli_path, commands::actions::clear_codex_cli_path, + commands::actions::redetect_codex_cli_path, commands::actions::cancel_codex_login, commands::switch::switch_profile, ]) diff --git a/src-tauri/win/front/index.html b/src-tauri/win/front/index.html index c4bfe79..9161f6c 100644 --- a/src-tauri/win/front/index.html +++ b/src-tauri/win/front/index.html @@ -170,6 +170,7 @@

Profiles

Codex CLI path

--

+
diff --git a/src-tauri/win/runtime/process.rs b/src-tauri/win/runtime/process.rs index 716bbad..9cc847d 100644 --- a/src-tauri/win/runtime/process.rs +++ b/src-tauri/win/runtime/process.rs @@ -511,6 +511,10 @@ impl CodexPathResolver for WindowsCodexPathResolver { fn suggested_paths(&self, codex_home: &Path) -> Vec { suggested_codex_cli_paths(Some(codex_home)) } + + fn redetect_runnable_paths(&self, codex_home: &Path) -> Vec { + redetect_runnable_codex_cli_paths(Some(codex_home)) + } } /// Return common codex CLI install locations on Windows that actually @@ -569,6 +573,71 @@ pub fn suggested_codex_cli_paths(codex_home: Option<&Path>) -> Vec { suggestions } +/// How long a single `codex --version` probe may run before we kill it +/// and treat the candidate as unusable. A little more generous than +/// macOS: a Windows `.cmd` shim plus npm wrapper has a slower cold +/// start, but a healthy codex still answers well under this. +const RUNNABLE_PROBE_TIMEOUT: Duration = Duration::from_secs(5); + +/// Upper bound on how many candidates the auto-detect scan will probe. +/// Each probe spawns a child (up to `RUNNABLE_PROBE_TIMEOUT`), so without +/// a cap a machine with many `where codex` hits could stall the scan. +/// Realistic machines have 1-3 candidates. +const MAX_PROBE_CANDIDATES: usize = 12; + +/// Whether `path` is an actually-runnable codex CLI: it must be a file +/// and `codex --version` must exit successfully within the probe +/// timeout. This is what lets auto-detect reject a stale or broken path +/// that a mere existence check would accept. The "ran but failed" and +/// "couldn't spawn" cases are logged so a broken install leaves a +/// diagnostic trail instead of looking identical to "not found". +fn probe_codex_runnable(path: &Path) -> bool { + if !path.is_file() { + return false; + } + let mut command = Command::new(path); + command + .arg("--version") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + match hide_console_window(&mut command).spawn() { + Ok(child) => match crate::shared::codex_cli_path::wait_child_with_timeout( + child, + RUNNABLE_PROBE_TIMEOUT, + ) { + Some(status) if status.success() => true, + Some(status) => { + eprintln!( + "codex probe: {} ran but `--version` exited unsuccessfully ({status})", + path.display() + ); + false + } + None => false, + }, + Err(error) => { + eprintln!("codex probe: failed to spawn {}: {error}", path.display()); + false + } + } +} + +/// Force a fresh scan for the Settings auto-detect button, keeping only +/// candidates that pass the runnable probe. Reuses +/// `suggested_codex_cli_paths`, which already resolves Windows +/// extensions, filters the managed shim / Windows Apps aliases, and +/// folds in `where codex` (every PATH match) — so it is the full +/// candidate set. Ignores the cached/override path so a wrong saved +/// path can be corrected. +pub fn redetect_runnable_codex_cli_paths(codex_home: Option<&Path>) -> Vec { + suggested_codex_cli_paths(codex_home) + .into_iter() + .take(MAX_PROBE_CANDIDATES) + .filter(|path| probe_codex_runnable(path)) + .collect() +} + pub fn quit_codex_app_if_running() -> AppResult { if !is_codex_app_running() { return Ok(false); @@ -678,8 +747,9 @@ impl PlatformHooks for WindowsPlatformHooks { mod tests { use super::{ build_app_server_command, discover_real_codex_cli_path, is_acceptable_real_codex_cli_path, - load_install_state, resolve_real_codex_cli, resolve_windows_app_target, - windows_store_shell_target, AppLaunchTarget, InstallState, WINDOWS_STORE_APP_ID, + load_install_state, probe_codex_runnable, resolve_real_codex_cli, + resolve_windows_app_target, windows_store_shell_target, AppLaunchTarget, InstallState, + WINDOWS_STORE_APP_ID, }; use crate::windows::env_guard; use serde_json::to_string_pretty; @@ -695,6 +765,44 @@ mod tests { std::env::temp_dir().join(format!("codex-switch-process-{name}-{unique}")) } + // Runs on the Linux `cargo test --lib` job (the win module compiles on + // non-macOS): a `#!/bin/sh` candidate is spawnable there, and + // `hide_console_window` is a no-op off Windows. Pins the three + // behaviours auto-detect depends on: a non-file is rejected without + // spawning, a binary that runs but exits non-zero is rejected (broken + // install), and only a zero-exit binary is accepted. + #[cfg(unix)] + #[test] + fn probe_codex_runnable_rejects_missing_and_failing_accepts_zero_exit() { + use std::os::unix::fs::PermissionsExt; + + let codex_home = temp_codex_home("probe-runnable"); + fs::create_dir_all(&codex_home).unwrap(); + + // (a) non-file path → false, never spawned. + assert!(!probe_codex_runnable(&codex_home.join("does-not-exist"))); + + let set_exec = |path: &std::path::Path| { + let mut perm = fs::metadata(path).unwrap().permissions(); + perm.set_mode(0o755); + fs::set_permissions(path, perm).unwrap(); + }; + + // (b) exists + runs but exits non-zero → false (broken install). + let bad = codex_home.join("bad-codex"); + fs::write(&bad, "#!/bin/sh\nexit 3\n").unwrap(); + set_exec(&bad); + assert!(!probe_codex_runnable(&bad)); + + // (c) exists + exits zero → true. + let good = codex_home.join("good-codex"); + fs::write(&good, "#!/bin/sh\nexit 0\n").unwrap(); + set_exec(&good); + assert!(probe_codex_runnable(&good)); + + let _ = fs::remove_dir_all(&codex_home); + } + #[test] fn discover_real_codex_cli_path_prefers_cmd_and_skips_managed_shim() { let _guard = env_guard(); From 50b7e9363a5732fd65bb11076f3fa7a33a06a44b Mon Sep 17 00:00:00 2001 From: Cmochance <3216202644@qq.com> Date: Fri, 29 May 2026 12:52:40 +0800 Subject: [PATCH 2/4] feat(settings): make auto-detect comprehensive + list multiple installs Reworks the auto-detect button per hands-on testing feedback (the button is the small-user last-resort, so it must find codex broadly and, when there are several, list the real ones to pick from): - More comprehensive discovery: add login-shell resolution (`$SHELL -lc 'command -v codex'`) so installs on the user's PATH (nvm / asdf / brew / fnm) are found even under Finder's narrow launchd PATH, while bypassing any `codex` shell function. Joined with the managed-shim resolver, Codex.app bundle, fixed locations, and PATH walk. - Confirm authenticity + capture version: each candidate is probed with `codex --version` via a new shared `probe_version_with_timeout` (poll + kill + stdout capture), replacing the bool-only `wait_child_with_timeout`. - Multiple installs -> list to pick: the dialog shows each verified candidate as `path | version` under a "detected (verified runnable)" heading with a "pick one" copy -- not the misleading "couldn't find codex" copy that previously appeared even when two were found. - One install -> applied directly (one-click fallback); none -> manual dialog, distinguishing "broken install" from "not installed". CodexCliCandidate{path, version} replaces bare path strings end to end (model, trait, mac/win impls, TS types, dialog rendering). Tests updated: probe_version_with_timeout (capture / non-zero / overrun), redetect multi/empty, win probe_codex_version. --- src-tauri/mac/runtime/process.rs | 110 ++++++++------ src-tauri/shared/front/actions.ts | 53 ++++--- src-tauri/shared/front/i18n.ts | 4 + src-tauri/shared/front/tauri.ts | 2 +- src-tauri/shared/front/types.ts | 11 +- src-tauri/shared/runtime/codex_cli_path.rs | 158 ++++++++++++--------- src-tauri/shared/runtime/models.rs | 24 +++- src-tauri/win/runtime/process.rs | 82 +++++------ 8 files changed, 267 insertions(+), 177 deletions(-) diff --git a/src-tauri/mac/runtime/process.rs b/src-tauri/mac/runtime/process.rs index 3b09354..4ca1636 100644 --- a/src-tauri/mac/runtime/process.rs +++ b/src-tauri/mac/runtime/process.rs @@ -9,6 +9,7 @@ use std::time::Duration; use crate::errors::{AppError, AppResult}; use crate::platform::hooks::PlatformHooks; use crate::shared::codex_app_server::{fetch_account_snapshot, AppServerSnapshot}; +use crate::models::CodexCliCandidate; use crate::shared::codex_cli_path::CodexPathResolver; pub use crate::shared::codex_cli_path::{InstallState, RealCodexPathSource}; use crate::shared::login_cancel::wait_for_login_or_cancel; @@ -253,7 +254,7 @@ impl CodexPathResolver for MacosCodexPathResolver { suggested_codex_cli_paths(Some(codex_home)) } - fn redetect_runnable_paths(&self, codex_home: &Path) -> Vec { + fn redetect_runnable_paths(&self, codex_home: &Path) -> Vec { redetect_runnable_codex_cli_paths(Some(codex_home)) } } @@ -317,56 +318,78 @@ const RUNNABLE_PROBE_TIMEOUT: Duration = Duration::from_secs(3); /// scan. Realistic machines have 1-3 candidates. const MAX_PROBE_CANDIDATES: usize = 12; -/// Whether `path` is an actually-runnable codex CLI: it must be a file -/// and `codex --version` must exit successfully within the probe -/// timeout. This is what lets auto-detect reject a stale or broken path -/// that a mere existence check would accept. The "ran but failed" and -/// "couldn't spawn" cases are logged so a broken install leaves a -/// diagnostic trail instead of looking identical to "not found". -fn probe_codex_runnable(path: &Path) -> bool { +/// Probe whether `path` is a runnable codex CLI and capture its version. +/// `Some(version)` (possibly empty) means it's a file that ran and exited +/// 0; `None` means not-a-file, couldn't spawn, exited non-zero, or timed +/// out. The failure is logged so a broken install leaves a diagnostic +/// trail instead of looking identical to "not found". +fn probe_codex_version(path: &Path) -> Option { if !path.is_file() { - return false; + return None; } let mut command = Command::new(path); - command - .arg("--version") - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()); - match command.spawn() { - Ok(child) => match crate::shared::codex_cli_path::wait_child_with_timeout( - child, - RUNNABLE_PROBE_TIMEOUT, - ) { - Some(status) if status.success() => true, - Some(status) => { - eprintln!( - "codex probe: {} ran but `--version` exited unsuccessfully ({status})", - path.display() - ); - false - } - None => false, - }, - Err(error) => { - eprintln!("codex probe: failed to spawn {}: {error}", path.display()); - false - } + command.arg("--version"); + let result = + crate::shared::codex_cli_path::probe_version_with_timeout(command, RUNNABLE_PROBE_TIMEOUT); + if result.is_none() { + eprintln!( + "codex probe: {} is not a runnable codex (spawn / non-zero exit / timeout)", + path.display() + ); + } + result +} + +/// Resolve codex through the user's login shell, so installs on the +/// shell's PATH (nvm / asdf / brew / fnm / any rc-managed location) are +/// found even when the app was launched from Finder with the narrow +/// launchd PATH. A non-interactive login shell + `command -v` avoids +/// loading the user's `codex` shell *function* (if any), so we resolve to +/// the real binary; the result is verified to be an absolute file. +fn discover_codex_via_login_shell(managed_shim_path: Option<&Path>) -> Option { + let shell = env::var_os("SHELL")?; + let output = Command::new(&shell) + .args(["-lc", "command -v codex"]) + .output() + .ok()?; + if !output.status.success() { + return None; + } + let stdout = String::from_utf8_lossy(&output.stdout); + let resolved = stdout + .lines() + .map(str::trim) + .find(|value| !value.is_empty())?; + let candidate = PathBuf::from(resolved); + // `command -v` of a function/alias returns a bare name, not a path — + // require an absolute path to a real file so we never feed that back. + if !candidate.is_absolute() || !candidate.is_file() { + return None; } + if managed_shim_path.is_some_and(|managed| managed == candidate.as_path()) { + return None; + } + Some(candidate) } /// Force a fresh scan for the Settings auto-detect button: gather every /// candidate the discovery + suggestion paths know about (login-shell -/// resolver, fixed install locations, PATH), then keep only those that -/// pass the runnable probe. Ignores the cached/override path entirely so -/// a wrong saved path can be corrected. -pub fn redetect_runnable_codex_cli_paths(codex_home: Option<&Path>) -> Vec { +/// resolution, managed-shim resolver, Codex.app bundle, fixed install +/// locations, PATH), then keep only those that pass the runnable probe, +/// capturing each one's version. Ignores the cached/override path so a +/// wrong saved path can be corrected. +pub fn redetect_runnable_codex_cli_paths(codex_home: Option<&Path>) -> Vec { let managed_shim = codex_home.map(managed_shim_path); let mut candidates: Vec = Vec::new(); - // The login-shell resolver catches version-manager installs (nvm / - // asdf) that aren't on the GUI app's PATH; suggestions cover the - // fixed locations plus a bounded PATH walk. + // Login-shell resolution first — catches nvm / asdf / brew / fnm + // installs on the user's PATH even under Finder's narrow launchd PATH. + if let Some(path) = discover_codex_via_login_shell(managed_shim.as_deref()) { + push_candidate(&mut candidates, path); + } + // The managed-shim resolver (present when codex_switch installed its + // shim) and the suggestion list (Codex.app bundle, fixed locations, + // bounded PATH walk) fill in the rest. if let Some(shell_path) = discover_real_codex_cli_from_shell(managed_shim.as_deref()) { push_candidate(&mut candidates, shell_path); } @@ -377,7 +400,12 @@ pub fn redetect_runnable_codex_cli_paths(codex_home: Option<&Path>) -> Vec 0; - const chips = showingDetected ? detected : status.suggested_paths; elements.codexCliSuggestionsHeading.textContent = showingDetected ? t(state.locale, "codexCliDetectedHeading") : t(state.locale, "codexCliSuggestionsHeading"); + // Normalise both sources to { path, version } so one render loop serves + // detected candidates (with versions) and plain suggestion hints. + const chips: CodexCliCandidate[] = showingDetected + ? detected + : status.suggested_paths.map((path) => ({ path, version: null })); + elements.codexCliSuggestions.replaceChildren(); if (chips.length === 0) { const empty = document.createElement("p"); @@ -710,13 +715,15 @@ function renderCodexCliStatus(status: CodexCliStatus, detected?: string[]): void return; } - for (const path of chips) { + for (const candidate of chips) { const button = document.createElement("button"); button.type = "button"; button.className = "codex-cli-suggestion"; - button.textContent = path; + button.textContent = candidate.version + ? `${candidate.path} · ${candidate.version}` + : candidate.path; button.addEventListener("click", () => { - elements.codexCliInput.value = path; + elements.codexCliInput.value = candidate.path; elements.codexCliInput.focus(); elements.codexCliInput.select(); clearDialogError(elements.codexCliDialogError); @@ -727,7 +734,7 @@ function renderCodexCliStatus(status: CodexCliStatus, detected?: string[]): void async function openCodexCliDialog( onSavedRetry?: () => Promise, - detectedCandidates?: string[], + detectedCandidates?: CodexCliCandidate[], ): Promise { pendingLoginRetry = onSavedRetry ?? null; clearDialogError(elements.codexCliDialogError); @@ -747,10 +754,17 @@ async function openCodexCliDialog( ); } - // Prefill the first detected candidate when auto-detect routed here - // with several runnable hits; otherwise fall back to the resolved path. + // Detected-mode opens after auto-detect found several runnable codex, + // so use a "pick one" copy — NOT the default "couldn't find it" copy + // that misled users into thinking detection had failed. + const hasDetected = detectedCandidates !== undefined && detectedCandidates.length > 0; + elements.codexCliDialogCopy.textContent = hasDetected + ? t(state.locale, "codexCliDetectPickCopy") + : t(state.locale, "codexCliDialogCopy"); + + // Prefill the first detected candidate; otherwise the resolved path. if (detectedCandidates !== undefined && detectedCandidates.length > 0) { - elements.codexCliInput.value = detectedCandidates[0]; + elements.codexCliInput.value = detectedCandidates[0].path; } else if (status.resolved_path) { elements.codexCliInput.value = status.resolved_path; } @@ -777,18 +791,19 @@ async function handleDetectCodexCli(): Promise { try { const result: CodexCliRedetectResult = await redetectCodexCliPath(); if (result.candidates.length === 1) { - // Lone runnable hit → apply it straight away. If the backend's - // set/validate then rejects it (managed shim, or the file vanished - // between probe and set), don't dump the raw error — fall back to - // the dialog prefilled with the candidate so the user can adjust. + // Lone runnable hit → apply it straight away (the small-user + // fallback: one click, done). If the backend's set/validate then + // rejects it (managed shim, or the file vanished between probe and + // set), don't dump the raw error — fall back to the dialog with the + // candidate so the user can adjust. const only = result.candidates[0]; try { - const status = await setCodexCliPath(only); + const status = await setCodexCliPath(only.path); applyCodexCliSettingsDisplay(status); - showToast(t(state.locale, "codexCliDetectApplied", { path: only })); + showToast(t(state.locale, "codexCliDetectApplied", { path: only.path })); } catch { applyCodexCliSettingsDisplay(result.status); - void openCodexCliDialog(undefined, [only]); + void openCodexCliDialog(undefined, result.candidates); } } else if (result.candidates.length === 0) { // Nothing runnable. Distinguish "no codex anywhere" from "codex diff --git a/src-tauri/shared/front/i18n.ts b/src-tauri/shared/front/i18n.ts index 0eaebc1..cf4a369 100644 --- a/src-tauri/shared/front/i18n.ts +++ b/src-tauri/shared/front/i18n.ts @@ -249,6 +249,8 @@ const enMessages = { codexCliDetectNone: "Couldn't auto-detect codex. Set the path manually below.", codexCliDetectFoundButBroken: "Found codex on disk, but none of them would run. Reinstall codex or set a working path below.", + codexCliDetectPickCopy: + "Found multiple runnable codex installs — pick the one to use, or set a path manually below.", codexCliDetectMultiple: "Found {count} runnable codex binaries — pick one.", codexCliDetectFailed: "Auto-detect failed.", } as const; @@ -504,6 +506,8 @@ const messages: Record = { codexCliDetectNone: "未能自动检测到 codex,请在下方手动设置路径。", codexCliDetectFoundButBroken: "找到了 codex,但都无法运行。请重装 codex 或在下方指定一个可用路径。", + codexCliDetectPickCopy: + "检测到多个可运行的 codex,选择要使用的那个;也可以在下方手动指定。", codexCliDetectMultiple: "检测到 {count} 个可运行的 codex,请选择一个。", codexCliDetectFailed: "自动检测失败。", }, diff --git a/src-tauri/shared/front/tauri.ts b/src-tauri/shared/front/tauri.ts index 0b05e36..1ff0af9 100644 --- a/src-tauri/shared/front/tauri.ts +++ b/src-tauri/shared/front/tauri.ts @@ -337,7 +337,7 @@ async function invokeCommand(command: string, args?: Record) }) as Promise; case "redetect_codex_cli_path": return Promise.resolve({ - candidates: ["/preview/codex"], + candidates: [{ path: "/preview/codex", version: "codex-cli 0.133.0" }], status: { resolved_path: "/preview/codex", source: "user_override", diff --git a/src-tauri/shared/front/types.ts b/src-tauri/shared/front/types.ts index 9187073..5314c1f 100644 --- a/src-tauri/shared/front/types.ts +++ b/src-tauri/shared/front/types.ts @@ -98,9 +98,16 @@ export interface CodexCliStatus { suggested_paths: string[]; } +export interface CodexCliCandidate { + /** Absolute path to the verified-runnable codex binary. */ + path: string; + /** `codex --version` line (e.g. "codex-cli 0.133.0"), or null if it ran but printed nothing parseable. */ + version: string | null; +} + export interface CodexCliRedetectResult { - /** Paths verified runnable by the forced scan, deduped, best-first. */ - candidates: string[]; + /** Candidates verified runnable by the forced scan, deduped, best-first. */ + candidates: CodexCliCandidate[]; /** Refreshed status snapshot so the Settings row can update in step. */ status: CodexCliStatus; } diff --git a/src-tauri/shared/runtime/codex_cli_path.rs b/src-tauri/shared/runtime/codex_cli_path.rs index 0d4538e..af58359 100644 --- a/src-tauri/shared/runtime/codex_cli_path.rs +++ b/src-tauri/shared/runtime/codex_cli_path.rs @@ -13,15 +13,16 @@ //! managed-shim filtering — are kept per-platform and reached through //! the `CodexPathResolver` trait so this shared layer is OS-agnostic. +use std::io::Read; use std::path::{Path, PathBuf}; -use std::process::{Child, ExitStatus}; +use std::process::{Command, Stdio}; use std::thread; use std::time::{Duration, Instant}; use serde::{Deserialize, Serialize}; use crate::errors::AppResult; -use crate::models::{CodexCliRedetectResult, CodexCliStatus}; +use crate::models::{CodexCliCandidate, CodexCliRedetectResult, CodexCliStatus}; /// Persistent install metadata. Both mac and Windows used to declare /// this struct independently; consolidating here keeps the on-disk @@ -83,11 +84,12 @@ pub trait CodexPathResolver { fn suggested_paths(&self, codex_home: &Path) -> Vec; /// Force a fresh scan that ignores the cached/override path and - /// returns only candidates verified runnable via `codex --version` - /// (deduped, best-first). Backs the Settings "auto-detect" button: - /// `resolve_with_source` trusts a previously-saved path, so when - /// that path is wrong the user needs this to rescan from scratch. - fn redetect_runnable_paths(&self, codex_home: &Path) -> Vec; + /// returns every candidate verified runnable via `codex --version` + /// (deduped, best-first, with the version string captured). Backs the + /// Settings "auto-detect" button: `resolve_with_source` trusts a + /// previously-saved path, so when that path is wrong — or there are + /// several installs — the user needs this to rescan from scratch. + fn redetect_runnable_paths(&self, codex_home: &Path) -> Vec; } /// Build the snapshot the front-end consumes. Used by both @@ -148,27 +150,46 @@ pub fn redetect_codex_cli_path( resolver: &dyn CodexPathResolver, codex_home: &Path, ) -> CodexCliRedetectResult { - let candidates = resolver - .redetect_runnable_paths(codex_home) - .into_iter() - .map(|path| path.to_string_lossy().into_owned()) - .collect(); CodexCliRedetectResult { - candidates, + candidates: resolver.redetect_runnable_paths(codex_home), status: build_codex_cli_status(resolver, codex_home), } } -/// Poll-wait for a child process, killing it if it outlives `timeout`. -/// Shared by the per-platform `codex --version` runnable probes so a -/// hung or input-waiting candidate can't wedge the re-detection scan. -/// The platforms own `Command` construction (console hiding, Windows -/// extension resolution); only this bounded wait is common. -pub fn wait_child_with_timeout(mut child: Child, timeout: Duration) -> Option { +/// Run a configured `codex --version` `command`, bounded by `timeout`, +/// and return its trimmed first stdout line on a successful exit. This is +/// the one-shot "is this a real, runnable codex, and which version" probe +/// behind auto-detect: `Some(version)` means it ran and exited 0 (the +/// string may be empty if it printed nothing parseable); `None` means it +/// couldn't spawn, exited non-zero, errored, or overran the timeout (the +/// child is then killed so a hung / input-waiting binary can't wedge the +/// scan). Shared so mac + Windows reuse the poll/kill/capture logic; each +/// platform only builds the `Command` (console hiding, extension res). +pub fn probe_version_with_timeout(mut command: Command, timeout: Duration) -> Option { + command + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + let mut child = command.spawn().ok()?; let deadline = Instant::now() + timeout; loop { match child.try_wait() { - Ok(Some(status)) => return Some(status), + Ok(Some(status)) => { + if !status.success() { + return None; + } + let mut out = String::new(); + if let Some(mut stdout) = child.stdout.take() { + let _ = stdout.read_to_string(&mut out); + } + return Some( + out.lines() + .map(str::trim) + .find(|line| !line.is_empty()) + .unwrap_or("") + .to_owned(), + ); + } Ok(None) => { if Instant::now() >= deadline { let _ = child.kill(); @@ -182,7 +203,7 @@ pub fn wait_child_with_timeout(mut child: Child, timeout: Duration) -> Option>, suggestions: Vec, // What `redetect_runnable_paths` returns — the "verified - // runnable" subset, independent of `suggestions`. RefCell so a - // test can vary it (empty / multiple) per case. - runnable: RefCell>, + // runnable" candidates, independent of `suggestions`. RefCell so + // a test can vary it (empty / multiple) per case. + runnable: RefCell>, clear_calls: RefCell, } @@ -223,7 +244,10 @@ mod tests { state: RefCell::new(None), set_error: RefCell::new(None), suggestions: vec![PathBuf::from("/fake/suggested/codex")], - runnable: RefCell::new(vec![PathBuf::from("/fake/runnable/codex")]), + runnable: RefCell::new(vec![CodexCliCandidate { + path: "/fake/runnable/codex".to_string(), + version: Some("codex-cli 1.2.3".to_string()), + }]), clear_calls: RefCell::new(0), } } @@ -256,7 +280,7 @@ mod tests { self.suggestions.clone() } - fn redetect_runnable_paths(&self, _codex_home: &Path) -> Vec { + fn redetect_runnable_paths(&self, _codex_home: &Path) -> Vec { self.runnable.borrow().clone() } } @@ -341,8 +365,14 @@ mod tests { let result = redetect_codex_cli_path(&resolver, &codex_home); // Candidates come straight from the resolver's runnable probe, - // not from the suggestion list. - assert_eq!(result.candidates, vec!["/fake/runnable/codex".to_string()]); + // not from the suggestion list — version included. + assert_eq!( + result.candidates, + vec![CodexCliCandidate { + path: "/fake/runnable/codex".to_string(), + version: Some("codex-cli 1.2.3".to_string()), + }] + ); // The bundled status is rebuilt live, so the Settings row can // refresh from the same call. assert_eq!( @@ -379,63 +409,63 @@ mod tests { let resolver = FakeResolver::new(); let codex_home = PathBuf::from("/fake/home"); *resolver.runnable.borrow_mut() = vec![ - PathBuf::from("/fake/first/codex"), - PathBuf::from("/fake/second/codex"), + CodexCliCandidate { + path: "/fake/first/codex".to_string(), + version: Some("codex-cli 0.133.0".to_string()), + }, + CodexCliCandidate { + path: "/fake/second/codex".to_string(), + version: None, + }, ]; let result = redetect_codex_cli_path(&resolver, &codex_home); // Order is the contract the front-end relies on (it prefills [0]). + let paths: Vec<&str> = result.candidates.iter().map(|c| c.path.as_str()).collect(); + assert_eq!(paths, vec!["/fake/first/codex", "/fake/second/codex"]); + // Version rides along per candidate (one known, one unparsed). assert_eq!( - result.candidates, - vec![ - "/fake/first/codex".to_string(), - "/fake/second/codex".to_string(), - ] + result.candidates[0].version.as_deref(), + Some("codex-cli 0.133.0") ); + assert_eq!(result.candidates[1].version, None); } #[cfg(unix)] #[test] - fn wait_child_with_timeout_returns_status_for_fast_exit() { - use std::process::Command; - - // Absolute path + shell builtin so a sibling test that mutates + fn probe_version_with_timeout_captures_version_on_success() { + // Absolute path + shell builtins so a sibling test that mutates // PATH (the mac/win `discover_*` tests do) can't make these spawns - // fail with NotFound. `exit N` is a builtin, so no PATH lookup. - let zero = Command::new("/bin/sh") - .args(["-c", "exit 0"]) - .spawn() - .expect("spawn /bin/sh"); - assert!(wait_child_with_timeout(zero, Duration::from_secs(5)) - .is_some_and(|status| status.success())); - - // A clean non-zero exit must be observed as Some(!success), never - // collapsed into the timeout/error None — `probe_codex_runnable` - // relies on telling "ran but failed" from "didn't run". - let nonzero = Command::new("/bin/sh") - .args(["-c", "exit 3"]) - .spawn() - .expect("spawn /bin/sh"); - assert!(wait_child_with_timeout(nonzero, Duration::from_secs(5)) - .is_some_and(|status| !status.success())); + // fail with NotFound. `echo` / `exit` are builtins — no PATH lookup. + let mut ok = Command::new("/bin/sh"); + ok.args(["-c", "echo codex-cli 9.9.9"]); + assert_eq!( + probe_version_with_timeout(ok, Duration::from_secs(5)).as_deref(), + Some("codex-cli 9.9.9") + ); + + // A clean non-zero exit → None (never a captured string): redetect + // relies on this to reject "ran but failed" candidates. + let mut bad = Command::new("/bin/sh"); + bad.args(["-c", "exit 3"]); + assert_eq!(probe_version_with_timeout(bad, Duration::from_secs(5)), None); } #[cfg(unix)] #[test] - fn wait_child_with_timeout_kills_and_returns_none_on_overrun() { - use std::process::Command; - + fn probe_version_with_timeout_kills_and_returns_none_on_overrun() { let start = Instant::now(); // Absolute path (present on macOS + Ubuntu) so a PATH-mutating // sibling test can't break the spawn. - let sleeper = Command::new("/bin/sleep") - .arg("30") - .spawn() - .expect("spawn /bin/sleep"); + let mut sleeper = Command::new("/bin/sleep"); + sleeper.arg("30"); // 200ms budget against a 30s sleep: it must be killed and reported // as None, and we must not actually block anywhere near 30s. - assert!(wait_child_with_timeout(sleeper, Duration::from_millis(200)).is_none()); + assert_eq!( + probe_version_with_timeout(sleeper, Duration::from_millis(200)), + None + ); assert!(start.elapsed() < Duration::from_secs(5)); } } diff --git a/src-tauri/shared/runtime/models.rs b/src-tauri/shared/runtime/models.rs index 3bb30db..6fc5431 100644 --- a/src-tauri/shared/runtime/models.rs +++ b/src-tauri/shared/runtime/models.rs @@ -214,16 +214,28 @@ pub struct CodexCliStatus { pub suggested_paths: Vec, } +/// A codex CLI candidate confirmed runnable by the re-detection scan. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CodexCliCandidate { + /// Absolute path to the verified-runnable codex binary. + pub path: String, + /// Version line from `codex --version` (e.g. "codex-cli 0.133.0"), or + /// None if the binary ran successfully but printed nothing parseable. + /// Shown next to the path so the user can tell several installs apart. + pub version: Option, +} + /// Result of a forced re-detection scan triggered by the Settings /// "auto-detect" button. Unlike `get_codex_cli_status` (which honours -/// the cached/override path), this rescans from scratch and keeps only -/// the candidates that pass a `codex --version` runnable probe. +/// the cached/override path), this rescans from scratch across every +/// known source and keeps only the candidates that pass a +/// `codex --version` runnable probe. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct CodexCliRedetectResult { - /// Paths that were verified runnable, deduped and best-first. The - /// front-end auto-applies a lone hit and lets the user pick when - /// there are several. - pub candidates: Vec, + /// Verified-runnable candidates, deduped and best-first. The + /// front-end auto-applies a lone hit and lets the user pick (with + /// versions shown) when there are several. + pub candidates: Vec, /// Refreshed status snapshot so the Settings row and the dialog can /// update in lock-step after the scan. pub status: CodexCliStatus, diff --git a/src-tauri/win/runtime/process.rs b/src-tauri/win/runtime/process.rs index 9cc847d..d5315ae 100644 --- a/src-tauri/win/runtime/process.rs +++ b/src-tauri/win/runtime/process.rs @@ -11,6 +11,7 @@ use std::time::Duration; use crate::errors::{AppError, AppResult}; use crate::platform::hooks::PlatformHooks; use crate::shared::codex_app_server::{fetch_account_snapshot, AppServerSnapshot}; +use crate::models::CodexCliCandidate; use crate::shared::codex_cli_path::CodexPathResolver; pub use crate::shared::codex_cli_path::{InstallState, RealCodexPathSource}; use crate::shared::login_cancel::wait_for_login_or_cancel; @@ -512,7 +513,7 @@ impl CodexPathResolver for WindowsCodexPathResolver { suggested_codex_cli_paths(Some(codex_home)) } - fn redetect_runnable_paths(&self, codex_home: &Path) -> Vec { + fn redetect_runnable_paths(&self, codex_home: &Path) -> Vec { redetect_runnable_codex_cli_paths(Some(codex_home)) } } @@ -585,42 +586,27 @@ const RUNNABLE_PROBE_TIMEOUT: Duration = Duration::from_secs(5); /// Realistic machines have 1-3 candidates. const MAX_PROBE_CANDIDATES: usize = 12; -/// Whether `path` is an actually-runnable codex CLI: it must be a file -/// and `codex --version` must exit successfully within the probe -/// timeout. This is what lets auto-detect reject a stale or broken path -/// that a mere existence check would accept. The "ran but failed" and -/// "couldn't spawn" cases are logged so a broken install leaves a -/// diagnostic trail instead of looking identical to "not found". -fn probe_codex_runnable(path: &Path) -> bool { +/// Probe whether `path` is a runnable codex CLI and capture its version. +/// `Some(version)` (possibly empty) means it's a file that ran and exited +/// 0; `None` means not-a-file, couldn't spawn, exited non-zero, or timed +/// out. The failure is logged so a broken install leaves a diagnostic +/// trail instead of looking identical to "not found". +fn probe_codex_version(path: &Path) -> Option { if !path.is_file() { - return false; + return None; } let mut command = Command::new(path); - command - .arg("--version") - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()); - match hide_console_window(&mut command).spawn() { - Ok(child) => match crate::shared::codex_cli_path::wait_child_with_timeout( - child, - RUNNABLE_PROBE_TIMEOUT, - ) { - Some(status) if status.success() => true, - Some(status) => { - eprintln!( - "codex probe: {} ran but `--version` exited unsuccessfully ({status})", - path.display() - ); - false - } - None => false, - }, - Err(error) => { - eprintln!("codex probe: failed to spawn {}: {error}", path.display()); - false - } + command.arg("--version"); + hide_console_window(&mut command); + let result = + crate::shared::codex_cli_path::probe_version_with_timeout(command, RUNNABLE_PROBE_TIMEOUT); + if result.is_none() { + eprintln!( + "codex probe: {} is not a runnable codex (spawn / non-zero exit / timeout)", + path.display() + ); } + result } /// Force a fresh scan for the Settings auto-detect button, keeping only @@ -630,11 +616,16 @@ fn probe_codex_runnable(path: &Path) -> bool { /// folds in `where codex` (every PATH match) — so it is the full /// candidate set. Ignores the cached/override path so a wrong saved /// path can be corrected. -pub fn redetect_runnable_codex_cli_paths(codex_home: Option<&Path>) -> Vec { +pub fn redetect_runnable_codex_cli_paths(codex_home: Option<&Path>) -> Vec { suggested_codex_cli_paths(codex_home) .into_iter() .take(MAX_PROBE_CANDIDATES) - .filter(|path| probe_codex_runnable(path)) + .filter_map(|path| { + probe_codex_version(&path).map(|version| CodexCliCandidate { + path: path.to_string_lossy().into_owned(), + version: (!version.is_empty()).then_some(version), + }) + }) .collect() } @@ -747,7 +738,7 @@ impl PlatformHooks for WindowsPlatformHooks { mod tests { use super::{ build_app_server_command, discover_real_codex_cli_path, is_acceptable_real_codex_cli_path, - load_install_state, probe_codex_runnable, resolve_real_codex_cli, + load_install_state, probe_codex_version, resolve_real_codex_cli, resolve_windows_app_target, windows_store_shell_target, AppLaunchTarget, InstallState, WINDOWS_STORE_APP_ID, }; @@ -773,14 +764,14 @@ mod tests { // install), and only a zero-exit binary is accepted. #[cfg(unix)] #[test] - fn probe_codex_runnable_rejects_missing_and_failing_accepts_zero_exit() { + fn probe_codex_version_rejects_missing_and_failing_captures_zero_exit() { use std::os::unix::fs::PermissionsExt; let codex_home = temp_codex_home("probe-runnable"); fs::create_dir_all(&codex_home).unwrap(); - // (a) non-file path → false, never spawned. - assert!(!probe_codex_runnable(&codex_home.join("does-not-exist"))); + // (a) non-file path → None, never spawned. + assert_eq!(probe_codex_version(&codex_home.join("does-not-exist")), None); let set_exec = |path: &std::path::Path| { let mut perm = fs::metadata(path).unwrap().permissions(); @@ -788,17 +779,20 @@ mod tests { fs::set_permissions(path, perm).unwrap(); }; - // (b) exists + runs but exits non-zero → false (broken install). + // (b) exists + runs but exits non-zero → None (broken install). let bad = codex_home.join("bad-codex"); fs::write(&bad, "#!/bin/sh\nexit 3\n").unwrap(); set_exec(&bad); - assert!(!probe_codex_runnable(&bad)); + assert_eq!(probe_codex_version(&bad), None); - // (c) exists + exits zero → true. + // (c) exists + exits zero, prints a version → Some(version). let good = codex_home.join("good-codex"); - fs::write(&good, "#!/bin/sh\nexit 0\n").unwrap(); + fs::write(&good, "#!/bin/sh\necho codex-cli 0.133.0\n").unwrap(); set_exec(&good); - assert!(probe_codex_runnable(&good)); + assert_eq!( + probe_codex_version(&good).as_deref(), + Some("codex-cli 0.133.0") + ); let _ = fs::remove_dir_all(&codex_home); } From 380e98d0b9f77d8486961a7cc969d45321e9242f Mon Sep 17 00:00:00 2001 From: Cmochance <3216202644@qq.com> Date: Fri, 29 May 2026 13:04:37 +0800 Subject: [PATCH 3/4] fix(settings): bound login-shell resolve + drain probe stdout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local silent-failure review of the auto-detect rework flagged a CRITICAL: - discover_codex_via_login_shell ran an UNBOUNDED `$SHELL -lc` — a slow or hung login profile (nvm / asdf / brew shellenv / network-y rc files) would wedge the whole scan: button stuck on "Detecting...", a leaked blocking-pool thread, and no diagnostic. It runs first and synchronously, so one wedged profile froze the whole feature. It now goes through a shared, timeout-bounded helper (8s), matching the per-candidate probes' own stated design intent. - Extracted `run_capturing_stdout_with_timeout` (reader thread + poll + kill) out of `probe_version_with_timeout` so a child that writes more than the ~64KB pipe buffer before exiting can't backpressure its own pipe and deadlock until the timeout (MEDIUM). The version probe is now a thin wrapper taking the first stdout line; the login-shell resolve reuses the helper and takes the LAST line (skipping any profile banner). - spawn errors are now logged instead of being dropped by `.ok()?` (LOW). Adds a regression test that >64KB of stdout is drained, not deadlocked. --- src-tauri/mac/runtime/process.rs | 26 ++++--- src-tauri/shared/runtime/codex_cli_path.rs | 81 +++++++++++++++++----- 2 files changed, 80 insertions(+), 27 deletions(-) diff --git a/src-tauri/mac/runtime/process.rs b/src-tauri/mac/runtime/process.rs index 4ca1636..c64a8f0 100644 --- a/src-tauri/mac/runtime/process.rs +++ b/src-tauri/mac/runtime/process.rs @@ -318,6 +318,12 @@ const RUNNABLE_PROBE_TIMEOUT: Duration = Duration::from_secs(3); /// scan. Realistic machines have 1-3 candidates. const MAX_PROBE_CANDIDATES: usize = 12; +/// Login shells source the user's full profile chain (nvm / asdf / brew +/// shellenv / network-y rc files), which can be slower than a bare +/// `--version`, so give the login-shell resolve a more generous budget — +/// but still hard-bounded so a hung profile can't wedge the whole scan. +const LOGIN_SHELL_PROBE_TIMEOUT: Duration = Duration::from_secs(8); + /// Probe whether `path` is a runnable codex CLI and capture its version. /// `Some(version)` (possibly empty) means it's a file that ran and exited /// 0; `None` means not-a-file, couldn't spawn, exited non-zero, or timed @@ -348,17 +354,21 @@ fn probe_codex_version(path: &Path) -> Option { /// the real binary; the result is verified to be an absolute file. fn discover_codex_via_login_shell(managed_shim_path: Option<&Path>) -> Option { let shell = env::var_os("SHELL")?; - let output = Command::new(&shell) - .args(["-lc", "command -v codex"]) - .output() - .ok()?; - if !output.status.success() { - return None; - } - let stdout = String::from_utf8_lossy(&output.stdout); + let mut command = Command::new(&shell); + command.args(["-lc", "command -v codex"]); + // Bounded via the shared helper: a slow / hung login profile (nvm, + // asdf, network-y rc files) must NOT wedge the scan the way an + // unbounded `.output()` would — this runs first and synchronously. + let stdout = crate::shared::codex_cli_path::run_capturing_stdout_with_timeout( + command, + LOGIN_SHELL_PROBE_TIMEOUT, + )?; + // `command -v` prints the resolved path last — after any banner a noisy + // profile may have echoed to stdout — so take the last non-empty line. let resolved = stdout .lines() .map(str::trim) + .rev() .find(|value| !value.is_empty())?; let candidate = PathBuf::from(resolved); // `command -v` of a function/alias returns a bare name, not a path — diff --git a/src-tauri/shared/runtime/codex_cli_path.rs b/src-tauri/shared/runtime/codex_cli_path.rs index af58359..a379665 100644 --- a/src-tauri/shared/runtime/codex_cli_path.rs +++ b/src-tauri/shared/runtime/codex_cli_path.rs @@ -165,30 +165,60 @@ pub fn redetect_codex_cli_path( /// child is then killed so a hung / input-waiting binary can't wedge the /// scan). Shared so mac + Windows reuse the poll/kill/capture logic; each /// platform only builds the `Command` (console hiding, extension res). -pub fn probe_version_with_timeout(mut command: Command, timeout: Duration) -> Option { +pub fn probe_version_with_timeout(command: Command, timeout: Duration) -> Option { + run_capturing_stdout_with_timeout(command, timeout).map(|stdout| { + stdout + .lines() + .map(str::trim) + .find(|line| !line.is_empty()) + .unwrap_or("") + .to_owned() + }) +} + +/// Run `command` bounded by `timeout`, draining stdout on a reader thread +/// so a chatty child can't backpressure its own pipe and deadlock. Returns +/// the full captured stdout on a successful (exit-0) run; `None` if it +/// couldn't spawn, exited non-zero, errored, or overran the timeout (the +/// child is then killed). Shared by the `codex --version` probe (caller +/// takes the first line = version) and the login-shell resolver (caller +/// takes the last line = path), so the poll / kill / drain logic — and the +/// hard timeout that keeps an arbitrary user login shell from wedging the +/// scan — lives in exactly one place. +pub fn run_capturing_stdout_with_timeout( + mut command: Command, + timeout: Duration, +) -> Option { command .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::null()); - let mut child = command.spawn().ok()?; + let mut child = match command.spawn() { + Ok(child) => child, + Err(error) => { + eprintln!("run_capturing_stdout_with_timeout: spawn failed: {error}"); + return None; + } + }; + // Drain stdout concurrently: a child that writes more than the pipe + // buffer (~64 KB) before exiting would otherwise block on write() + // forever while we poll try_wait. Joined only after exit/kill. + let mut reader = child.stdout.take().map(|mut stdout| { + thread::spawn(move || { + let mut buf = String::new(); + let _ = stdout.read_to_string(&mut buf); + buf + }) + }); let deadline = Instant::now() + timeout; loop { match child.try_wait() { Ok(Some(status)) => { - if !status.success() { - return None; - } - let mut out = String::new(); - if let Some(mut stdout) = child.stdout.take() { - let _ = stdout.read_to_string(&mut out); - } - return Some( - out.lines() - .map(str::trim) - .find(|line| !line.is_empty()) - .unwrap_or("") - .to_owned(), - ); + let stdout = reader + .take() + .and_then(|handle| handle.join().ok()) + .unwrap_or_default(); + return status.success().then_some(stdout); } Ok(None) => { if Instant::now() >= deadline { @@ -201,9 +231,8 @@ pub fn probe_version_with_timeout(mut command: Command, timeout: Duration) -> Op Err(error) => { // A real try_wait failure (EINTR, ECHILD, a Windows handle // error) is indistinguishable from a clean timeout to the - // caller — both return None — so leave a diagnostic trail - // instead of silently demoting the candidate. - eprintln!("probe_version_with_timeout: try_wait failed: {error}"); + // caller — both return None — so leave a diagnostic trail. + eprintln!("run_capturing_stdout_with_timeout: try_wait failed: {error}"); let _ = child.kill(); let _ = child.wait(); return None; @@ -468,4 +497,18 @@ mod tests { ); assert!(start.elapsed() < Duration::from_secs(5)); } + + #[cfg(unix)] + #[test] + fn run_capturing_does_not_deadlock_on_large_stdout() { + // Emit well over the ~64 KB pipe buffer before exiting. Without the + // reader thread the child would block on write() and we'd hit the + // timeout; with it, this drains and returns well under the budget. + let mut command = Command::new("/bin/sh"); + command.args(["-c", "yes 0123456789abcdef | head -n 20000; echo done"]); + let start = Instant::now(); + let out = run_capturing_stdout_with_timeout(command, Duration::from_secs(5)); + assert!(start.elapsed() < Duration::from_secs(4)); + assert!(out.expect("exit 0 with captured stdout").contains("done")); + } } From f9d09476e582083cfefd04c2d93623e14ea19d40 Mon Sep 17 00:00:00 2001 From: Cmochance <3216202644@qq.com> Date: Fri, 29 May 2026 13:14:17 +0800 Subject: [PATCH 4/4] fix(settings): bound shell-resolver spawn + cap stdout drain Addresses 3 chatgpt-codex-connector P2 threads on the auto-detect PR: - run_capturing_stdout_with_timeout: the stdout reader was join()'d unbounded after the child exited. If the child exits but leaves a background process holding the stdout pipe, read_to_string never sees EOF and join() blocks forever, defeating the timeout. The reader now posts to a channel and we wait with a bounded recv_timeout, abandoning the detached reader after a short grace. - discover_real_codex_cli_from_shell: the managed-shim resolver script ran via unbounded `.output()`; it now goes through the same bounded helper as the login-shell resolve and the per-candidate probes, so no shell-touching spawn in the scan is unbounded anymore. (The login-shell `$SHELL -lc` thread was already bounded in 380e98d.) --- src-tauri/mac/runtime/process.rs | 17 ++++---- src-tauri/shared/runtime/codex_cli_path.rs | 48 +++++++++++++++------- 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/src-tauri/mac/runtime/process.rs b/src-tauri/mac/runtime/process.rs index c64a8f0..ece196d 100644 --- a/src-tauri/mac/runtime/process.rs +++ b/src-tauri/mac/runtime/process.rs @@ -82,15 +82,14 @@ fn discover_real_codex_cli_from_shell(managed_shim_path: Option<&Path>) -> Optio let managed_shim_text = managed_shim_path .map(|path| path.to_string_lossy().into_owned()) .unwrap_or_default(); - let output = Command::new(&resolver_path) - .arg(managed_shim_text) - .output() - .ok()?; - if !output.status.success() { - return None; - } - - let stdout = String::from_utf8_lossy(&output.stdout); + let mut command = Command::new(&resolver_path); + command.arg(managed_shim_text); + // Bounded like the other shell spawns: even this first-party resolver + // script sources the login environment, which can stall on a hung rc. + let stdout = crate::shared::codex_cli_path::run_capturing_stdout_with_timeout( + command, + LOGIN_SHELL_PROBE_TIMEOUT, + )?; let resolved = stdout .lines() .map(str::trim) diff --git a/src-tauri/shared/runtime/codex_cli_path.rs b/src-tauri/shared/runtime/codex_cli_path.rs index a379665..8986abd 100644 --- a/src-tauri/shared/runtime/codex_cli_path.rs +++ b/src-tauri/shared/runtime/codex_cli_path.rs @@ -176,6 +176,12 @@ pub fn probe_version_with_timeout(command: Command, timeout: Duration) -> Option }) } +/// After the probed child exits, how long to wait for its stdout to finish +/// draining before giving up on capturing it. A child that left a +/// background process holding the pipe never yields EOF, so this bounds +/// that case instead of blocking forever. +const STDOUT_DRAIN_GRACE: Duration = Duration::from_secs(1); + /// Run `command` bounded by `timeout`, draining stdout on a reader thread /// so a chatty child can't backpressure its own pipe and deadlock. Returns /// the full captured stdout on a successful (exit-0) run; `None` if it @@ -200,25 +206,39 @@ pub fn run_capturing_stdout_with_timeout( return None; } }; - // Drain stdout concurrently: a child that writes more than the pipe - // buffer (~64 KB) before exiting would otherwise block on write() - // forever while we poll try_wait. Joined only after exit/kill. - let mut reader = child.stdout.take().map(|mut stdout| { - thread::spawn(move || { - let mut buf = String::new(); - let _ = stdout.read_to_string(&mut buf); - buf + // Drain stdout on a thread that posts to a channel — then wait with a + // bounded recv_timeout, NOT an unbounded join. Two hazards this guards: + // (1) a child that writes more than the pipe buffer (~64 KB) before + // exiting would block on write() while we poll try_wait; (2) a child + // that exits but leaves a background process holding the stdout pipe + // never yields EOF, so read_to_string — and an unbounded join() after + // it — would block forever and defeat the timeout. We abandon the + // (detached) reader after STDOUT_DRAIN_GRACE in that case. + let (tx, rx) = std::sync::mpsc::channel(); + let has_reader = child + .stdout + .take() + .map(|mut stdout| { + thread::spawn(move || { + let mut buf = String::new(); + let _ = stdout.read_to_string(&mut buf); + let _ = tx.send(buf); + }); }) - }); + .is_some(); let deadline = Instant::now() + timeout; loop { match child.try_wait() { Ok(Some(status)) => { - let stdout = reader - .take() - .and_then(|handle| handle.join().ok()) - .unwrap_or_default(); - return status.success().then_some(stdout); + if !status.success() { + return None; + } + let stdout = if has_reader { + rx.recv_timeout(STDOUT_DRAIN_GRACE).unwrap_or_default() + } else { + String::new() + }; + return Some(stdout); } Ok(None) => { if Instant::now() >= deadline {