mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Guard npm update readiness (#19389)
## Why For npm/Bun-managed installs, the update prompt was treating the latest GitHub release as ready to install. During the `0.124.0` release, GitHub and npm visibility were not atomic: the root npm wrapper could become visible before the npm registry marked that version as the package `latest`. That left a window where users could be prompted to upgrade before npm was ready for the release. ## What changed - Keep GitHub Releases as the candidate latest-version source for npm/Bun installs, but only write the existing `version.json` cache after npm registry metadata proves that same root version is ready. - Add `codex-rs/tui/src/npm_registry.rs` to validate npm readiness by checking `dist-tags.latest` and root package `dist` metadata for the GitHub candidate version. - Move version parsing helpers into `codex-rs/tui/src/update_versions.rs` so that logic can be tested without compiling the release-only `updates.rs` module under tests. - Update `.github/workflows/rust-release.yml` so the six known platform tarballs publish before the root `@openai/codex` wrapper. Other npm tarballs publish before the root wrapper, and the SDK publishes after the root package it depends on.
This commit is contained in:
committed by
GitHub
Unverified
parent
9881dc7306
commit
4e30281a13
@@ -651,11 +651,59 @@ jobs:
|
||||
prefix="${NPM_TAG}-"
|
||||
fi
|
||||
|
||||
root_tarball="dist/npm/codex-npm-${VERSION}.tgz"
|
||||
sdk_tarball="dist/npm/codex-sdk-npm-${VERSION}.tgz"
|
||||
# Keep this list in sync with CODEX_PLATFORM_PACKAGES in
|
||||
# codex-cli/scripts/build_npm_package.py. The root wrapper advances
|
||||
# @openai/codex@latest as soon as it publishes, so every platform
|
||||
# package it aliases must already exist in the registry first.
|
||||
platform_tarballs=(
|
||||
"dist/npm/codex-npm-linux-x64-${VERSION}.tgz"
|
||||
"dist/npm/codex-npm-linux-arm64-${VERSION}.tgz"
|
||||
"dist/npm/codex-npm-darwin-x64-${VERSION}.tgz"
|
||||
"dist/npm/codex-npm-darwin-arm64-${VERSION}.tgz"
|
||||
"dist/npm/codex-npm-win32-x64-${VERSION}.tgz"
|
||||
"dist/npm/codex-npm-win32-arm64-${VERSION}.tgz"
|
||||
)
|
||||
|
||||
for required_tarball in "${platform_tarballs[@]}" "${root_tarball}"; do
|
||||
if [[ ! -f "${required_tarball}" ]]; then
|
||||
echo "Missing npm tarball: ${required_tarball}"
|
||||
exit 1
|
||||
fi
|
||||
done
|
||||
|
||||
shopt -s nullglob
|
||||
tarballs=(dist/npm/*-"${VERSION}".tgz)
|
||||
if [[ ${#tarballs[@]} -eq 0 ]]; then
|
||||
echo "No npm tarballs found in dist/npm for version ${VERSION}"
|
||||
exit 1
|
||||
other_tarballs=()
|
||||
for tarball in dist/npm/*-"${VERSION}".tgz; do
|
||||
if [[ "${tarball}" == "${root_tarball}" || "${tarball}" == "${sdk_tarball}" ]]; then
|
||||
continue
|
||||
fi
|
||||
|
||||
is_platform_tarball=false
|
||||
for platform_tarball in "${platform_tarballs[@]}"; do
|
||||
if [[ "${tarball}" == "${platform_tarball}" ]]; then
|
||||
is_platform_tarball=true
|
||||
break
|
||||
fi
|
||||
done
|
||||
if [[ "${is_platform_tarball}" == true ]]; then
|
||||
continue
|
||||
fi
|
||||
|
||||
other_tarballs+=("${tarball}")
|
||||
done
|
||||
|
||||
# Publish the platform packages before the root CLI wrapper. The root
|
||||
# wrapper advances @openai/codex@latest, so it should only publish
|
||||
# after the optional dependency versions it references exist.
|
||||
tarballs=(
|
||||
"${platform_tarballs[@]}"
|
||||
"${other_tarballs[@]}"
|
||||
"${root_tarball}"
|
||||
)
|
||||
if [[ -f "${sdk_tarball}" ]]; then
|
||||
tarballs+=("${sdk_tarball}")
|
||||
fi
|
||||
|
||||
for tarball in "${tarballs[@]}"; do
|
||||
|
||||
@@ -142,6 +142,8 @@ mod model_catalog;
|
||||
mod model_migration;
|
||||
mod multi_agents;
|
||||
mod notifications;
|
||||
#[cfg(any(not(debug_assertions), test))]
|
||||
mod npm_registry;
|
||||
pub(crate) mod onboarding;
|
||||
mod oss_selection;
|
||||
mod pager_overlay;
|
||||
@@ -167,6 +169,8 @@ mod ui_consts;
|
||||
pub(crate) mod update_action;
|
||||
pub use update_action::UpdateAction;
|
||||
mod update_prompt;
|
||||
#[cfg(any(not(debug_assertions), test))]
|
||||
mod update_versions;
|
||||
mod updates;
|
||||
mod version;
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
|
||||
@@ -0,0 +1,130 @@
|
||||
use serde::Deserialize;
|
||||
use std::collections::HashMap;
|
||||
|
||||
#[cfg(not(debug_assertions))]
|
||||
pub(crate) const PACKAGE_URL: &str = "https://registry.npmjs.org/@openai%2fcodex";
|
||||
|
||||
#[derive(Deserialize, Debug, Clone)]
|
||||
pub(crate) struct NpmPackageInfo {
|
||||
#[serde(rename = "dist-tags")]
|
||||
dist_tags: HashMap<String, String>,
|
||||
versions: HashMap<String, NpmPackageVersionInfo>,
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug, Clone)]
|
||||
struct NpmPackageVersionInfo {
|
||||
dist: Option<NpmPackageDist>,
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug, Clone)]
|
||||
struct NpmPackageDist {
|
||||
tarball: Option<String>,
|
||||
integrity: Option<String>,
|
||||
}
|
||||
|
||||
pub(crate) fn ensure_version_ready(
|
||||
package_info: &NpmPackageInfo,
|
||||
version: &str,
|
||||
) -> anyhow::Result<()> {
|
||||
let version = version.trim();
|
||||
|
||||
match package_info.dist_tags.get("latest").map(String::as_str) {
|
||||
Some(latest) if latest == version => {}
|
||||
Some(latest) => anyhow::bail!(
|
||||
"npm latest dist-tag points to {latest}, expected GitHub release {version}"
|
||||
),
|
||||
None => anyhow::bail!("npm package is missing latest dist-tag"),
|
||||
}
|
||||
|
||||
version_info_with_dist(package_info, version)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn version_info_with_dist<'a>(
|
||||
package_info: &'a NpmPackageInfo,
|
||||
version: &str,
|
||||
) -> anyhow::Result<&'a NpmPackageVersionInfo> {
|
||||
let info = package_info
|
||||
.versions
|
||||
.get(version)
|
||||
.ok_or_else(|| anyhow::anyhow!("npm package version {version} is missing"))?;
|
||||
let Some(dist) = info.dist.as_ref() else {
|
||||
anyhow::bail!("npm package version {version} is missing dist metadata");
|
||||
};
|
||||
let has_tarball = dist
|
||||
.tarball
|
||||
.as_deref()
|
||||
.is_some_and(|tarball| !tarball.is_empty());
|
||||
if !has_tarball {
|
||||
anyhow::bail!("npm package version {version} is missing dist.tarball");
|
||||
}
|
||||
let has_integrity = dist
|
||||
.integrity
|
||||
.as_ref()
|
||||
.is_some_and(|integrity| !integrity.is_empty());
|
||||
if !has_integrity {
|
||||
anyhow::bail!("npm package version {version} is missing dist.integrity");
|
||||
}
|
||||
Ok(info)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn version_json(version: &str) -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
"dist": {
|
||||
"integrity": format!("sha512-{version}"),
|
||||
"tarball": format!("https://registry.npmjs.org/@openai/codex/-/codex-{version}.tgz"),
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn package_info(github_latest: &str, npm_latest: &str) -> NpmPackageInfo {
|
||||
let mut versions = serde_json::Map::new();
|
||||
versions.insert(github_latest.to_string(), version_json(github_latest));
|
||||
|
||||
serde_json::from_value(serde_json::json!({
|
||||
"dist-tags": { "latest": npm_latest },
|
||||
"versions": serde_json::Value::Object(versions),
|
||||
}))
|
||||
.expect("valid npm package metadata")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ready_version_requires_latest_dist_tag_and_root_dist() {
|
||||
let latest = "1.2.3";
|
||||
let package_info = package_info(latest, latest);
|
||||
|
||||
ensure_version_ready(&package_info, latest).expect("npm package is ready");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ready_version_rejects_stale_latest_dist_tag() {
|
||||
let package_info = package_info("1.2.3", "1.2.2");
|
||||
|
||||
let err = ensure_version_ready(&package_info, "1.2.3")
|
||||
.expect_err("npm latest dist-tag must match GitHub latest");
|
||||
assert!(
|
||||
err.to_string().contains("latest dist-tag"),
|
||||
"error should name stale latest dist-tag: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ready_version_rejects_missing_root_dist() {
|
||||
let package_info: NpmPackageInfo = serde_json::from_value(serde_json::json!({
|
||||
"dist-tags": { "latest": "1.2.3" },
|
||||
"versions": { "1.2.3": {} },
|
||||
}))
|
||||
.expect("valid npm package metadata");
|
||||
|
||||
let err = ensure_version_ready(&package_info, "1.2.3")
|
||||
.expect_err("root package must have dist metadata");
|
||||
assert!(
|
||||
err.to_string().contains("missing dist metadata"),
|
||||
"error should name missing dist metadata: {err}"
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,70 @@
|
||||
pub(crate) fn is_newer(latest: &str, current: &str) -> Option<bool> {
|
||||
match (parse_version(latest), parse_version(current)) {
|
||||
(Some(l), Some(c)) => Some(l > c),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn extract_version_from_latest_tag(latest_tag_name: &str) -> anyhow::Result<String> {
|
||||
latest_tag_name
|
||||
.strip_prefix("rust-v")
|
||||
.map(str::to_owned)
|
||||
.ok_or_else(|| anyhow::anyhow!("Failed to parse latest tag name '{latest_tag_name}'"))
|
||||
}
|
||||
|
||||
pub(crate) fn is_source_build_version(version: &str) -> bool {
|
||||
parse_version(version) == Some((0, 0, 0))
|
||||
}
|
||||
|
||||
fn parse_version(v: &str) -> Option<(u64, u64, u64)> {
|
||||
let mut iter = v.trim().split('.');
|
||||
let maj = iter.next()?.parse::<u64>().ok()?;
|
||||
let min = iter.next()?.parse::<u64>().ok()?;
|
||||
let pat = iter.next()?.parse::<u64>().ok()?;
|
||||
Some((maj, min, pat))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn extracts_version_from_latest_tag() {
|
||||
assert_eq!(
|
||||
extract_version_from_latest_tag("rust-v1.5.0").expect("failed to parse version"),
|
||||
"1.5.0"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn latest_tag_without_prefix_is_invalid() {
|
||||
assert!(extract_version_from_latest_tag("v1.5.0").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prerelease_version_is_not_considered_newer() {
|
||||
assert_eq!(is_newer("0.11.0-beta.1", "0.11.0"), None);
|
||||
assert_eq!(is_newer("1.0.0-rc.1", "1.0.0"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plain_semver_comparisons_work() {
|
||||
assert_eq!(is_newer("0.11.1", "0.11.0"), Some(true));
|
||||
assert_eq!(is_newer("0.11.0", "0.11.1"), Some(false));
|
||||
assert_eq!(is_newer("1.0.0", "0.9.9"), Some(true));
|
||||
assert_eq!(is_newer("0.9.9", "1.0.0"), Some(false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn source_build_version_is_not_checked() {
|
||||
assert!(is_source_build_version("0.0.0"));
|
||||
assert!(!is_source_build_version("0.1.0"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn whitespace_is_ignored() {
|
||||
assert_eq!(parse_version(" 1.2.3 \n"), Some((1, 2, 3)));
|
||||
assert_eq!(is_newer(" 1.2.3 ", "1.2.2"), Some(true));
|
||||
}
|
||||
}
|
||||
+30
-87
@@ -1,8 +1,13 @@
|
||||
#![cfg(not(debug_assertions))]
|
||||
|
||||
use crate::legacy_core::config::Config;
|
||||
use crate::npm_registry;
|
||||
use crate::npm_registry::NpmPackageInfo;
|
||||
use crate::update_action;
|
||||
use crate::update_action::UpdateAction;
|
||||
use crate::update_versions::extract_version_from_latest_tag;
|
||||
use crate::update_versions::is_newer;
|
||||
use crate::update_versions::is_source_build_version;
|
||||
use chrono::DateTime;
|
||||
use chrono::Duration;
|
||||
use chrono::Utc;
|
||||
@@ -19,6 +24,7 @@ pub fn get_upgrade_version(config: &Config) -> Option<String> {
|
||||
return None;
|
||||
}
|
||||
|
||||
let action = update_action::get_update_action();
|
||||
let version_file = version_filepath(config);
|
||||
let info = read_version_info(&version_file).ok();
|
||||
|
||||
@@ -30,7 +36,7 @@ pub fn get_upgrade_version(config: &Config) -> Option<String> {
|
||||
// isn’t blocked by a network call. The UI reads the previously cached
|
||||
// value (if any) for this run; the next run shows the banner if needed.
|
||||
tokio::spawn(async move {
|
||||
check_for_update(&version_file)
|
||||
check_for_update(&version_file, action)
|
||||
.await
|
||||
.inspect_err(|e| tracing::error!("Failed to update version: {e}"))
|
||||
});
|
||||
@@ -78,8 +84,8 @@ fn read_version_info(version_file: &Path) -> anyhow::Result<VersionInfo> {
|
||||
Ok(serde_json::from_str(&contents)?)
|
||||
}
|
||||
|
||||
async fn check_for_update(version_file: &Path) -> anyhow::Result<()> {
|
||||
let latest_version = match update_action::get_update_action() {
|
||||
async fn check_for_update(version_file: &Path, action: Option<UpdateAction>) -> anyhow::Result<()> {
|
||||
let latest_version = match action {
|
||||
Some(UpdateAction::BrewUpgrade) => {
|
||||
let HomebrewCaskInfo { version } = create_client()
|
||||
.get(HOMEBREW_CASK_API_URL)
|
||||
@@ -90,17 +96,20 @@ async fn check_for_update(version_file: &Path) -> anyhow::Result<()> {
|
||||
.await?;
|
||||
version
|
||||
}
|
||||
_ => {
|
||||
let ReleaseInfo {
|
||||
tag_name: latest_tag_name,
|
||||
} = create_client()
|
||||
.get(LATEST_RELEASE_URL)
|
||||
Some(UpdateAction::NpmGlobalLatest) | Some(UpdateAction::BunGlobalLatest) => {
|
||||
let latest_version = fetch_latest_github_release_version().await?;
|
||||
let package_info = create_client()
|
||||
.get(npm_registry::PACKAGE_URL)
|
||||
.send()
|
||||
.await?
|
||||
.error_for_status()?
|
||||
.json::<ReleaseInfo>()
|
||||
.json::<NpmPackageInfo>()
|
||||
.await?;
|
||||
extract_version_from_latest_tag(&latest_tag_name)?
|
||||
npm_registry::ensure_version_ready(&package_info, &latest_version)?;
|
||||
latest_version
|
||||
}
|
||||
Some(UpdateAction::StandaloneUnix) | Some(UpdateAction::StandaloneWindows) | None => {
|
||||
fetch_latest_github_release_version().await?
|
||||
}
|
||||
};
|
||||
|
||||
@@ -120,18 +129,17 @@ async fn check_for_update(version_file: &Path) -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn is_newer(latest: &str, current: &str) -> Option<bool> {
|
||||
match (parse_version(latest), parse_version(current)) {
|
||||
(Some(l), Some(c)) => Some(l > c),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn extract_version_from_latest_tag(latest_tag_name: &str) -> anyhow::Result<String> {
|
||||
latest_tag_name
|
||||
.strip_prefix("rust-v")
|
||||
.map(str::to_owned)
|
||||
.ok_or_else(|| anyhow::anyhow!("Failed to parse latest tag name '{latest_tag_name}'"))
|
||||
async fn fetch_latest_github_release_version() -> anyhow::Result<String> {
|
||||
let ReleaseInfo {
|
||||
tag_name: latest_tag_name,
|
||||
} = create_client()
|
||||
.get(LATEST_RELEASE_URL)
|
||||
.send()
|
||||
.await?
|
||||
.error_for_status()?
|
||||
.json::<ReleaseInfo>()
|
||||
.await?;
|
||||
extract_version_from_latest_tag(&latest_tag_name)
|
||||
}
|
||||
|
||||
/// Returns the latest version to show in a popup, if it should be shown.
|
||||
@@ -168,68 +176,3 @@ pub async fn dismiss_version(config: &Config, version: &str) -> anyhow::Result<(
|
||||
tokio::fs::write(version_file, json_line).await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn parse_version(v: &str) -> Option<(u64, u64, u64)> {
|
||||
let mut iter = v.trim().split('.');
|
||||
let maj = iter.next()?.parse::<u64>().ok()?;
|
||||
let min = iter.next()?.parse::<u64>().ok()?;
|
||||
let pat = iter.next()?.parse::<u64>().ok()?;
|
||||
Some((maj, min, pat))
|
||||
}
|
||||
|
||||
fn is_source_build_version(version: &str) -> bool {
|
||||
parse_version(version) == Some((0, 0, 0))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn extract_version_from_brew_api_json() {
|
||||
//
|
||||
// https://formulae.brew.sh/api/cask/codex.json
|
||||
let cask_json = r#"{
|
||||
"token": "codex",
|
||||
"full_token": "codex",
|
||||
"tap": "homebrew/cask",
|
||||
"version": "0.96.0",
|
||||
}"#;
|
||||
let HomebrewCaskInfo { version } = serde_json::from_str::<HomebrewCaskInfo>(cask_json)
|
||||
.expect("failed to parse version from cask json");
|
||||
assert_eq!(version, "0.96.0");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extracts_version_from_latest_tag() {
|
||||
assert_eq!(
|
||||
extract_version_from_latest_tag("rust-v1.5.0").expect("failed to parse version"),
|
||||
"1.5.0"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn latest_tag_without_prefix_is_invalid() {
|
||||
assert!(extract_version_from_latest_tag("v1.5.0").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prerelease_version_is_not_considered_newer() {
|
||||
assert_eq!(is_newer("0.11.0-beta.1", "0.11.0"), None);
|
||||
assert_eq!(is_newer("1.0.0-rc.1", "1.0.0"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plain_semver_comparisons_work() {
|
||||
assert_eq!(is_newer("0.11.1", "0.11.0"), Some(true));
|
||||
assert_eq!(is_newer("0.11.0", "0.11.1"), Some(false));
|
||||
assert_eq!(is_newer("1.0.0", "0.9.9"), Some(true));
|
||||
assert_eq!(is_newer("0.9.9", "1.0.0"), Some(false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn whitespace_is_ignored() {
|
||||
assert_eq!(parse_version(" 1.2.3 \n"), Some((1, 2, 3)));
|
||||
assert_eq!(is_newer(" 1.2.3 ", "1.2.2"), Some(true));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user