diff --git a/Cargo.lock b/Cargo.lock index dc62d2c6..1e7065c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2012,6 +2012,7 @@ dependencies = [ "base64", "crossterm", "ed25519-dalek", + "fs4", "gethostname", "nix", "opener", diff --git a/GLOSSARY.md b/GLOSSARY.md index 3a9b79fd..9213a614 100644 --- a/GLOSSARY.md +++ b/GLOSSARY.md @@ -456,7 +456,7 @@ The digest reports changes in watched Repositories. **Is:** replacing the skilld CLI executable with a newer release of the skilld CLI. -**Use for:** the upgrade notice, the standalone background upgrade, and `SKILLD_NO_UPGRADE`. +**Use for:** the upgrade banner, the upgrade prompt, executable replacement, and `SKILLD_NO_UPGRADE`. **Never:** update, self-update. `update` means a Skill update. diff --git a/README.md b/README.md index 7991d15c..5b9d02f3 100644 --- a/README.md +++ b/README.md @@ -166,9 +166,14 @@ The script installs one native binary to `~/.skilld/bin`. It adds that directory to PATH: in your shell profile on macOS and Linux, in your user PATH on Windows. On macOS and Linux, set `SKILLD_NO_MODIFY_PATH=1` to leave your shell profile alone. -The script install upgrades itself in the background after it verifies the signed release manifest. +skilld checks for CLI upgrades in the background once a day. +If a newer release is cached, the next terminal invocation offers to upgrade. +Choose `Upgrade now`, `Not now`, or `Don't remind me for this version`. +The script install verifies the signed release manifest before replacing the executable. Restart skilld to use the new version. -An npm install prints the upgrade command instead. +Recognized global npm, pnpm, Yarn, and Bun installs run their package manager after you confirm. +Local and transient installs show upgrade guidance instead. +After an upgrade, run your command again. Set `SKILLD_NO_UPGRADE=1` to turn off upgrade checks. See [SECURITY.md](./SECURITY.md#how-the-cli-upgrades-itself) for each check. diff --git a/SECURITY.md b/SECURITY.md index b5f34913..0af2df3a 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -32,7 +32,8 @@ The release manifest names the CLI version and the SHA-256 digest of each releas The public release key is compiled into the CLI. The private release key exists only as a GitHub Actions secret, in the one step that signs. -A standalone install upgrades itself in the background. Before it replaces the executable, the CLI checks: +A standalone install checks for releases in the background and asks before upgrading. +Before it replaces the executable, the CLI checks: - the release manifest signature against the compiled release key - that the signed version is the requested version and is newer than the running version @@ -45,7 +46,9 @@ Set `SKILLD_NO_UPGRADE=1` to turn off upgrade checks. `install.sh` checks the same signature when OpenSSL supports Ed25519. Otherwise, `install.sh` and `install.ps1` trust the HTTPS download and the SHA-256 digest for the first install. -npm installs never upgrade themselves. They print the upgrade command instead. +Recognized global package installations run their package manager only after you confirm the displayed command. +The command installs the exact version shown in the prompt. +Local and transient package installations show upgrade guidance instead. ## Account credentials diff --git a/crates/skilld-command/src/upgrade.rs b/crates/skilld-command/src/upgrade.rs index 864de8ce..a52e91da 100644 --- a/crates/skilld-command/src/upgrade.rs +++ b/crates/skilld-command/src/upgrade.rs @@ -7,8 +7,6 @@ use skilld_core::is_release_version; /// How long a latest-version check stays fresh. pub const CHECK_INTERVAL_SECONDS: u64 = 24 * 60 * 60; -/// How long a failed standalone upgrade waits before it retries the same version. -pub const RETRY_INTERVAL_SECONDS: u64 = 60 * 60; /// The largest release binary skilld downloads. pub const MAX_BINARY_BYTES: usize = 64 * 1024 * 1024; pub const RELEASE_MANIFEST_ASSET: &str = "skilld-release.txt"; @@ -49,16 +47,6 @@ impl PackageRunner { _ => None, } } - - const fn upgrade_command(self) -> &'static str { - match self { - Self::Npx => "npx skilld@latest", - Self::Npm => "npm install --global skilld", - Self::Pnpm => "pnpm add --global skilld", - Self::Yarn => "yarn global add skilld", - Self::Bun => "bun add --global skilld", - } - } } /// The upgrade state skilld keeps in its data directory. @@ -71,11 +59,7 @@ pub struct UpgradeState { #[serde(default)] pub latest: Option, #[serde(default)] - pub attempted_version: Option, - #[serde(default)] - pub attempted_at: Option, - /// The error code of the last failed check or upgrade, for diagnosis. - #[serde(default)] + /// The error code of the last failed check, for diagnosis. pub last_error: Option, } @@ -89,42 +73,11 @@ impl UpgradeState { #[derive(Clone, Debug, Eq, PartialEq)] pub enum UpgradeWorker { Check, - Install { version: String }, } #[derive(Clone, Debug, Eq, PartialEq)] -pub enum UpgradeNotice { - Available { - version: String, - command: String, - }, - Installing { - version: String, - last_error: Option, - }, -} - -impl UpgradeNotice { - pub fn message(&self) -> String { - match self { - Self::Available { version, command } => { - format!("skilld {version} is available. Run {command} to upgrade.") - } - Self::Installing { - version, - last_error: Some(code), - } => format!( - "The last upgrade attempt failed: {code}. \ - Retrying skilld {version} in the background. Restart skilld to use it." - ), - Self::Installing { - version, - last_error: None, - } => format!( - "Upgrading skilld to {version} in the background. Restart skilld to use it." - ), - } - } +pub struct UpgradeNotice { + pub version: String, } #[derive(Clone, Debug, Default, Eq, PartialEq)] @@ -155,38 +108,11 @@ pub fn plan_upgrade( else { return check; }; - match channel { - InstallChannel::Npm(runner) => UpgradePlan { - worker: check.worker, - notice: Some(UpgradeNotice::Available { - version: latest.to_owned(), - command: runner.upgrade_command().to_owned(), - }), - }, - InstallChannel::Standalone => { - let retry = state.attempted_version.as_deref() == Some(latest); - let waiting = retry - && state.attempted_at.is_some_and(|attempted| { - attempted <= now && now - attempted < RETRY_INTERVAL_SECONDS - }); - if waiting { - return check; - } - UpgradePlan { - worker: Some(UpgradeWorker::Install { - version: latest.to_owned(), - }), - notice: Some(UpgradeNotice::Installing { - version: latest.to_owned(), - last_error: if retry { - state.last_error.clone() - } else { - None - }, - }), - } - } - InstallChannel::Unmanaged => UpgradePlan::default(), + UpgradePlan { + worker: check.worker, + notice: Some(UpgradeNotice { + version: latest.to_owned(), + }), } } diff --git a/crates/skilld-command/tests/upgrade.rs b/crates/skilld-command/tests/upgrade.rs index d7184ac2..b68ea931 100644 --- a/crates/skilld-command/tests/upgrade.rs +++ b/crates/skilld-command/tests/upgrade.rs @@ -6,12 +6,23 @@ use skilld_command::upgrade::{ const DAY: u64 = 86_400; const NOW: u64 = 1_800_000_000; +#[test] +fn a_known_standalone_upgrade_never_installs_without_an_answer() { + let plan = plan_upgrade( + "3.0.0", + &InstallChannel::Standalone, + &state(NOW - 60, Some("3.1.0")), + NOW, + ); + + assert_eq!(plan.worker, None); + assert!(plan.notice.is_some()); +} + fn state(checked_at: u64, latest: Option<&str>) -> UpgradeState { UpgradeState { checked_at, latest: latest.map(str::to_owned), - attempted_version: None, - attempted_at: None, last_error: None, } } @@ -51,109 +62,27 @@ fn a_stale_check_refreshes_in_the_background_without_a_notice() { } #[test] -fn an_npm_install_names_the_upgrade_command_for_its_runner() { +fn a_cached_newer_release_offers_an_upgrade_for_every_managed_channel() { let fresh = state(NOW - 60, Some("3.1.0")); - for (runner, command) in [ - (PackageRunner::Npx, "npx skilld@latest"), - (PackageRunner::Npm, "npm install --global skilld"), - (PackageRunner::Pnpm, "pnpm add --global skilld"), - (PackageRunner::Yarn, "yarn global add skilld"), - (PackageRunner::Bun, "bun add --global skilld"), + for channel in [ + InstallChannel::Standalone, + InstallChannel::Npm(PackageRunner::Npx), + InstallChannel::Npm(PackageRunner::Npm), + InstallChannel::Npm(PackageRunner::Pnpm), + InstallChannel::Npm(PackageRunner::Yarn), + InstallChannel::Npm(PackageRunner::Bun), ] { - let plan = plan_upgrade("3.0.0", &InstallChannel::Npm(runner), &fresh, NOW); - + let plan = plan_upgrade("3.0.0", &channel, &fresh, NOW); assert_eq!(plan.worker, None); - let notice = plan.notice.expect("a newer version shows a notice"); - assert_eq!( - notice, - UpgradeNotice::Available { - version: "3.1.0".to_owned(), - command: command.to_owned(), - } - ); assert_eq!( - notice.message(), - format!("skilld 3.1.0 is available. Run {command} to upgrade.") + plan.notice, + Some(UpgradeNotice { + version: "3.1.0".to_owned() + }) ); } } -#[test] -fn a_standalone_install_upgrades_itself_and_asks_for_a_restart() { - let plan = plan_upgrade( - "3.0.0", - &InstallChannel::Standalone, - &state(NOW - 60, Some("3.1.0")), - NOW, - ); - - assert_eq!( - plan.worker, - Some(UpgradeWorker::Install { - version: "3.1.0".to_owned() - }) - ); - assert_eq!( - plan.notice.unwrap().message(), - "Upgrading skilld to 3.1.0 in the background. Restart skilld to use it." - ); -} - -#[test] -fn a_recent_failed_upgrade_waits_before_it_retries() { - let mut recent = state(NOW - 60, Some("3.1.0")); - recent.attempted_version = Some("3.1.0".to_owned()); - recent.attempted_at = Some(NOW - 60); - - assert_eq!( - plan_upgrade("3.0.0", &InstallChannel::Standalone, &recent, NOW), - UpgradePlan::default() - ); - - recent.attempted_at = Some(NOW - DAY); - assert!(matches!( - plan_upgrade("3.0.0", &InstallChannel::Standalone, &recent, NOW).worker, - Some(UpgradeWorker::Install { .. }) - )); -} - -#[test] -fn a_retry_after_a_failed_upgrade_names_the_failure_in_its_notice() { - let mut failed = state(NOW - 60, Some("3.1.0")); - failed.attempted_version = Some("3.1.0".to_owned()); - failed.attempted_at = Some(NOW - DAY); - failed.last_error = Some("UPGRADE_DOWNLOAD_FAILED".to_owned()); - - let plan = plan_upgrade("3.0.0", &InstallChannel::Standalone, &failed, NOW); - - assert_eq!( - plan.notice.map(|notice| notice.message()), - Some( - "The last upgrade attempt failed: UPGRADE_DOWNLOAD_FAILED. \ - Retrying skilld 3.1.0 in the background. Restart skilld to use it." - .to_owned() - ) - ); -} - -#[test] -fn a_first_install_ignores_a_failure_from_another_version() { - let mut failed = state(NOW - 60, Some("3.2.0")); - failed.attempted_version = Some("3.1.0".to_owned()); - failed.attempted_at = Some(NOW - DAY); - failed.last_error = Some("UPGRADE_DOWNLOAD_FAILED".to_owned()); - - let plan = plan_upgrade("3.0.0", &InstallChannel::Standalone, &failed, NOW); - - assert_eq!( - plan.notice, - Some(UpgradeNotice::Installing { - version: "3.2.0".to_owned(), - last_error: None, - }) - ); -} - #[test] fn versions_compare_by_number_and_never_downgrade() { let channel = InstallChannel::Npm(PackageRunner::Npm); diff --git a/crates/skilld-native/Cargo.toml b/crates/skilld-native/Cargo.toml index b0d898b7..86e5e9a4 100644 --- a/crates/skilld-native/Cargo.toml +++ b/crates/skilld-native/Cargo.toml @@ -18,6 +18,8 @@ path = "src/main.rs" [dependencies] gethostname.workspace = true +fs4.workspace = true + skilld-auth.workspace = true skilld-command.workspace = true diff --git a/crates/skilld-native/src/lib.rs b/crates/skilld-native/src/lib.rs index 2cc0c340..48c4aeae 100644 --- a/crates/skilld-native/src/lib.rs +++ b/crates/skilld-native/src/lib.rs @@ -15,6 +15,7 @@ pub mod select_ui; pub mod update_ui; #[cfg(not(target_os = "wasi"))] pub mod upgrade; +pub mod upgrade_ui; pub mod weekly; #[derive(Clone, Debug, Eq, PartialEq)] diff --git a/crates/skilld-native/src/main.rs b/crates/skilld-native/src/main.rs index 654ac5f2..0592d2d5 100644 --- a/crates/skilld-native/src/main.rs +++ b/crates/skilld-native/src/main.rs @@ -11,7 +11,7 @@ use std::sync::Arc; use embedded_skill::EmbeddedSkilld; use native_auth::NativeAccount; use skilld_command::AccountProvider; -use skilld_command::upgrade::{InstallChannel, UpgradeNotice}; +use skilld_command::upgrade::InstallChannel; use skilld_command::weekly::{self, NoticeContext}; use skilld_command::{ CommandError, CommandPlatform, DetectionEnvironment, Host, InstalledSkill, LocalHost, @@ -54,9 +54,11 @@ fn main() -> ExitCode { run_upgrade_worker(&worker); return ExitCode::SUCCESS; } - let upgrade_notice = start_upgrade(); - let args = env::args_os().collect::>(); + let upgrade_session = match start_upgrade(&args) { + UpgradeStartup::Continue(session) => session, + UpgradeStartup::Exit(exit) => return exit, + }; let interactive = interactive_update_requested(args.clone()).is_ok_and(|requested| requested); if interactive && let Err(error) = require_interactive_tty( @@ -193,7 +195,7 @@ fn main() -> ExitCode { { eprintln!("{notice}"); } - print_upgrade_notice(upgrade_notice.as_ref()); + print_upgrade_notice(upgrade_session.as_ref()); print_weekly_notice( ¬ice_root, account.as_ref(), @@ -219,7 +221,7 @@ fn main() -> ExitCode { { eprintln!("{notice}"); } - print_upgrade_notice(upgrade_notice.as_ref()); + print_upgrade_notice(upgrade_session.as_ref()); print_weekly_notice( ¬ice_root, account.as_ref(), @@ -247,29 +249,128 @@ fn upgrade_channel(executable: &std::path::Path) -> InstallChannel { } } -/// Starts background upgrade work for a person at a terminal. -fn start_upgrade() -> Option { +struct UpgradeSession { + root: PathBuf, + channel: InstallChannel, + executable: PathBuf, +} + +enum UpgradeStartup { + Continue(Option), + Exit(ExitCode), +} + +/// Offer the cached release first, then check asynchronously for the next run. +fn start_upgrade(args: &[std::ffi::OsString]) -> UpgradeStartup { if environment_enabled("CI") || environment_present("SKILLD_NO_UPGRADE") || active_agent_detected() + || !std::io::stdin().is_terminal() + || !std::io::stdout().is_terminal() || !std::io::stderr().is_terminal() + || args.iter().any(|arg| { + matches!( + arg.to_str(), + Some("--json" | "--plain" | "--help" | "-h" | "--version" | "-V") + ) + }) { - return None; + return UpgradeStartup::Continue(None); } - let executable = env::current_exe().ok()?; + let Ok(executable) = env::current_exe() else { + return UpgradeStartup::Continue(None); + }; let channel = upgrade_channel(&executable); + if channel == InstallChannel::Unmanaged { + return UpgradeStartup::Continue(None); + } + let root = global_root(); #[cfg(windows)] if channel == InstallChannel::Standalone { // An earlier upgrade left the replaced executable here; it is safe to lose. let _ = std::fs::remove_file(cli_upgrade::previous_executable(&executable)); } + if let Some(notice) = cli_upgrade::available_upgrade(&root, channel, VERSION) + && !cli_upgrade::version_dismissed(&root, ¬ice.version) + { + let package = match channel { + InstallChannel::Npm(runner) => { + cli_upgrade::package_upgrade_args(runner, &executable, ¬ice.version) + } + _ => None, + }; + let action = match (&channel, &package) { + (InstallChannel::Standalone, _) => { + Some("Download and verify the signed release, then upgrade skilld.".to_owned()) + } + (_, Some((program, args))) => Some(format!("Run {} {}", program, args.join(" "))), + _ => None, + }; + if let Some(action) = action { + use skilld_native::upgrade_ui::{self, UpgradeChoice}; + match upgrade_ui::ask( + VERSION, + ¬ice.version, + &action, + !environment_present("NO_COLOR"), + ) { + Ok(UpgradeChoice::Later) => {} + Ok(UpgradeChoice::Dismiss) => { + if let Err(error) = cli_upgrade::dismiss_version(&root, ¬ice.version) { + eprintln!("UPGRADE_STATE_FAILED: Cannot save the upgrade choice: {error}"); + } + } + Ok(UpgradeChoice::Upgrade) => { + eprintln!("Upgrading skilld to {}...", notice.version); + let result = if let Some((program, args)) = package { + eprintln!("Running {program} {}", args.join(" ")); + cli_upgrade::run_package_upgrade(program, &args) + } else if let (Some(pin), Some(asset)) = + (release_pin(), cli_upgrade::current_release_asset()) + { + cli_upgrade::install_release( + &InstallTarget { + executable: &executable, + current_version: VERSION, + asset, + pin: &pin, + }, + &NativeReleaseFetcher::new(VERSION), + ¬ice.version, + ) + } else { + unreachable!("standalone upgrades require a release key and asset") + }; + return UpgradeStartup::Exit(match result { + Ok(()) => { + eprintln!("Upgrade complete. Run your command again."); + ExitCode::SUCCESS + } + Err(error) => { + eprintln!("{}: {}", error.code, error.message); + ExitCode::from(2) + } + }); + } + Err(error) => { + eprintln!("{error}"); + return UpgradeStartup::Exit(ExitCode::from(2)); + } + } + } + } cli_upgrade::before_command( - &global_root(), + &root, &executable, channel, VERSION, cli_upgrade::unix_now(), - ) + ); + UpgradeStartup::Continue(Some(UpgradeSession { + root, + channel, + executable, + })) } fn run_upgrade_worker(value: &str) { @@ -277,29 +378,47 @@ fn run_upgrade_worker(value: &str) { return; }; let channel = upgrade_channel(&executable); - let pin = release_pin(); - let target = match (channel, &pin, cli_upgrade::current_release_asset()) { - (InstallChannel::Standalone, Some(pin), Some(asset)) => Some(InstallTarget { - executable: &executable, - current_version: VERSION, - asset, - pin, - }), - _ => None, - }; cli_upgrade::run_worker( value, &global_root(), channel, &NativeReleaseFetcher::new(VERSION), - target, cli_upgrade::unix_now(), ); } -fn print_upgrade_notice(notice: Option<&UpgradeNotice>) { - if let Some(notice) = notice { - eprintln!("{}", notice.message()); +fn print_upgrade_notice(session: Option<&UpgradeSession>) { + if let Some(session) = session + && let Some(notice) = + cli_upgrade::available_upgrade(&session.root, session.channel, VERSION) + { + let guidance = match session.channel { + InstallChannel::Npm(runner) + if cli_upgrade::package_upgrade_args( + runner, + &session.executable, + ¬ice.version, + ) + .is_none() => + { + if runner == skilld_command::upgrade::PackageRunner::Npx { + "Run npx skilld@latest to upgrade." + } else { + "Use your package manager to upgrade this skilld installation." + } + } + _ => "Restart skilld to upgrade.", + }; + eprintln!( + "\n{}", + skilld_native::upgrade_ui::render_banner( + VERSION, + ¬ice.version, + guidance, + terminal_width(), + !environment_present("NO_COLOR") + ) + ); } } diff --git a/crates/skilld-native/src/upgrade.rs b/crates/skilld-native/src/upgrade.rs index 540a1f53..214ee630 100644 --- a/crates/skilld-native/src/upgrade.rs +++ b/crates/skilld-native/src/upgrade.rs @@ -77,7 +77,7 @@ pub fn current_release_asset() -> Option<&'static str> { /// Plans the upgrade for one command, records it, and starts the worker. /// /// Returns the notice to print after the command. Upgrade problems never change -/// the command result, so this reports nothing when state or spawning fails. +/// the command result. A failed check still allows an existing cached notice. pub fn before_command( data_root: &Path, executable: &Path, @@ -88,29 +88,23 @@ pub fn before_command( let mut state = read_state(data_root); let plan = plan_upgrade(current_version, &channel, &state, now); if let Some(worker) = &plan.worker { - match worker { - UpgradeWorker::Check => state.checked_at = now, - UpgradeWorker::Install { version } => { - state.attempted_version = Some(version.clone()); - state.attempted_at = Some(now); - } - } + state.checked_at = now; // Recording first stops concurrent commands from starting duplicate workers. - write_state(data_root, &state).ok()?; - spawn_worker(executable, worker).ok()?; + if write_state(data_root, &state).is_ok() { + // A failed optional check must not hide an already cached upgrade. + let _ = spawn_worker(executable, worker); + } } plan.notice } /// Runs the background worker named by `WORKER_VARIABLE`. /// -/// `target` is `None` unless this is a standalone install with a compiled release key. pub fn run_worker( value: &str, data_root: &Path, channel: InstallChannel, fetcher: &dyn ReleaseFetcher, - target: Option>, now: u64, ) { let Some(_lock) = WorkerLock::acquire(data_root) else { @@ -130,16 +124,6 @@ pub fn run_worker( state.latest = Some(latest); Ok(()) }) - } else if let Some(version) = value.strip_prefix("install:") { - state.attempted_version = Some(version.to_owned()); - state.attempted_at = Some(now); - match target { - Some(target) => install_release(&target, fetcher, version), - None => Err(RemoteError::new( - "UPGRADE_UNAVAILABLE", - "only a standalone install with a release key upgrades itself", - )), - } } else { return; }; @@ -150,8 +134,8 @@ pub fn run_worker( /// Downloads, verifies, and installs one release over the running executable. /// -/// Nothing is written beside the executable until the signed manifest and the -/// binary digest both verify. The new binary runs only after verification. +/// Release bytes reach disk only after the signature and digest verify. +/// The new binary runs only after verification. pub fn install_release( target: &InstallTarget<'_>, fetcher: &dyn ReleaseFetcher, @@ -163,6 +147,26 @@ pub fn install_release( "skilld only upgrades to a newer version", )); } + let lock = OpenOptions::new() + .write(true) + .create(true) + .truncate(false) + .open(target.executable.with_file_name(".skilld-install.lock")) + .map_err(upgrade_io)?; + fs4::FileExt::try_lock(&lock).map_err(|error| { + RemoteError::new( + "UPGRADE_LOCK_FAILED", + format!("Cannot lock skilld for an upgrade: {error}"), + ) + })?; + // Another terminal may have upgraded while this process waited for consent. + let installed = executable_version(target.executable)?; + if !is_newer(version, &installed) { + return Err(RemoteError::new( + "UPGRADE_NOT_NEWER", + "The installed skilld is already current. Run your command again.", + )); + } let url = |asset: &str| { release_asset_url(version, asset) .ok_or_else(|| RemoteError::new("UPGRADE_CHECK_INVALID", "the version is invalid")) @@ -181,6 +185,65 @@ pub fn install_release( replace_executable(target.executable, &binary, version) } +/// Upgrade only a recognized global package installation. Project dependencies +/// and transient npx installs remain under their caller's package manager. +pub fn package_upgrade_args( + runner: PackageRunner, + executable: &Path, + version: &str, +) -> Option<(&'static str, Vec)> { + if !skilld_core::is_release_version(version) { + return None; + } + let path = executable.to_string_lossy().replace('\\', "/"); + let global = match runner { + PackageRunner::Npm => { + path.contains("/lib/node_modules/") || path.contains("/npm/node_modules/") + } + PackageRunner::Pnpm => path.contains("/pnpm/global/"), + PackageRunner::Bun => path.contains("/.bun/install/global/"), + PackageRunner::Yarn => path.contains("/yarn/global/"), + PackageRunner::Npx => false, + }; + if !global { + return None; + } + let (program, args): (_, &[&str]) = match runner { + PackageRunner::Npm => ("npm", &["install", "--global"]), + PackageRunner::Pnpm => ("pnpm", &["add", "--global"]), + PackageRunner::Bun => ("bun", &["add", "--global"]), + PackageRunner::Yarn => ("yarn", &["global", "add"]), + PackageRunner::Npx => return None, + }; + let mut args = args.iter().map(|arg| (*arg).to_owned()).collect::>(); + args.push(format!("skilld@{version}")); + Some((program, args)) +} + +/// Run the package manager with inherited terminal streams after consent. +pub fn run_package_upgrade(program: &str, args: &[String]) -> Result<(), RemoteError> { + #[cfg(windows)] + let mut command = { + let mut command = Command::new("cmd.exe"); + command.args(["/d", "/s", "/c", program]); + command + }; + #[cfg(not(windows))] + let mut command = Command::new(program); + let status = command + .args(args) + .env_remove(WORKER_VARIABLE) + .status() + .map_err(upgrade_io)?; + if !status.success() { + return Err(RemoteError::new( + "UPGRADE_FAILED", + format!("{program} could not upgrade skilld. Retry the command shown above."), + )); + } + Ok(()) +} + fn replace_executable(executable: &Path, binary: &[u8], version: &str) -> Result<(), RemoteError> { let staged = executable.with_file_name(format!(".skilld-upgrade-{}", std::process::id())); let result = stage(&staged, binary) @@ -207,6 +270,16 @@ fn stage(path: &Path, binary: &[u8]) -> Result<(), RemoteError> { } fn check_version(path: &Path, version: &str) -> Result<(), RemoteError> { + if executable_version(path)? != version { + return Err(RemoteError::new( + "UPGRADE_VERSION_MISMATCH", + "the downloaded skilld reports another version", + )); + } + Ok(()) +} + +fn executable_version(path: &Path) -> Result { let output = Command::new(path) .arg("--version") .env("SKILLD_NO_UPGRADE", "1") @@ -215,13 +288,18 @@ fn check_version(path: &Path, version: &str) -> Result<(), RemoteError> { .stderr(Stdio::null()) .output() .map_err(upgrade_io)?; - if !output.status.success() || output.stdout != format!("skilld {version}\n").as_bytes() { + let version = std::str::from_utf8(&output.stdout) + .ok() + .and_then(|value| value.strip_prefix("skilld ")) + .and_then(|value| value.strip_suffix('\n')) + .filter(|version| skilld_core::is_release_version(version)); + if !output.status.success() || version.is_none() { return Err(RemoteError::new( "UPGRADE_VERSION_MISMATCH", - "the downloaded skilld reports another version", + "Cannot read the skilld executable version.", )); } - Ok(()) + Ok(version.expect("validated version").to_owned()) } #[cfg(not(windows))] @@ -251,7 +329,6 @@ pub fn previous_executable(executable: &Path) -> PathBuf { fn spawn_worker(executable: &Path, worker: &UpgradeWorker) -> io::Result<()> { let value = match worker { UpgradeWorker::Check => "check".to_owned(), - UpgradeWorker::Install { version } => format!("install:{version}"), }; let mut command = Command::new(executable); command @@ -281,6 +358,30 @@ pub fn read_state(data_root: &Path) -> UpgradeState { .unwrap_or_default() } +/// Dismissals live separately, so an in-flight check cannot overwrite an answer. +pub fn dismiss_version(data_root: &Path, version: &str) -> io::Result<()> { + fs::create_dir_all(data_root)?; + let mut file = tempfile::NamedTempFile::new_in(data_root)?; + file.write_all(version.as_bytes())?; + file.persist(data_root.join("upgrade-dismissed")) + .map(drop) + .map_err(|error| error.error) +} + +pub fn version_dismissed(data_root: &Path, version: &str) -> bool { + fs::read_to_string(data_root.join("upgrade-dismissed")) + .is_ok_and(|dismissed| dismissed == version) +} + +/// Recheck cached results after a command without waiting for the network worker. +pub fn available_upgrade( + data_root: &Path, + channel: InstallChannel, + current: &str, +) -> Option { + plan_upgrade(current, &channel, &read_state(data_root), unix_now()).notice +} + fn write_state(data_root: &Path, state: &UpgradeState) -> io::Result<()> { fs::create_dir_all(data_root)?; let bytes = serde_json::to_vec(state).map_err(io::Error::other)?; diff --git a/crates/skilld-native/src/upgrade_ui.rs b/crates/skilld-native/src/upgrade_ui.rs new file mode 100644 index 00000000..0440d78c --- /dev/null +++ b/crates/skilld-native/src/upgrade_ui.rs @@ -0,0 +1,321 @@ +//! Cached CLI upgrade prompt. Network checks never own the terminal. + +use std::io; +use std::time::Duration; + +use crossterm::event::{self, Event, KeyCode, KeyEventKind, KeyModifiers}; +use ratatui::Terminal; +use ratatui::backend::{CrosstermBackend, TestBackend}; +use ratatui::layout::{Constraint, Layout}; +use ratatui::style::{Color, Modifier, Style}; +use ratatui::text::{Line, Span}; +use ratatui::widgets::{Block, List, ListItem, ListState, Paragraph, Wrap}; + +use crate::update_ui::{InteractiveUpdateError, NativeTerminalLifecycle, with_restored_terminal}; + +const ACCENT: Color = Color::Rgb(103, 232, 249); +const SUCCESS: Color = Color::Rgb(134, 239, 172); +const MUTED: Color = Color::Rgb(148, 163, 184); +const COMMAND: Color = Color::Rgb(253, 224, 71); +const SELECTED: Color = Color::Rgb(22, 78, 99); + +fn tone(color: bool, foreground: Color) -> Style { + if color { + Style::default().fg(foreground) + } else { + Style::default() + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum UpgradeChoice { + Upgrade, + Later, + Dismiss, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum UpgradeKey { + Up, + Down, + Confirm, + Cancel, +} + +#[derive(Default)] +pub struct UpgradePrompt { + cursor: usize, +} + +impl UpgradePrompt { + pub fn update(&mut self, key: UpgradeKey) -> Option { + match key { + UpgradeKey::Up => self.cursor = (self.cursor + 2) % 3, + UpgradeKey::Down => self.cursor = (self.cursor + 1) % 3, + UpgradeKey::Cancel => return Some(UpgradeChoice::Later), + UpgradeKey::Confirm => { + return Some( + [ + UpgradeChoice::Upgrade, + UpgradeChoice::Later, + UpgradeChoice::Dismiss, + ][self.cursor], + ); + } + } + None + } +} + +pub fn banner(current: &str, latest: &str) -> String { + format!( + "Upgrade available: {current} -> {latest}\nhttps://github.com/skilld-dev/skilld/releases/latest" + ) +} + +/// A static banner can be appended after the command without moving its cursor. +pub fn render_banner( + current: &str, + latest: &str, + guidance: &str, + width: u16, + color: bool, +) -> String { + let width = width.clamp(20, 90); + let lines = format!("{}\n\n{guidance}", banner(current, latest)) + .lines() + .enumerate() + .flat_map(|(index, line)| { + let style = tone( + color, + match index { + 0 => SUCCESS, + 1 => MUTED, + _ => ACCENT, + }, + ); + skilld_ui::text::wrap(line, usize::from(width - 2)) + .into_iter() + .map(move |line| Line::styled(line, style)) + }) + .collect::>(); + let height = lines.len() as u16 + 2; + let paragraph = + Paragraph::new(lines).block(Block::bordered().border_style(tone(color, ACCENT))); + let mut terminal = Terminal::new(TestBackend::new(width, height)).expect("in-memory terminal"); + terminal + .draw(|frame| frame.render_widget(paragraph, frame.area())) + .expect("render banner"); + snapshot_text(&terminal, color) +} + +pub fn ask( + current: &str, + latest: &str, + action: &str, + color: bool, +) -> Result { + with_restored_terminal(NativeTerminalLifecycle, || { + let mut terminal = + Terminal::new(CrosstermBackend::new(io::stdout())).map_err(terminal_error)?; + let mut model = UpgradePrompt::default(); + // An Enter already buffered for the original command is not upgrade consent. + while event::poll(Duration::ZERO).map_err(terminal_error)? { + let _ = event::read().map_err(terminal_error)?; + } + loop { + terminal + .draw(|frame| view(frame, &model, current, latest, action, color)) + .map_err(terminal_error)?; + let Event::Key(key) = event::read().map_err(terminal_error)? else { + continue; + }; + if key.kind != KeyEventKind::Press { + continue; + } + let key = match key.code { + KeyCode::Up | KeyCode::Char('k') => UpgradeKey::Up, + KeyCode::Down | KeyCode::Char('j') => UpgradeKey::Down, + KeyCode::Enter => UpgradeKey::Confirm, + KeyCode::Esc | KeyCode::Char('q') => UpgradeKey::Cancel, + KeyCode::Char('c') if key.modifiers.contains(KeyModifiers::CONTROL) => { + UpgradeKey::Cancel + } + _ => continue, + }; + if let Some(choice) = model.update(key) { + return Ok(choice); + } + } + }) +} + +fn view( + frame: &mut ratatui::Frame<'_>, + model: &UpgradePrompt, + current: &str, + latest: &str, + action: &str, + color: bool, +) { + let areas = Layout::vertical([ + Constraint::Length(7), + Constraint::Length(3), + Constraint::Min(1), + ]) + .split(frame.area()); + let header = Paragraph::new(vec![ + Line::from(vec![ + Span::styled( + "Upgrade available: ", + tone(color, ACCENT).add_modifier(Modifier::BOLD), + ), + Span::styled(current, tone(color, MUTED)), + Span::styled(" -> ", tone(color, MUTED)), + Span::styled(latest, tone(color, SUCCESS).add_modifier(Modifier::BOLD)), + ]), + Line::styled( + "https://github.com/skilld-dev/skilld/releases/latest", + tone(color, MUTED), + ), + Line::from(""), + Line::styled(action, tone(color, COMMAND)), + ]) + .wrap(Wrap { trim: true }) + .block( + Block::bordered() + .border_style(tone(color, ACCENT)) + .title(Span::styled( + " skilld CLI upgrade ", + tone(color, ACCENT).add_modifier(Modifier::BOLD), + )), + ); + frame.render_widget(header, areas[0]); + let options = List::new([ + ListItem::new("Upgrade now"), + ListItem::new("Not now"), + ListItem::new("Don't remind me for this version"), + ]) + .highlight_symbol("> ") + .highlight_style(if color { + Style::default() + .fg(ACCENT) + .bg(SELECTED) + .add_modifier(Modifier::BOLD) + } else { + Style::default().add_modifier(Modifier::BOLD) + }); + frame.render_stateful_widget( + options, + areas[1], + &mut ListState::default().with_selected(Some(model.cursor)), + ); + frame.render_widget( + Paragraph::new(vec![ + Line::from(""), + Line::from(vec![ + Span::styled("Up/Down", tone(color, ACCENT)), + Span::styled(" to choose. ", tone(color, MUTED)), + Span::styled("Enter", tone(color, ACCENT)), + Span::styled(" to confirm. ", tone(color, MUTED)), + Span::styled("Esc", tone(color, ACCENT)), + Span::styled(" to continue.", tone(color, MUTED)), + ]), + ]) + .wrap(Wrap { trim: true }), + areas[2], + ); +} + +pub fn render_snapshot( + current: &str, + latest: &str, + action: &str, + width: u16, + height: u16, + color: bool, +) -> String { + let mut terminal = Terminal::new(TestBackend::new(width, height)).expect("in-memory terminal"); + terminal + .draw(|frame| { + view( + frame, + &UpgradePrompt::default(), + current, + latest, + action, + color, + ) + }) + .expect("render prompt"); + snapshot_text(&terminal, color) +} + +fn snapshot_text(terminal: &Terminal, color: bool) -> String { + let buffer = terminal.backend().buffer(); + let width = buffer.area.width; + let height = buffer.area.height; + (0..height) + .map(|y| { + if color { + let mut line = String::new(); + let mut previous = None; + for x in 0..width { + let cell = &buffer[(x, y)]; + let style = cell.style(); + if previous != Some(style) { + line.push_str("\x1b[0m"); + line.push_str(&ansi_color(cell.fg, false)); + line.push_str(&ansi_color(cell.bg, true)); + if cell.modifier.contains(Modifier::BOLD) { + line.push_str("\x1b[1m"); + } + previous = Some(style); + } + line.push_str(cell.symbol()); + } + line.push_str("\x1b[0m"); + return line; + } + (0..width) + .map(|x| buffer[(x, y)].symbol()) + .collect::() + .trim_end() + .to_owned() + }) + .collect::>() + .join("\n") +} + +fn ansi_color(color: Color, background: bool) -> String { + let base = if background { 48 } else { 38 }; + let index = match color { + Color::Reset => return format!("\x1b[{}m", if background { 49 } else { 39 }), + Color::Rgb(red, green, blue) => return format!("\x1b[{base};2;{red};{green};{blue}m"), + Color::Indexed(index) => index, + Color::Black => 0, + Color::Red => 1, + Color::Green => 2, + Color::Yellow => 3, + Color::Blue => 4, + Color::Magenta => 5, + Color::Cyan => 6, + Color::Gray => 7, + Color::DarkGray => 8, + Color::LightRed => 9, + Color::LightGreen => 10, + Color::LightYellow => 11, + Color::LightBlue => 12, + Color::LightMagenta => 13, + Color::LightCyan => 14, + Color::White => 15, + }; + format!("\x1b[{base};5;{index}m") +} + +fn terminal_error(error: io::Error) -> InteractiveUpdateError { + InteractiveUpdateError::new( + "UPGRADE_TERMINAL_FAILED", + format!("Cannot read the upgrade choice: {error}"), + ) +} diff --git a/crates/skilld-native/tests/upgrade.rs b/crates/skilld-native/tests/upgrade.rs index 487793fc..ec0890c4 100644 --- a/crates/skilld-native/tests/upgrade.rs +++ b/crates/skilld-native/tests/upgrade.rs @@ -16,6 +16,83 @@ use skilld_native::upgrade::{ install_release, read_state, run_worker, }; +#[test] +fn dismissal_survives_an_in_flight_check_and_only_matches_one_version() { + use skilld_native::upgrade::{dismiss_version, version_dismissed}; + let data = tempfile::tempdir().unwrap(); + let mut fetcher = FakeFetcher::default(); + fetcher.responses.insert( + "https://registry.npmjs.org/skilld/latest".to_owned(), + br#"{"version":"3.2.0"}"#.to_vec(), + ); + dismiss_version(data.path(), "3.2.0").unwrap(); + run_worker( + "check", + data.path(), + InstallChannel::Npm(PackageRunner::Npm), + &fetcher, + 1_000, + ); + assert!(version_dismissed(data.path(), "3.2.0")); + assert!(!version_dismissed(data.path(), "3.3.0")); +} + +#[test] +fn only_global_package_installs_offer_an_executable_upgrade_command() { + use skilld_native::upgrade::package_upgrade_args; + for (runner, path, program, arguments) in [ + ( + PackageRunner::Npm, + "/usr/lib/node_modules/skilld-cli/bin/skilld", + "npm", + vec!["install", "--global", "skilld@3.7.0"], + ), + ( + PackageRunner::Pnpm, + "/home/me/.local/share/pnpm/global/v11/pkg/node_modules/skilld-cli/bin/skilld", + "pnpm", + vec!["add", "--global", "skilld@3.7.0"], + ), + ( + PackageRunner::Bun, + "/home/me/.bun/install/global/node_modules/skilld-cli/bin/skilld", + "bun", + vec!["add", "--global", "skilld@3.7.0"], + ), + ( + PackageRunner::Yarn, + "/home/me/.config/yarn/global/node_modules/skilld-cli/bin/skilld", + "yarn", + vec!["global", "add", "skilld@3.7.0"], + ), + ] { + assert_eq!( + package_upgrade_args(runner, Path::new(path), "3.7.0"), + Some((program, arguments.into_iter().map(str::to_owned).collect())) + ); + assert_eq!( + package_upgrade_args( + runner, + Path::new("/project/node_modules/skilld-cli/bin/skilld"), + "3.7.0" + ), + None + ); + assert_eq!( + package_upgrade_args(runner, Path::new(path), "3.7.0;bad"), + None + ); + } + assert_eq!( + package_upgrade_args( + PackageRunner::Npx, + Path::new("/cache/_npx/pkg/skilld"), + "3.7.0" + ), + None + ); +} + const ASSET: &str = "skilld-cli-linux-x64-gnu"; const BASE: &str = "https://github.com/skilld-dev/skilld/releases/download/v9.0.0"; @@ -175,6 +252,21 @@ fn an_older_or_equal_version_is_refused_before_any_download() { assert!(fetcher.requested.borrow().is_empty()); } +#[test] +fn a_prompt_from_an_old_process_cannot_replace_a_newer_installed_release() { + let directory = tempfile::tempdir().unwrap(); + let executable = installed(directory.path()); + fs::write(&executable, script("10.0.0")).unwrap(); + let pin = pin(); + let fetcher = FakeFetcher::default(); + + let error = install_release(&target(&executable, &pin), &fetcher, "9.0.0").unwrap_err(); + + assert_eq!(error.code, "UPGRADE_NOT_NEWER"); + assert!(fetcher.requested.borrow().is_empty()); + assert_eq!(fs::read(executable).unwrap(), script("10.0.0")); +} + #[test] fn the_channel_comes_from_the_loader_or_a_regular_install_marker() { let directory = tempfile::tempdir().unwrap(); @@ -220,7 +312,6 @@ fn the_check_worker_records_the_latest_npm_version() { data.path(), InstallChannel::Npm(PackageRunner::Npm), &fetcher, - None, 1_000, ); @@ -228,64 +319,36 @@ fn the_check_worker_records_the_latest_npm_version() { assert_eq!(state.latest.as_deref(), Some("3.2.0")); assert_eq!(state.checked_at, 1_000); assert_eq!(state.last_error, None); + assert_eq!( + skilld_native::upgrade::available_upgrade( + data.path(), + InstallChannel::Npm(PackageRunner::Npm), + "3.0.0" + ), + Some(UpgradeNotice { + version: "3.2.0".to_owned() + }), + "the command footer sees a completed asynchronous check" + ); } #[test] -fn the_install_worker_records_a_failed_verification() { +fn a_background_worker_cannot_install_a_release() { let directory = tempfile::tempdir().unwrap(); - let data = tempfile::tempdir().unwrap(); let executable = installed(directory.path()); - let pin = pin(); - let fetcher = release(&signing_key(), &script("9.0.0"), &script("tampered")); + let data = tempfile::tempdir().unwrap(); + let fetcher = release(&signing_key(), &script("9.0.0"), &script("9.0.0")); run_worker( "install:9.0.0", data.path(), InstallChannel::Standalone, &fetcher, - Some(target(&executable, &pin)), 1_000, ); - let state = read_state(data.path()); - assert_eq!(state.last_error.as_deref(), Some("RELEASE_DIGEST_MISMATCH")); - assert_eq!(state.attempted_version.as_deref(), Some("9.0.0")); - assert_eq!(fs::read(&executable).unwrap(), script("3.0.0")); -} - -#[test] -fn a_retry_after_a_failed_upgrade_names_the_previous_error() { - let directory = tempfile::tempdir().unwrap(); - let data = tempfile::tempdir().unwrap(); - let executable = installed(directory.path()); - let state = UpgradeState { - checked_at: 1_000, - latest: Some("3.2.0".to_owned()), - attempted_version: Some("3.2.0".to_owned()), - attempted_at: Some(1_000), - last_error: Some("UPGRADE_DOWNLOAD_FAILED".to_owned()), - }; - fs::write( - data.path().join("upgrade.json"), - serde_json::to_vec(&state).unwrap(), - ) - .unwrap(); - - let notice = before_command( - data.path(), - &executable, - InstallChannel::Standalone, - "3.0.0", - 5_000, - ); - - let message = notice - .expect("an overdue retry still plans an upgrade") - .message(); - assert!( - message.contains("UPGRADE_DOWNLOAD_FAILED"), - "the notice must name the previous failure: {message}" - ); + assert_eq!(fs::read(executable).unwrap(), script("3.0.0")); + assert!(fetcher.requested.borrow().is_empty()); } #[test] @@ -312,9 +375,8 @@ fn a_known_npm_upgrade_prints_its_notice_without_a_worker() { assert_eq!( notice, - Some(UpgradeNotice::Available { + Some(UpgradeNotice { version: "3.2.0".to_owned(), - command: "npx skilld@latest".to_owned(), }) ); } diff --git a/crates/skilld-native/tests/upgrade_flow.rs b/crates/skilld-native/tests/upgrade_flow.rs new file mode 100644 index 00000000..a4f3f5c4 --- /dev/null +++ b/crates/skilld-native/tests/upgrade_flow.rs @@ -0,0 +1,227 @@ +#![cfg(unix)] + +use std::fs::{self, File}; +use std::io::{Read, Write}; +use std::os::unix::fs::PermissionsExt; +use std::os::unix::process::CommandExt; +use std::path::PathBuf; +use std::process::{Command, Stdio}; +use std::sync::mpsc; +use std::thread; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; + +use nix::pty::{Winsize, openpty}; + +struct Fixture { + root: tempfile::TempDir, + binary: PathBuf, +} + +impl Fixture { + fn new(package_exit: u8) -> Self { + let root = tempfile::tempdir().unwrap(); + let binary = root.path().join("pnpm/global/v11/native/skilld"); + fs::create_dir_all(binary.parent().unwrap()).unwrap(); + fs::hard_link(env!("CARGO_BIN_EXE_skilld"), &binary).unwrap(); + let bin = root.path().join("bin"); + fs::create_dir_all(&bin).unwrap(); + let manager = bin.join("pnpm"); + fs::write( + &manager, + format!( + "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$HOME/manager-args\"\nexit {package_exit}\n" + ), + ) + .unwrap(); + fs::set_permissions(manager, fs::Permissions::from_mode(0o755)).unwrap(); + let fixture = Self { root, binary }; + fixture.cache("9.0.0"); + fixture + } + + fn cache(&self, version: &str) { + fs::write( + self.root.path().join("upgrade.json"), + serde_json::to_vec(&serde_json::json!({ + "checkedAt": SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_secs(), + "latest": version + })) + .unwrap(), + ) + .unwrap(); + } + + fn command(&self) -> Command { + let mut command = Command::new(&self.binary); + command + .env_clear() + .env("HOME", self.root.path()) + .env("PATH", self.root.path().join("bin")) + .env("SKILLD_DATA_DIR", self.root.path()) + .env("SKILLD_LAUNCHER", "pnpm") + .env("SKILLD_NO_WEEKLY", "1") + .env("TERM", "xterm-256color") + .current_dir(self.root.path()); + command + } + + fn terminal( + &self, + args: &[&str], + answer: Option<&[u8]>, + env: Option<(&str, &str)>, + ) -> (bool, String) { + let pair = openpty( + Some(&Winsize { + ws_row: 20, + ws_col: 80, + ws_xpixel: 0, + ws_ypixel: 0, + }), + None, + ) + .unwrap(); + let mut writer = File::from(pair.master); + let mut reader = writer.try_clone().unwrap(); + let slave = File::from(pair.slave); + let mut command = self.command(); + command + .args(args) + .stdin(Stdio::from(slave.try_clone().unwrap())) + .stdout(Stdio::from(slave.try_clone().unwrap())) + .stderr(Stdio::from(slave)); + if let Some((key, value)) = env { + command.env(key, value); + } + // The child needs its own controlling terminal for crossterm input. + unsafe { + command.pre_exec(|| { + if nix::libc::setsid() < 0 || nix::libc::ioctl(0, nix::libc::TIOCSCTTY, 0) < 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(()) + }); + } + let mut child = command.spawn().unwrap(); + drop(command); + let (sender, receiver) = mpsc::channel(); + thread::spawn(move || { + let mut buffer = [0; 4096]; + while let Ok(count) = reader.read(&mut buffer) { + if count == 0 || sender.send(buffer[..count].to_vec()).is_err() { + break; + } + } + }); + let mut output = String::new(); + let mut answer = answer; + loop { + match receiver.recv_timeout(Duration::from_secs(10)) { + Ok(bytes) => { + output.push_str(&String::from_utf8_lossy(&bytes)); + if output.contains("continue.") + && let Some(keys) = answer.take() + { + writer.write_all(keys).unwrap(); + writer.flush().unwrap(); + } + } + Err(mpsc::RecvTimeoutError::Disconnected) => break, + Err(mpsc::RecvTimeoutError::Timeout) => { + let _ = child.kill(); + let _ = child.wait(); + panic!("upgrade terminal timed out: {output}"); + } + } + } + (child.wait().unwrap().success(), output) + } +} + +#[test] +fn restarting_with_a_cached_release_prompts_and_runs_the_package_manager() { + let fixture = Fixture::new(0); + let (success, output) = fixture.terminal(&["list", "--global"], Some(b"\r"), None); + assert!(success, "{output}"); + assert!(output.contains("\x1b[38;2;"), "{output}"); + assert!( + output.contains("Upgrade complete. Run your command again."), + "{output}" + ); + assert_eq!( + fs::read_to_string(fixture.root.path().join("manager-args")).unwrap(), + "add\n--global\nskilld@9.0.0\n" + ); + let restore = output.find("\x1b[?1049l").expect("terminal restored"); + assert!(restore < output.find("Running pnpm").unwrap()); +} + +#[test] +fn not_now_runs_the_original_command_without_installing() { + let fixture = Fixture::new(0); + let (success, output) = fixture.terminal( + &["list", "--global"], + Some(b"\x1b[B\r"), + Some(("NO_COLOR", "1")), + ); + assert!(success, "{output}"); + assert!(output.contains("Restart skilld to upgrade."), "{output}"); + assert!(!output.contains("\x1b[38;"), "{output}"); + assert!(!output.contains("\x1b[48;"), "{output}"); + assert!(!fixture.root.path().join("manager-args").exists()); + assert!(!fixture.root.path().join("upgrade-dismissed").exists()); +} + +#[test] +fn dismissal_skips_the_next_prompt_but_a_new_release_prompts_again() { + let fixture = Fixture::new(0); + let (success, output) = fixture.terminal(&["list", "--global"], Some(b"\x1b[A\r"), None); + assert!(success, "{output}"); + let (success, output) = fixture.terminal(&["list", "--global"], None, None); + assert!(success, "{output}"); + assert!(!output.contains("Don't"), "{output}"); + fixture.cache("9.1.0"); + let (success, output) = fixture.terminal(&["list", "--global"], Some(b"q"), None); + assert!(success && output.contains("Don't"), "{output}"); +} + +#[test] +fn package_manager_failure_is_visible_and_never_reports_success() { + let fixture = Fixture::new(7); + let (success, output) = fixture.terminal(&["list", "--global"], Some(b"\r"), None); + assert!(!success, "{output}"); + assert!(output.contains("UPGRADE_FAILED"), "{output}"); + assert!(!output.contains("Upgrade complete"), "{output}"); +} + +#[test] +fn agents_ci_opt_out_and_machine_output_never_prompt() { + let fixture = Fixture::new(0); + for signal in [ + ("AGENT_SESSION_ID", "test"), + ("CI", "1"), + ("SKILLD_NO_UPGRADE", "1"), + ] { + let (success, output) = fixture.terminal(&["list", "--global"], None, Some(signal)); + assert!(success, "{output}"); + assert!(!output.contains("Upgrade available"), "{output}"); + } + for args in [ + &["update", "--check", "--global", "--json"][..], + &["list", "--global", "--plain"][..], + &["--version"][..], + &["--help"][..], + ] { + let (success, output) = fixture.terminal(args, None, None); + assert!(success, "{output}"); + assert!(!output.contains("\x1b[?1049h"), "{output}"); + } + let output = fixture + .command() + .args(["list", "--global"]) + .output() + .unwrap(); + assert!(output.status.success()); + assert!(!String::from_utf8_lossy(&output.stderr).contains("Upgrade available")); + assert!(!fixture.root.path().join("manager-args").exists()); +} diff --git a/crates/skilld-native/tests/upgrade_ui.rs b/crates/skilld-native/tests/upgrade_ui.rs new file mode 100644 index 00000000..fb41ae30 --- /dev/null +++ b/crates/skilld-native/tests/upgrade_ui.rs @@ -0,0 +1,94 @@ +use skilld_native::upgrade_ui::{ + UpgradeChoice, UpgradeKey, UpgradePrompt, render_banner, render_snapshot, +}; + +#[test] +fn the_prompt_accepts_defers_and_dismisses() { + let mut prompt = UpgradePrompt::default(); + assert_eq!( + prompt.update(UpgradeKey::Confirm), + Some(UpgradeChoice::Upgrade) + ); + let mut prompt = UpgradePrompt::default(); + assert_eq!(prompt.update(UpgradeKey::Down), None); + assert_eq!( + prompt.update(UpgradeKey::Confirm), + Some(UpgradeChoice::Later) + ); + let mut prompt = UpgradePrompt::default(); + assert_eq!(prompt.update(UpgradeKey::Up), None); + assert_eq!( + prompt.update(UpgradeKey::Confirm), + Some(UpgradeChoice::Dismiss) + ); + assert_eq!( + prompt.update(UpgradeKey::Cancel), + Some(UpgradeChoice::Later) + ); +} + +#[test] +fn the_prompt_shows_versions_action_and_choices_without_color() { + let output = render_snapshot( + "3.6.7", + "3.7.0", + "Run pnpm add --global skilld@3.7.0", + 80, + 16, + false, + ); + assert!( + output.contains("Upgrade available: 3.6.7 -> 3.7.0"), + "{output}" + ); + assert!( + output.contains("Run pnpm add --global skilld@3.7.0"), + "{output}" + ); + assert!(output.contains("> Upgrade now"), "{output}"); + assert!(output.contains("Not now"), "{output}"); + assert!( + output.contains("Don't remind me for this version"), + "{output}" + ); + assert!(output.contains("Enter to confirm"), "{output}"); + assert!(!output.contains('\x1b')); +} + +#[test] +fn color_output_resets_styles_and_plain_banners_have_no_escape_codes() { + let prompt = render_snapshot( + "3.6.7", + "3.7.0", + "Run pnpm add --global skilld@3.7.0", + 80, + 13, + true, + ); + let banner = render_banner("3.6.7", "3.7.0", "Restart skilld to upgrade.", 80, true); + println!("COLOR_PREVIEW_START\n{prompt}\n\n{banner}\nCOLOR_PREVIEW_END"); + for output in [&prompt, &banner] { + assert!(output.contains("\x1b[38;2;"), "{output}"); + assert!(output.lines().all(|line| line.ends_with("\x1b[0m"))); + } + let plain = render_banner("3.6.7", "3.7.0", "Restart skilld to upgrade.", 40, false); + assert!(plain.contains("Upgrade available: 3.6.7 -> 3.7.0")); + assert!(!plain.contains('\x1b')); +} + +#[test] +fn the_prompt_fits_a_narrow_terminal() { + let output = render_snapshot( + "3.6.7", + "3.7.0", + "Run pnpm add --global skilld@3.7.0", + 40, + 16, + false, + ); + assert!(output.contains("Upgrade now"), "{output}"); + assert!( + output.contains("Don't remind me for this version"), + "{output}" + ); +}