From 20cc287ed4d01454b6b02dccb12bac56c1d1eecc Mon Sep 17 00:00:00 2001 From: Fzhiyu1 <2403809502@qq.com> Date: Wed, 11 Mar 2026 11:13:44 +0800 Subject: [PATCH] fix(switching): improve account switching reliability and UX --- src-tauri/src/auth/fs_utils.rs | 95 +++++++++++++++- src-tauri/src/commands/account.rs | 63 +++++++++-- src-tauri/src/commands/process.rs | 122 ++++++++++++++++----- src/App.tsx | 84 ++++++++++---- src/components/SwitchConfirmationModal.tsx | 61 +++++++++++ src/components/index.ts | 1 + src/hooks/useAccounts.ts | 5 +- src/types/index.ts | 2 + src/utils/switching.ts | 18 +++ tests/switching.test.ts | 41 +++++++ 10 files changed, 433 insertions(+), 59 deletions(-) create mode 100644 src/components/SwitchConfirmationModal.tsx create mode 100644 src/utils/switching.ts create mode 100644 tests/switching.test.ts diff --git a/src-tauri/src/auth/fs_utils.rs b/src-tauri/src/auth/fs_utils.rs index b25baae..4ea4b81 100644 --- a/src-tauri/src/auth/fs_utils.rs +++ b/src-tauri/src/auth/fs_utils.rs @@ -1,7 +1,9 @@ use std::ffi::OsString; use std::fs::{self, File, OpenOptions}; +use std::io::Read; use std::io::Write; use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; use std::thread; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -32,6 +34,7 @@ impl FileLock { return Ok(Self { path: lock_path }); } Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => { + maybe_clear_stale_lock(&lock_path); thread::sleep(Duration::from_millis(50)); } Err(err) => { @@ -42,8 +45,8 @@ impl FileLock { } } - anyhow::bail!("Timed out waiting for file lock: {}", lock_path.display()); - } + anyhow::bail!("Timed out waiting for file lock: {}", lock_path.display()); +} } impl Drop for FileLock { @@ -106,3 +109,91 @@ fn temp_path_for(path: &Path) -> PathBuf { .unwrap_or(0); sibling_with_suffix(path, &format!(".tmp-{}-{nanos}", std::process::id())) } + +fn maybe_clear_stale_lock(lock_path: &Path) { + let Ok(Some(pid)) = read_lock_pid(lock_path) else { + return; + }; + + if !process_is_alive(pid) { + let _ = fs::remove_file(lock_path); + } +} + +fn read_lock_pid(lock_path: &Path) -> Result> { + let mut content = String::new(); + File::open(lock_path) + .with_context(|| format!("Failed to open lock file: {}", lock_path.display()))? + .read_to_string(&mut content) + .with_context(|| format!("Failed to read lock file: {}", lock_path.display()))?; + + let trimmed = content.trim(); + if trimmed.is_empty() { + return Ok(None); + } + + let pid = trimmed + .parse::() + .with_context(|| format!("Invalid PID in lock file: {}", lock_path.display()))?; + Ok(Some(pid)) +} + +#[cfg(unix)] +fn process_is_alive(pid: u32) -> bool { + Command::new("kill") + .args(["-0", &pid.to_string()]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .map(|status| status.success()) + .unwrap_or(false) +} + +#[cfg(windows)] +fn process_is_alive(pid: u32) -> bool { + Command::new("tasklist") + .args(["/FI", &format!("PID eq {pid}")]) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .output() + .map(|output| { + let stdout = String::from_utf8_lossy(&output.stdout); + stdout.lines().any(|line| line.contains(&pid.to_string())) + }) + .unwrap_or(false) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn test_dir() -> PathBuf { + let path = std::env::temp_dir().join(format!( + "codex-switcher-fs-utils-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| duration.as_nanos()) + .unwrap_or(0) + )); + fs::create_dir_all(&path).unwrap(); + path + } + + #[test] + fn acquire_reclaims_stale_lock_file() { + let dir = test_dir(); + let target = dir.join("auth.json"); + let lock_path = sibling_with_suffix(&target, ".lock"); + fs::write(&lock_path, format!("{}\n", u32::MAX)).unwrap(); + + let lock = FileLock::acquire(&target).expect("stale lock should be reclaimed"); + let content = fs::read_to_string(&lock_path).unwrap(); + + assert_eq!(content.trim(), std::process::id().to_string()); + + drop(lock); + assert!(!lock_path.exists()); + fs::remove_dir_all(dir).unwrap(); + } +} diff --git a/src-tauri/src/commands/account.rs b/src-tauri/src/commands/account.rs index b5e3981..9dea9b4 100644 --- a/src-tauri/src/commands/account.rs +++ b/src-tauri/src/commands/account.rs @@ -55,6 +55,15 @@ const MAX_IMPORT_JSON_BYTES: u64 = 2 * 1024 * 1024; const MAX_IMPORT_FILE_BYTES: u64 = 8 * 1024 * 1024; const SLIM_IMPORT_CONCURRENCY: usize = 6; +#[derive(Debug, Clone, Copy, Default, serde::Serialize, serde::Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum SwitchAccountMode { + #[default] + RequireRestart, + KeepRunning, + RestartRunning, +} + #[derive(Debug, serde::Serialize, serde::Deserialize)] struct SlimPayload { #[serde(rename = "v")] @@ -124,7 +133,7 @@ pub async fn add_account_from_file(path: String, name: String) -> Result, + switch_mode: Option, ) -> Result<(), String> { let store = load_accounts().map_err(|e| e.to_string())?; let running_processes = collect_running_codex_processes().map_err(|e| e.to_string())?; @@ -136,12 +145,8 @@ pub async fn switch_account( .find(|a| a.id == account_id) .ok_or_else(|| format!("Account not found: {account_id}"))?; - let should_restart = restart_running_codex.unwrap_or(false); - if !running_processes.is_empty() && !should_restart { - return Err(String::from( - "Codex is currently running. Confirm a graceful restart before switching accounts.", - )); - } + let should_restart = + resolve_switch_behavior(!running_processes.is_empty(), switch_mode.unwrap_or_default())?; if should_restart { gracefully_stop_codex_processes(&running_processes).map_err(|e| e.to_string())?; @@ -158,6 +163,50 @@ pub async fn switch_account( Ok(()) } +fn resolve_switch_behavior( + has_running_processes: bool, + switch_mode: SwitchAccountMode, +) -> Result { + if !has_running_processes { + return Ok(false); + } + + match switch_mode { + SwitchAccountMode::RequireRestart => Err(String::from( + "Codex is currently running. Choose whether to keep current sessions running or restart them before switching accounts.", + )), + SwitchAccountMode::KeepRunning => Ok(false), + SwitchAccountMode::RestartRunning => Ok(true), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn keep_running_mode_allows_switch_with_running_processes() { + let should_restart = resolve_switch_behavior(true, SwitchAccountMode::KeepRunning).unwrap(); + + assert!(!should_restart); + } + + #[test] + fn restart_running_mode_requests_restart_when_processes_are_running() { + let should_restart = + resolve_switch_behavior(true, SwitchAccountMode::RestartRunning).unwrap(); + + assert!(should_restart); + } + + #[test] + fn missing_mode_requires_explicit_choice_when_processes_are_running() { + let error = resolve_switch_behavior(true, SwitchAccountMode::RequireRestart).unwrap_err(); + + assert!(error.contains("Choose whether to keep current sessions running")); + } +} + #[tauri::command] pub async fn get_app_settings() -> Result { load_settings().map_err(|e| e.to_string()) diff --git a/src-tauri/src/commands/process.rs b/src-tauri/src/commands/process.rs index 89028b8..b678710 100644 --- a/src-tauri/src/commands/process.rs +++ b/src-tauri/src/commands/process.rs @@ -107,7 +107,7 @@ pub fn gracefully_stop_codex_processes(processes: &[RunningCodexProcess]) -> any return Ok(()); } - anyhow::bail!("Timed out waiting for Codex processes to close gracefully"); + anyhow::bail!(format_graceful_shutdown_timeout(processes)); } pub fn restart_codex_processes(processes: &[RunningCodexProcess]) -> anyhow::Result<()> { @@ -157,33 +157,11 @@ fn collect_running_codex_processes_unix() -> anyhow::Result() { - if pid != std::process::id() - && !processes.iter().any(|p: &RunningCodexProcess| p.pid == pid) - { - processes.push(RunningCodexProcess { - pid, - command: command.to_string(), - is_background, - }); - } - } + if let Some(process) = parse_unix_process_line(line) { + if process.pid != std::process::id() + && !processes.iter().any(|p: &RunningCodexProcess| p.pid == process.pid) + { + processes.push(process); } } } @@ -191,6 +169,55 @@ fn collect_running_codex_processes_unix() -> anyhow::Result Option { + let line = line.trim(); + if line.is_empty() { + return None; + } + + let (pid_str, command) = line.split_once(' ')?; + let command = command.trim(); + let executable = command.split_whitespace().next().unwrap_or(""); + let is_codex = executable == "codex" || executable.ends_with("/codex"); + let is_background = + command.contains(".antigravity") || command.contains("openai.chatgpt") || command.contains(".vscode"); + let is_switcher = command.contains("codex-switcher") || command.contains("Codex Switcher"); + let is_codex_app_server = + command.contains("/Codex.app/Contents/Resources/codex app-server") + || command.contains("/Applications/Codex.app/Contents/Resources/codex app-server"); + + if !is_codex || is_switcher || is_codex_app_server { + return None; + } + + let pid = pid_str.trim().parse::().ok()?; + Some(RunningCodexProcess { + pid, + command: command.to_string(), + is_background, + }) +} + +fn format_graceful_shutdown_timeout(processes: &[RunningCodexProcess]) -> String { + let details = processes + .iter() + .map(|process| format!("pid {} ({})", process.pid, summarize_command(&process.command))) + .collect::>() + .join(", "); + format!("Timed out waiting for Codex processes to close gracefully: {details}") +} + +fn summarize_command(command: &str) -> String { + const MAX_LEN: usize = 80; + if command.chars().count() <= MAX_LEN { + return command.to_string(); + } + + let summary = command.chars().take(MAX_LEN - 3).collect::(); + format!("{summary}...") +} + #[cfg(windows)] fn collect_running_codex_processes_windows() -> anyhow::Result> { let mut processes = Vec::new(); @@ -221,3 +248,42 @@ fn collect_running_codex_processes_windows() -> anyhow::Result(null); const [isRunningMissedWarmup, setIsRunningMissedWarmup] = useState(false); + const [pendingSwitch, setPendingSwitch] = useState<{ + accountId: string; + processInfo: CodexProcessInfo; + } | null>(null); const actionsMenuRef = useRef(null); const toggleMask = (accountId: string) => { @@ -268,9 +278,24 @@ function App() { return () => document.removeEventListener("mousedown", handleClickOutside); }, [isActionsMenuOpen]); + const performSwitch = useCallback( + async (accountId: string, switchMode?: SwitchAccountMode) => { + try { + setSwitchingId(accountId); + await switchAccount(accountId, switchMode); + await checkProcesses(); + } catch (err) { + console.error("Failed to switch account:", err); + showWarmupToast(`Failed to switch account: ${getSwitchErrorMessage(err)}`, true); + } finally { + setSwitchingId(null); + } + }, + [checkProcesses, switchAccount] + ); + const handleSwitch = async (accountId: string) => { try { - setSwitchingId(accountId); const latestProcessInfo = await invoke("check_codex_processes").catch( (err) => { console.error("Failed to check processes before switching:", err); @@ -282,30 +307,37 @@ function App() { setProcessInfo(latestProcessInfo); } - const hasRunningCodex = - !!latestProcessInfo && - (latestProcessInfo.count > 0 || latestProcessInfo.background_count > 0); - - let restartRunningCodex = false; - if (hasRunningCodex) { - restartRunningCodex = window.confirm( - `Codex is running (${latestProcessInfo.count} foreground, ${latestProcessInfo.background_count} background). Do you want Codex Switcher to close and reopen it gracefully before switching accounts?` - ); - - if (!restartRunningCodex) { - return; - } + if (latestProcessInfo && hasRunningCodexProcesses(latestProcessInfo)) { + setPendingSwitch({ accountId, processInfo: latestProcessInfo }); + return; } - - await switchAccount(accountId, restartRunningCodex); - await checkProcesses(); + await performSwitch(accountId); } catch (err) { - console.error("Failed to switch account:", err); - } finally { - setSwitchingId(null); + console.error("Failed to prepare account switch:", err); + showWarmupToast(`Failed to switch account: ${getSwitchErrorMessage(err)}`, true); } }; + const handleCancelPendingSwitch = () => { + setPendingSwitch(null); + }; + + const handleKeepRunningPendingSwitch = async () => { + if (!pendingSwitch) return; + + const { accountId } = pendingSwitch; + setPendingSwitch(null); + await performSwitch(accountId, "keep_running"); + }; + + const handleRestartPendingSwitch = async () => { + if (!pendingSwitch) return; + + const { accountId } = pendingSwitch; + setPendingSwitch(null); + await performSwitch(accountId, "restart_running"); + }; + const handleDelete = async (accountId: string) => { if (deleteConfirmId !== accountId) { setDeleteConfirmId(accountId); @@ -1046,6 +1078,18 @@ function App() { onSkipToday={handleSkipMissedScheduledWarmup} /> + { + void handleKeepRunningPendingSwitch(); + }} + onRestartRunning={() => { + void handleRestartPendingSwitch(); + }} + /> + {/* Import/Export Config Modal */} {isConfigModalOpen && (
diff --git a/src/components/SwitchConfirmationModal.tsx b/src/components/SwitchConfirmationModal.tsx new file mode 100644 index 0000000..5d80f4e --- /dev/null +++ b/src/components/SwitchConfirmationModal.tsx @@ -0,0 +1,61 @@ +import type { CodexProcessInfo } from "../types"; +import { getSwitchConfirmationMessage } from "../utils/switching"; + +interface SwitchConfirmationModalProps { + isOpen: boolean; + processInfo: CodexProcessInfo | null; + onCancel: () => void; + onKeepRunning: () => void; + onRestartRunning: () => void; +} + +export function SwitchConfirmationModal({ + isOpen, + processInfo, + onCancel, + onKeepRunning, + onRestartRunning, +}: SwitchConfirmationModalProps) { + if (!isOpen || !processInfo) return null; + + return ( +
+
+
+

Switch Account

+

+ {getSwitchConfirmationMessage(processInfo)} +

+
+ +
+
+ Keeping current sessions running only affects future Codex launches. Existing sessions + will continue using their current login until you restart them manually. +
+
+ +
+ + + +
+
+
+ ); +} diff --git a/src/components/index.ts b/src/components/index.ts index 2936fe5..91a005e 100644 --- a/src/components/index.ts +++ b/src/components/index.ts @@ -3,3 +3,4 @@ export { UsageBar } from "./UsageBar"; export { AddAccountModal } from "./AddAccountModal"; export { ScheduledWarmupsModal } from "./ScheduledWarmupsModal"; export { MissedScheduledWarmupModal } from "./MissedScheduledWarmupModal"; +export { SwitchConfirmationModal } from "./SwitchConfirmationModal"; diff --git a/src/hooks/useAccounts.ts b/src/hooks/useAccounts.ts index 90e81b4..29aa505 100644 --- a/src/hooks/useAccounts.ts +++ b/src/hooks/useAccounts.ts @@ -10,6 +10,7 @@ import type { ScheduledWarmupSettings, ScheduledWarmupStatus, AppSettings, + SwitchAccountMode, } from "../types"; export function useAccounts() { @@ -100,9 +101,9 @@ export function useAccounts() { }, []); const switchAccount = useCallback( - async (accountId: string, restartRunningCodex?: boolean) => { + async (accountId: string, switchMode?: SwitchAccountMode) => { try { - await invoke("switch_account", { accountId, restartRunningCodex }); + await invoke("switch_account", { accountId, switchMode }); await loadAccounts(true); // Preserve usage data } catch (err) { throw err; diff --git a/src/types/index.ts b/src/types/index.ts index 04c169b..b8a61e6 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -45,6 +45,8 @@ export interface CodexProcessInfo { pids: number[]; } +export type SwitchAccountMode = "keep_running" | "restart_running"; + export interface WarmupSummary { total_accounts: number; warmed_accounts: number; diff --git a/src/utils/switching.ts b/src/utils/switching.ts new file mode 100644 index 0000000..9d876e1 --- /dev/null +++ b/src/utils/switching.ts @@ -0,0 +1,18 @@ +import type { CodexProcessInfo } from "../types"; + +export function hasRunningCodexProcesses(processInfo: CodexProcessInfo | null | undefined) { + return !!processInfo && (processInfo.count > 0 || processInfo.background_count > 0); +} + +export function getSwitchConfirmationMessage(processInfo: CodexProcessInfo) { + return `Codex is running (${processInfo.count} foreground, ${processInfo.background_count} background). Do you want Codex Switcher to close and reopen it gracefully before switching accounts?`; +} + +export function getSwitchErrorMessage(error: unknown) { + if (typeof error === "string") return error; + if (error && typeof error === "object" && "message" in error) { + const message = (error as { message?: unknown }).message; + if (typeof message === "string") return message; + } + return "Unknown error"; +} diff --git a/tests/switching.test.ts b/tests/switching.test.ts new file mode 100644 index 0000000..7a38268 --- /dev/null +++ b/tests/switching.test.ts @@ -0,0 +1,41 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + getSwitchConfirmationMessage, + getSwitchErrorMessage, + hasRunningCodexProcesses, +} from "../src/utils/switching.ts"; + +test("hasRunningCodexProcesses returns true when foreground or background codex is present", () => { + assert.equal( + hasRunningCodexProcesses({ count: 6, background_count: 0, can_switch: false, pids: [1] }), + true + ); + assert.equal( + hasRunningCodexProcesses({ count: 0, background_count: 2, can_switch: false, pids: [] }), + true + ); + assert.equal( + hasRunningCodexProcesses({ count: 0, background_count: 0, can_switch: true, pids: [] }), + false + ); +}); + +test("getSwitchConfirmationMessage includes both foreground and background counts", () => { + assert.equal( + getSwitchConfirmationMessage({ + count: 6, + background_count: 0, + can_switch: false, + pids: [1, 2, 3], + }), + "Codex is running (6 foreground, 0 background). Do you want Codex Switcher to close and reopen it gracefully before switching accounts?" + ); +}); + +test("getSwitchErrorMessage unwraps strings and Error-like objects", () => { + assert.equal(getSwitchErrorMessage("plain failure"), "plain failure"); + assert.equal(getSwitchErrorMessage({ message: "object failure" }), "object failure"); + assert.equal(getSwitchErrorMessage(null), "Unknown error"); +});