diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 98370c4fc5..3ec2708880 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -924,6 +924,7 @@ name = "codex-auto-updater" version = "0.0.0" dependencies = [ "async-trait", + "codex-internal-storage", "pretty_assertions", "semver", "serde", @@ -931,6 +932,7 @@ dependencies = [ "tempfile", "thiserror 2.0.16", "tokio", + "tracing", "which", ] @@ -1239,6 +1241,14 @@ dependencies = [ "walkdir", ] +[[package]] +name = "codex-internal-storage" +version = "0.0.0" +dependencies = [ + "serde_json", + "thiserror 2.0.16", +] + [[package]] name = "codex-keyring-store" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 5c6c1b859b..8738e43739 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -41,6 +41,7 @@ members = [ "utils/string", "utils/tokenizer", "auto-updater", + "internal-storage", ] resolver = "2" @@ -83,6 +84,7 @@ codex-responses-api-proxy = { path = "responses-api-proxy" } codex-rmcp-client = { path = "rmcp-client" } codex-stdio-to-uds = { path = "stdio-to-uds" } codex-tui = { path = "tui" } +codex-internal-storage = { path = "internal-storage" } codex-utils-cache = { path = "utils/cache" } codex-utils-image = { path = "utils/image" } codex-utils-json-to-toml = { path = "utils/json-to-toml" } diff --git a/codex-rs/auto-updater/Cargo.toml b/codex-rs/auto-updater/Cargo.toml index 7df5de1e1d..f647fddb81 100644 --- a/codex-rs/auto-updater/Cargo.toml +++ b/codex-rs/auto-updater/Cargo.toml @@ -11,6 +11,8 @@ serde_json = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true, features = ["macros", "process", "rt-multi-thread"] } which = { workspace = true } +tracing = { workspace = true } +codex-internal-storage = { workspace = true } [dev-dependencies] pretty_assertions = { workspace = true } diff --git a/codex-rs/auto-updater/src/brew.rs b/codex-rs/auto-updater/src/brew.rs index bda0c6c046..3f9faa6f0e 100644 --- a/codex-rs/auto-updater/src/brew.rs +++ b/codex-rs/auto-updater/src/brew.rs @@ -4,9 +4,11 @@ use crate::errors::Error; use async_trait::async_trait; use semver::Version; use serde::Deserialize; +use std::cmp::Ordering; use std::path::Path; use std::path::PathBuf; use std::process::Command; +use std::time::Instant; const CODENAME: &str = "codex"; @@ -33,18 +35,16 @@ impl BrewInstaller { fn install_status(&self) -> Result { if let Some(info) = self.formula_info()? { - let current_version = self.formula_current_version()?; return Ok(InstallStatus { method: InstallMethod::Formula, - current_version, + current_version: info.current_version, latest_version: info.latest_version, }); } if let Some(info) = self.cask_info()? { - let current_version = self.cask_current_version()?; return Ok(InstallStatus { method: InstallMethod::Cask, - current_version, + current_version: info.current_version, latest_version: info.latest_version, }); } @@ -79,14 +79,24 @@ impl BrewInstaller { Some(value) => value, None => return Ok(None), }; - if formula.installed.is_empty() { + let installed_versions: Vec = formula + .installed + .into_iter() + .filter_map(|entry| entry.version) + .collect(); + let current_version = if installed_versions.is_empty() { return Ok(None); - } + } else { + select_highest_brew_version(&installed_versions)? + }; let latest_version = formula .versions .stable .ok_or_else(|| Error::Version("missing stable formula version".into()))?; - Ok(Some(BrewFormulaInfo { latest_version })) + Ok(Some(BrewFormulaInfo { + current_version, + latest_version, + })) } fn cask_info(&self) -> Result, Error> { @@ -102,13 +112,23 @@ impl BrewInstaller { Some(value) => value, None => return Ok(None), }; - if cask.installed.is_empty() { + let installed_versions: Vec = cask + .installed + .into_iter() + .filter_map(|entry| entry.version) + .collect(); + let current_version = if installed_versions.is_empty() { return Ok(None); - } + } else { + select_highest_brew_version(&installed_versions)? + }; let latest_version = cask .version .ok_or_else(|| Error::Version("missing cask version".into()))?; - Ok(Some(BrewCaskInfo { latest_version })) + Ok(Some(BrewCaskInfo { + current_version, + latest_version, + })) } fn formula_current_version(&self) -> Result { @@ -128,18 +148,34 @@ impl BrewInstaller { #[async_trait] impl Installer for BrewInstaller { fn version_status(&self) -> Result { - let status = self.install_status()?; - let update_available = status.needs_update()?; - let InstallStatus { - method: _, - current_version, - latest_version, - } = status; - Ok(UpdateStatus { - current_version, - latest_version, - update_available, - }) + let started = Instant::now(); + let outcome = (|| { + let status = self.install_status()?; + let update_available = status.needs_update()?; + let InstallStatus { + method: _, + current_version, + latest_version, + } = status; + Ok(UpdateStatus { + current_version, + latest_version, + update_available, + }) + })(); + let elapsed = started.elapsed(); + match &outcome { + Ok(_) => tracing::info!( + elapsed_ms = elapsed.as_millis(), + "brew version status completed" + ), + Err(err) => tracing::info!( + elapsed_ms = elapsed.as_millis(), + error = %err, + "brew version status failed" + ), + } + outcome } async fn update(&self) -> Result { @@ -191,10 +227,15 @@ struct BrewFormulaInfoResponse { #[derive(Debug, Deserialize)] struct BrewFormulaEntry { - installed: Vec, + installed: Vec, versions: BrewFormulaVersions, } +#[derive(Debug, Deserialize)] +struct BrewFormulaInstalledEntry { + version: Option, +} + #[derive(Debug, Deserialize)] struct BrewFormulaVersions { stable: Option, @@ -202,6 +243,7 @@ struct BrewFormulaVersions { #[derive(Debug)] struct BrewFormulaInfo { + current_version: String, latest_version: String, } @@ -212,12 +254,18 @@ struct BrewCaskInfoResponse { #[derive(Debug, Deserialize)] struct BrewCaskEntry { - installed: Vec, + installed: Vec, + version: Option, +} + +#[derive(Debug, Deserialize)] +struct BrewCaskInstalledEntry { version: Option, } #[derive(Debug)] struct BrewCaskInfo { + current_version: String, latest_version: String, } @@ -228,6 +276,7 @@ struct CommandOutput { fn run_command_sync(path: &Path, args: &[&str]) -> Result { let output = Command::new(path) .args(args) + .env("HOMEBREW_NO_AUTO_UPDATE", "1") .output() .map_err(|err| Error::Io(err.to_string()))?; handle_command_output(path, args, output) @@ -240,6 +289,7 @@ async fn run_command_async( ) -> Result { let mut command = tokio::process::Command::new(path); command.args(args); + command.env("HOMEBREW_NO_AUTO_UPDATE", "1"); if let Some((key, value)) = env { command.env(key, value); } @@ -298,16 +348,55 @@ fn parse_brew_list_version(stdout: &str) -> Result { Ok(version.to_string()) } -fn compare_versions(current: &str, latest: &str) -> Result { - match (Version::parse(current), Version::parse(latest)) { - (Ok(current_semver), Ok(latest_semver)) => Ok(latest_semver > current_semver), - (Err(_), Err(_)) | (Ok(_), Err(_)) | (Err(_), Ok(_)) => { - if let Some(result) = compare_brew_versions(current, latest) { - return Ok(result); +fn select_highest_brew_version(versions: &[String]) -> Result { + let mut best: Option = None; + for version in versions { + let trimmed = version.trim(); + if trimmed.is_empty() { + continue; + } + let candidate = trimmed.to_string(); + match &best { + Some(current_best) => { + if version_ordering(current_best, &candidate) == Ordering::Less { + best = Some(candidate); + } } - Ok(latest > current) + None => best = Some(candidate), } } + match best { + Some(value) => Ok(value), + None => Err(Error::Version("missing installed brew version".into())), + } +} + +fn version_ordering(lhs: &str, rhs: &str) -> Ordering { + match (Version::parse(lhs), Version::parse(rhs)) { + (Ok(lhs_semver), Ok(rhs_semver)) => lhs_semver.cmp(&rhs_semver), + _ => match (parse_brew_version(lhs), parse_brew_version(rhs)) { + (Some((lhs_semver, lhs_revision)), Some((rhs_semver, rhs_revision))) => { + match lhs_semver.cmp(&rhs_semver) { + Ordering::Equal => lhs_revision.cmp(&rhs_revision), + other => other, + } + } + _ => lhs.cmp(rhs), + }, + } +} + +fn compare_versions(current: &str, latest: &str) -> Result { + Ok(true) + // match (Version::parse(current), Version::parse(latest)) { + // (Ok(current_semver), Ok(latest_semver)) => Ok(latest_semver > current_semver), + // (Err(_), Err(_)) | (Ok(_), Err(_)) | (Err(_), Ok(_)) => { + // if let Some(result) = compare_brew_versions(current, latest) { + // return Ok(result); + // } + // Ok(latest > current) + // } + // } } fn compare_brew_versions(current: &str, latest: &str) -> Option { diff --git a/codex-rs/auto-updater/src/lib.rs b/codex-rs/auto-updater/src/lib.rs index 8ac555eb79..6930d59010 100644 --- a/codex-rs/auto-updater/src/lib.rs +++ b/codex-rs/auto-updater/src/lib.rs @@ -3,10 +3,15 @@ mod errors; use async_trait::async_trait; pub use errors::Error; +use serde::Deserialize; +use serde::Serialize; +use std::path::Path; use crate::brew::BrewInstaller; -#[derive(Debug)] +const AUTO_UPDATER_STATUS_KEY: &str = "auto_updater.status"; + +#[derive(Debug, Serialize, Deserialize)] pub struct UpdateStatus { pub current_version: String, pub latest_version: String, @@ -31,10 +36,14 @@ pub fn installer() -> Result, Error> { Err(Error::Unsupported) } -pub fn update_status() -> Result { +fn compute_update_status() -> Result { installer()?.version_status() } +pub fn update_status() -> Result { + compute_update_status() +} + pub fn update_available() -> Result { installer()?.update_available() } @@ -42,3 +51,38 @@ pub fn update_available() -> Result { pub async fn update() -> Result { installer()?.update().await } + +pub fn initialize_storage(codex_home: &Path) -> Result<(), Error> { + codex_internal_storage::initialize(codex_home.to_path_buf()); + Ok(()) +} + +pub fn read_cached_status() -> Result, Error> { + match codex_internal_storage::read(AUTO_UPDATER_STATUS_KEY) { + Ok(Some(value)) => { + let status = + serde_json::from_str(&value).map_err(|err| Error::Json(err.to_string()))?; + Ok(Some(status)) + } + Ok(None) => Ok(None), + Err(err) => Err(map_storage_error(err)), + } +} + +pub async fn refresh_status() -> Result { + let status = compute_update_status()?; + let serialized = serde_json::to_string(&status).map_err(|err| Error::Json(err.to_string()))?; + codex_internal_storage::write(AUTO_UPDATER_STATUS_KEY, &serialized) + .map_err(map_storage_error)?; + Ok(status) +} + +fn map_storage_error(err: codex_internal_storage::InternalStorageError) -> Error { + match err { + codex_internal_storage::InternalStorageError::Io(err) => Error::Io(err.to_string()), + codex_internal_storage::InternalStorageError::Json(err) => Error::Json(err.to_string()), + codex_internal_storage::InternalStorageError::Uninitialized => { + Error::Io("internal storage not initialized".into()) + } + } +} diff --git a/codex-rs/internal-storage/Cargo.toml b/codex-rs/internal-storage/Cargo.toml new file mode 100644 index 0000000000..cd419326c8 --- /dev/null +++ b/codex-rs/internal-storage/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "codex-internal-storage" +version.workspace = true +edition.workspace = true + +[dependencies] +serde_json = { workspace = true } +thiserror = { workspace = true } + +[lints] +workspace = true diff --git a/codex-rs/internal-storage/src/lib.rs b/codex-rs/internal-storage/src/lib.rs new file mode 100644 index 0000000000..6a2efbaca5 --- /dev/null +++ b/codex-rs/internal-storage/src/lib.rs @@ -0,0 +1,113 @@ +use serde_json::Map as JsonMap; +use serde_json::Value; +use std::fs; +use std::io; +use std::path::Path; +use std::path::PathBuf; +use std::sync::Mutex; +use std::sync::OnceLock; +use thiserror::Error; + +const INTERNAL_STORAGE_FILENAME: &str = "internal_storage.json"; + +#[derive(Debug, Error)] +pub enum InternalStorageError { + #[error("{0}")] + Io(#[from] io::Error), + #[error("{0}")] + Json(#[from] serde_json::Error), + #[error("internal storage has not been initialized")] + Uninitialized, +} + +#[derive(Debug)] +struct Storage { + path: PathBuf, + lock: Mutex<()>, +} + +impl Storage { + fn new(path: PathBuf) -> Self { + Self { + path, + lock: Mutex::new(()), + } + } + + fn read_map(&self) -> Result, InternalStorageError> { + match fs::read_to_string(&self.path) { + Ok(contents) => { + let value: Value = serde_json::from_str(&contents)?; + match value { + Value::Object(map) => Ok(map), + Value::Null => Ok(JsonMap::new()), + _ => Ok(JsonMap::new()), + } + } + Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(JsonMap::new()), + Err(err) => Err(err.into()), + } + } + + fn write_map(&self, map: &JsonMap) -> Result<(), InternalStorageError> { + if let Some(parent) = self.path.parent() { + fs::create_dir_all(parent)?; + } + let payload = serde_json::to_string_pretty(&Value::Object(map.clone()))?; + fs::write(&self.path, payload)?; + Ok(()) + } + + fn read(&self, key: &str) -> Result, InternalStorageError> { + let _guard = self.lock.lock().expect("internal storage lock poisoned"); + let map = self.read_map()?; + Ok(map.get(key).map(value_to_string)) + } + + fn write(&self, key: &str, value: &str) -> Result<(), InternalStorageError> { + let _guard = self.lock.lock().expect("internal storage lock poisoned"); + let mut map = self.read_map()?; + map.insert(key.to_string(), Value::String(value.to_string())); + self.write_map(&map) + } +} + +static STORAGE: OnceLock = OnceLock::new(); + +pub fn initialize(codex_home: PathBuf) { + let path = build_storage_path(&codex_home); + let storage = Storage::new(path.clone()); + match STORAGE.get() { + Some(existing) if existing.path != path => {} + Some(_) => {} + None => { + let _ = STORAGE.set(storage); + } + } +} + +pub fn read(key: &str) -> Result, InternalStorageError> { + storage()?.read(key) +} + +pub fn write(key: &str, value: &str) -> Result<(), InternalStorageError> { + storage()?.write(key, value) +} + +fn storage() -> Result<&'static Storage, InternalStorageError> { + STORAGE.get().ok_or(InternalStorageError::Uninitialized) +} + +fn build_storage_path(codex_home: &Path) -> PathBuf { + codex_home.join(INTERNAL_STORAGE_FILENAME) +} + +fn value_to_string(value: &Value) -> String { + match value { + Value::String(s) => s.clone(), + Value::Number(n) => n.to_string(), + Value::Bool(b) => b.to_string(), + Value::Null => "null".to_string(), + _ => value.to_string(), + } +} diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 429c5400fd..32ca564694 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -8,7 +8,6 @@ use app::App; pub use app::AppExitInfo; use codex_app_server_protocol::AuthMode; use codex_auto_updater::Error as AutoUpdateError; -use codex_auto_updater::update_status; use codex_core::AuthManager; use codex_core::BUILT_IN_OSS_MODEL_PROVIDER_ID; use codex_core::CodexAuth; @@ -258,19 +257,39 @@ pub async fn run_main( .try_init(); }; - match update_status() { - Ok(status) if status.update_available => { - let current = status.current_version; - let latest = status.latest_version; - tracing::error!( - current_version = current.as_str(), - latest_version = latest.as_str(), - "A newer Codex release is available. Update Codex from {current} to {latest} with `brew upgrade codex`." - ); + match codex_auto_updater::initialize_storage(&config.codex_home) { + Ok(_) => { + let t1 = std::time::Instant::now(); + match codex_auto_updater::read_cached_status() { + Ok(Some(status)) if status.update_available => { + let t2 = std::time::Instant::now(); + tracing::warn!("Diff: {:?}", t2 - t1); + let current = status.current_version; + let latest = status.latest_version; + tracing::error!( + current_version = current.as_str(), + latest_version = latest.as_str(), + "A newer Codex release is available. Update Codex from {current} to {latest} with `brew upgrade codex`." + ); + } + Ok(_) | Err(AutoUpdateError::Unsupported) => {} + Err(err) => { + tracing::error!(error = ?err, "Failed to read cached Codex update status"); + } + } + + tokio::spawn(async move { + if let Err(err) = codex_auto_updater::refresh_status().await { + tracing::error!(error = ?err, "Failed to refresh Codex update status"); + } + error!("Prop done"); + }); } - Ok(_) | Err(AutoUpdateError::Unsupported) => {} Err(err) => { - tracing::debug!(error = ?err, "Failed to check for Codex updates"); + tracing::error!( + error = ?err, + "Failed to initialize internal storage for Codex updates" + ); } }