diff --git a/Cargo.lock b/Cargo.lock index c86cd2578e..bfd39f32e8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8494,6 +8494,7 @@ dependencies = [ "node-semver", "owo-colors", "oxc_resolver", + "same-file", "serde", "serde_json", "serial_test", diff --git a/Cargo.toml b/Cargo.toml index d94e4453ba..f152818f44 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -251,6 +251,7 @@ rolldown-notify = "10.2.0" rolldown-notify-debouncer-full = "0.7.5" rustc-hash = "2.1.1" rustls = { version = "0.23", default-features = false, features = ["ring", "std", "tls12"] } +same-file = "1.0.6" schemars = "1.0.0" self_cell = "1.2.0" node-semver = "2.2.0" diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_check/mock-manifest.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_check/mock-manifest.json new file mode 100644 index 0000000000..aec06b40b2 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_check/mock-manifest.json @@ -0,0 +1,9 @@ +{ + "vite-plus/latest": { + "version": "999.0.0", + "dist": { + "tarball": "unused", + "integrity": "sha512-unused" + } + } +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_check/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_check/snapshots.toml index d36e269d3a..493fa12dca 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_check/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_check/snapshots.toml @@ -5,3 +5,18 @@ skip-platforms = ["windows", { os = "linux", libc = "musl" }] steps = [ { argv = ["vp", "upgrade", "--check", "--tag", "alpha"], comment = "alpha tag avoids release-day flake (dev version equals npm latest right after a release, hiding the Update-available branch)", continue-on-failure = true }, ] + +[[case]] +name = "command_upgrade_background_notice" +vp = "global" +local-registry = true +unset-env = ["VP_CLI_TEST"] +comment = "A foreground command launches a detached update check without waiting for it, then later commands show the cached notice at most once per prompt interval." +steps = [ + { argv = ["vp", "env", "list"], comment = "The foreground command launches the detached checker and returns without waiting for registry I/O.", snapshot = false }, + { argv = ["node", "-e", "(async()=>{const fs=require('node:fs');const path=require('node:path');const file=path.join(process.env.VP_HOME,'cache','upgrade-check.json');const deadline=Date.now()+5000;for(;;){try{if(JSON.parse(fs.readFileSync(file,'utf8')).status==='available')return}catch{}if(Date.now()>=deadline)process.exit(1);await new Promise(resolve=>setTimeout(resolve,25))}})()"], snapshot = false }, + { argv = ["vpt", "grep-file", "$VP_HOME/cache/upgrade-check.json", '"status":"available"'], snapshot = false }, + { argv = ["vp", "env", "list", "--json"], comment = "Machine-readable output does not consume the pending notice.", snapshot = false }, + { argv = ["vp", "env", "off"], comment = "The next interactive command displays the cached update notice." }, + { argv = ["vp", "env", "off"], comment = "A subsequent command stays quiet after the notice timestamp is recorded." }, +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_check/snapshots/command_upgrade_background_notice.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_check/snapshots/command_upgrade_background_notice.md new file mode 100644 index 0000000000..5a9aeb8657 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_check/snapshots/command_upgrade_background_notice.md @@ -0,0 +1,46 @@ +# command_upgrade_background_notice + +A foreground command launches a detached update check without waiting for it, then later commands show the cached notice at most once per prompt interval. + +## `vp env list` + +The foreground command launches the detached checker and returns without waiting for registry I/O. + + +## `node -e '(async()=>{const fs=require('\''node:fs'\'');const path=require('\''node:path'\'');const file=path.join(process.env.VP_HOME,'\''cache'\'','\''upgrade-check.json'\'');const deadline=Date.now()+5000;for(;;){try{if(JSON.parse(fs.readFileSync(file,'\''utf8'\'')).status==='\''available'\'')return}catch{}if(Date.now()>=deadline)process.exit(1);await new Promise(resolve=>setTimeout(resolve,25))}})()'` + + +## `vpt grep-file $VP_HOME/cache/upgrade-check.json '"status":"available"'` + + +## `vp env list --json` + +Machine-readable output does not consume the pending notice. + + +## `vp env off` + +The next interactive command displays the cached update notice. + +``` +VITE+ - The Unified Toolchain for the Web + +✓ Node.js management set to system-first. + +All vp commands and shims will now prefer system Node.js, falling back to managed if not found. + +Run `vp env on` to always use Vite+ managed Node.js. + +A new version of vp is available. Run `vp upgrade` to update. +``` + +## `vp env off` + +A subsequent command stays quiet after the notice timestamp is recorded. + +``` +VITE+ - The Unified Toolchain for the Web + +Node.js management is already set to system-first. +All vp commands and shims will prefer system Node.js, falling back to managed if not found. +``` diff --git a/crates/vp_global_cli/Cargo.toml b/crates/vp_global_cli/Cargo.toml index 94dff268b9..e4a9c8acad 100644 --- a/crates/vp_global_cli/Cargo.toml +++ b/crates/vp_global_cli/Cargo.toml @@ -24,9 +24,11 @@ serde_json = { workspace = true } node-semver = { workspace = true } thiserror = { workspace = true } tar = { workspace = true } +tempfile = { workspace = true } tokio = { workspace = true, features = ["full"] } tracing = { workspace = true } owo-colors = { workspace = true } +same-file = { workspace = true } oxc_resolver = { workspace = true } crossterm = { workspace = true } indexmap = { workspace = true } @@ -45,7 +47,6 @@ uuid = { workspace = true, features = ["v4"] } [dev-dependencies] serial_test = { workspace = true } -tempfile = { workspace = true } [lints] workspace = true diff --git a/crates/vp_global_cli/src/cli.rs b/crates/vp_global_cli/src/cli.rs index 5337894919..ccd0b03481 100644 --- a/crates/vp_global_cli/src/cli.rs +++ b/crates/vp_global_cli/src/cli.rs @@ -252,6 +252,10 @@ pub enum Commands { /// Custom npm registry URL #[arg(long)] registry: Option, + + /// Refresh the cached update status without producing output + #[arg(long, hide = true)] + background_check: bool, }, /// Remove vp and all related data @@ -1093,7 +1097,16 @@ pub async fn run_command_with_options( Commands::Env(args) => commands::env::execute(cwd, args).await, // Self-Management - Commands::Upgrade { version, tag, check, rollback, force, silent, registry } => { + Commands::Upgrade { + version, + tag, + check, + rollback, + force, + silent, + registry, + background_check, + } => { commands::upgrade::execute(commands::upgrade::UpgradeOptions { version, tag, @@ -1102,6 +1115,7 @@ pub async fn run_command_with_options( force, silent, registry, + background_check, }) .await } diff --git a/crates/vp_global_cli/src/commands/upgrade/mod.rs b/crates/vp_global_cli/src/commands/upgrade/mod.rs index c853e84881..dee70e07f1 100644 --- a/crates/vp_global_cli/src/commands/upgrade/mod.rs +++ b/crates/vp_global_cli/src/commands/upgrade/mod.rs @@ -29,11 +29,18 @@ pub struct UpgradeOptions { pub silent: bool, /// Custom npm registry URL pub registry: Option, + /// Refresh cached update status in a background helper + pub background_check: bool, } /// Execute the upgrade command. #[allow(clippy::print_stdout, clippy::print_stderr)] pub async fn execute(options: UpgradeOptions) -> Result { + if options.background_check { + crate::upgrade_check::run_background_check().await; + return Ok(ExitStatus::default()); + } + let install_dir = get_vp_home()?; // Handle --rollback diff --git a/crates/vp_global_cli/src/main.rs b/crates/vp_global_cli/src/main.rs index 14deffa4e1..861793d203 100644 --- a/crates/vp_global_cli/src/main.rs +++ b/crates/vp_global_cli/src/main.rs @@ -436,13 +436,11 @@ async fn main() -> ExitCode { // Parse CLI arguments (using custom help formatting) let parse_result = try_parse_args_from(normalized_args); - // Spawn background upgrade check for eligible commands - let upgrade_handle = match &parse_result { - Ok(args) if upgrade_check::should_run_for_command(args) => { - Some(tokio::spawn(upgrade_check::check_for_update())) - } - _ => None, - }; + let should_run_upgrade_check = + parse_result.as_ref().is_ok_and(upgrade_check::should_run_for_command); + if should_run_upgrade_check { + upgrade_check::spawn_background_check_if_needed(); + } let exit_code = match parse_result { Err(e) => { @@ -510,12 +508,8 @@ async fn main() -> ExitCode { }, }; - // Display upgrade notice if a newer version is available - if let Some(handle) = upgrade_handle - && let Ok(Ok(Some(result))) = - tokio::time::timeout(std::time::Duration::from_millis(500), handle).await - { - upgrade_check::display_upgrade_notice(&result); + if should_run_upgrade_check { + upgrade_check::display_cached_upgrade_notice(); } exit_code diff --git a/crates/vp_global_cli/src/upgrade_check.rs b/crates/vp_global_cli/src/upgrade_check.rs index 6cd8826d18..beaa06030d 100644 --- a/crates/vp_global_cli/src/upgrade_check.rs +++ b/crates/vp_global_cli/src/upgrade_check.rs @@ -1,68 +1,172 @@ -//! Background upgrade check for the vp CLI. +//! Background upgrade check state for the vp CLI. //! -//! Periodically queries the npm registry for the latest version and caches the -//! result to `~/.vite-plus/.upgrade-check.json`. Displays a one-line notice on -//! stderr when a newer version is available, at most once per 24 hours. +//! Eligible foreground commands launch `vp upgrade --background-check` as a +//! detached process when the cache is stale. That command records a retry +//! cooldown before touching the network, then queries the npm registry and +//! caches only whether an update is available. + +use std::{ + fs::{File, OpenOptions}, + io::Write, + process::{Command, Stdio}, + time::{SystemTime, UNIX_EPOCH}, +}; -use std::time::{SystemTime, UNIX_EPOCH}; - -use owo_colors::OwoColorize; use serde::{Deserialize, Serialize}; use vp_setup::registry; const CHECK_INTERVAL_SECS: u64 = 24 * 60 * 60; const PROMPT_INTERVAL_SECS: u64 = 24 * 60 * 60; -const CACHE_FILE_NAME: &str = ".upgrade-check.json"; +const CACHE_DIR_NAME: &str = "cache"; +const CACHE_FILE_NAME: &str = "upgrade-check.json"; +const LOCK_FILE_NAME: &str = "upgrade-check.lock"; +const UPGRADE_NOTICE: &str = "A new version of vp is available. Run `vp upgrade` to update."; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +enum UpgradeCheckStatus { + Unknown, + Current, + Available, +} #[expect(clippy::disallowed_types)] // String required for serde JSON round-trip #[derive(Debug, Clone, Serialize, Deserialize)] struct UpgradeCheckCache { - latest: String, + checked_for: String, + status: UpgradeCheckStatus, checked_at: u64, prompted_at: u64, } +impl UpgradeCheckCache { + fn needs_check(&self, current_version: &str, now: u64) -> bool { + self.checked_for != current_version + || now.saturating_sub(self.checked_at) > CHECK_INTERVAL_SECS + } + + fn notice_due(&self, current_version: &str, now: u64) -> bool { + self.checked_for == current_version + && self.status == UpgradeCheckStatus::Available + && now.saturating_sub(self.prompted_at) > PROMPT_INTERVAL_SECS + } +} + +struct UpgradeCheckLock { + _file: File, + cache_dir: vt_path::AbsolutePathBuf, + identity: same_file::Handle, +} + +impl UpgradeCheckLock { + fn is_current(&self) -> bool { + same_file::Handle::from_path(self.cache_dir.join(LOCK_FILE_NAME).as_path()) + .is_ok_and(|identity| identity == self.identity) + } + + fn write_cache(&self, cache: &UpgradeCheckCache) -> std::io::Result<()> { + if !self.is_current() { + return Err(std::io::ErrorKind::NotFound.into()); + } + + persist_cache(&self.cache_dir, cache) + } +} + +fn cache_dir(install_dir: &vt_path::AbsolutePath) -> vt_path::AbsolutePathBuf { + install_dir.join(CACHE_DIR_NAME) +} + +fn cache_path(install_dir: &vt_path::AbsolutePath) -> vt_path::AbsolutePathBuf { + cache_dir(install_dir).join(CACHE_FILE_NAME) +} + fn read_cache(install_dir: &vt_path::AbsolutePath) -> Option { - let cache_path = install_dir.join(CACHE_FILE_NAME); - let data = std::fs::read_to_string(cache_path.as_path()).ok()?; + let data = std::fs::read_to_string(cache_path(install_dir).as_path()).ok()?; serde_json::from_str(&data).ok() } -fn write_cache(install_dir: &vt_path::AbsolutePath, cache: &UpgradeCheckCache) { - let cache_path = install_dir.join(CACHE_FILE_NAME); - if let Ok(data) = serde_json::to_string(cache) { - let _ = std::fs::write(cache_path.as_path(), &data); +#[cfg(test)] +fn write_cache( + install_dir: &vt_path::AbsolutePath, + cache: &UpgradeCheckCache, +) -> std::io::Result<()> { + let cache_dir = cache_dir(install_dir); + std::fs::create_dir_all(cache_dir.as_path())?; + persist_cache(&cache_dir, cache) +} + +fn persist_cache( + cache_dir: &vt_path::AbsolutePath, + cache: &UpgradeCheckCache, +) -> std::io::Result<()> { + let cache_path = cache_dir.join(CACHE_FILE_NAME); + let data = serde_json::to_vec(cache).map_err(std::io::Error::other)?; + let mut temp = tempfile::NamedTempFile::new_in(cache_dir.as_path())?; + temp.write_all(&data)?; + temp.as_file().sync_all()?; + temp.persist(cache_path.as_path()).map_err(|error| error.error)?; + Ok(()) +} + +fn try_acquire_lock(install_dir: &vt_path::AbsolutePath) -> Option { + let cache_dir = cache_dir(install_dir); + if let Err(error) = std::fs::create_dir(cache_dir.as_path()) + && error.kind() != std::io::ErrorKind::AlreadyExists + { + return None; } + let path = cache_dir.join(LOCK_FILE_NAME); + let file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(path.as_path()) + .ok()?; + let identity = same_file::Handle::from_file(file.try_clone().ok()?).ok()?; + file.try_lock().ok()?; + Some(UpgradeCheckLock { _file: file, cache_dir, identity }) } fn now_secs() -> u64 { SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or_default().as_secs() } -fn should_check(cache: Option<&UpgradeCheckCache>, now: u64) -> bool { - if std::env::var_os("VP_NO_UPDATE_CHECK").is_some() - || std::env::var_os("CI").is_some() +fn checks_disabled() -> bool { + std::env::var_os("VP_NO_UPDATE_CHECK").is_some() + || vp_shared::EnvConfig::get().is_ci || std::env::var_os("VP_CLI_TEST").is_some() - { - return false; - } +} - cache.is_none_or(|c| now.saturating_sub(c.checked_at) > CHECK_INTERVAL_SECS) +fn should_check(cache: Option<&UpgradeCheckCache>, current_version: &str, now: u64) -> bool { + !checks_disabled() && cache.is_none_or(|cache| cache.needs_check(current_version, now)) } -fn should_prompt(cache: Option<&UpgradeCheckCache>, now: u64) -> bool { - cache.is_none_or(|c| now.saturating_sub(c.prompted_at) > PROMPT_INTERVAL_SECS) +fn read_due_notice( + install_dir: &vt_path::AbsolutePath, + current_version: &str, + now: u64, +) -> Option { + read_cache(install_dir).filter(|cache| cache.notice_due(current_version, now)) } /// Returns `true` if `latest` is strictly newer than `current` per semver. /// Returns `false` for equal versions, downgrades, or unparsable strings. +#[cfg(test)] fn is_newer_version(current: &str, latest: &str) -> bool { - if latest.is_empty() || current == "0.0.0" { - return false; + status_for_versions(current, latest) == UpgradeCheckStatus::Available +} + +fn status_for_versions(current: &str, latest: &str) -> UpgradeCheckStatus { + if current == "0.0.0" { + return UpgradeCheckStatus::Current; } + match (node_semver::Version::parse(current), node_semver::Version::parse(latest)) { - (Ok(current), Ok(latest)) => latest > current, - _ => false, + (Ok(current), Ok(latest)) if latest > current => UpgradeCheckStatus::Available, + (Ok(_), Ok(_)) => UpgradeCheckStatus::Current, + _ => UpgradeCheckStatus::Unknown, } } @@ -71,72 +175,132 @@ async fn resolve_version_string() -> Option { registry::resolve_version_string("latest", None).await.ok() } -pub struct UpgradeCheckResult { - install_dir: vt_path::AbsolutePathBuf, - cache: UpgradeCheckCache, -} - -/// Returns an upgrade check result if a newer version is available and the user -/// hasn't been prompted within the last 24 hours. Returns `None` otherwise. -pub async fn check_for_update() -> Option { - let install_dir = vp_shared::get_vp_home().ok()?; +pub(crate) fn spawn_background_check_if_needed() { + let Ok(install_dir) = vp_shared::get_vp_home() else { + return; + }; let current_version = env!("CARGO_PKG_VERSION"); - let now = now_secs(); - let mut cache = read_cache(&install_dir); + if !should_check(read_cache(&install_dir).as_ref(), current_version, now_secs()) { + return; + } - if should_check(cache.as_ref(), now) { - let prompted_at = cache.as_ref().map_or(0, |c| c.prompted_at); + let Ok(current_exe) = std::env::current_exe() else { + return; + }; + let mut command = Command::new(current_exe); + command + .args(["upgrade", "--background-check"]) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + configure_background_process(&mut command); - match resolve_version_string().await { - Some(latest) => { - let new_cache = UpgradeCheckCache { latest, checked_at: now, prompted_at }; - write_cache(&install_dir, &new_cache); - cache = Some(new_cache); - } - None => { - // Still update checked_at so we back off for 24h instead of - // retrying on every command when the registry is unreachable. - let latest = cache.as_ref().map(|c| c.latest.clone()).unwrap_or_default(); - let failed_cache = UpgradeCheckCache { latest, checked_at: now, prompted_at }; - write_cache(&install_dir, &failed_cache); - cache = Some(failed_cache); - } - } + if let Ok(mut child) = command.spawn() { + // A long-running foreground command must still reap a helper that exits first. + let _ = std::thread::spawn(move || child.wait()); } +} - let cache = cache?; +fn configure_background_process(command: &mut Command) { + #[cfg(unix)] + { + use std::os::unix::process::CommandExt as _; - if !is_newer_version(current_version, &cache.latest) { - return None; + command.process_group(0); } - if !should_prompt(Some(&cache), now) { - return None; + #[cfg(windows)] + { + use std::os::windows::process::CommandExt as _; + + const CREATE_NEW_PROCESS_GROUP: u32 = 0x0000_0200; + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + command.creation_flags(CREATE_NEW_PROCESS_GROUP | CREATE_NO_WINDOW); } - Some(UpgradeCheckResult { install_dir, cache }) + #[cfg(not(any(unix, windows)))] + let _ = command; } -/// Print a one-line upgrade notice to stderr and record the prompt time. +/// Refresh the cached update status. This function intentionally runs in the +/// helper process so the foreground command never waits for registry I/O. +pub async fn run_background_check() { + let Ok(install_dir) = vp_shared::get_vp_home() else { + return; + }; + let current_version = env!("CARGO_PKG_VERSION"); + let now = now_secs(); + + if !should_check(read_cache(&install_dir).as_ref(), current_version, now) { + return; + } + + let Some(lock) = try_acquire_lock(&install_dir) else { + return; + }; + + // Another process may have refreshed the cache before this process won the + // lock, so check again while holding it. + let cache = read_cache(&install_dir); + let now = now_secs(); + if !should_check(cache.as_ref(), current_version, now) { + return; + } + + let prompted_at = cache + .as_ref() + .filter(|cache| cache.checked_for == current_version) + .map_or(0, |cache| cache.prompted_at); + let pending = UpgradeCheckCache { + checked_for: current_version.to_owned(), + status: UpgradeCheckStatus::Unknown, + checked_at: now, + prompted_at, + }; + + // Persist the cooldown before the first await. If the shell or OS ends this + // process during the request, subsequent commands still avoid a retry storm. + if lock.write_cache(&pending).is_err() { + return; + } + + let status = resolve_version_string().await.map_or(UpgradeCheckStatus::Unknown, |latest| { + status_for_versions(current_version, &latest) + }); + let completed = UpgradeCheckCache { status, checked_at: now_secs(), ..pending }; + let _ = lock.write_cache(&completed); +} + +/// Print a generic one-line upgrade notice from cache and record the prompt time. #[expect(clippy::print_stderr, clippy::disallowed_macros)] -pub fn display_upgrade_notice(result: &UpgradeCheckResult) { +pub fn display_cached_upgrade_notice() { + if checks_disabled() { + return; + } + + let Ok(install_dir) = vp_shared::get_vp_home() else { + return; + }; let current_version = env!("CARGO_PKG_VERSION"); - eprintln!( - "\n{} {} {} {}{} {}", - "vp update available:".bright_black(), - current_version.bright_black(), - "\u{2192}".bright_black(), - result.cache.latest.bright_green().bold(), - ", run".bright_black(), - "vp upgrade".bright_green().bold(), - ); - - let mut cache = result.cache.clone(); - cache.prompted_at = now_secs(); - write_cache(&result.install_dir, &cache); + let now = now_secs(); + if read_due_notice(&install_dir, current_version, now).is_none() { + return; + } + + let Some(lock) = try_acquire_lock(&install_dir) else { + return; + }; + let Some(mut cache) = read_due_notice(&install_dir, current_version, now) else { + return; + }; + + eprintln!("\n{UPGRADE_NOTICE}"); + + cache.prompted_at = now; + let _ = lock.write_cache(&cache); } -/// Whether the upgrade check should run for the given command args. +/// Whether a foreground command may run the upgrade check and display its cached notice. /// Returns `false` for commands excluded by design, quiet modes, and /// machine-readable output flags (--silent, -s, --json, --parseable, --format json). pub fn should_run_for_command(args: &crate::cli::Args) -> bool { @@ -162,7 +326,16 @@ pub fn should_run_for_command(args: &crate::cli::Args) -> bool { #[cfg(test)] mod tests { + use std::{ + sync::{ + Arc, + atomic::{AtomicUsize, Ordering}, + }, + time::Duration, + }; + use serial_test::serial; + use tokio::net::TcpListener; use super::*; @@ -171,16 +344,82 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let dir_path = vt_path::AbsolutePathBuf::new(dir.path().to_path_buf()).unwrap(); - let cache = - UpgradeCheckCache { latest: "1.2.3".to_owned(), checked_at: 1000, prompted_at: 900 }; - write_cache(&dir_path, &cache); + let cache = UpgradeCheckCache { + checked_for: "1.2.3".to_owned(), + status: UpgradeCheckStatus::Available, + checked_at: 1000, + prompted_at: 900, + }; + write_cache(&dir_path, &cache).unwrap(); let loaded = read_cache(&dir_path).expect("should read back cache"); - assert_eq!(loaded.latest, "1.2.3"); + let expected_path = dir_path.join("cache").join("upgrade-check.json"); + assert_eq!(cache_path(&dir_path), expected_path); + assert!(expected_path.as_path().exists()); + assert_eq!(loaded.checked_for, "1.2.3"); + assert_eq!(loaded.status, UpgradeCheckStatus::Available); assert_eq!(loaded.checked_at, 1000); assert_eq!(loaded.prompted_at, 900); } + #[test] + fn cache_write_atomically_replaces_existing_state() { + let dir = tempfile::tempdir().unwrap(); + let dir_path = vt_path::AbsolutePathBuf::new(dir.path().to_path_buf()).unwrap(); + let mut cache = UpgradeCheckCache { + checked_for: "1.2.3".to_owned(), + status: UpgradeCheckStatus::Unknown, + checked_at: 1000, + prompted_at: 0, + }; + write_cache(&dir_path, &cache).unwrap(); + + cache.status = UpgradeCheckStatus::Available; + cache.checked_at = 2000; + write_cache(&dir_path, &cache).unwrap(); + + let loaded = read_cache(&dir_path).unwrap(); + assert_eq!(loaded.status, UpgradeCheckStatus::Available); + assert_eq!(loaded.checked_at, 2000); + } + + #[test] + fn lock_is_exclusive_and_released_by_its_owner() { + let dir = tempfile::tempdir().unwrap(); + let dir_path = vt_path::AbsolutePathBuf::new(dir.path().to_path_buf()).unwrap(); + + let lock = try_acquire_lock(&dir_path).expect("first process should acquire the lock"); + assert!(try_acquire_lock(&dir_path).is_none(), "second process must not acquire the lock"); + drop(lock); + + try_acquire_lock(&dir_path).expect("lock should be reusable after owner exits"); + } + + #[cfg(unix)] + #[test] + fn locked_cache_write_does_not_recreate_a_moved_install() { + let dir = tempfile::tempdir().unwrap(); + let install_path = dir.path().join("vite-plus"); + std::fs::create_dir(&install_path).unwrap(); + let install_dir = vt_path::AbsolutePathBuf::new(install_path.clone()).unwrap(); + let lock = try_acquire_lock(&install_dir).expect("worker should acquire the lock"); + let moved_path = dir.path().join("vite-plus.removing"); + std::fs::rename(&install_path, &moved_path).unwrap(); + let cache = UpgradeCheckCache { + checked_for: "1.2.3".to_owned(), + status: UpgradeCheckStatus::Available, + checked_at: 1000, + prompted_at: 0, + }; + + assert!(!lock.is_current(), "moving the install should invalidate the worker"); + assert!( + lock.write_cache(&cache).is_err(), + "an invalidated worker must not write its result" + ); + assert!(!install_path.exists(), "the removed install path must stay absent"); + } + #[test] fn read_cache_returns_none_for_missing_file() { let dir = tempfile::tempdir().unwrap(); @@ -192,7 +431,8 @@ mod tests { fn read_cache_returns_none_for_corrupt_file() { let dir = tempfile::tempdir().unwrap(); let dir_path = vt_path::AbsolutePathBuf::new(dir.path().to_path_buf()).unwrap(); - std::fs::write(dir_path.join(CACHE_FILE_NAME).as_path(), "not json").unwrap(); + std::fs::create_dir_all(cache_dir(&dir_path).as_path()).unwrap(); + std::fs::write(cache_path(&dir_path).as_path(), "not json").unwrap(); assert!(read_cache(&dir_path).is_none()); } @@ -211,12 +451,18 @@ mod tests { unsafe { if let Some(v) = ci { std::env::set_var("CI", v); + } else { + std::env::remove_var("CI"); } if let Some(v) = test { std::env::set_var("VP_CLI_TEST", v); + } else { + std::env::remove_var("VP_CLI_TEST"); } if let Some(v) = no_check { std::env::set_var("VP_NO_UPDATE_CHECK", v); + } else { + std::env::remove_var("VP_NO_UPDATE_CHECK"); } } } @@ -225,7 +471,7 @@ mod tests { #[serial] fn should_check_returns_true_when_no_cache() { with_env_vars_cleared(|| { - assert!(should_check(None, now_secs())); + assert!(should_check(None, "1.0.0", now_secs())); }); } @@ -234,9 +480,13 @@ mod tests { fn should_check_returns_false_when_cache_fresh() { with_env_vars_cleared(|| { let now = now_secs(); - let cache = - UpgradeCheckCache { latest: "1.0.0".to_owned(), checked_at: now, prompted_at: 0 }; - assert!(!should_check(Some(&cache), now)); + let cache = UpgradeCheckCache { + checked_for: "1.0.0".to_owned(), + status: UpgradeCheckStatus::Current, + checked_at: now, + prompted_at: 0, + }; + assert!(!should_check(Some(&cache), "1.0.0", now)); }); } @@ -247,11 +497,27 @@ mod tests { let now = now_secs(); let stale_time = now - CHECK_INTERVAL_SECS - 1; let cache = UpgradeCheckCache { - latest: "1.0.0".to_owned(), + checked_for: "1.0.0".to_owned(), + status: UpgradeCheckStatus::Current, checked_at: stale_time, prompted_at: 0, }; - assert!(should_check(Some(&cache), now)); + assert!(should_check(Some(&cache), "1.0.0", now)); + }); + } + + #[test] + #[serial] + fn should_check_returns_true_for_a_different_cli_version() { + with_env_vars_cleared(|| { + let now = now_secs(); + let cache = UpgradeCheckCache { + checked_for: "1.0.0".to_owned(), + status: UpgradeCheckStatus::Current, + checked_at: now, + prompted_at: 0, + }; + assert!(should_check(Some(&cache), "1.0.1", now)); }); } @@ -262,40 +528,44 @@ mod tests { unsafe { std::env::set_var("VP_NO_UPDATE_CHECK", "1"); } - assert!(!should_check(None, now_secs())); + assert!(!should_check(None, "1.0.0", now_secs())); }); } #[test] - fn should_prompt_returns_true_when_no_cache() { - assert!(should_prompt(None, now_secs())); - } - - #[test] - fn should_prompt_returns_true_when_never_prompted() { + fn notice_is_due_when_never_prompted() { let cache = UpgradeCheckCache { - latest: "2.0.0".to_owned(), + checked_for: "1.0.0".to_owned(), + status: UpgradeCheckStatus::Available, checked_at: now_secs(), prompted_at: 0, }; - assert!(should_prompt(Some(&cache), now_secs())); + assert!(cache.notice_due("1.0.0", now_secs())); } #[test] - fn should_prompt_returns_false_when_recently_prompted() { + fn notice_is_not_due_when_recently_prompted() { let now = now_secs(); - let cache = - UpgradeCheckCache { latest: "2.0.0".to_owned(), checked_at: now, prompted_at: now }; - assert!(!should_prompt(Some(&cache), now)); + let cache = UpgradeCheckCache { + checked_for: "1.0.0".to_owned(), + status: UpgradeCheckStatus::Available, + checked_at: now, + prompted_at: now, + }; + assert!(!cache.notice_due("1.0.0", now)); } #[test] - fn should_prompt_returns_true_when_prompt_stale() { + fn notice_is_due_when_prompt_stale() { let now = now_secs(); let stale = now - PROMPT_INTERVAL_SECS - 1; - let cache = - UpgradeCheckCache { latest: "2.0.0".to_owned(), checked_at: now, prompted_at: stale }; - assert!(should_prompt(Some(&cache), now)); + let cache = UpgradeCheckCache { + checked_for: "1.0.0".to_owned(), + status: UpgradeCheckStatus::Available, + checked_at: now, + prompted_at: stale, + }; + assert!(cache.notice_due("1.0.0", now)); } #[test] @@ -408,4 +678,51 @@ mod tests { fn should_run_for_outdated_without_format() { assert!(should_run_for_command(&parse_args(&["outdated"]))); } + + #[tokio::test(flavor = "current_thread")] + #[serial] + async fn concurrent_slow_checks_start_one_request_and_back_off_before_it_finishes() { + let home = tempfile::tempdir().unwrap(); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let registry = format!("http://{}", listener.local_addr().unwrap()); + let request_count = Arc::new(AtomicUsize::new(0)); + let server_request_count = Arc::clone(&request_count); + let server = tokio::spawn(async move { + loop { + let (connection, _) = listener.accept().await.unwrap(); + server_request_count.fetch_add(1, Ordering::SeqCst); + tokio::spawn(async move { + let _connection = connection; + std::future::pending::<()>().await; + }); + } + }); + + let _env = vp_shared::EnvConfig::test_guard(vp_shared::EnvConfig { + vite_plus_home: Some(home.path().to_path_buf()), + npm_registry: registry, + ..vp_shared::EnvConfig::for_test() + }); + + let checks = (0..5).map(|_| tokio::spawn(run_background_check())).collect::>(); + + tokio::time::timeout(Duration::from_secs(2), async { + while request_count.load(Ordering::SeqCst) == 0 { + tokio::task::yield_now().await; + } + }) + .await + .expect("at least one registry request should start"); + tokio::time::sleep(Duration::from_millis(200)).await; + + let observed_requests = request_count.load(Ordering::SeqCst); + let cache_exists = home.path().join(CACHE_DIR_NAME).join(CACHE_FILE_NAME).exists(); + for check in checks { + check.abort(); + } + server.abort(); + + assert_eq!(observed_requests, 1, "concurrent checks must share one registry request"); + assert!(cache_exists, "the retry cooldown must be persisted before awaiting the registry"); + } } diff --git a/rfcs/upgrade-check.md b/rfcs/upgrade-check.md index 41893e6bde..b6a9363fad 100644 --- a/rfcs/upgrade-check.md +++ b/rfcs/upgrade-check.md @@ -38,7 +38,7 @@ The upgrade-command RFC explicitly listed "auto-update on every command invocati $ vp build ...build output... -vp update available: 0.1.0 → 0.2.0, run `vp upgrade` +A new version of vp is available. Run `vp upgrade` to update. ``` ### Story 2: Already Up to Date (no notice) @@ -82,43 +82,50 @@ The check fails silently. No notice, no error, no retry spam. ### Overview ``` -Command starts +Foreground `vp` starts │ - ├──────────────────────────────┐ - │ │ - ▼ ▼ - Run the actual command Spawn background task: - │ 1. Check if cache is fresh (<24h) - │ → Yes: read cached version - │ → No: query npm registry, - │ write result to cache file - │ │ - ▼ ▼ - Command finishes Background task finishes - │ │ - ▼ ▼ - If newer version found, print one-line notice - Show tip (existing behavior) - Exit + ├── parse the requested command and read the local cache + │ │ + │ ├── ineligible command or fresh cache → spawn nothing + │ └── stale/missing cache → launch detached + │ `vp upgrade --background-check` + │ │ + │ ├── fresh cache or lock held → exit silently + │ └── acquire OS file lock → atomically write + │ `unknown` cooldown → query registry + │ → atomically write final status + │ + └── run the requested command without waiting for registry I/O + │ + └── after completion, read cache and optionally print notice ``` -The background task runs concurrently with the command. When the command finishes, we check if the background task has a result (with a very short timeout — if it hasn't finished, skip the notice this time). +The foreground process performs only argument and cache checks before launching the helper, avoiding a new process while the cache is fresh. The helper is placed in a separate process group with standard streams disconnected, so it can finish after the foreground command and its shell prompt return. If the check is still running when the command finishes, the notice can appear after a later command. ### Cache File -Location: `~/.vite-plus/.upgrade-check.json` +Locations: + +- Cache: `~/.vite-plus/cache/upgrade-check.json` +- Cross-process lock: `~/.vite-plus/cache/upgrade-check.lock` Format (single JSON line for simplicity): ```json -{ "latest": "0.2.0", "checked_at": 1711500000, "prompted_at": 1711500000 } +{ + "checked_for": "0.1.0", + "status": "available", + "checked_at": 1711500000, + "prompted_at": 1711500000 +} ``` -- `latest`: The version string returned by the npm registry for the `latest` dist-tag -- `checked_at`: Unix timestamp (seconds) of when the registry was last queried +- `checked_for`: The installed `vp` version this result applies to +- `status`: `available`, `current`, or `unknown`; the target version is not persisted +- `checked_at`: Unix timestamp (seconds) of when the latest check attempt began or completed - `prompted_at`: Unix timestamp (seconds) of when the user was last shown the notice -The file is small and cheap to read. A direct overwrite is sufficient — if corruption occurs (e.g., process killed mid-write), the worst case is one extra registry query. +Cache writes use a temporary file plus atomic replacement. An OS file lock serializes workers and prompt timestamp updates, releases automatically when a process exits, and stores a generation token so a worker cannot write into an install that was removed or replaced while its request was in flight. The worker writes an `unknown` result with a fresh `checked_at` before its first network await, so cancellation, offline registries, and abrupt shell exit cannot cause a request on every invocation. ### Check Logic (Pseudocode) @@ -134,13 +141,13 @@ This means: the registry is queried at most once per day, and even if an update The upgrade notice is printed to **stderr** (like tips), after the command output and before the tip line: ``` -vp update available: 0.1.0 → 0.2.0, run `vp upgrade` +A new version of vp is available. Run `vp upgrade` to update. ``` Styling: - Single line, no indentation -- Dimmed text with version numbers highlighted (current in dim, new in green bold) and `vp upgrade` highlighted +- Does not reveal either the installed or target version The notice is printed **after** the command output and **before** any tip, so it feels like a natural postscript rather than an interruption. @@ -159,9 +166,11 @@ The notice is **not shown** when: | Stderr is not a TTY | Non-interactive / piped / redirected output | | Already prompted within 24h | Show at most once per day, not on every run | -### Commands That Trigger the Check +### Check Triggers and Foreground Suppression -The background check runs on **all** commands except: +After parsing an eligible foreground command, `vp` checks opt-out and CI state plus the local cache. It launches a detached worker only when that cache is stale or missing. The worker repeats the cache check after startup, then uses the cross-process lock and another cache check to coordinate concurrent invocations. Its standard streams are discarded, and foreground commands never wait for its registry request. + +The cached notice is not displayed after: - `vp upgrade` (already handles version checking) - `vp implode` (removing the tool) @@ -170,45 +179,33 @@ The background check runs on **all** commands except: - Any command with quiet/machine-readable flags (`--silent`, `-s`, `--json`, `--parseable`, `--format json/list`) - Shim invocations (`node`, `npm`, `npx` via vp) -This keeps the check broadly useful without interfering with special commands. +Shim invocations do not pass through the foreground notice path. ### File Structure ``` crates/vp_global_cli/src/ ├── upgrade_check.rs # New: cache read/write, background check, display -├── main.rs # Modified: spawn check, display result after command +├── main.rs # Modified: conditionally launch helper and display cached result +└── cli.rs # Modified: hidden background-check option ``` No new crate — this is a small, focused module in the existing `vp_global_cli` crate. It imports `resolve_version` from the existing `commands/upgrade/registry.rs`. ### Implementation Details -#### Async Background Check +#### Background Check Command ```rust -// In main.rs, before running the command: -let update_handle = if should_run_for_command(&args, &raw_args) { - Some(tokio::spawn(check_for_update())) -} else { - None -}; - -// After command completes: -if let Some(handle) = update_handle { - // Wait up to 500ms for the result — if the network is slow, skip it - match tokio::time::timeout(Duration::from_millis(500), handle).await { - Ok(Ok(Some(result))) => { - display_upgrade_notice(&result); // also records prompted_at - } - _ => {} // Timeout, error, or no update — silent - } +if options.background_check { + run_background_check().await; + return Ok(ExitStatus::default()); } ``` -The 500ms timeout ensures that even if the registry is slow, the user's command exits promptly. In practice, most checks will read from cache (instant) or complete the network request during the time the actual command runs. +`--background-check` is hidden because it is an implementation detail of the foreground launcher. The foreground `vp` process configures and spawns this command as a detached child before running the requested command. The hidden command repeats the cheap policy and cache checks to close races between concurrent foreground invocations. -`display_upgrade_notice` updates `prompted_at` in the cache file after showing the notice, so subsequent runs within 24h are silent. +Foreground commands call `display_cached_upgrade_notice` after completing. This path performs no network work and only acquires the lock when an available, unprompted cached result exists. ## Design Decisions @@ -223,16 +220,18 @@ The 500ms timeout ensures that even if the registry is slow, the user's command **Rationale**: Deterministic behavior, no surprises. The cache file is tiny and cheap to read. 24 hours is long enough to not annoy, short enough to be useful. -### 2. Background Async (Not Post-Command Blocking) +### 2. Detached Background Process (Not an In-Process Task) -**Decision**: Spawn the registry query concurrently with the command. +**Decision**: Let eligible foreground `vp` commands launch the hidden Rust check command as a detached process, but only after determining that the cache is stale. **Alternatives considered**: - Check after the command finishes — adds visible latency +- Let shell integrations launch a worker — excludes shells without an integration and starts unnecessary processes while the cache is fresh +- Spawn a Tokio task inside the foreground CLI — its runtime must wait or cancel the request when the CLI exits - Separate background daemon — heavyweight, harder to manage -**Rationale**: The registry query runs in parallel with the actual command. By the time the command finishes, the check is usually done. The 500ms timeout is a safety net for slow networks. +**Rationale**: The foreground process can avoid almost all helper launches with a cheap cache read, while the detached process has no registry-request latency tail. Keeping the launcher in `vp` also provides consistent behavior across shells without maintaining a daemon. ### 3. Stderr for the Notice @@ -261,7 +260,8 @@ The 500ms timeout ensures that even if the registry is slow, the user's command ### Unit Tests -- Cache read/write: valid JSON, corrupt file, missing file +- Cache read/write: valid JSON, atomic replacement, corrupt/missing files +- OS file-lock exclusivity, automatic release, and install-generation invalidation - `should_check`: respects env vars, cache freshness, TTY detection - Version comparison: same version, different version, pre-release @@ -270,18 +270,19 @@ The 500ms timeout ensures that even if the registry is slow, the user's command - Mock registry server returning a version, verify notice is displayed - Verify no notice when cache is fresh - Verify no notice in CI mode -- Verify timeout behavior (slow mock server) +- Start concurrent checks against a slow mock registry; verify exactly one request and that the cooldown is persisted before the response +- Verify an eligible foreground command launches a detached check only when the cache is stale ### Manual Testing ```bash # Clear cache to force a fresh check -rm ~/.vite-plus/.upgrade-check.json +rm ~/.vite-plus/cache/upgrade-check.json -# Run any command — should show notice if behind latest -vp --version +# Run an eligible foreground command to launch the check +vp build -# Run again immediately — should not re-query (cached) +# Run again after the background request completes — should not re-query (cached) vp build # Disable and verify