From bfae65005527abfbac0d7bb44b1bd1f82b5cc57d Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Thu, 6 Aug 2026 23:48:29 -0700 Subject: [PATCH 1/3] Honest install-only UX and Nu-agnostic try fallbacks. Label scripts/completions as install-only in search/info, and let numan try install wttr/nufetch when no activatable starter fits without calling activate. Co-authored-by: Cursor --- src/cmd/info.rs | 25 ++++--- src/cmd/search.rs | 22 ++++-- src/cmd/try_cmd.rs | 162 ++++++++++++++++++++++++++++++++++++++------- 3 files changed, 171 insertions(+), 38 deletions(-) diff --git a/src/cmd/info.rs b/src/cmd/info.rs index c7132256..01c999d3 100644 --- a/src/cmd/info.rs +++ b/src/cmd/info.rs @@ -26,6 +26,15 @@ pub fn format_info(pkg: &Package, platform: &Platform, nu: Option<&NuVersion>) - let mut out = String::new(); out.push_str(&format!("Package: {}/{}\n", pkg.id.owner, pkg.id.name)); out.push_str(&format!("Type: {}\n", pkg.package_type)); + match pkg.package_type { + crate::core::package::PackageType::Script + | crate::core::package::PackageType::Completion => { + out.push_str( + "Activation: install-only (activation deferred; install does not wire Nu)\n", + ); + } + _ => {} + } out.push_str("Status: verified upstream artifact\n"); out.push_str(&format!("Description: {}\n", pkg.description)); out.push_str(&format!("Repository: {}\n", pkg.repo)); @@ -157,16 +166,12 @@ mod tests { } #[test] - fn format_info_includes_verified_status_and_disclaimer() { - let pkg = sample_plugin(false); - let nu = NuVersion::parse("0.113.1").unwrap(); - let out = format_info(&pkg, &linux_platform(), Some(&nu)); - assert!( - out.contains("Status: verified upstream artifact"), - "{out}" - ); - assert!(out.contains("has not security-audited"), "{out}"); - assert!(!out.to_lowercase().contains("approved"), "{out}"); + fn format_info_marks_script_install_only() { + let mut pkg = sample_plugin(false); + pkg.package_type = PackageType::Script; + let out = format_info(&pkg, &linux_platform(), None); + assert!(out.contains("Activation: install-only"), "{out}"); + assert!(out.contains("Type: script"), "{out}"); } #[test] diff --git a/src/cmd/search.rs b/src/cmd/search.rs index 34d93033..a727df4e 100644 --- a/src/cmd/search.rs +++ b/src/cmd/search.rs @@ -135,8 +135,8 @@ fn format_search_header(nu: Option<&NuVersion>, triple: &str) -> String { /// Row status suffix (leading space + brackets), or empty. /// -/// Plugins get a hard evaluated verdict. Non-plugins never use `[compatible]`; -/// they use not-ABI-locked wording and surface `verified_with` when present. +/// Plugins get a hard evaluated verdict. Modules use not-ABI-locked wording. +/// Scripts and completions are install-only until activation contracts land. fn format_row_status( pkg_type: &PackageType, compatible: bool, @@ -159,7 +159,17 @@ fn format_row_status( String::new() } } - _ => { + PackageType::Script | PackageType::Completion => { + if verified_with.is_empty() { + " [install-only; activation deferred]".to_string() + } else { + format!( + " [install-only; activation deferred; verified with {}]", + verified_with.join(", ") + ) + } + } + PackageType::Module => { if verified_with.is_empty() { " [not ABI-locked]".to_string() } else { @@ -297,11 +307,13 @@ mod tests { } #[test] - fn script_and_completion_use_module_style_labels() { + fn script_and_completion_use_install_only_labels() { for ty in [PackageType::Script, PackageType::Completion] { let status = format_row_status(&ty, true, true, None, &["0.113.1".to_string()]); - assert!(status.contains("not ABI-locked"), "{ty}"); + assert!(status.contains("install-only"), "{ty}"); + assert!(status.contains("activation deferred"), "{ty}"); assert!(!status.contains("[compatible]"), "{ty}"); + assert!(!status.contains("not ABI-locked"), "{ty}"); } } diff --git a/src/cmd/try_cmd.rs b/src/cmd/try_cmd.rs index 2d943ea5..4d4866cf 100644 --- a/src/cmd/try_cmd.rs +++ b/src/cmd/try_cmd.rs @@ -1,7 +1,3 @@ -use anyhow::{bail, Context, Result}; -use clap::Parser; -use std::path::Path; - use crate::cmd::activate::{self, ActivateArgs}; use crate::cmd::nu_pin_offer; use crate::core::nu_version::NuVersion; @@ -10,8 +6,12 @@ use crate::core::platform::{Os, Platform}; use crate::core::registry::RegistryManager; use crate::core::resolve::Resolver; use crate::install::transaction; +use crate::state::lockfile::Lockfile; use crate::util::fs_safety::acquire_mutation_lock; use crate::util::hints::{self, CMD_REGISTRY_SYNC}; +use anyhow::{bail, Context, Result}; +use clap::Parser; +use std::path::{Path, PathBuf}; /// Install and activate a starter package that fits your current Nu. /// @@ -56,6 +56,17 @@ const STARTERS: &[StarterSpec] = &[ nu_minor: None, os: None, }, + // Nu-agnostic install-only fallbacks (prefer activatable starters above). + StarterSpec { + id: "SuaveIV/nu_script_wttr", + nu_minor: None, + os: None, + }, + StarterSpec { + id: "Sanceilaks/nufetch", + nu_minor: None, + os: None, + }, ]; pub fn execute(args: &TryArgs, root: &Path) -> Result<()> { @@ -144,8 +155,24 @@ pub fn execute(args: &TryArgs, root: &Path) -> Result<()> { transaction::install_package(&package_id, None, &options)?; } - if args.no_activate { - println!("Installed '{package_id}' (not activated). Run `numan activate {package_id}`."); + let selected = packages.iter().find(|p| p.id.to_string() == package_id); + let install_only = selected + .map(|p| { + matches!( + p.package_type, + PackageType::Script | PackageType::Completion + ) + }) + .unwrap_or(false); + + if args.no_activate || install_only { + if install_only { + print_install_only_hint(root, &package_id, selected); + } else { + println!( + "Installed '{package_id}' (not activated). Run `numan activate {package_id}`." + ); + } return Ok(()); } @@ -177,13 +204,17 @@ enum StarterSelection { }, } +fn package_is_activatable(pkg: &Package) -> bool { + matches!(pkg.package_type, PackageType::Plugin | PackageType::Module) +} + fn select_starter( packages: &[Package], resolver: &Resolver<'_>, platform: &Platform, nu: &NuVersion, ) -> StarterSelection { - // 1. Curated starters that match OS + Nu minor and are compatible. + // 1. Curated activatable starters that match OS + Nu minor and are compatible. for spec in STARTERS { if let Some(os) = spec.os { if platform.os != os { @@ -196,13 +227,27 @@ fn select_starter( } } if let Some(pkg) = packages.iter().find(|p| p.id.to_string() == spec.id) { - if resolver.has_compatible_version(pkg) { + if package_is_activatable(pkg) && resolver.has_compatible_version(pkg) { + return StarterSelection::Compatible(spec.id.to_string()); + } + } + } + + // 2. Any curated activatable starter that is compatible regardless of Nu minor. + for spec in STARTERS { + if let Some(os) = spec.os { + if platform.os != os { + continue; + } + } + if let Some(pkg) = packages.iter().find(|p| p.id.to_string() == spec.id) { + if package_is_activatable(pkg) && resolver.has_compatible_version(pkg) { return StarterSelection::Compatible(spec.id.to_string()); } } } - // 2. Any curated starter that is compatible regardless of Nu minor table miss. + // 3. Nu-agnostic install-only script/completion starters (never pin-offer). for spec in STARTERS { if let Some(os) = spec.os { if platform.os != os { @@ -210,13 +255,13 @@ fn select_starter( } } if let Some(pkg) = packages.iter().find(|p| p.id.to_string() == spec.id) { - if resolver.has_compatible_version(pkg) { + if !package_is_activatable(pkg) && resolver.has_compatible_version(pkg) { return StarterSelection::Compatible(spec.id.to_string()); } } } - // 3. Curated starter with a suggested Nu pin (prefer skim / Windows semver / nutest). + // 4. Curated activatable starter with a suggested Nu pin. let mut suggested_pin = None; for spec in STARTERS { if let Some(os) = spec.os { @@ -225,6 +270,9 @@ fn select_starter( } } if let Some(pkg) = packages.iter().find(|p| p.id.to_string() == spec.id) { + if !package_is_activatable(pkg) { + continue; + } let diagnosis = resolver.diagnose_package(pkg); if nu_pin_offer::is_nu_mismatch(&diagnosis) && diagnosis.suggested_pin.is_some() { return StarterSelection::NeedsPin { @@ -232,7 +280,6 @@ fn select_starter( diagnosis, }; } - // Remember first pin discovered for None fallback. if suggested_pin.is_none() && nu_pin_offer::is_nu_mismatch(&diagnosis) { suggested_pin = diagnosis.suggested_pin; } @@ -282,6 +329,29 @@ fn print_usage_hint(package_id: &str, packages: &[Package]) { } } +fn print_install_only_hint(root: &Path, package_id: &str, pkg: Option<&Package>) { + println!("Installed '{package_id}' (install-only; activation deferred)."); + let entry = pkg + .and_then(|p| p.versions.last()) + .and_then(|v| v.artifact.entry.as_deref()); + let payload = Lockfile::load(root) + .ok() + .and_then(|lf| lf.packages.get(package_id).map(|e| e.payload_path.clone())); + match (payload, entry) { + (Some(rel), Some(entry_name)) => { + let full: PathBuf = root.join(rel).join(entry_name); + println!("In Nu: overlay use {}", full.display()); + } + (Some(rel), None) => { + let full: PathBuf = root.join(rel); + println!("Installed under {}", full.display()); + } + _ => { + println!("Use `numan list` to find the installed payload path."); + } + } +} + fn detect_nu(root: &Path) -> Result { NuVersion::from_paths_or_detect(root) .context("Could not detect Nu version. Run `numan init` first.") @@ -310,6 +380,18 @@ mod tests { } fn pkg(id: &str, constraint: &str, plugin: bool) -> Package { + pkg_typed( + id, + constraint, + if plugin { + PackageType::Plugin + } else { + PackageType::Module + }, + ) + } + + fn pkg_typed(id: &str, constraint: &str, package_type: PackageType) -> Package { let (owner, name) = id.split_once('/').unwrap(); let mut targets = HashMap::new(); targets.insert( @@ -328,39 +410,40 @@ mod tests { executable_path: "p".to_string(), }, ); + let is_plugin = package_type == PackageType::Plugin; Package { id: ScopedId::new(owner, name), description: "d".to_string(), repo: "https://example.com".to_string(), - package_type: if plugin { - PackageType::Plugin - } else { - PackageType::Module - }, + package_type, tags: vec![], versions: vec![VersionEntry { version: semver::Version::new(1, 0, 0), nu_version: constraint.to_string(), verified_with: vec!["0.113.1".to_string()], artifact: Artifact { - kind: if plugin { + kind: if is_plugin { "binary".to_string() } else { "archive".to_string() }, - url: if plugin { + url: if is_plugin { None } else { Some("https://example.com/m.zip".to_string()) }, - sha256: if plugin { None } else { Some("cc".to_string()) }, - targets: if plugin { targets } else { HashMap::new() }, + sha256: if is_plugin { + None + } else { + Some("cc".to_string()) + }, + targets: if is_plugin { targets } else { HashMap::new() }, archive_root: None, include: None, - entry: if plugin { + entry: if is_plugin { None } else { - Some("mod.nu".to_string()) + Some("entry.nu".to_string()) }, }, source: None, @@ -388,6 +471,8 @@ mod tests { pkg("idanarye/nu_plugin_skim", ">=0.114.0 <0.115.0", true), pkg("vyadh/nutest", ">=0.103.0", false), pkg("abusch/nu_plugin_semver", ">=0.113.0 <0.114.0", true), + pkg_typed("SuaveIV/nu_script_wttr", "*", PackageType::Script), + pkg_typed("Sanceilaks/nufetch", "*", PackageType::Script), ]; match select_starter(&packages, &resolver, &platform, &nu) { StarterSelection::Compatible(id) => assert_eq!(id, "idanarye/nu_plugin_skim"), @@ -425,6 +510,37 @@ mod tests { } } + #[test] + fn select_starter_falls_back_to_script_before_pin_offer() { + let platform = windows_platform(); + let nu = NuVersion::parse("0.114.1").unwrap(); + let resolver = Resolver::new(&platform, &nu); + let packages = vec![ + pkg("abusch/nu_plugin_semver", ">=0.113.0 <0.114.0", true), + pkg_typed("SuaveIV/nu_script_wttr", "*", PackageType::Script), + pkg_typed("Sanceilaks/nufetch", "*", PackageType::Script), + ]; + match select_starter(&packages, &resolver, &platform, &nu) { + StarterSelection::Compatible(id) => assert_eq!(id, "SuaveIV/nu_script_wttr"), + other => panic!("unexpected selection: {other:?}"), + } + } + + #[test] + fn select_starter_prefers_wttr_when_only_scripts_present() { + let platform = windows_platform(); + let nu = NuVersion::parse("0.115.0").unwrap(); + let resolver = Resolver::new(&platform, &nu); + let packages = vec![ + pkg_typed("Sanceilaks/nufetch", "*", PackageType::Script), + pkg_typed("SuaveIV/nu_script_wttr", "*", PackageType::Script), + ]; + match select_starter(&packages, &resolver, &platform, &nu) { + StarterSelection::Compatible(id) => assert_eq!(id, "SuaveIV/nu_script_wttr"), + other => panic!("unexpected selection: {other:?}"), + } + } + #[test] fn format_no_compatible_starter_with_pin_is_honest() { let msg = From 2fdd2d31b2749d431dbd29aca014fa2b891cd548 Mon Sep 17 00:00:00 2001 From: "qodo-code-review[bot]" <151058649+qodo-code-review[bot]@users.noreply.github.com> Date: Fri, 7 Aug 2026 06:54:10 +0000 Subject: [PATCH 2/3] =?UTF-8?q?fix:=202=20findings=20=E2=80=94=20Use=20ins?= =?UTF-8?q?talled=20lockfile=20entry=20metadata;=20Surface=20lockfil?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use installed lockfile entry metadata - Surface lockfile loading errors --- src/cmd/try_cmd.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/cmd/try_cmd.rs b/src/cmd/try_cmd.rs index 4d4866cf..b7b706b8 100644 --- a/src/cmd/try_cmd.rs +++ b/src/cmd/try_cmd.rs @@ -167,7 +167,7 @@ pub fn execute(args: &TryArgs, root: &Path) -> Result<()> { if args.no_activate || install_only { if install_only { - print_install_only_hint(root, &package_id, selected); + print_install_only_hint(root, &package_id)?; } else { println!( "Installed '{package_id}' (not activated). Run `numan activate {package_id}`." @@ -329,14 +329,15 @@ fn print_usage_hint(package_id: &str, packages: &[Package]) { } } -fn print_install_only_hint(root: &Path, package_id: &str, pkg: Option<&Package>) { +fn print_install_only_hint(root: &Path, package_id: &str) -> Result<()> { println!("Installed '{package_id}' (install-only; activation deferred)."); - let entry = pkg - .and_then(|p| p.versions.last()) - .and_then(|v| v.artifact.entry.as_deref()); - let payload = Lockfile::load(root) - .ok() - .and_then(|lf| lf.packages.get(package_id).map(|e| e.payload_path.clone())); + let lockfile = Lockfile::load(root) + .with_context(|| format!("Failed to load lockfile for installed package '{package_id}'"))?; + let (payload, entry) = lockfile + .packages + .get(package_id) + .map(|e| (Some(e.payload_path.clone()), e.entry.as_deref())) + .unwrap_or((None, None)); match (payload, entry) { (Some(rel), Some(entry_name)) => { let full: PathBuf = root.join(rel).join(entry_name); @@ -350,6 +351,7 @@ fn print_install_only_hint(root: &Path, package_id: &str, pkg: Option<&Package>) println!("Use `numan list` to find the installed payload path."); } } + Ok(()) } fn detect_nu(root: &Path) -> Result { From 467d80baaeac01402a2773146518df2d45248cb4 Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Fri, 7 Aug 2026 00:17:09 -0700 Subject: [PATCH 3/3] Harden install-only try hints and search honesty. Address PR review: require lockfile records, quote Nu overlay paths, label scripts when Nu is unknown, and cover completion info. Co-authored-by: Cursor --- src/cmd/info.rs | 9 ++++ src/cmd/search.rs | 9 +++- src/cmd/try_cmd.rs | 121 +++++++++++++++++++++++++++++++++++++++------ 3 files changed, 123 insertions(+), 16 deletions(-) diff --git a/src/cmd/info.rs b/src/cmd/info.rs index 01c999d3..a6d27c8c 100644 --- a/src/cmd/info.rs +++ b/src/cmd/info.rs @@ -174,6 +174,15 @@ mod tests { assert!(out.contains("Type: script"), "{out}"); } + #[test] + fn format_info_marks_completion_install_only() { + let mut pkg = sample_plugin(false); + pkg.package_type = PackageType::Completion; + let out = format_info(&pkg, &linux_platform(), None); + assert!(out.contains("Activation: install-only"), "{out}"); + assert!(out.contains("Type: completion"), "{out}"); + } + #[test] fn format_info_prints_source_when_present() { let pkg = sample_plugin(true); diff --git a/src/cmd/search.rs b/src/cmd/search.rs index a727df4e..ac3d9bb4 100644 --- a/src/cmd/search.rs +++ b/src/cmd/search.rs @@ -82,6 +82,8 @@ pub fn execute(args: &SearchArgs, root: &Path) -> Result<()> { // Status when Nu is known and the row needs a verdict: incompatibles, // --all listings, and all non-plugin compatible rows (asymmetric label). + // Install-only labels do not depend on Nu detection, so script/completion + // rows stay labeled even when the resolver is absent. let status = if resolver.is_some() { format_row_status( &pkg.package_type, @@ -91,7 +93,12 @@ pub fn execute(args: &SearchArgs, root: &Path) -> Result<()> { verified_with, ) } else { - String::new() + match pkg.package_type { + PackageType::Script | PackageType::Completion => { + format_row_status(&pkg.package_type, true, args.all, None, verified_with) + } + PackageType::Plugin | PackageType::Module => String::new(), + } }; println!( diff --git a/src/cmd/try_cmd.rs b/src/cmd/try_cmd.rs index b7b706b8..46eb806a 100644 --- a/src/cmd/try_cmd.rs +++ b/src/cmd/try_cmd.rs @@ -329,27 +329,32 @@ fn print_usage_hint(package_id: &str, packages: &[Package]) { } } +/// Nu `overlay use` hint with the same path-literal escaping as +/// [`crate::nu::autoload::render_use_statement`]. +fn format_overlay_use_hint(path: &Path) -> Result { + let path_str = path + .to_str() + .with_context(|| format!("Installed path '{}' is not valid UTF-8", path.display()))?; + let escaped = path_str.replace('\\', "\\\\").replace('"', "\\\""); + Ok(format!("overlay use \"{escaped}\"")) +} + fn print_install_only_hint(root: &Path, package_id: &str) -> Result<()> { - println!("Installed '{package_id}' (install-only; activation deferred)."); let lockfile = Lockfile::load(root) .with_context(|| format!("Failed to load lockfile for installed package '{package_id}'"))?; - let (payload, entry) = lockfile - .packages - .get(package_id) - .map(|e| (Some(e.payload_path.clone()), e.entry.as_deref())) - .unwrap_or((None, None)); - match (payload, entry) { - (Some(rel), Some(entry_name)) => { - let full: PathBuf = root.join(rel).join(entry_name); - println!("In Nu: overlay use {}", full.display()); + let installed = lockfile.packages.get(package_id).with_context(|| { + format!("Installed '{package_id}' but no lockfile record was found; refuse usage hint") + })?; + println!("Installed '{package_id}' (install-only; activation deferred)."); + match installed.entry.as_deref() { + Some(entry_name) => { + let full: PathBuf = root.join(&installed.payload_path).join(entry_name); + println!("In Nu: {}", format_overlay_use_hint(&full)?); } - (Some(rel), None) => { - let full: PathBuf = root.join(rel); + None => { + let full: PathBuf = root.join(&installed.payload_path); println!("Installed under {}", full.display()); } - _ => { - println!("Use `numan list` to find the installed payload path."); - } } Ok(()) } @@ -543,6 +548,92 @@ mod tests { } } + fn script_lock_entry( + payload_path: &str, + entry: Option<&str>, + ) -> crate::state::lockfile::LockfileEntry { + crate::state::lockfile::LockfileEntry { + version: "0.1.0".to_string(), + package_type: "script".to_string(), + source: "registry".to_string(), + target: None, + artifact_url: None, + artifact_sha256: None, + executable_path: None, + archive_root: None, + include: None, + entry: entry.map(str::to_string), + installed_at: "now".to_string(), + nu_version_at_install: None, + activation: None, + registry_url: None, + registry_revision: None, + index_sha256: None, + signing_key_fingerprint: None, + git_url: None, + git_rev: None, + cargo_name: None, + cargo_lock_sha256: None, + built_sha256: None, + payload_path: payload_path.to_string(), + revision_id: None, + payload_sha256: None, + executable_sha256: None, + selection_reason: None, + origin: None, + module_activation: None, + module_import_mode: None, + locked_dependencies: BTreeMap::new(), + } + } + + #[test] + fn format_overlay_use_hint_quotes_and_escapes_path() { + let path = PathBuf::from(r#"C:\Numan Root\pkg\wttr.nu"#); + let hint = format_overlay_use_hint(&path).unwrap(); + assert_eq!(hint, r#"overlay use "C:\\Numan Root\\pkg\\wttr.nu""#); + } + + #[test] + fn print_install_only_hint_uses_lockfile_entry_and_quotes_path() { + let root = tempfile::tempdir().unwrap(); + let mut lock = Lockfile::empty(); + lock.packages.insert( + "SuaveIV/nu_script_wttr".to_string(), + script_lock_entry( + "packages/scripts/SuaveIV/nu_script_wttr/0.1.0-deadbeef", + Some("wttr.nu"), + ), + ); + lock.save(root.path()).unwrap(); + + let err = print_install_only_hint(root.path(), "missing/pkg").unwrap_err(); + assert!( + err.to_string().contains("no lockfile record"), + "missing record must fail: {err:#}" + ); + + print_install_only_hint(root.path(), "SuaveIV/nu_script_wttr").unwrap(); + let full = root + .path() + .join("packages/scripts/SuaveIV/nu_script_wttr/0.1.0-deadbeef/wttr.nu"); + let expected = format_overlay_use_hint(&full).unwrap(); + assert!(expected.starts_with("overlay use \"")); + assert!(expected.contains("wttr.nu")); + } + + #[test] + fn print_install_only_hint_rejects_malformed_lockfile() { + let root = tempfile::tempdir().unwrap(); + std::fs::write(root.path().join("lockfile"), "{not-json").unwrap(); + let err = print_install_only_hint(root.path(), "any/pkg").unwrap_err(); + let msg = format!("{err:#}"); + assert!( + msg.contains("Failed to load lockfile") || msg.contains("expected"), + "malformed lockfile must surface: {msg}" + ); + } + #[test] fn format_no_compatible_starter_with_pin_is_honest() { let msg =