From 967b1bc073a87e31468a939706b959ef811dabce Mon Sep 17 00:00:00 2001 From: Cmochance <3216202644@qq.com> Date: Sun, 12 Jul 2026 07:18:52 +0800 Subject: [PATCH 1/2] =?UTF-8?q?fix(win):=20adapt=20desktop-host=20discover?= =?UTF-8?q?y=20to=20the=20Codex=20=E2=86=92=20ChatGPT=20app=20merge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merged Windows host runs as ChatGPT.exe (MSIX package family unchanged: OpenAI.Codex_2p2nqsd0c76g0; non-Store installs under %LOCALAPPDATA%\Programs\ChatGPT), so the Codex.exe-only tasklist / taskkill checks missed the running UI: switch skipped quitting the host, leaving its in-memory app-server on the old account, and never relaunched it. - is-running / quit enumerate both host names via one Win32_Process CIM query and classify each PID by executable install identity: OpenAI.Codex* package family (incl. beta) or a non-Store install embedding resources\codex.exe => ours; ChatGPT Classic (OpenAI.ChatGPT-Desktop_*, same exe name) => positively excluded; unattributable => counts as running, never signalled (mirrors the macOS ours/other/unknown semantics) - quit signals by PID (graceful taskkill /PID, then /F) instead of /IM , with the differentiated APP_EXIT_FAILED message - launch prefers the detected Store package (shell:AppsFolder id is unchanged post-merge), then a qualified non-Store executable - CLI discovery + Settings auto-detect gain the desktop-bundled CLI tiers: \app\resources\codex.exe and Programs\{ChatGPT,Codex}\resources\codex.exe Facts anchored from public sources (winget manifests, KAPE targets, third-party Codex tooling paths); pending verification on a live Windows install — noted in CHANGELOG. --- CHANGELOG.md | 4 + src-tauri/win/runtime/process.rs | 399 ++++++++++++++++++++++++++++--- 2 files changed, 369 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de99ad3..a237ba3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- **Critical** — adapted the Windows desktop-host discovery for the Codex → ChatGPT app merge (2026-07-09; the Windows counterpart of the macOS adaptation shipped in 1.6.0). The merged host runs as `ChatGPT.exe` (MSIX package family still `OpenAI.Codex_2p2nqsd0c76g0`, non-Store installs under `%LOCALAPPDATA%\Programs\ChatGPT\`), so the old `Codex.exe`-only checks missed the running UI: switching skipped quitting the host (stale in-memory app-server on the old account) and never relaunched it. is-running / quit now enumerate both host names and classify each PID by its executable's install identity — the `OpenAI.Codex*` package family (incl. the beta channel) or a non-Store install that embeds the bundled CLI (`resources\codex.exe`, the Windows analog of the macOS `Contents/Resources/codex` qualifier) counts as ours; ChatGPT Classic (`OpenAI.ChatGPT-Desktop_…`, same `ChatGPT.exe` name) is positively excluded and never killed; unattributable PIDs still count as running (the switch aborts rather than proceeding over a possibly-live host) but are never signalled. Quit signals by PID (graceful `taskkill /PID` then `/F`) instead of `/IM `. Launch prefers the detected Store package (unchanged `shell:AppsFolder` id), falling back to a qualified non-Store `ChatGPT.exe` / `Codex.exe`. CLI discovery and the Settings auto-detect suggestions gain the desktop-bundled CLI locations (MSIX `\app\resources\codex.exe` and the non-Store `Programs\{ChatGPT,Codex}\resources\codex.exe`). Facts anchored from public manifests and tooling (winget, KAPE, third-party Codex tools); not yet verified on a live Windows install. + ## 1.6.0 - 2026-07-11 - **Critical** — fixed the quota backfill reverting fresh data and bleeding across accounts. The root `~/.codex/profile.json` copy was only ever written at switch-in, so every write-back (switch, app launch, window close) copied that frozen snapshot over the profile's card and silently reverted the quota / plan updates recorded while it was active — permanently for API-key profiles, which no API refresh repairs; Windows had patched the switch path only (keyed off the possibly-stale `.current_profile` marker) and macOS had nothing. The write-back now folds the freshest live-session usage into the identity-verified target's card and refreshes the root copy first, on both platforms and on all four write-back paths (switch, app launch, window close, current-profile login); the legacy `codex-switch.sh` no longer copies the root `profile.json` back at all, closing its variant of the same clobber. Live-session data is scoped to entries written since the profile's activation (`activated_at` from the active marker): `~/.codex/sessions` is not swapped on switch, so the previously-live account's newer entries used to win the freshness race and show up as the current profile's quota for minutes after a switch — and when a profile has no parseable activation marker (an identity-drift target, a hand-edited marker) the fold is skipped outright rather than attributing unattributable sessions. A present-but-unreadable `profile.json` now aborts the write-back loudly instead of laundering a transient read failure into a blanked card. The duplicated Windows display path (which had already drifted: it lacked the unmanaged-account guard) now re-exports the shared implementation; an empty stored card with a newer timestamp (the deliberate downgrade-to-free clear) is no longer overridden by older live sessions; and switching kicks an immediate silent API refresh for the new card instead of priming its stored timestamp and waiting out the 5-minute tick. Known residual: a codex CLI session started under the previous account that keeps running across the switch keeps appending to a session file whose mtime passes the activation cutoff, so its usage can still be misattributed until that session exits. diff --git a/src-tauri/win/runtime/process.rs b/src-tauri/win/runtime/process.rs index bf599b8..bdd41a1 100644 --- a/src-tauri/win/runtime/process.rs +++ b/src-tauri/win/runtime/process.rs @@ -18,19 +18,39 @@ use crate::shared::login_cancel::wait_for_login_or_cancel; use super::paths::{get_codex_home, get_install_state_file}; -const APP_PROCESS_NAME: &str = "Codex.exe"; +/// Post-merge desktop host process name. OpenAI folded Codex into the +/// ChatGPT desktop app on Windows on 2026-07-09 (executable +/// `ChatGPT.exe`, installed under `%LOCALAPPDATA%\Programs\ChatGPT\` or +/// inside the MSIX package's `app\` dir); exact-name checks against +/// `Codex.exe` no longer see the running UI. +const APP_PROCESS_NAME_CHATGPT: &str = "ChatGPT.exe"; +/// Historical standalone process name, still shipped by +/// not-yet-updated `Codex.exe` installs. +const APP_PROCESS_NAME_CODEX: &str = "Codex.exe"; +/// Lower-cased path marker of the MSIX package family shared by the +/// pre-merge Codex app and the merged ChatGPT host. The Store package +/// "updates as usual" into the merged app and a package family name +/// never changes across updates, so `OpenAI.Codex_2p2nqsd0c76g0` stays +/// (the prefix also covers the `OpenAI.CodexBeta_…` channel). +const WINDOWSAPPS_CODEX_MARKER: &str = r"\windowsapps\openai.codex"; +/// Lower-cased path marker of ChatGPT Classic — the pre-merge consumer +/// chat app (`OpenAI.ChatGPT-Desktop_2p2nqsd0c76g0`). Its executable is +/// also named `ChatGPT.exe`, but it is NOT a codex host and must never +/// be counted as running or killed. +const WINDOWSAPPS_CLASSIC_CHATGPT_MARKER: &str = r"\windowsapps\openai.chatgpt-desktop_"; const WINDOWS_INVOKABLE_SUFFIXES: [&str; 4] = ["cmd", "exe", "bat", "com"]; const WINDOWS_APPS_PATH_SEGMENT: &str = r"\microsoft\windowsapps\"; const WINDOWS_STORE_APP_ID: &str = "OpenAI.Codex_2p2nqsd0c76g0!App"; const WINDOWS_STORE_SHELL_PREFIX: &str = r"shell:AppsFolder\"; #[cfg(target_os = "windows")] const CREATE_NO_WINDOW: u32 = 0x08000000; -static WINDOWS_APP_TARGET_CACHE: OnceLock = OnceLock::new(); +static WINDOWS_APP_TARGET_CACHE: OnceLock> = OnceLock::new(); static WINDOWS_PLATFORM_HOOKS: WindowsPlatformHooks = WindowsPlatformHooks; #[derive(Debug, Clone, PartialEq, Eq)] enum AppLaunchTarget { WindowsStore(String), + Executable(PathBuf), } pub struct WindowsPlatformHooks; @@ -180,6 +200,13 @@ pub(super) fn discover_real_codex_cli_path(managed_shim_path: Option<&Path>) -> } } + // Desktop-host bundled CLI as the last tier — mirrors the macOS + // app-bundle fallback so a machine whose only codex is the one + // inside the (merged) desktop app still resolves. + for candidate in desktop_host_bundled_cli_candidates() { + push_real_codex_candidate(&mut candidates, candidate, managed_shim_path); + } + candidates.into_iter().next() } @@ -215,36 +242,215 @@ fn detect_windows_store_app_target() -> Option { is_valid_windows_store_app_id(&app_id).then(|| windows_store_shell_target(&app_id)) } -fn resolve_windows_store_shell_target() -> String { +/// Detected Store shell target, cached for the process lifetime. +/// `None` when the package is absent or PowerShell is unavailable. +fn detected_windows_store_target() -> Option { WINDOWS_APP_TARGET_CACHE + .get_or_init(detect_windows_store_app_target) + .clone() +} + +/// InstallLocation of the (merged) codex host's MSIX package, cached +/// for the process lifetime. `None` when the Store package is absent +/// (non-Store install) or PowerShell is unavailable. +static WINDOWS_STORE_INSTALL_LOCATION_CACHE: OnceLock> = OnceLock::new(); + +fn windows_store_install_location() -> Option { + WINDOWS_STORE_INSTALL_LOCATION_CACHE .get_or_init(|| { - detect_windows_store_app_target() - .unwrap_or_else(|| windows_store_shell_target(WINDOWS_STORE_APP_ID)) + if !cfg!(target_os = "windows") { + return None; + } + let mut command = Command::new("powershell"); + command.args([ + "-NoProfile", + "-Command", + "Get-AppxPackage -Name 'OpenAI.Codex' -ErrorAction SilentlyContinue \ + | Select-Object -First 1 -ExpandProperty InstallLocation", + ]); + let output = hide_console_window(&mut command).output().ok()?; + if !output.status.success() { + return None; + } + let location = String::from_utf8_lossy(&output.stdout).trim().to_string(); + (!location.is_empty()).then_some(location) }) .clone() } +/// Non-Store install locations of the desktop host executable, current +/// host first. Each candidate must embed the codex CLI +/// (`resources\codex.exe`) to qualify — a ChatGPT.exe without it could +/// be an unrelated install and must not be launched as "Codex". +fn non_store_host_executables() -> Vec { + let Some(local_app_data) = env::var_os("LOCALAPPDATA") else { + return Vec::new(); + }; + let programs = PathBuf::from(local_app_data).join("Programs"); + [ + programs.join("ChatGPT").join(APP_PROCESS_NAME_CHATGPT), + programs.join("Codex").join(APP_PROCESS_NAME_CODEX), + ] + .into_iter() + .filter(|exe| exe.is_file() && host_dir_embeds_codex_cli(exe)) + .collect() +} + +/// Codex CLI copies bundled inside the desktop host installs — the +/// Windows analog of the macOS `Contents/Resources/codex` fallback. +/// Order is discovery preference: the MSIX package +/// (`\app\resources\codex.exe`), then the non-Store +/// ChatGPT install, then the legacy non-Store Codex install. +fn desktop_host_bundled_cli_candidates() -> Vec { + let mut candidates = Vec::new(); + if let Some(install_location) = windows_store_install_location() { + candidates.push( + PathBuf::from(install_location) + .join("app") + .join("resources") + .join("codex.exe"), + ); + } + if let Some(local_app_data) = env::var_os("LOCALAPPDATA") { + let programs = PathBuf::from(local_app_data).join("Programs"); + candidates.push(programs.join("ChatGPT").join("resources").join("codex.exe")); + candidates.push(programs.join("Codex").join("resources").join("codex.exe")); + } + candidates +} + fn resolve_windows_app_target() -> AppLaunchTarget { - AppLaunchTarget::WindowsStore(resolve_windows_store_shell_target()) + // Prefer the Store package when it is actually installed (the + // merged app keeps the OpenAI.Codex package family). Otherwise fall + // back to a qualified non-Store executable; the hardcoded shell + // target stays as the historical last resort. + if let Some(target) = detected_windows_store_target() { + return AppLaunchTarget::WindowsStore(target); + } + if let Some(executable) = non_store_host_executables().into_iter().next() { + return AppLaunchTarget::Executable(executable); + } + AppLaunchTarget::WindowsStore(windows_store_shell_target(WINDOWS_STORE_APP_ID)) } -pub fn is_codex_app_running() -> bool { - let mut command = Command::new("tasklist"); - command.args([ - "/FI", - &format!("IMAGENAME eq {APP_PROCESS_NAME}"), - "/FO", - "CSV", - "/NH", - ]); +/// Process names that may own the Codex desktop UI after the ChatGPT +/// merge. Order only matters for probe latency (current host first). +fn desktop_app_process_names() -> &'static [&'static str] { + &[APP_PROCESS_NAME_CHATGPT, APP_PROCESS_NAME_CODEX] +} + +/// Install identity of a name-matched PID. `Other` is the only verdict +/// that positively rules a process out (ChatGPT Classic). `Unknown` +/// (no executable path, unrecognized location) is treated +/// asymmetrically: it COUNTS for is-running — proceeding with a +/// possibly-live host risks account cross-contamination, so the switch +/// must abort instead — but it is NEVER signalled (we do not kill what +/// we cannot identify). Mirrors the macOS classification in +/// `mac/runtime/process.rs`. +#[derive(Debug, PartialEq)] +enum HostIdentity { + Ours, + Other, + Unknown, +} + +/// Classify a host process by its executable path. The bare name match +/// is not enough: ChatGPT Classic ships an executable that is also +/// named `ChatGPT.exe`. +fn classify_host_executable(executable_path: &str) -> HostIdentity { + let normalized = executable_path.replace('/', "\\").to_ascii_lowercase(); + if normalized.trim().is_empty() { + return HostIdentity::Unknown; + } + if normalized.contains(WINDOWSAPPS_CODEX_MARKER) { + return HostIdentity::Ours; + } + if normalized.contains(WINDOWSAPPS_CLASSIC_CHATGPT_MARKER) { + return HostIdentity::Other; + } + // Non-Store installs (%LOCALAPPDATA%\Programs\ChatGPT, portable + // copies): the codex host embeds the codex CLI next to its + // executable (`resources\codex.exe` — the Windows analog of the + // macOS `Contents/Resources/codex` qualifier). Classic has no such + // file, but its absence alone cannot rule a host out (probe + // failures), so it stays Unknown rather than Other. + if host_dir_embeds_codex_cli(Path::new(executable_path)) { + return HostIdentity::Ours; + } + HostIdentity::Unknown +} + +fn host_dir_embeds_codex_cli(executable: &Path) -> bool { + executable + .parent() + .map(|dir| dir.join("resources").join("codex.exe").is_file()) + .unwrap_or(false) +} + +/// Parse the `|` lines emitted by the PowerShell +/// CIM query in [`desktop_app_pid_classifications`]. Extracted for unit +/// tests (the query itself needs a live Windows host). +fn parse_pid_classification_lines(stdout: &str) -> Vec<(u32, HostIdentity)> { + stdout + .lines() + .filter_map(|line| { + let (pid, path) = line.trim().split_once('|')?; + let pid = pid.trim().parse::().ok()?; + Some((pid, classify_host_executable(path))) + }) + .collect() +} +/// Name-matched desktop host PIDs with their install classification, +/// via one PowerShell CIM query (~150-400ms — only paid after the cheap +/// tasklist pre-check matched a name). Returns an empty list when +/// PowerShell itself is unavailable; callers must treat that as +/// "unattributable", not as "not running". +fn desktop_app_pid_classifications() -> Vec<(u32, HostIdentity)> { + let script = format!( + "Get-CimInstance Win32_Process -Filter \"Name='{APP_PROCESS_NAME_CHATGPT}' OR Name='{APP_PROCESS_NAME_CODEX}'\" \ + | ForEach-Object {{ \"$($_.ProcessId)|$($_.ExecutablePath)\" }}" + ); + let mut command = Command::new("powershell"); + command.args(["-NoProfile", "-Command", &script]); let output = match hide_console_window(&mut command).output() { - Ok(value) => value, - Err(_) => return false, + Ok(output) if output.status.success() => output, + _ => return Vec::new(), }; + parse_pid_classification_lines(&String::from_utf8_lossy(&output.stdout)) +} - let stdout = String::from_utf8_lossy(&output.stdout).to_ascii_lowercase(); - stdout.contains(&APP_PROCESS_NAME.to_ascii_lowercase()) +/// Cheap pre-check: does any process with a known host name exist at +/// all? Avoids paying the PowerShell classification cost on every +/// 200ms quit-wait poll when nothing is running. +fn any_host_process_name_running() -> bool { + desktop_app_process_names().iter().any(|name| { + let mut command = Command::new("tasklist"); + command.args(["/FI", &format!("IMAGENAME eq {name}"), "/FO", "CSV", "/NH"]); + let output = match hide_console_window(&mut command).output() { + Ok(value) => value, + Err(_) => return false, + }; + String::from_utf8_lossy(&output.stdout) + .to_ascii_lowercase() + .contains(&name.to_ascii_lowercase()) + }) +} + +pub fn is_codex_app_running() -> bool { + if !any_host_process_name_running() { + return false; + } + let classified = desktop_app_pid_classifications(); + if classified.is_empty() { + // A host name is running but PowerShell could not attribute it: + // count as running so the switch aborts instead of proceeding + // over a possibly-live host (quit will refuse to signal it). + return true; + } + classified + .iter() + .any(|(_, identity)| *identity != HostIdentity::Other) } pub fn open_or_activate_codex_app(_codex_home: Option<&Path>) -> AppResult { @@ -260,6 +466,14 @@ pub fn open_or_activate_codex_app(_codex_home: Option<&Path>) -> AppResult { + let mut command = Command::new(&executable); + hide_console_window(&mut command).spawn().map_err(|error| { + AppError::new("APP_OPEN_FAILED", format!("Failed to open Codex: {error}")) + })?; + + Ok(executable.to_string_lossy().into_owned()) + } } } @@ -555,6 +769,9 @@ pub fn suggested_codex_cli_paths(codex_home: Option<&Path>) -> Vec { push(base.join("codex.exe")); push(base.join("codex.cmd")); } + for candidate in desktop_host_bundled_cli_candidates() { + push(candidate); + } let mut where_command = Command::new("where"); where_command.arg("codex"); @@ -628,35 +845,78 @@ pub fn redetect_runnable_codex_cli_paths(codex_home: Option<&Path>) -> Vec` keeps ChatGPT +/// Classic (same executable name, different install) untouched, and +/// Unknown PIDs are never signalled. +fn signal_desktop_app_processes(force: bool) -> usize { + let mut signalled = 0; + for (pid, identity) in desktop_app_pid_classifications() { + if identity != HostIdentity::Ours { + continue; + } + let pid_text = pid.to_string(); + let mut taskkill = Command::new("taskkill"); + if force { + taskkill.args(["/F", "/PID", &pid_text]); + } else { + taskkill.args(["/PID", &pid_text]); + } + match hide_console_window(&mut taskkill).output() { + Ok(output) if output.status.success() => signalled += 1, + // Non-zero exit: the process exited between enumeration and + // signalling — the wait loop below re-checks, nothing to do. + Ok(_) => {} + Err(error) => { + eprintln!("codex_switch: failed to spawn taskkill for pid {pid}: {error}"); + } + } + } + signalled +} + pub fn quit_codex_app_if_running() -> AppResult { if !is_codex_app_running() { return Ok(false); } - let mut taskkill = Command::new("taskkill"); - taskkill.args(["/IM", APP_PROCESS_NAME]); - let _ = hide_console_window(&mut taskkill).output(); + let mut signalled = signal_desktop_app_processes(false); + let mut exited = false; for _ in 0..20 { if !is_codex_app_running() { - return Ok(true); + exited = true; + break; } thread::sleep(Duration::from_millis(200)); } - let mut force_taskkill = Command::new("taskkill"); - force_taskkill.args(["/F", "/IM", APP_PROCESS_NAME]); - let _ = hide_console_window(&mut force_taskkill).output(); - for _ in 0..10 { - if !is_codex_app_running() { - return Ok(true); + if !exited { + signalled += signal_desktop_app_processes(true); + for _ in 0..10 { + if !is_codex_app_running() { + exited = true; + break; + } + thread::sleep(Duration::from_millis(200)); } - thread::sleep(Duration::from_millis(200)); } - Err(AppError::new( - "APP_EXIT_FAILED", - "Codex did not exit cleanly. Close it manually and retry.", - )) + if exited { + return Ok(true); + } + + // Distinguish "we signalled it and it would not die" from "we never + // managed to signal anything" — the latter means the failure is on + // our side (no identifiable PID / taskkill unavailable), not the + // app's. Mirrors the macOS quit path. + let message = if signalled == 0 { + "Codex/ChatGPT still appears to be running, but no matching process could be \ + identified and signalled. Close it manually and retry." + } else { + "Codex/ChatGPT did not exit cleanly. Close it manually and retry." + }; + Err(AppError::new("APP_EXIT_FAILED", message)) } pub fn reopen_codex_app_if_needed( @@ -991,4 +1251,75 @@ mod tests { ); let _ = fs::remove_dir_all(&codex_home); } + + #[test] + fn classify_host_executable_identifies_msix_packages() { + use super::{classify_host_executable, HostIdentity}; + // Merged host: the OpenAI.Codex package family survives the + // in-place Store update (display/executable renamed to ChatGPT). + assert_eq!( + classify_host_executable( + r"C:\Program Files\WindowsApps\OpenAI.Codex_26.707.0.0_x64__2p2nqsd0c76g0\app\ChatGPT.exe" + ), + HostIdentity::Ours + ); + // Beta channel shares the OpenAI.Codex prefix. + assert_eq!( + classify_host_executable( + r"C:\Program Files\WindowsApps\OpenAI.CodexBeta_26.513.4821.0_x64__2p2nqsd0c76g0\app\Codex (Beta).exe" + ), + HostIdentity::Ours + ); + // ChatGPT Classic: same executable name, NOT a codex host. + assert_eq!( + classify_host_executable( + r"C:\Program Files\WindowsApps\OpenAI.ChatGPT-Desktop_1.2025.112.0_x64__2p2nqsd0c76g0\app\ChatGPT.exe" + ), + HostIdentity::Other + ); + assert_eq!( + super::classify_host_executable(""), + HostIdentity::Unknown, + "missing executable path must stay unattributable" + ); + assert_eq!( + classify_host_executable(r"C:\Users\me\Desktop\ChatGPT.exe"), + HostIdentity::Unknown + ); + } + + #[test] + fn host_dir_embeds_codex_cli_detects_bundled_cli() { + use super::host_dir_embeds_codex_cli; + let root = temp_codex_home("host-embed-cli"); + fs::create_dir_all(root.join("resources")).unwrap(); + let exe = root.join("ChatGPT.exe"); + fs::write(&exe, "stub").unwrap(); + assert!(!host_dir_embeds_codex_cli(&exe)); + fs::write(root.join("resources").join("codex.exe"), "stub").unwrap(); + assert!(host_dir_embeds_codex_cli(&exe)); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn parse_pid_classification_lines_parses_and_skips_malformed() { + use super::{parse_pid_classification_lines, HostIdentity}; + let stdout = "123|C:\\Program Files\\WindowsApps\\OpenAI.Codex_26.707.0.0_x64__2p2nqsd0c76g0\\app\\ChatGPT.exe\r\n\ + 456|C:\\Program Files\\WindowsApps\\OpenAI.ChatGPT-Desktop_1.0_x64__2p2nqsd0c76g0\\app\\ChatGPT.exe\r\n\ + 789|\r\n\ + not-a-line\r\n"; + let parsed = parse_pid_classification_lines(stdout); + assert_eq!(parsed.len(), 3); + assert_eq!(parsed[0], (123, HostIdentity::Ours)); + assert_eq!(parsed[1], (456, HostIdentity::Other)); + assert_eq!(parsed[2], (789, HostIdentity::Unknown)); + } + + #[test] + fn desktop_app_process_names_cover_merged_and_legacy_hosts() { + assert_eq!( + super::desktop_app_process_names(), + &["ChatGPT.exe", "Codex.exe"] + ); + } } From 1cdaf6f06b013581757f7305e289f065dd51d875 Mon Sep 17 00:00:00 2001 From: Cmochance <3216202644@qq.com> Date: Sun, 12 Jul 2026 07:31:28 +0800 Subject: [PATCH 2/2] =?UTF-8?q?fix(win):=20address=20local=20review=20?= =?UTF-8?q?=E2=80=94=20exhaustive=20match,=20resilient=20probes,=20beta=20?= =?UTF-8?q?coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Findings from the pre-push review pass (1 BLOCKER, 4 MEDIUM, 1 LOW): - BLOCKER: reopen_codex_app_if_needed's match was non-exhaustive after AppLaunchTarget gained Executable — E0004, the whole win module (and the Linux CI job that compiles it) failed to build. Added the Executable arm so non-Store installs relaunch too. - both PowerShell-backed OnceLock caches (Store shell target, MSIX InstallLocation) no longer pin a transient probe failure as 'absent': they cache only an authoritative answer and retry otherwise, so a login-storm spawn failure can't wedge the bundled CLI candidate for the whole session. - tasklist is no longer the sole short-circuit: a broken tasklist now returns Unavailable (not 'not running'), and is_codex_app_running falls through to PowerShell classification instead of silently skipping the host quit (the switch-time contamination direction). Both probe failures log once via OnceLock sentinels. - Get-AppxPackage widened to 'OpenAI.Codex*' and the beta standalone process name 'Codex (Beta).exe' added to enumeration, so the beta channel is actually covered rather than only named in comments. - quit's could-not-attribute message now hints at PowerShell being blocked; the wait loop uses classification directly and documents the ~7-12s worst-case latency. --- src-tauri/win/runtime/process.rs | 228 ++++++++++++++++++++++--------- 1 file changed, 165 insertions(+), 63 deletions(-) diff --git a/src-tauri/win/runtime/process.rs b/src-tauri/win/runtime/process.rs index bdd41a1..c0b2ed1 100644 --- a/src-tauri/win/runtime/process.rs +++ b/src-tauri/win/runtime/process.rs @@ -27,6 +27,10 @@ const APP_PROCESS_NAME_CHATGPT: &str = "ChatGPT.exe"; /// Historical standalone process name, still shipped by /// not-yet-updated `Codex.exe` installs. const APP_PROCESS_NAME_CODEX: &str = "Codex.exe"; +/// Pre-merge beta channel standalone process name (`Codex (Beta).exe`), +/// still running on not-yet-updated beta installs. Post-merge beta +/// updates to `ChatGPT.exe` and is covered by that name instead. +const APP_PROCESS_NAME_CODEX_BETA: &str = "Codex (Beta).exe"; /// Lower-cased path marker of the MSIX package family shared by the /// pre-merge Codex app and the merged ChatGPT host. The Store package /// "updates as usual" into the merged app and a package family name @@ -216,16 +220,24 @@ fn windows_store_shell_target(app_id: &str) -> String { fn is_valid_windows_store_app_id(app_id: &str) -> bool { let trimmed = app_id.trim(); - trimmed.starts_with("OpenAI.Codex_") && trimmed.ends_with("!App") + // Accept both `OpenAI.Codex_…!App` and the `OpenAI.CodexBeta_…!App` + // channel; the `OpenAI.Codex` prefix cannot reach ChatGPT Classic + // (`OpenAI.ChatGPT-Desktop_…`). + trimmed.starts_with("OpenAI.Codex") && trimmed.ends_with("!App") } -fn detect_windows_store_app_target() -> Option { +/// Probe LaunchServices/AppX for the codex host's Store shell target. +/// Returns `(authoritative, value)`: `authoritative` is false only when +/// the probe itself failed (spawn error, non-zero exit) and the answer +/// is therefore unknown — callers must not cache a non-authoritative +/// `None` as "absent". +fn detect_windows_store_app_target_probe() -> (bool, Option) { if !cfg!(target_os = "windows") { - return None; + return (true, None); } let script = format!( - "$package = Get-AppxPackage -Name 'OpenAI.Codex' -ErrorAction SilentlyContinue; \ + "$package = Get-AppxPackage -Name 'OpenAI.Codex*' -ErrorAction SilentlyContinue; \ if ($package) {{ \ $appId = Get-StartApps | Where-Object {{ $_.AppID -like 'OpenAI.Codex*' }} | Select-Object -First 1 -ExpandProperty AppID; \ if ($appId) {{ $appId }} else {{ '{WINDOWS_STORE_APP_ID}' }} \ @@ -233,49 +245,64 @@ fn detect_windows_store_app_target() -> Option { ); let mut command = Command::new("powershell"); command.args(["-NoProfile", "-Command", &script]); - let output = hide_console_window(&mut command).output().ok()?; - if !output.status.success() { - return None; - } + let output = match hide_console_window(&mut command).output() { + Ok(output) if output.status.success() => output, + _ => return (false, None), + }; let app_id = String::from_utf8_lossy(&output.stdout).trim().to_string(); - is_valid_windows_store_app_id(&app_id).then(|| windows_store_shell_target(&app_id)) + ( + true, + is_valid_windows_store_app_id(&app_id).then(|| windows_store_shell_target(&app_id)), + ) } -/// Detected Store shell target, cached for the process lifetime. -/// `None` when the package is absent or PowerShell is unavailable. +/// Detected Store shell target, cached only once a query authoritatively +/// resolves. A transient PowerShell failure (login-storm spawn failure, +/// non-zero exit) is NOT cached, so a later call retries instead of +/// being pinned to "absent" for the whole process lifetime. fn detected_windows_store_target() -> Option { - WINDOWS_APP_TARGET_CACHE - .get_or_init(detect_windows_store_app_target) - .clone() + if let Some(cached) = WINDOWS_APP_TARGET_CACHE.get() { + return cached.clone(); + } + let (authoritative, value) = detect_windows_store_app_target_probe(); + if authoritative { + let _ = WINDOWS_APP_TARGET_CACHE.set(value.clone()); + } + value } -/// InstallLocation of the (merged) codex host's MSIX package, cached -/// for the process lifetime. `None` when the Store package is absent -/// (non-Store install) or PowerShell is unavailable. +/// InstallLocation of the (merged) codex host's MSIX package. Cached +/// only on an authoritative answer (see [`detected_windows_store_target`]). static WINDOWS_STORE_INSTALL_LOCATION_CACHE: OnceLock> = OnceLock::new(); fn windows_store_install_location() -> Option { - WINDOWS_STORE_INSTALL_LOCATION_CACHE - .get_or_init(|| { - if !cfg!(target_os = "windows") { - return None; - } - let mut command = Command::new("powershell"); - command.args([ - "-NoProfile", - "-Command", - "Get-AppxPackage -Name 'OpenAI.Codex' -ErrorAction SilentlyContinue \ - | Select-Object -First 1 -ExpandProperty InstallLocation", - ]); - let output = hide_console_window(&mut command).output().ok()?; - if !output.status.success() { - return None; - } - let location = String::from_utf8_lossy(&output.stdout).trim().to_string(); - (!location.is_empty()).then_some(location) - }) - .clone() + if let Some(cached) = WINDOWS_STORE_INSTALL_LOCATION_CACHE.get() { + return cached.clone(); + } + if !cfg!(target_os = "windows") { + let _ = WINDOWS_STORE_INSTALL_LOCATION_CACHE.set(None); + return None; + } + // `-Name 'OpenAI.Codex*'` also matches the OpenAI.CodexBeta channel; + // the trailing `*` cannot reach OpenAI.ChatGPT-Desktop (Classic). + let mut command = Command::new("powershell"); + command.args([ + "-NoProfile", + "-Command", + "Get-AppxPackage -Name 'OpenAI.Codex*' -ErrorAction SilentlyContinue \ + | Select-Object -First 1 -ExpandProperty InstallLocation", + ]); + let output = match hide_console_window(&mut command).output() { + Ok(output) if output.status.success() => output, + // Transient failure: leave the cache empty so the next call + // retries instead of pinning "no bundled CLI" for the session. + _ => return None, + }; + let location = String::from_utf8_lossy(&output.stdout).trim().to_string(); + let value = (!location.is_empty()).then_some(location); + let _ = WINDOWS_STORE_INSTALL_LOCATION_CACHE.set(value.clone()); + value } /// Non-Store install locations of the desktop host executable, current @@ -336,7 +363,11 @@ fn resolve_windows_app_target() -> AppLaunchTarget { /// Process names that may own the Codex desktop UI after the ChatGPT /// merge. Order only matters for probe latency (current host first). fn desktop_app_process_names() -> &'static [&'static str] { - &[APP_PROCESS_NAME_CHATGPT, APP_PROCESS_NAME_CODEX] + &[ + APP_PROCESS_NAME_CHATGPT, + APP_PROCESS_NAME_CODEX, + APP_PROCESS_NAME_CODEX_BETA, + ] } /// Install identity of a name-matched PID. `Other` is the only verdict @@ -401,6 +432,23 @@ fn parse_pid_classification_lines(stdout: &str) -> Vec<(u32, HostIdentity)> { .collect() } +/// One-shot markers so a persistent probe failure (stripped PATH, +/// blocked PowerShell) is logged once instead of every poll — without +/// them the degradation is invisible when diagnosing a switch that +/// silently stopped quitting the host. +static TASKLIST_PROBE_DEGRADED_WARNING: OnceLock<()> = OnceLock::new(); +static CIM_PROBE_DEGRADED_WARNING: OnceLock<()> = OnceLock::new(); + +/// Outcome of the cheap host-name pre-check. `Absent` is authoritative +/// (`tasklist` ran and matched nothing); `Unavailable` means the probe +/// itself failed and the answer is unknown — the caller must fall +/// through to classification rather than treat it as "not running". +enum NameProbe { + Present, + Absent, + Unavailable, +} + /// Name-matched desktop host PIDs with their install classification, /// via one PowerShell CIM query (~150-400ms — only paid after the cheap /// tasklist pre-check matched a name). Returns an empty list when @@ -408,51 +456,95 @@ fn parse_pid_classification_lines(stdout: &str) -> Vec<(u32, HostIdentity)> { /// "unattributable", not as "not running". fn desktop_app_pid_classifications() -> Vec<(u32, HostIdentity)> { let script = format!( - "Get-CimInstance Win32_Process -Filter \"Name='{APP_PROCESS_NAME_CHATGPT}' OR Name='{APP_PROCESS_NAME_CODEX}'\" \ + "Get-CimInstance Win32_Process -Filter \"Name='{APP_PROCESS_NAME_CHATGPT}' OR Name='{APP_PROCESS_NAME_CODEX}' OR Name='{APP_PROCESS_NAME_CODEX_BETA}'\" \ | ForEach-Object {{ \"$($_.ProcessId)|$($_.ExecutablePath)\" }}" ); let mut command = Command::new("powershell"); command.args(["-NoProfile", "-Command", &script]); let output = match hide_console_window(&mut command).output() { Ok(output) if output.status.success() => output, - _ => return Vec::new(), + result => { + CIM_PROBE_DEGRADED_WARNING.get_or_init(|| { + eprintln!( + "codex_switch: PowerShell host classification unavailable ({}); \ + process attribution disabled", + match &result { + Ok(output) => format!("exit {:?}", output.status.code()), + Err(error) => error.to_string(), + } + ); + }); + return Vec::new(); + } }; parse_pid_classification_lines(&String::from_utf8_lossy(&output.stdout)) } /// Cheap pre-check: does any process with a known host name exist at /// all? Avoids paying the PowerShell classification cost on every -/// 200ms quit-wait poll when nothing is running. -fn any_host_process_name_running() -> bool { - desktop_app_process_names().iter().any(|name| { +/// 200ms quit-wait poll when nothing is running. Distinguishes +/// "definitely absent" from "probe unavailable" so a broken `tasklist` +/// does not masquerade as "not running". +fn host_name_probe() -> NameProbe { + let mut any_unavailable = false; + for name in desktop_app_process_names() { let mut command = Command::new("tasklist"); command.args(["/FI", &format!("IMAGENAME eq {name}"), "/FO", "CSV", "/NH"]); - let output = match hide_console_window(&mut command).output() { - Ok(value) => value, - Err(_) => return false, - }; - String::from_utf8_lossy(&output.stdout) - .to_ascii_lowercase() - .contains(&name.to_ascii_lowercase()) - }) + match hide_console_window(&mut command).output() { + Ok(output) if output.status.success() => { + if String::from_utf8_lossy(&output.stdout) + .to_ascii_lowercase() + .contains(&name.to_ascii_lowercase()) + { + return NameProbe::Present; + } + } + _ => any_unavailable = true, + } + } + if any_unavailable { + TASKLIST_PROBE_DEGRADED_WARNING.get_or_init(|| { + eprintln!( + "codex_switch: tasklist pre-check unavailable; \ + falling back to PowerShell classification" + ); + }); + NameProbe::Unavailable + } else { + NameProbe::Absent + } } -pub fn is_codex_app_running() -> bool { - if !any_host_process_name_running() { - return false; - } +/// Classification-only running check, for callers that already know a +/// host name exists (the quit wait loops): skips the tasklist +/// pre-check and pays a single PowerShell CIM query per poll (~150-400 +/// ms — the quit worst case is documented on the loop below). +fn is_codex_host_running_via_classification() -> bool { let classified = desktop_app_pid_classifications(); if classified.is_empty() { - // A host name is running but PowerShell could not attribute it: - // count as running so the switch aborts instead of proceeding - // over a possibly-live host (quit will refuse to signal it). - return true; + // PowerShell could not attribute anything: fall back to the + // name probe and stay conservative — a bare name match counts + // as running so the switch aborts instead of proceeding over a + // possibly-live host (quit refuses to signal it). + return matches!(host_name_probe(), NameProbe::Present); } classified .iter() .any(|(_, identity)| *identity != HostIdentity::Other) } +pub fn is_codex_app_running() -> bool { + match host_name_probe() { + // Authoritatively no host process: skip the expensive query. + NameProbe::Absent => false, + // A host name is present, or the pre-check is broken and we + // cannot rule a host out: consult classification (which itself + // fails conservative — an unattributable host counts as + // running so the switch aborts rather than risk contamination). + NameProbe::Present | NameProbe::Unavailable => is_codex_host_running_via_classification(), + } +} + pub fn open_or_activate_codex_app(_codex_home: Option<&Path>) -> AppResult { let target = resolve_windows_app_target(); @@ -881,10 +973,15 @@ pub fn quit_codex_app_if_running() -> AppResult { return Ok(false); } + // Wait-loop cost note: each poll pays one PowerShell CIM query + // (~150-400ms) on top of the 200ms sleep, because classification is + // genuinely required — ChatGPT Classic shares the ChatGPT.exe name + // and a cheap name check would never observe "exited". Worst case + // quit latency is therefore ~7-12s graceful + ~4-6s forced. let mut signalled = signal_desktop_app_processes(false); let mut exited = false; for _ in 0..20 { - if !is_codex_app_running() { + if !is_codex_host_running_via_classification() { exited = true; break; } @@ -894,7 +991,7 @@ pub fn quit_codex_app_if_running() -> AppResult { if !exited { signalled += signal_desktop_app_processes(true); for _ in 0..10 { - if !is_codex_app_running() { + if !is_codex_host_running_via_classification() { exited = true; break; } @@ -912,7 +1009,8 @@ pub fn quit_codex_app_if_running() -> AppResult { // app's. Mirrors the macOS quit path. let message = if signalled == 0 { "Codex/ChatGPT still appears to be running, but no matching process could be \ - identified and signalled. Close it manually and retry." + identified and signalled (process attribution may be unavailable if PowerShell \ + is blocked). Close it manually and retry." } else { "Codex/ChatGPT did not exit cleanly. Close it manually and retry." }; @@ -935,6 +1033,10 @@ pub fn reopen_codex_app_if_needed( command.arg(shell_target); hide_console_window(&mut command).spawn() } + AppLaunchTarget::Executable(executable) => { + let mut command = Command::new(&executable); + hide_console_window(&mut command).spawn() + } }; if let Err(error) = result { @@ -1319,7 +1421,7 @@ mod tests { fn desktop_app_process_names_cover_merged_and_legacy_hosts() { assert_eq!( super::desktop_app_process_names(), - &["ChatGPT.exe", "Codex.exe"] + &["ChatGPT.exe", "Codex.exe", "Codex (Beta).exe"] ); } }