diff --git a/src/cmd/activation_switch.rs b/src/cmd/activation_switch.rs index f15b9dc..232ca43 100644 --- a/src/cmd/activation_switch.rs +++ b/src/cmd/activation_switch.rs @@ -617,7 +617,7 @@ pub fn sync_profile_after_user_deactivate( } /// Keep `nu_state/paths.json` aligned with the newly selected active Nu. -fn refresh_cached_nu_paths_after_switch(root: &Path) -> Result<()> { +pub(crate) fn refresh_cached_nu_paths_after_switch(root: &Path) -> Result<()> { let paths_file = root.join("nu_state").join("paths.json"); if !paths_file.is_file() { return Ok(()); diff --git a/src/cmd/doctor.rs b/src/cmd/doctor.rs index 20d050b..2299459 100644 --- a/src/cmd/doctor.rs +++ b/src/cmd/doctor.rs @@ -18,7 +18,7 @@ use crate::nu::bootstrap::managed_nu_binary; use crate::nu::paths::{ discover_nu_off_path, find_nu_executable_with_root, find_nu_on_path, NuPaths, }; -use crate::nu::version_manager; +use crate::nu::version_manager::{self, VersionManagerError}; use crate::nupm_compat::NupmCompatibility; use crate::nupm_compat::{ count_drifted_imports, resolve_nupm_home, scan_nupm_home, NupmHomeResolution, @@ -517,23 +517,60 @@ fn check_nu_environments(root: &Path, options: &DoctorOptions, findings: &mut Ve None, RepairTier::None, )), - Err(e) => findings.push(finding( + Err(e) => findings.push(managed_nu_resolve_finding(e)), + } +} + +/// Map managed-Nu resolution failures to actionable doctor findings. +/// +/// `CMD_USE` is only appropriate for a dangling active-version marker. +/// Filesystem scan / marker parse failures need a different message and no +/// `numan use` hint. +fn managed_nu_resolve_finding(err: VersionManagerError) -> Finding { + match &err { + VersionManagerError::DanglingActive { .. } + | VersionManagerError::DanglingActiveWithOffTree { .. } => finding( "nu.managed.version", Severity::Warn, - format!("Managed Nu: could not resolve active managed binary ({e})"), + format!("Managed Nu: could not resolve active managed binary ({err})"), Some(CMD_USE), RepairTier::Manual, - )), + ), + VersionManagerError::ReadVersionsDir { .. } + | VersionManagerError::ReadLegacyVersion { .. } + | VersionManagerError::ReadMarker { .. } + | VersionManagerError::MalformedMarker { .. } + | VersionManagerError::InvalidVersion { .. } => finding( + "nu.managed.version", + Severity::Warn, + format!( + "Managed Nu: could not resolve managed Nu binary ({err}). \ + Check permissions on tools/nushell and nu_state, or reinstall with \ + `{CMD_SETUP_NU}`." + ), + None, + RepairTier::Manual, + ), + _ => finding( + "nu.managed.version", + Severity::Warn, + format!("Managed Nu: could not resolve managed Nu binary ({err})"), + None, + RepairTier::Manual, + ), } } -fn resolve_managed_nu_binary(root: &Path) -> Result> { +/// Resolve the managed Nu binary for doctor reporting. +/// +/// Prefers the versioned active install (`tools/nushell//nu`), then +/// falls back to the legacy single-binary path (`tools/nushell/nu`) so older +/// roots still report correctly before migration. +fn resolve_managed_nu_binary(root: &Path) -> Result, VersionManagerError> { match version_manager::active_nu_binary(root) { Ok(Some(path)) => return Ok(Some(path)), Ok(None) => {} - Err(e) => { - return Err(e).with_context(|| "Failed to resolve active managed Nu binary"); - } + Err(e) => return Err(e), } let legacy = managed_nu_binary(root); @@ -541,9 +578,7 @@ fn resolve_managed_nu_binary(root: &Path) -> Result> { return Ok(Some(legacy)); } - if let Some(latest) = version_manager::latest_installed_version(root) - .with_context(|| "Failed to list installed managed Nu versions")? - { + if let Some(latest) = version_manager::latest_installed_version(root)? { let path = version_manager::version_binary(root, &latest); if path.is_file() { return Ok(Some(path)); @@ -2401,6 +2436,12 @@ mod tests { .expect("nu.managed.version"); assert_eq!(managed.severity, Severity::Warn); assert_eq!(managed.repair, RepairTier::Manual); + assert_eq!(managed.fix.as_deref(), Some(CMD_USE)); + assert!( + managed.message.contains("active managed binary"), + "dangling active should keep active-binary wording: {}", + managed.message + ); assert!( !managed.message.contains("not installed"), "should not report 'not installed' when marker exists: {}", @@ -2408,6 +2449,52 @@ mod tests { ); } + #[test] + fn doctor_reports_managed_nu_scan_failure_without_use_hint() { + let dir = TempDir::new().unwrap(); + let root = dir.path(); + // Make tools/nushell a file so read_dir fails with ReadVersionsDir. + let tools = root.join("tools"); + std::fs::create_dir_all(&tools).unwrap(); + std::fs::write(tools.join("nushell"), b"not-a-directory").unwrap(); + + let report = run_checks_with_options( + &DoctorArgs { + scan: true, + json: true, + nupm_home: None, + }, + root, + &test_doctor_options(), + ) + .unwrap(); + + let managed = report + .findings + .iter() + .find(|f| f.id == "nu.managed.version") + .expect("nu.managed.version"); + assert_eq!(managed.severity, Severity::Warn); + assert_eq!(managed.repair, RepairTier::Manual); + assert!( + managed.fix.is_none(), + "filesystem scan failures must not hint `{CMD_USE}`: {:?}", + managed.fix + ); + assert!( + managed + .message + .contains("could not resolve managed Nu binary"), + "expected generic resolve wording: {}", + managed.message + ); + assert!( + !managed.message.contains("active managed binary"), + "scan failure must not use dangling-active wording: {}", + managed.message + ); + } + #[test] fn doctor_reports_managed_nu_not_installed() { let dir = TempDir::new().unwrap(); diff --git a/src/cmd/registry.rs b/src/cmd/registry.rs index 01ead5b..a4ce35a 100644 --- a/src/cmd/registry.rs +++ b/src/cmd/registry.rs @@ -207,6 +207,7 @@ fn list_packages(root: &Path) -> Result<()> { Ok(()) } +/// Convert terminal column count to usable description width. fn terminal_cols_to_description_width(terminal_cols: usize) -> usize { let available = terminal_cols.saturating_sub(4); if available >= 40 { @@ -216,6 +217,7 @@ fn terminal_cols_to_description_width(terminal_cols: usize) -> usize { } } +/// Usable width for indented package descriptions (leave room for ` ` prefix). fn package_description_width() -> usize { let cols = console::Term::stdout() .size_checked() @@ -224,6 +226,7 @@ fn package_description_width() -> usize { terminal_cols_to_description_width(cols) } +/// Soft-wrap `text` on whitespace so the terminal does not split mid-word. fn wrap_words(text: &str, width: usize) -> Vec { if width == 0 { return vec![text.to_string()]; diff --git a/src/cmd/remove.rs b/src/cmd/remove.rs index 3059fc9..b9dec65 100644 --- a/src/cmd/remove.rs +++ b/src/cmd/remove.rs @@ -131,6 +131,19 @@ fn execute_with_tty(args: &RemoveArgs, root: &Path, is_tty: bool) -> Result<()> }; journal.save(root)?; + // Clear desire before any destructive change so a profile-write failure + // aborts the remove while the lockfile and payload are still intact and + // the user can retry. A leftover profile entry would make later + // `numan use` restore attempts target a removed package. + crate::state::activation_profile::remove_from_all_minors(root, &args.package).with_context( + || { + format!( + "Failed to clear activation profile entries for '{}'", + args.package + ) + }, + )?; + // Remove from lockfile (atomic write). lockfile.packages.remove(&args.package); lockfile.save(root)?; @@ -159,20 +172,7 @@ fn execute_with_tty(args: &RemoveArgs, root: &Path, is_tty: bool) -> Result<()> } } - // Clear desire while the remove journal is still present. Failure here - // must not report success: a leftover profile entry would make later - // `numan use` restore attempts target a removed package. - crate::state::activation_profile::remove_from_all_minors(root, &args.package).with_context( - || { - format!( - "Failed to clear activation profile entries for '{}'", - args.package - ) - }, - )?; - PendingLifecycle::clear(root)?; - println!("{} Removed {}", console::style("✓").green(), args.package); Ok(()) diff --git a/src/cmd/use_cmd.rs b/src/cmd/use_cmd.rs index cb892a8..1e804db 100644 --- a/src/cmd/use_cmd.rs +++ b/src/cmd/use_cmd.rs @@ -144,6 +144,10 @@ fn execute_latest( if let Some(path) = existing.binary_path.as_deref() { if !std::path::Path::new(path).is_file() { version_manager::write_active_version(root, &version)?; + // The cached paths.json still names the now-missing off-tree + // binary; reconcile_target_profile would load it and fail + // validate_drift after the marker has already changed. + activation_switch::refresh_cached_nu_paths_after_switch(root)?; } } } @@ -369,6 +373,50 @@ mod tests { ); } + #[test] + fn test_use_latest_self_heal_clears_stale_paths_cache() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + create_fake_version(root, "0.113.1"); + version_manager::write_active_version_with_binary( + root, + "0.113.1", + std::path::Path::new("/nonexistent/nu"), + ) + .unwrap(); + + // Seed a paths.json that still points at the now-missing off-tree + // binary. Without a refresh after the marker repair, the same-target + // reconcile would load this cache and fail validate_drift. + let stale = NuPaths { + nu_executable: "/nonexistent/nu".to_string(), + nu_version: "0.113.1".to_string(), + plugin_registry_path: root.join("plugins.msgpackz").to_string_lossy().into_owned(), + nu_executable_hash: "deadbeef".to_string(), + platform: "test".to_string(), + data_dir: None, + vendor_autoload_dirs: vec![], + vendor_autoload_dir: None, + }; + stale.save(root).unwrap(); + assert!(root.join("nu_state/paths.json").is_file()); + + execute( + &UseArgs { + version: "latest".to_string(), + }, + root, + ) + .unwrap(); + + // The stale cache must be cleared so the repaired marker is the source + // of truth, not the dangling off-tree path. + assert!( + !root.join("nu_state/paths.json").exists(), + "stale paths.json must be cleared after dangling binary_path repair" + ); + } + #[test] fn test_use_latest_preserves_live_offtree_binary_path() { let tmp = TempDir::new().unwrap();