diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b4e9cc..0d1b6da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **First-class script package activation (#14)**: script packages whose registry entry declares `activation: {kind: "nu-script", entry_points: [{name, path}, ...]}` can now be activated with `numan activate`. Each declared name becomes a command in the Numan-managed autoload file that runs the package-relative `.nu` target under the activating Nu executable, forwarding arguments, stdout/stderr, and exit status (pipeline input is not forwarded; see the design spec). Command names are validated, collisions across active script packages are rejected (never shadowed), and install remains inert — package code runs only when a user invokes a declared command. Scripts without a declaration stay install-only; `numan info` shows declared entry points before activation. +- **First-class completion package activation (#15)**: completion packages whose registry entry declares `activation: {kind: "nu-completion", targets: [{command, path, kind}, ...]}` can now be activated with `numan activate`. Each declared target is composed into the Numan-managed autoload file as a `use "" *` contribution under an ownership comment; target ownership is exclusive across active completion packages (conflicts are rejected, never shadowed), and install remains inert — completion providers are never invoked at install or activation time, and `command-backed` providers run only when Nushell requests a completion. `numan info` shows declared targets and flags `command-backed` targets as able to execute code during completion. Completion packages without a declaration stay install-only. This is distinct from `numan completions `, which generates shell completions for Numan itself. + ## [0.3.0] - 2026-10-24 ### Added diff --git a/Cargo.lock b/Cargo.lock index 479b250..d74c163 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1481,9 +1481,9 @@ dependencies = [ [[package]] name = "libz-sys" -version = "1.1.29" +version = "1.1.31" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85bc9657773828b90eeb625adff10eeac83cc21bbfd8e23a03eaa8a33c9e28d9" +checksum = "ff20308ecabd635a2dc1dd5834a86c462cca20efba1d5d3ddab1f8aea3455cb2" dependencies = [ "cc", "libc", diff --git a/README.md b/README.md index 9e33e61..bdaeb1c 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ These workflows are covered by unit tests, hermetic integration tests, and real- - **Registry-backed installs**: Search packages, inspect available versions, and install `owner/name` or `owner/name@version`. - **Official registry**: `numan init` configures the `official` registry automatically. Its production trust root is built into numan, and `numan registry sync` verifies every signed index. -- **Package types**: Plugins and modules support `numan activate`. Hook profiles require explicit `numan hook enable`. Scripts and completion packages are currently install-only. +- **Package types**: Plugins, modules, scripts, and completion packages support `numan activate`. Hook profiles require explicit `numan hook enable`. Packages without a supported activation surface are install-only. - **Verified artifacts**: Plugin binaries require SHA-256 hashes, and registry indexes require valid Ed25519 signatures. - **Scoped activation**: Plugins remain active only while the Nushell executable hash, Nushell version, and plugin registry path match the recorded activation state. - **Module autoloads**: numan writes managed vendor autoload files with ownership markers and validates candidate files before promotion. @@ -62,12 +62,18 @@ These workflows are covered by unit tests, hermetic integration tests, and real- | Module | Yes | `numan activate` (managed vendor autoload) | Supported | | Overlay module | Yes | `numan overlay use` / `hide` (interactive Nu wrapper) | Explicit activation in the current shell; applied at the next prompt | | Hook profile | Yes | `numan hook enable` (owned vendor autoload) | Explicit opt-in; handlers run only on declared interactive events | -| Script | Yes | None | Install-only; activation is deferred | -| Completion package | Yes | None | Install-only; activation is deferred | - -Install-only packages remain inert: numan downloads, verifies, locks, lists, -removes, and garbage-collects their payloads, but does not execute them or -modify Nu configuration for them. Hook packages are also inert until explicitly +| Script | Yes | `numan activate` (managed vendor autoload) | Supported | +| Completion package | Yes | `numan activate` (managed vendor autoload) | Supported | + +Install remains inert for every package type: installation only downloads, +verifies, extracts, and locks payloads; nothing is executed or wired into +Nu until an explicit activation command. Listing, removal, and garbage +collection are separate lifecycle commands. Packages +whose registry entry declares no activation surface stay install-only. +Completion packages declare their targets (`activation: {kind: "nu-completion", +targets: [{command, path, kind}, ...]}`); a target is owned exclusively by one +active package, and `command-backed` providers run only when Nushell requests +a completion, never at install or activation time. Hook packages are also inert until explicitly enabled; inspect their declared events with `numan info` first. Hook changes load in new Nushell sessions. This is separate from numan's own shell completion installer: `numan completions ` is supported for bash, fish, @@ -362,8 +368,8 @@ Global flag: `--root ` — override the numan root directory (all commands | `numan info ` | Show package metadata and available versions | | `numan install ` | Download, verify, extract, and lock | | `numan list` | List installed packages and activation status | -| `numan activate [pkg...]` | Register plugins / write module autoloads (scripts and completion packages are deferred) | -| `numan deactivate [pkg...]` | Remove module autoload entries | +| `numan activate [pkg...]` | Register plugins / write managed autoloads (modules, scripts, completion packages) | +| `numan deactivate [pkg...]` | Remove managed autoload entries (modules, scripts, completion packages) / unregister plugins | | `numan hook enable [pkg...]` | Explicitly enable declared Nushell hook bindings | | `numan hook disable [pkg...]` | Disable hook bindings; no IDs disables all | | `numan hook list` | Show active hook bindings | diff --git a/docs/specs/2026-10-29-completion-activation-design.md b/docs/specs/2026-10-29-completion-activation-design.md new file mode 100644 index 0000000..fa7d412 --- /dev/null +++ b/docs/specs/2026-10-29-completion-activation-design.md @@ -0,0 +1,91 @@ +# Completion package activation (issue #15) + +## Contract + +- A completion package declares its targets in its registry activation spec: + `activation: {kind: "nu-completion", targets: [{command, path, kind}, ...]}`. + `command` is the external command surface the package augments (a single + command name, e.g. `git`); `path` is a package-relative `.nu` source file + contributing that command's completions; `kind` is the declared completion + kind, `"static"` or `"command-backed"`. At least one target is required. +- Command names use the same small grammar as script command names (ASCII + letter or `_`, then letters, digits, `_`, `-`) because the target name is + emitted into the generated managed file's ownership comments, and a + declared target is an ownership claim, not free text. Duplicate commands + or duplicate paths within a package are rejected, as are Nushell reserved + keywords. +- `kind` is declared, never inferred. `"command-backed"` means the source + defines completer functions that Nushell will execute when it requests a + completion; `"static"` means completions are literal values in the source. + Activation and install never invoke a completer either way. `numan info` + displays every declared target and, for `"command-backed"` targets, states + plainly that the provider can execute code during completion. +- Target ownership is exclusive across active completion packages: if a + command target is already owned by another active completion package, + activation of the second package is rejected. Numan never silently + replaces or shadows another active package's completion. +- Install persists the declaration in the lockfile (`completion_targets`); + install never executes package code. Activation validates every target + with the same containment/regular-file/`.nu` rules as script entry points. +- Activation composes each target into the Numan-managed autoload file + (`numan.nu`), alongside module `use` statements and script wrappers: + + ```nu + # numan:completion + use "" * + ``` + + The `use ... *` form matches how Nushell custom-completion files are + consumed (they export `extern` definitions plus completer functions). + Rendering is sorted by scoped package ID, then command, so the generated + file is deterministic. +- The managed file goes through the existing candidate-validation, journal, + drift, and recovery machinery; the lockfile ground truth is a per-package + `completion_activation` record with the same Nu-identity fields as + `script_activation` (Nu executable hash, Nu version, vendor autoload dir, + managed file path, resolved targets). +- Deactivation removes exactly the package's `use` lines and clears its + record, leaving other packages' contributions and all user-authored + configuration untouched. Update/remove follow the existing autoload + reconciliation used for script packages. +- `numan init --refresh` re-resolves active completion targets and + regenerates the managed file when the Nu identity changes, matching + script behaviour. + +## Acceptance + +- Fixture coverage for static and command-backed targets, multi-command + packages, cross-package target conflicts, malformed declarations, + traversal attempts, deactivation, interrupted activation (recovery), and + updates, mirroring `tests/script_activation_test.rs`. +- Real-Nu acceptance (ignored by default, run explicitly) proves a clean + interactive session receives a contributed completion after activation + and no longer receives it after deactivation, without modifying user + config files. +- Documentation distinguishes Numan's own shell-completion generator + (`numan completions `) from installable Nushell completion + packages, and the README package-support table lists completion packages + as activatable via `numan activate`. + +## Non-goals + +- Managing Bash/Fish/Zsh/PowerShell completion packages for arbitrary tools. +- Executing completion providers at install, inspect, lock, update, or + activation time. Providers run only when Nushell requests a completion. +- Merging arbitrary user-authored completion files, or detecting conflicts + with completions a user defined outside Numan. Declared targets are + registry-curated; the ownership check covers Numan-activated completion + packages. +- Inferring a target's kind by parsing its source. The declaration is the + contract; a package that mis-declares is a curation failure, surfaced by + `numan info` showing the declared kind before activation. + +## Registry dependency + +This contract is implemented on the client side only. The numan-registry +index schema (`schemas/index-v1.json` in numan-cli/numan-registry) does not +yet permit the `targets` field on an activation spec, so registry packages +cannot declare completion targets until that schema and its validation +path are updated to accept `nu-completion` targets. That schema change is +a tracked numan-registry follow-up; it is not part of the client PR that +implements this spec. diff --git a/src/cmd/activate.rs b/src/cmd/activate.rs index 2aa37f6..0456dff 100644 --- a/src/cmd/activate.rs +++ b/src/cmd/activate.rs @@ -5,7 +5,8 @@ use std::path::{Component, Path, PathBuf}; use crate::nu::autoload::{ generate_managed_content, replace_managed_file, resolve_entry, validate_candidate, - write_candidate, CandidateRunner, NuCandidateRunner, ResolvedScriptCommand, + write_candidate, CandidateRunner, NuCandidateRunner, ResolvedCompletionTarget, + ResolvedScriptCommand, }; use crate::nu::paths::NuPaths; use crate::state::autoload_journal::{sha256_file, SCHEMA_VERSION as AUTOLOAD_SCHEMA_VERSION}; @@ -14,7 +15,8 @@ use crate::state::autoload_recovery::reconcile_pending_autoload; use crate::state::autoload_state::AutoloadState; use crate::state::journal::{PendingActivation, PendingActivationEntry, PendingStatus}; use crate::state::lockfile::{ - Lockfile, ModuleActivation, PluginActivation, ScriptActivation, ScriptActivationCommand, + CompletionActivation, CompletionActivationTarget, Lockfile, ModuleActivation, PluginActivation, + ScriptActivation, ScriptActivationCommand, }; use crate::state::snapshot::{create_snapshot, SnapshotReason, SnapshotTrigger}; use crate::util::format_timestamp; @@ -68,6 +70,14 @@ struct ScriptTarget { entry_points: Vec, } +/// Resolved completion target ready for autoload generation (issue #15). +#[derive(Debug)] +struct CompletionTarget { + package_id: String, + payload_path: String, + targets: Vec, +} + /// `plugin add` Nu program — paths come only from environment variables. /// Never interpolated into the command string. const ADD_PLUGIN: &str = @@ -178,10 +188,14 @@ fn execute_activating( Some(acquire_mutation_lock(root)?) }; - let (plugin_targets, module_targets, script_targets, managed_file_path) = + let (plugin_targets, module_targets, script_targets, completion_targets, managed_file_path) = prepare_activation_targets(args, root, &nu_paths)?; - if plugin_targets.is_empty() && module_targets.is_empty() && script_targets.is_empty() { + if plugin_targets.is_empty() + && module_targets.is_empty() + && script_targets.is_empty() + && completion_targets.is_empty() + { let lockfile = Lockfile::load(root)?; snapshot_before_activate_profile_sync(root, &nu_paths, args, &lockfile)?; sync_user_activate_profile(root, &nu_paths, args, &lockfile)?; @@ -197,6 +211,8 @@ fn execute_activating( print_grouped_consent_table( &plugin_targets, &module_targets, + &script_targets, + &completion_targets, managed_file_path.as_deref(), &nu_paths.plugin_registry_path, ); @@ -209,6 +225,7 @@ fn execute_activating( plugin_targets, module_targets, script_targets, + completion_targets, managed_file_path, ); } @@ -217,15 +234,21 @@ fn execute_activating( print_grouped_consent_table( &plugin_targets, &module_targets, + &script_targets, + &completion_targets, managed_file_path.as_deref(), &nu_paths.plugin_registry_path, ); let _lock = acquire_mutation_lock(root)?; - let (plugin_targets, module_targets, script_targets, managed_file_path) = + let (plugin_targets, module_targets, script_targets, completion_targets, managed_file_path) = prepare_activation_targets(args, root, &nu_paths)?; - if plugin_targets.is_empty() && module_targets.is_empty() && script_targets.is_empty() { + if plugin_targets.is_empty() + && module_targets.is_empty() + && script_targets.is_empty() + && completion_targets.is_empty() + { let lockfile = Lockfile::load(root)?; snapshot_before_activate_profile_sync(root, &nu_paths, args, &lockfile)?; sync_user_activate_profile(root, &nu_paths, args, &lockfile)?; @@ -242,6 +265,7 @@ fn execute_activating( plugin_targets, module_targets, script_targets, + completion_targets, managed_file_path, ) } @@ -255,6 +279,7 @@ fn prepare_activation_targets( Vec, Vec, Vec, + Vec, Option, )> { let mut lockfile = Lockfile::load(root)?; @@ -264,7 +289,11 @@ fn prepare_activation_targets( let plugin_targets = resolve_plugin_targets(args, &lockfile, nu_paths, root)?; let module_targets = resolve_module_targets(args, &lockfile, nu_paths)?; let script_targets = resolve_script_targets(args, &lockfile, nu_paths)?; - let managed_file_path = if !module_targets.is_empty() || !script_targets.is_empty() { + let completion_targets = resolve_completion_targets(args, &lockfile, nu_paths)?; + let managed_file_path = if !module_targets.is_empty() + || !script_targets.is_empty() + || !completion_targets.is_empty() + { Some(resolve_managed_file_path(nu_paths)?) } else { None @@ -273,6 +302,7 @@ fn prepare_activation_targets( plugin_targets, module_targets, script_targets, + completion_targets, managed_file_path, )) } @@ -287,6 +317,7 @@ fn run_activation_lanes( plugin_targets: Vec, module_targets: Vec, script_targets: Vec, + completion_targets: Vec, managed_file_path: Option, ) -> Result<()> { let mut lockfile = Lockfile::load(root)?; @@ -316,7 +347,7 @@ fn run_activation_lanes( } } - if !module_targets.is_empty() || !script_targets.is_empty() { + if !module_targets.is_empty() || !script_targets.is_empty() || !completion_targets.is_empty() { let managed_file_path = managed_file_path.expect("autoload targets require managed file"); let real_runner; let runner_ref: &dyn CandidateRunner = if let Some(r) = runner { @@ -332,6 +363,7 @@ fn run_activation_lanes( &mut lockfile, &module_targets, &script_targets, + &completion_targets, &managed_file_path, runner_ref, Some(snapshot.id.clone()), @@ -382,7 +414,9 @@ fn snapshot_before_activate_profile_sync( let entry = lockfile.packages.get(id)?; let kind = match entry.package_type.as_str() { "plugin" => crate::state::activation_profile::ProfileKind::Plugin, - "module" | "script" => crate::state::activation_profile::ProfileKind::Module, + "module" | "script" | "completion" => { + crate::state::activation_profile::ProfileKind::Module + } _ => return None, }; Some((kind, id.as_str())) @@ -533,6 +567,7 @@ fn run_module_lane( lockfile: &mut Lockfile, targets: &[ModuleTarget], script_targets: &[ScriptTarget], + completion_targets: &[CompletionTarget], managed_file_path: &str, runner: &dyn CandidateRunner, pre_mutation_snapshot_id: Option, @@ -598,6 +633,49 @@ fn run_module_lane( } } + // Resolve all completion targets, checking target ownership against + // every other active or activating completion package: a command + // target is owned exclusively, and Numan never silently replaces + // another package's completion (issue #15). + let mut completion_resolved: Vec = Vec::new(); + let mut completion_claimed: std::collections::BTreeMap = + std::collections::BTreeMap::new(); + for target in completion_targets { + crate::core::package::validate_completion_targets(&target.targets).with_context(|| { + format!("Invalid completion declaration for '{}'", target.package_id) + })?; + for ct in &target.targets { + if let Some(owner) = + completion_claimed.insert(ct.command.clone(), target.package_id.clone()) + { + bail!( + "Completion target '{}' is claimed by both '{}' and '{}'; refusing to shadow", + ct.command, + owner, + target.package_id + ); + } + let resolved = resolve_entry( + root, + &target.payload_path, + &ct.path, + crate::core::package::ModuleImportMode::Module, + &target.package_id, + ) + .with_context(|| { + format!( + "Failed to resolve completion target '{}' for '{}'", + ct.path, target.package_id + ) + })?; + completion_resolved.push(ResolvedCompletionTarget { + scoped_id: target.package_id.clone(), + command: ct.command.clone(), + target_path: resolved.absolute_path, + }); + } + } + // Collect already-active autoload contributors (modules and scripts not // in this activation run) so the generated file includes ALL of them. let currently_active_ids: Vec = AutoloadState::active_autoload_ids_from_lockfile( @@ -664,6 +742,60 @@ fn run_module_lane( } } + // Add already-active completions not in this batch, ownership-checked + // against the batch's claimed commands (mirrors the script carry-in). + for (pkg_id, entry) in &lockfile.packages { + if entry.package_type != "completion" { + continue; + } + if completion_targets.iter().any(|t| &t.package_id == pkg_id) { + continue; + } + if currently_active_ids.contains(pkg_id) { + if let Some(ca) = &entry.completion_activation { + for tgt in &ca.targets { + if !crate::core::package::is_valid_script_command_name(&tgt.command) { + bail!( + "Active completion package '{}' has an invalid stored command name '{}'; deactivate and re-activate it", + pkg_id, + tgt.command + ); + } + if let Some(owner) = completion_claimed.get(&tgt.command) { + bail!( + "Completion target '{}' from active package '{}' collides with '{}'; deactivate one of them first", + tgt.command, + pkg_id, + owner + ); + } + // Re-resolve from the declaration, mirroring scripts: a + // deleted payload file must fail this activation rather + // than regenerate a dangling `use` line. + let resolved = resolve_entry( + root, + &entry.payload_path, + &tgt.path, + crate::core::package::ModuleImportMode::Module, + pkg_id, + ) + .with_context(|| { + format!( + "Active completion target '{}' for '{}' no longer resolves; deactivate and re-activate it", + tgt.path, pkg_id + ) + })?; + completion_claimed.insert(tgt.command.clone(), pkg_id.clone()); + completion_resolved.push(ResolvedCompletionTarget { + scoped_id: pkg_id.clone(), + command: tgt.command.clone(), + target_path: resolved.absolute_path, + }); + } + } + } + } + // Add already-active modules not in this batch to the entry list for (pkg_id, entry) in &lockfile.packages { if entry.package_type != "module" { @@ -687,7 +819,7 @@ fn run_module_lane( } // Step 4: Generate candidate content - let content = generate_managed_content(&entries, &script_commands) + let content = generate_managed_content(&entries, &script_commands, &completion_resolved) .context("Failed to generate autoload content")?; // Step 5: Write candidate file (same directory as managed file) @@ -726,6 +858,7 @@ fn run_module_lane( .iter() .map(|t| t.package_id.clone()) .chain(script_targets.iter().map(|t| t.package_id.clone())) + .chain(completion_targets.iter().map(|t| t.package_id.clone())) .chain(currently_active_ids.iter().cloned()) .collect(); desired_active_ids.sort(); @@ -735,6 +868,7 @@ fn run_module_lane( .iter() .map(|t| t.package_id.clone()) .chain(script_targets.iter().map(|t| t.package_id.clone())) + .chain(completion_targets.iter().map(|t| t.package_id.clone())) .collect(); // Step 5 (Nu validation): Execute candidate with the candidate runner @@ -849,6 +983,36 @@ fn run_module_lane( } } + for target in completion_targets { + if let Some(pkg) = lockfile.packages.get_mut(&target.package_id) { + let resolved_targets = target + .targets + .iter() + .filter_map(|ct| { + completion_resolved + .iter() + .find(|c| c.scoped_id == target.package_id && c.command == ct.command) + .map(|c| CompletionActivationTarget { + command: ct.command.clone(), + path: ct.path.clone(), + target_path: c.target_path.to_string_lossy().into_owned(), + kind: ct.kind, + }) + }) + .collect::>(); + if resolved_targets.len() == target.targets.len() { + pkg.completion_activation = Some(CompletionActivation { + targets: resolved_targets, + vendor_autoload_dir: vendor_autoload_dir.to_string(), + managed_file_path: managed_file_path.to_string(), + nu_executable_sha256: nu_paths.nu_executable_hash.clone(), + nu_version: nu_paths.nu_version.clone(), + activated_at: ts.clone(), + }); + } + } + } + if let Err(e) = lockfile.save(root) { // Lockfile save failed after replacement — preserve the Replaced journal eprintln!( @@ -914,6 +1078,15 @@ fn run_module_lane( names.join(", "), ); } + for target in completion_targets { + let names: Vec<&str> = target.targets.iter().map(|t| t.command.as_str()).collect(); + println!( + "{} Completion {} added to autoload (targets: {}) — takes effect in future Nu sessions", + console::style("✓").green(), + target.package_id, + names.join(", "), + ); + } Ok(false) // lane succeeded } @@ -944,7 +1117,8 @@ pub(crate) fn activate_modules_unlocked( }; let module_targets = resolve_module_targets(&args, &lockfile, &nu_paths)?; let script_targets = resolve_script_targets(&args, &lockfile, &nu_paths)?; - if module_targets.is_empty() && script_targets.is_empty() { + let completion_targets = resolve_completion_targets(&args, &lockfile, &nu_paths)?; + if module_targets.is_empty() && script_targets.is_empty() && completion_targets.is_empty() { return Ok(false); } let managed_file_path = resolve_managed_file_path(&nu_paths)?; @@ -954,6 +1128,7 @@ pub(crate) fn activate_modules_unlocked( &mut lockfile, &module_targets, &script_targets, + &completion_targets, &managed_file_path, runner, pre_mutation_snapshot_id, @@ -1023,12 +1198,24 @@ fn execute_list(lockfile: &Lockfile, nu_paths: &NuPaths) -> Result<()> { "inactive" } } + "completion" => { + if entry.is_completion_active_for( + &nu_paths.nu_executable_hash, + &nu_paths.nu_version, + vendor_dir, + &managed_path, + ) { + "active" + } else { + "inactive" + } + } _ => "n/a", }; let target = match entry.package_type.as_str() { "plugin" => nu_paths.plugin_registry_path.as_str(), - "module" | "script" => vendor_dir, + "module" | "script" | "completion" => vendor_dir, _ => "-", }; @@ -1092,6 +1279,7 @@ fn execute_check( .map(|e| { e.package_type == "module" || (e.package_type == "script" && e.is_script_activatable()) + || (e.package_type == "completion" && e.is_completion_activatable()) }) .unwrap_or(false) }) @@ -1229,6 +1417,82 @@ fn execute_check( continue; } + if entry.package_type == "completion" { + if !entry.is_completion_activatable() { + println!( + "{} {}: completion not activatable — no declared targets or unresolved dependencies", + console::style("✗").red(), + pkg_id + ); + any_issues = true; + continue; + } + + let declared = entry.completion_targets.as_deref().unwrap_or_default(); + let mut completion_ok = true; + if let Err(e) = crate::core::package::validate_completion_targets(declared) { + println!( + "{} {}: invalid completion declaration: {}", + console::style("✗").red(), + pkg_id, + e + ); + completion_ok = false; + } + for ct in declared { + if let Err(e) = resolve_entry( + root, + &entry.payload_path, + &ct.path, + crate::core::package::ModuleImportMode::Module, + pkg_id, + ) { + println!( + "{} {}: completion target '{}' does not resolve: {}", + console::style("✗").red(), + pkg_id, + ct.path, + e + ); + completion_ok = false; + } + } + if let Some(ca) = &entry.completion_activation { + if !entry.is_completion_active_for( + &nu_paths.nu_executable_hash, + &nu_paths.nu_version, + vendor_dir, + &managed_path_str, + ) { + println!( + "{} {}: completion activation record is stale for the current Nu identity", + console::style("✗").red(), + pkg_id + ); + completion_ok = false; + } else if ca.targets.is_empty() { + println!( + "{} {}: completion activation record has no targets", + console::style("✗").red(), + pkg_id + ); + completion_ok = false; + } + } + + if completion_ok { + println!( + "{} {}: completion check passed ({} target(s))", + console::style("✓").green(), + pkg_id, + declared.len() + ); + } else { + any_issues = true; + } + continue; + } + if entry.package_type != "module" { println!( " {}: not a module package (type: {}), skipping check", @@ -1359,9 +1623,8 @@ fn resolve_plugin_targets( continue; } "completion" => { - bail!( - "Package '{pkg_id}' is a completion — completion activation is deferred to a later phase." - ); + // Completion targets are handled in resolve_completion_targets + continue; } other => { bail!("Package '{pkg_id}' has unknown type '{other}' — cannot activate."); @@ -1613,6 +1876,93 @@ fn resolve_script_targets( Ok(targets) } +/// Resolve all completion targets to activate (issue #15). +fn resolve_completion_targets( + args: &ActivateArgs, + lockfile: &Lockfile, + nu_paths: &NuPaths, +) -> Result> { + let vendor_dir = nu_paths.vendor_autoload_dir.as_deref().unwrap_or(""); + let managed_path_str = if vendor_dir.is_empty() { + String::new() + } else { + format!("{vendor_dir}/numan.nu") + }; + + let mut targets = Vec::new(); + if args.packages.is_empty() { + for (id, entry) in &lockfile.packages { + if entry.package_type != "completion" { + continue; + } + if entry.is_completion_active_for( + &nu_paths.nu_executable_hash, + &nu_paths.nu_version, + vendor_dir, + &managed_path_str, + ) { + continue; + } + if !entry.is_completion_activatable() { + continue; + } + targets.push(CompletionTarget { + package_id: id.clone(), + payload_path: entry.payload_path.clone(), + targets: entry.completion_targets.clone().unwrap_or_default(), + }); + } + return Ok(targets); + } + + for pkg_id in &args.packages { + let entry = match lockfile.packages.get(pkg_id) { + Some(e) => e, + None => continue, + }; + if entry.package_type != "completion" { + continue; + } + if entry.is_completion_active_for( + &nu_paths.nu_executable_hash, + &nu_paths.nu_version, + vendor_dir, + &managed_path_str, + ) { + println!( + "{} {} is already active for Nu {} (skipped)", + console::style("✓").green(), + pkg_id, + nu_paths.nu_version, + ); + continue; + } + if !entry.is_completion_activatable() { + if entry + .completion_targets + .as_ref() + .map(|t| !t.is_empty()) + .unwrap_or(false) + { + bail!( + "Package '{pkg_id}' declares completion targets but has locked dependencies \ + that must be resolved before activation." + ); + } + bail!( + "Package '{pkg_id}' is a completion package without declared targets — it is install-only. \ + Completion activation requires the registry entry to declare activation targets (issue #15)." + ); + } + targets.push(CompletionTarget { + package_id: pkg_id.clone(), + payload_path: entry.payload_path.clone(), + targets: entry.completion_targets.clone().unwrap_or_default(), + }); + } + Ok(targets) +} + // ── Journal reconciliation ───────────────────────────────────────────────────── /// Read-only preflight check: bail if any pending journal has a mismatched Nu @@ -1696,10 +2046,13 @@ fn reconcile_plugin_journal( // ── Consent table ────────────────────────────────────────────────────────────── -/// Print the grouped consent table showing plugins and modules grouped by operation. +/// Print the grouped consent table showing plugins, modules, scripts, and +/// completions grouped by operation. fn print_grouped_consent_table( plugins: &[PluginTarget], modules: &[ModuleTarget], + scripts: &[ScriptTarget], + completions: &[CompletionTarget], managed_file: Option<&str>, registry_path: &str, ) { @@ -1721,6 +2074,37 @@ fn print_grouped_consent_table( } println!(); } + + if !scripts.is_empty() { + let target = managed_file.unwrap_or(""); + println!("Script startup autoload"); + for t in scripts { + let names: Vec<&str> = t.entry_points.iter().map(|e| e.name.as_str()).collect(); + println!(" {} ({}) -> {}", t.package_id, names.join(", "), target); + } + println!(); + } + + if !completions.is_empty() { + let target = managed_file.unwrap_or(""); + println!("Completion startup autoload"); + for t in completions { + for ct in &t.targets { + println!( + " {} {} -> {} [{}]", + t.package_id, ct.command, ct.path, ct.kind + ); + if ct.kind == crate::core::package::CompletionKind::CommandBacked { + println!( + " note: this completion provider can execute code during completion \ + (only when Nushell requests a completion; never at install or activation)" + ); + } + } + } + println!(" -> {target}"); + println!(); + } } // ── Path validation (plugin payloads) ───────────────────────────────────────── @@ -1973,6 +2357,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); @@ -2017,13 +2403,13 @@ mod tests { ); } - #[test] - fn completion_type_fails_with_deferred_error() { + fn completion_test_lockfile( + targets: Option>, + ) -> (Lockfile, crate::nu::paths::NuPaths, tempfile::TempDir) { use crate::state::lockfile::{Lockfile, LockfileEntry}; use std::collections::BTreeMap; let dir = tempfile::tempdir().unwrap(); - let root = dir.path(); let mut packages: BTreeMap = BTreeMap::new(); packages.insert( @@ -2064,6 +2450,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: targets, locked_dependencies: BTreeMap::new(), }, ); @@ -2087,21 +2475,51 @@ mod tests { vendor_autoload_dir: None, }; + (lockfile, fake_nu_paths, dir) + } + + #[test] + fn completion_without_targets_is_install_only() { + let (lockfile, fake_nu_paths, root) = completion_test_lockfile(None); let args = ActivateArgs { packages: vec!["owner/mycomp".to_string()], - verbose: false, list: false, check: false, }; - - let err = resolve_plugin_targets(&args, &lockfile, &fake_nu_paths, root).unwrap_err(); + // The plugin lane no longer rejects completions outright. + assert!( + resolve_plugin_targets(&args, &lockfile, &fake_nu_paths, root.path()) + .unwrap() + .is_empty() + ); + let err = resolve_completion_targets(&args, &lockfile, &fake_nu_paths).unwrap_err(); assert!( - err.to_string().contains("completion") && err.to_string().contains("deferred"), - "Expected deferred error for completion type, got: {err}" + err.to_string().contains("install-only"), + "Expected install-only error for completion without targets, got: {err}" ); } + #[test] + fn completion_with_targets_resolves_for_activation() { + let (lockfile, fake_nu_paths, _root) = + completion_test_lockfile(Some(vec![crate::core::package::CompletionTarget { + command: "git".to_string(), + path: "git.nu".to_string(), + kind: crate::core::package::CompletionKind::CommandBacked, + }])); + let args = ActivateArgs { + packages: vec!["owner/mycomp".to_string()], + verbose: false, + list: false, + check: false, + }; + let targets = resolve_completion_targets(&args, &lockfile, &fake_nu_paths).unwrap(); + assert_eq!(targets.len(), 1); + assert_eq!(targets[0].targets.len(), 1); + assert_eq!(targets[0].targets[0].command, "git"); + } + #[test] fn already_active_module_is_skipped() { use crate::core::package::ModuleImportMode; @@ -2155,6 +2573,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); @@ -2242,6 +2662,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: deps, // has dependencies — not activatable }, ); diff --git a/src/cmd/activation_switch.rs b/src/cmd/activation_switch.rs index 268578f..6ac5539 100644 --- a/src/cmd/activation_switch.rs +++ b/src/cmd/activation_switch.rs @@ -132,6 +132,17 @@ pub fn collect_currently_active(lockfile: &Lockfile, nu_paths: &NuPaths) -> Mino { set.modules.push(id.clone()); } + // Completions share the managed autoload file too (issue #15). + "completion" + if entry.is_completion_active_for( + &nu_paths.nu_executable_hash, + &nu_paths.nu_version, + vendor_dir, + &managed_path, + ) => + { + set.modules.push(id.clone()); + } _ => {} } } @@ -195,7 +206,10 @@ pub fn leave_current_nu( // activation record exists at all (plugins with activation or modules // with module_activation). let any_activation = lockfile.packages.values().any(|e| { - e.activation.is_some() || e.module_activation.is_some() || e.script_activation.is_some() + e.activation.is_some() + || e.module_activation.is_some() + || e.script_activation.is_some() + || e.completion_activation.is_some() }); if any_activation { bail!( @@ -334,9 +348,11 @@ fn restore_desired( .modules .iter() .filter_map(|id| { - // Autoload profile list holds modules and scripts (issue #14). + // Autoload profile list holds modules, scripts (issue #14), + // and completions (issue #15). let expected = match lockfile.packages.get(id) { Some(e) if e.package_type == "script" => "script", + Some(e) if e.package_type == "completion" => "completion", _ => "module", }; match classify_restore_target(&lockfile, registry.as_ref(), &resolver, id, expected) { @@ -600,8 +616,8 @@ pub fn sync_profile_after_user_activate( }; let kind = match entry.package_type.as_str() { "plugin" => ProfileKind::Plugin, - // Scripts share the module (autoload) profile list. - "module" | "script" => ProfileKind::Module, + // Scripts and completions share the module (autoload) profile list. + "module" | "script" | "completion" => ProfileKind::Module, _ => continue, }; activation_profile::ensure_contains_for_paths(root, nu_version, kind, id)?; @@ -625,7 +641,7 @@ pub fn sync_profile_after_user_deactivate( }; let kind = match entry.package_type.as_str() { "plugin" => ProfileKind::Plugin, - "module" | "script" => ProfileKind::Module, + "module" | "script" | "completion" => ProfileKind::Module, _ => continue, }; activation_profile::ensure_absent_for_paths(root, nu_version, kind, id)?; @@ -674,7 +690,10 @@ mod tests { use crate::nu::autoload::{CandidateRunner, FakeCandidateRunner}; use crate::nu::version_manager; use crate::state::activation_profile::{ActivationProfile, ProfileKind}; - use crate::state::lockfile::{LockfileEntry, ModuleActivation, PluginActivation}; + use crate::state::lockfile::{ + CompletionActivation, CompletionActivationTarget, LockfileEntry, ModuleActivation, + PluginActivation, + }; use crate::util::fs_safety::acquire_mutation_lock; use std::cell::RefCell; use std::collections::BTreeMap; @@ -769,6 +788,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), } } @@ -820,6 +841,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), } } @@ -836,6 +859,40 @@ mod tests { std::fs::write(dir.join("mod.nu"), b"export def main [] {}").unwrap(); } + fn completion_entry(paths: &NuPaths, payload: &str, active: bool) -> LockfileEntry { + let vendor = paths.vendor_autoload_dir.clone().unwrap(); + let managed = format!("{vendor}/numan.nu"); + let mut entry = module_entry(paths, payload, false); + entry.package_type = "completion".to_string(); + entry.entry = None; + entry.module_import_mode = None; + entry.completion_targets = Some(vec![crate::core::package::CompletionTarget { + command: "git".to_string(), + path: "git.nu".to_string(), + kind: crate::core::package::CompletionKind::Static, + }]); + entry.completion_activation = active.then(|| CompletionActivation { + targets: vec![CompletionActivationTarget { + command: "git".to_string(), + path: "git.nu".to_string(), + target_path: format!("{payload}/git.nu"), + kind: crate::core::package::CompletionKind::Static, + }], + vendor_autoload_dir: vendor, + managed_file_path: managed, + nu_executable_sha256: paths.nu_executable_hash.clone(), + nu_version: paths.nu_version.clone(), + activated_at: "0".to_string(), + }); + entry + } + + fn seed_completion_payload(root: &Path, payload: &str) { + let dir = root.join(payload); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("git.nu"), b"export extern \"git\" []\n").unwrap(); + } + #[test] fn leave_merge_unions_never_shrinks_on_retry() { let tmp = TempDir::new().unwrap(); @@ -1039,6 +1096,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); @@ -1155,6 +1214,112 @@ mod tests { .is_active_for(&hash113, "0.113.1", &paths113.plugin_registry_path)); } + #[test] + fn collect_currently_active_includes_active_completion() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + std::fs::create_dir_all(root.join("state")).unwrap(); + let (exe, hash) = write_fake_nu(root, "0.113.1", b"nu-113"); + let vendor = root.join("vendor"); + let paths = save_paths(root, &exe, "0.113.1", &hash, &vendor); + + let payload = "packages/completions/o/comp/1.0.0-aaa"; + seed_completion_payload(root, payload); + let mut lockfile = Lockfile::empty(); + lockfile + .packages + .insert("o/comp".into(), completion_entry(&paths, payload, true)); + lockfile.packages.insert( + "o/inactive".into(), + completion_entry(&paths, payload, false), + ); + + let active = collect_currently_active(&lockfile, &paths); + assert_eq!(active.modules, vec!["o/comp".to_string()]); + assert!(active.plugins.is_empty()); + } + + #[test] + fn round_trip_cross_minor_restores_completion() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + std::fs::create_dir_all(root.join("state")).unwrap(); + let (exe113, hash113) = write_fake_nu(root, "0.113.1", b"nu-113-bytes"); + let (exe114, hash114) = write_fake_nu(root, "0.114.0", b"nu-114-bytes"); + let vendor = root.join("vendor"); + let paths113 = save_paths(root, &exe113, "0.113.1", &hash113, &vendor); + version_manager::write_active_version(root, "0.113.1").unwrap(); + + let payload = "packages/completions/o/comp/1.0.0-aaa"; + seed_completion_payload(root, payload); + let mut lockfile = Lockfile::empty(); + lockfile + .packages + .insert("o/comp".into(), completion_entry(&paths113, payload, true)); + lockfile.save(root).unwrap(); + + let reg = |_a: &str, _b: &str, _c: &str| Ok(()); + let unreg = |_a: &str, _b: &str, _c: &str| Ok(()); + let runner = FakeCandidateRunner::success(); + + let refresh114 = move |root: &Path| -> Result<()> { + let vendor = root.join("vendor"); + save_paths(root, &exe114, "0.114.0", &hash114, &vendor); + Ok(()) + }; + let refresh113 = { + let exe113 = exe113.clone(); + let hash113 = hash113.clone(); + move |root: &Path| -> Result<()> { + let vendor = root.join("vendor"); + save_paths(root, &exe113, "0.113.1", &hash113, &vendor); + Ok(()) + } + }; + + let hooks_to_114 = SwitchHooks { + registrar: ®, + unregistrar: &unreg, + runner: Some(&runner), + path_refresh: Some(&refresh114), + }; + let _lock = acquire_mutation_lock(root).unwrap(); + switch_active_nu_version(root, "0.114.0", &hooks_to_114, None).unwrap(); + assert!(Lockfile::load(root) + .unwrap() + .packages + .get("o/comp") + .unwrap() + .completion_activation + .is_none()); + let profile = ActivationProfile::load(root).unwrap().unwrap(); + assert_eq!( + profile.set_for_minor("0.113").modules, + vec!["o/comp".to_string()] + ); + + let hooks_to_113 = SwitchHooks { + registrar: ®, + unregistrar: &unreg, + runner: Some(&runner), + path_refresh: Some(&refresh113), + }; + switch_active_nu_version(root, "0.113.1", &hooks_to_113, None).unwrap(); + let vendor_str = vendor.to_string_lossy().into_owned(); + let managed = format!("{vendor_str}/numan.nu"); + let entry = Lockfile::load(root).unwrap(); + let entry = entry.packages.get("o/comp").unwrap(); + assert!( + entry.is_completion_active_for(&hash113, "0.113.1", &vendor_str, &managed), + "completion must be restored after switching back" + ); + let content = std::fs::read_to_string(&managed).unwrap(); + assert!( + content.contains("# numan:completion o/comp git"), + "managed file must carry the completion target after restore: {content}" + ); + } + #[test] fn same_target_restore_only_does_not_deactivate() { let tmp = TempDir::new().unwrap(); diff --git a/src/cmd/deactivate.rs b/src/cmd/deactivate.rs index 8cc2e32..1272637 100644 --- a/src/cmd/deactivate.rs +++ b/src/cmd/deactivate.rs @@ -122,7 +122,7 @@ pub fn execute_with_candidate_runner_and_unregistrar( print_autoload_recovery(reconcile_pending_autoload(root, &nu_paths, &mut lockfile)?); let lockfile = Lockfile::load(root)?; - // 3. Classify explicit package IDs (plugins + modules; script/completion deferred). + // 3. Classify explicit package IDs (plugins + modules/scripts/completions). let targets_requested = classify_and_validate_packages(args, &lockfile, root, &nu_paths)?; if targets_requested.is_empty() { @@ -245,6 +245,7 @@ pub fn execute_with_candidate_runner_and_unregistrar( Some(e) if e.package_type == "plugin" => e.activation.is_none(), Some(e) if e.package_type == "module" => e.module_activation.is_none(), Some(e) if e.package_type == "script" => e.script_activation.is_none(), + Some(e) if e.package_type == "completion" => e.completion_activation.is_none(), _ => false, }; if clear { @@ -301,7 +302,7 @@ fn snapshot_before_deactivate_profile_sync( Some(entry) => { let kind = match entry.package_type.as_str() { "plugin" => Some(crate::state::activation_profile::ProfileKind::Plugin), - "module" | "script" => { + "module" | "script" | "completion" => { Some(crate::state::activation_profile::ProfileKind::Module) } _ => None, @@ -346,6 +347,7 @@ fn is_full_module_deactivation(lockfile: &Lockfile, requested: &[ActiveModule]) .filter(|(_, pkg)| { (pkg.package_type == "module" && pkg.module_activation.is_some()) || (pkg.package_type == "script" && pkg.script_activation.is_some()) + || (pkg.package_type == "completion" && pkg.completion_activation.is_some()) }) .map(|(id, _)| id.as_str()) .collect(); @@ -520,9 +522,18 @@ pub(crate) fn deactivate_modules_unlocked( vendor_autoload_dir: sa.vendor_autoload_dir.clone(), managed_file_path: sa.managed_file_path.clone(), }); + } else if entry.package_type == "completion" { + let Some(ca) = &entry.completion_activation else { + continue; + }; + targets.push(ActiveModule { + package_id: pkg_id.clone(), + vendor_autoload_dir: ca.vendor_autoload_dir.clone(), + managed_file_path: ca.managed_file_path.clone(), + }); } else { bail!( - "Package '{pkg_id}' is not a module or script (type: {})", + "Package '{pkg_id}' is not a module, script, or completion (type: {})", entry.package_type ); } @@ -565,7 +576,8 @@ pub fn plugin_name_from_executable_path(executable_path: &str) -> String { /// /// - Plugins with `activation.is_some()` are collected as `ActivePlugin`. /// - Modules with `module_activation` are collected as `ActiveModule`. -/// - Script/completion packages return a deferred-feature error. +/// - Completion packages with `completion_activation` are collected as +/// `ActiveModule` alongside modules and scripts (issue #15). /// - Unknown package IDs fail. /// - Plugins that are not currently active are skipped when listed explicitly /// (idempotent after journal reconcile). Modules that are not active are also @@ -577,7 +589,7 @@ fn classify_and_validate_packages( root: &Path, nu_paths: &NuPaths, ) -> Result { - // First pass: reject deferred / unknown types in the explicit list. + // First pass: reject unknown types in the explicit list. for pkg_id in &args.packages { let entry = match lockfile.packages.get(pkg_id) { Some(e) => e, @@ -586,12 +598,7 @@ fn classify_and_validate_packages( } }; match entry.package_type.as_str() { - "module" | "plugin" | "script" => {} - "completion" => { - bail!( - "Package '{pkg_id}' is a completion — completion deactivation is deferred to a later phase." - ); - } + "module" | "plugin" | "script" | "completion" => {} other => { bail!("Package '{pkg_id}' has unknown type '{other}' — cannot deactivate."); } @@ -633,6 +640,14 @@ fn classify_and_validate_packages( managed_file_path: sa.managed_file_path.clone(), }); } + "completion" if entry.completion_activation.is_some() => { + let ca = entry.completion_activation.as_ref().unwrap(); + modules.push(ActiveModule { + package_id: pkg_id.clone(), + vendor_autoload_dir: ca.vendor_autoload_dir.clone(), + managed_file_path: ca.managed_file_path.clone(), + }); + } _ => {} } } @@ -685,7 +700,19 @@ fn classify_and_validate_packages( }); } }, - _ => unreachable!("deferred types rejected in first pass"), + "completion" => match &entry.completion_activation { + None => { + continue; + } + Some(ca) => { + modules.push(ActiveModule { + package_id: pkg_id.clone(), + vendor_autoload_dir: ca.vendor_autoload_dir.clone(), + managed_file_path: ca.managed_file_path.clone(), + }); + } + }, + _ => unreachable!("unknown types rejected in first pass"), } } } @@ -1174,6 +1201,7 @@ fn run_partial_deactivation( // Resolve remaining module entries and script commands from the lockfile. let mut remaining_entries = Vec::new(); let mut remaining_scripts: Vec = Vec::new(); + let mut remaining_completions: Vec = Vec::new(); for pkg_id in remaining_ids { let entry = lockfile.packages.get(pkg_id).with_context(|| { format!("Package '{pkg_id}' in remaining set not found in lockfile") @@ -1197,6 +1225,24 @@ fn run_partial_deactivation( }); } } + if let Some(ca) = &entry.completion_activation { + for tgt in &ca.targets { + // Re-resolve instead of trusting the stored absolute path, + // mirroring the script branch above. + let resolved = crate::nu::autoload::resolve_entry( + root, + &entry.payload_path, + &tgt.path, + crate::core::package::ModuleImportMode::Module, + pkg_id, + )?; + remaining_completions.push(crate::nu::autoload::ResolvedCompletionTarget { + scoped_id: pkg_id.clone(), + command: tgt.command.clone(), + target_path: resolved.absolute_path, + }); + } + } if let Some(ma) = &entry.module_activation { let resolved_entry = crate::nu::autoload::ResolvedEntry { absolute_path: PathBuf::from(&ma.entry_path), @@ -1204,8 +1250,8 @@ fn run_partial_deactivation( scoped_id: pkg_id.clone(), }; remaining_entries.push(resolved_entry); - } else if entry.script_activation.is_some() { - // Script contributor: wrappers already collected above. + } else if entry.script_activation.is_some() || entry.completion_activation.is_some() { + // Script/completion contributor: contributions already collected above. } else { // Remaining module has no activation record — skip (shouldn't happen). eprintln!( @@ -1222,8 +1268,12 @@ fn run_partial_deactivation( // post-deactivation set. // Step 4: Generate candidate content for remaining modules. - let content = generate_managed_content(&remaining_entries, &remaining_scripts) - .context("Failed to generate autoload content for partial deactivation")?; + let content = generate_managed_content( + &remaining_entries, + &remaining_scripts, + &remaining_completions, + ) + .context("Failed to generate autoload content for partial deactivation")?; // Step 5: Write candidate file (same directory as managed file). let candidate_path = write_candidate(managed_path, &content) @@ -1353,6 +1403,7 @@ fn clear_module_activations( if let Some(pkg) = lockfile.packages.get_mut(&target.package_id) { pkg.module_activation = None; pkg.script_activation = None; + pkg.completion_activation = None; } } lockfile.save(root) @@ -1437,6 +1488,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); @@ -1675,7 +1728,7 @@ mod tests { } #[test] - fn completion_type_returns_deferred_error() { + fn completion_type_inactive_is_skipped() { let dir = TempDir::new().unwrap(); let lockfile = make_lockfile_with_modules(vec![("owner/mycomp", "completion", false)]); @@ -1685,11 +1738,11 @@ mod tests { verbose: false, }; - let err = classify_and_validate_packages(&args, &lockfile, dir.path(), &fake_nu_paths()) - .unwrap_err(); + let targets = + classify_and_validate_packages(&args, &lockfile, dir.path(), &fake_nu_paths()).unwrap(); assert!( - err.to_string().contains("deferred"), - "Expected deferred error for completion type, got: {err}" + targets.modules.is_empty(), + "inactive completion must be skipped" ); } @@ -2071,6 +2124,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }; diff --git a/src/cmd/doctor.rs b/src/cmd/doctor.rs index ae89d4f..5638edf 100644 --- a/src/cmd/doctor.rs +++ b/src/cmd/doctor.rs @@ -948,10 +948,11 @@ fn check_lockfile( } if let Some(paths) = nu_paths { - let has_active_modules = lockfile - .packages - .values() - .any(|e| e.module_activation.is_some() || e.script_activation.is_some()); + let has_active_modules = lockfile.packages.values().any(|e| { + e.module_activation.is_some() + || e.script_activation.is_some() + || e.completion_activation.is_some() + }); if has_active_modules && paths.vendor_autoload_dir.is_none() { findings.push(finding( "nu_paths.vendor_missing", @@ -1044,6 +1045,23 @@ fn check_activation( RepairTier::Confirm, )); } + if entry.package_type == "completion" + && entry.completion_activation.is_some() + && !entry.is_completion_active_for( + &paths.nu_executable_hash, + &paths.nu_version, + vendor_dir, + &managed_path, + ) + { + findings.push(finding( + "activation.completion_stale", + Severity::Warn, + format!("Completion '{id}' activation is stale for current Nu"), + Some(CMD_ACTIVATE), + RepairTier::Confirm, + )); + } } if let Ok(Some(state)) = AutoloadState::load(root) { @@ -1058,10 +1076,11 @@ fn check_activation( } } - let has_active_modules = lockfile - .packages - .values() - .any(|e| e.module_activation.is_some() || e.script_activation.is_some()); + let has_active_modules = lockfile.packages.values().any(|e| { + e.module_activation.is_some() + || e.script_activation.is_some() + || e.completion_activation.is_some() + }); if has_active_modules && !managed_path.is_empty() { let managed = Path::new(&managed_path); if !managed.is_file() { @@ -1674,6 +1693,8 @@ fn apply_repairs( | "journal.autoload_pending" | "activation.plugin_stale" | "activation.module_stale" + | "activation.script_stale" + | "activation.completion_stale" | "autoload.projection" | "autoload.managed_missing" ) @@ -1877,6 +1898,8 @@ fn print_report_to( "activation.plugin_mutation_gated", "activation.plugin_stale", "activation.module_stale", + "activation.script_stale", + "activation.completion_stale", "autoload.projection", "autoload.managed_missing", "autoload.managed_foreign", @@ -2228,6 +2251,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); @@ -2302,6 +2327,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); @@ -2393,6 +2420,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); diff --git a/src/cmd/gc.rs b/src/cmd/gc.rs index 7e31215..a448f15 100644 --- a/src/cmd/gc.rs +++ b/src/cmd/gc.rs @@ -250,6 +250,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); @@ -332,6 +334,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); @@ -406,6 +410,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); diff --git a/src/cmd/hook.rs b/src/cmd/hook.rs index 48bd5f5..6926417 100644 --- a/src/cmd/hook.rs +++ b/src/cmd/hook.rs @@ -701,6 +701,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), } } diff --git a/src/cmd/info.rs b/src/cmd/info.rs index 3303980..0b55a29 100644 --- a/src/cmd/info.rs +++ b/src/cmd/info.rs @@ -55,9 +55,21 @@ pub fn format_info(pkg: &Package, platform: &Platform, nu: Option<&NuVersion>) - } } crate::core::package::PackageType::Completion => { - out.push_str( - "Activation: install-only (activation deferred; install does not wire Nu)\n", - ); + let declared = pkg.versions.iter().any(|v| { + v.activation + .as_ref() + .map(|spec| spec.kind == "nu-completion" && !spec.targets.is_empty()) + .unwrap_or(false) + }); + if declared { + out.push_str( + "Activation: explicit `numan activate` required; install does not wire Nu\n", + ); + } else { + out.push_str( + "Activation: install-only (no declared targets; install does not wire Nu)\n", + ); + } } _ => {} } @@ -115,6 +127,24 @@ pub fn format_info(pkg: &Package, platform: &Platform, nu: Option<&NuVersion>) - } } + if let Some(activation) = ver + .activation + .as_ref() + .filter(|spec| spec.kind == "nu-completion") + { + for target in &activation.targets { + out.push_str(&format!( + " completion: {} -> {} [{}] (inactive until activated)\n", + target.command, target.path, target.kind + )); + if target.kind == crate::core::package::CompletionKind::CommandBacked { + out.push_str( + " note: this completion provider can execute code during completion (only when Nushell requests a completion; never at install or activation)\n", + ); + } + } + } + if let Some(activation) = ver .activation .as_ref() @@ -284,6 +314,7 @@ mod tests { name: "wttr".into(), path: "wttr.nu".into(), }], + targets: Vec::new(), }); let out = format_info(&pkg, &linux_platform(), None); assert!( @@ -305,6 +336,47 @@ mod tests { assert!(out.contains("Type: completion"), "{out}"); } + #[test] + fn format_info_shows_completion_targets_and_command_backed_disclosure() { + let mut pkg = sample_plugin(false); + pkg.package_type = PackageType::Completion; + pkg.versions[0].activation = Some(crate::core::package::RegistryActivationSpec { + kind: "nu-completion".into(), + import: Default::default(), + entry: None, + overlay_name: None, + exports: None, + bindings: None, + entry_points: Vec::new(), + targets: vec![ + crate::core::package::CompletionTarget { + command: "git".into(), + path: "git.nu".into(), + kind: crate::core::package::CompletionKind::CommandBacked, + }, + crate::core::package::CompletionTarget { + command: "cargo".into(), + path: "cargo.nu".into(), + kind: crate::core::package::CompletionKind::Static, + }, + ], + }); + let out = format_info(&pkg, &linux_platform(), None); + assert!( + out.contains("Activation: explicit `numan activate` required"), + "{out}" + ); + assert!( + out.contains("completion: git -> git.nu [command-backed] (inactive until activated)"), + "{out}" + ); + assert!( + out.contains("completion: cargo -> cargo.nu [static] (inactive until activated)"), + "{out}" + ); + assert!(out.contains("can execute code during completion"), "{out}"); + } + #[test] fn format_info_shows_hook_triggers_before_explicit_activation() { let mut pkg = sample_plugin(false); @@ -321,6 +393,7 @@ mod tests { filter: None, }]), entry_points: Vec::new(), + targets: Vec::new(), }); let out = format_info(&pkg, &linux_platform(), None); assert!(out.contains("high-risk Nushell hooks"), "{out}"); diff --git a/src/cmd/init.rs b/src/cmd/init.rs index 99adf6f..2e8fce0 100644 --- a/src/cmd/init.rs +++ b/src/cmd/init.rs @@ -211,10 +211,11 @@ where let old_paths = NuPaths::load(root)?; let mut lockfile = Lockfile::load(root)?; - let has_active_modules = lockfile - .packages - .values() - .any(|entry| entry.module_activation.is_some() || entry.script_activation.is_some()); + let has_active_modules = lockfile.packages.values().any(|entry| { + entry.module_activation.is_some() + || entry.script_activation.is_some() + || entry.completion_activation.is_some() + }); if has_active_modules { validate_refresh_for_active_modules( @@ -309,13 +310,33 @@ fn validate_refresh_for_active_modules( let active_ids: Vec = lockfile .packages .iter() - .filter(|(_, entry)| entry.module_activation.is_some() || entry.script_activation.is_some()) + .filter(|(_, entry)| { + entry.module_activation.is_some() + || entry.script_activation.is_some() + || entry.completion_activation.is_some() + }) .map(|(id, _)| id.clone()) .collect(); let scoped_refs: Vec<&str> = active_ids.iter().map(String::as_str).collect(); let runner = runner_factory(&new_paths.nu_executable); - validate_candidate(&managed_file, runner.as_ref(), &scoped_refs)?; + // Validate a copy, never the live file: `validate_candidate` deletes + // its argument on failure, and a failing refresh must not remove the + // managed autoload file (issue #15 review). + let content = std::fs::read_to_string(&managed_file).with_context(|| { + format!( + "Failed to read managed autoload file '{}'", + managed_file.display() + ) + })?; + let candidate = write_candidate(&managed_file, &content) + .context("Failed to write managed autoload validation copy for `numan init --refresh`")?; + let result = validate_candidate(&candidate, runner.as_ref(), &scoped_refs); + if candidate.exists() { + // On failure validate_candidate already removed the copy. + let _ = std::fs::remove_file(&candidate); + } + result?; // Touch autoload-state path consistency — projection is updated after lockfile save. let _ = root; @@ -341,6 +362,10 @@ fn refresh_activation_records(lockfile: &mut Lockfile, new_paths: &NuPaths) -> R activation.nu_executable_sha256 = new_paths.nu_executable_hash.clone(); activation.nu_version = new_paths.nu_version.clone(); } + if let Some(ref mut activation) = entry.completion_activation { + activation.nu_executable_sha256 = new_paths.nu_executable_hash.clone(); + activation.nu_version = new_paths.nu_version.clone(); + } } Ok(()) } @@ -349,7 +374,10 @@ fn refresh_activation_records(lockfile: &mut Lockfile, new_paths: &NuPaths) -> R /// rewrite `numan.nu` so script wrappers invoke the refreshed executable. /// /// Module `use` statements do not embed the Nu path, so only script wrappers -/// need regeneration. Candidate validation runs before the atomic replace. +/// need regeneration on a path change; active completion targets are +/// re-resolved and revalidated on the same pass (issue #15), and included +/// whenever regeneration does happen. Candidate validation runs before the +/// atomic replace. fn regenerate_managed_autoload_for_refresh( root: &Path, old_paths: &NuPaths, @@ -361,7 +389,11 @@ fn regenerate_managed_autoload_for_refresh( .packages .values() .any(|entry| entry.script_activation.is_some()); - if !has_scripts { + let has_completions = lockfile + .packages + .values() + .any(|entry| entry.completion_activation.is_some()); + if !has_scripts && !has_completions { return Ok(()); } @@ -373,6 +405,7 @@ fn regenerate_managed_autoload_for_refresh( let mut entries = Vec::new(); let mut scripts = Vec::new(); + let mut completions = Vec::new(); let mut active_ids = Vec::new(); for (pkg_id, entry) in &lockfile.packages { if let Some(ma) = &entry.module_activation { @@ -408,6 +441,30 @@ fn regenerate_managed_autoload_for_refresh( } active_ids.push(pkg_id.clone()); } + if let Some(ca) = &entry.completion_activation { + for tgt in &ca.targets { + let resolved = crate::nu::autoload::resolve_entry( + root, + &entry.payload_path, + &tgt.path, + crate::core::package::ModuleImportMode::Module, + pkg_id, + ) + .with_context(|| { + format!( + "Active completion target '{}' for '{}' no longer resolves; \ + deactivate and re-activate it", + tgt.path, pkg_id + ) + })?; + completions.push(crate::nu::autoload::ResolvedCompletionTarget { + scoped_id: pkg_id.clone(), + command: tgt.command.clone(), + target_path: resolved.absolute_path, + }); + } + active_ids.push(pkg_id.clone()); + } } if old_paths.nu_executable == new_paths.nu_executable { @@ -415,8 +472,9 @@ fn regenerate_managed_autoload_for_refresh( return Ok(()); } - let content = crate::nu::autoload::generate_managed_content(&entries, &scripts) - .context("Failed to regenerate managed autoload content for `numan init --refresh`")?; + let content = + crate::nu::autoload::generate_managed_content(&entries, &scripts, &completions) + .context("Failed to regenerate managed autoload content for `numan init --refresh`")?; let candidate = write_candidate(&managed_file, &content) .context("Failed to write managed autoload candidate for `numan init --refresh`")?; let runner = runner_factory(&new_paths.nu_executable); @@ -533,6 +591,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), } } diff --git a/src/cmd/list.rs b/src/cmd/list.rs index 43e14e1..20f05ea 100644 --- a/src/cmd/list.rs +++ b/src/cmd/list.rs @@ -39,6 +39,11 @@ pub(crate) fn execute_to(root: &Path, out: &mut dyn Write) -> Result<()> { writeln!(out, "Installed packages ({}):\n", lockfile.packages.len())?; for (id, entry) in &lockfile.packages { + // Status here distinguishes plugin and hook activation only. + // Managed-file activations (module, script, completion) all report + // "installed", matching the pre-existing module/script treatment; + // `numan activate --list` is the surface that reports their + // per-Nu active state. let status = match &nu_paths { Some(p) if active_hook_packages.as_ref().is_some_and(|profile| { @@ -130,6 +135,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: Default::default(), } } diff --git a/src/cmd/plugin_lifecycle.rs b/src/cmd/plugin_lifecycle.rs index 716fd6e..fbe3e3a 100644 --- a/src/cmd/plugin_lifecycle.rs +++ b/src/cmd/plugin_lifecycle.rs @@ -515,6 +515,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); diff --git a/src/cmd/remove.rs b/src/cmd/remove.rs index 68a99ac..4262b7c 100644 --- a/src/cmd/remove.rs +++ b/src/cmd/remove.rs @@ -78,6 +78,14 @@ pub fn execute_reviewed( args.package ); } + if entry.completion_activation.is_some() { + bail!( + "Package '{}' is currently active as a completion. \ + Run `numan deactivate {}` first.", + args.package, + args.package + ); + } if !args.force && entry.module_activation.is_some() { bail!( "Package '{}' is currently active. \ @@ -124,6 +132,14 @@ pub fn execute_reviewed( args.package ); } + if entry.completion_activation.is_some() { + bail!( + "Package '{}' is currently active as a completion. \ + Run `numan deactivate {}` first.", + args.package, + args.package + ); + } if !args.force && entry.module_activation.is_some() { bail!( "Package '{}' is currently active. \ @@ -254,7 +270,7 @@ mod tests { use super::*; use crate::core::package::ModuleImportMode; use crate::state::lockfile::{ - LockfileEntry, ModuleActivation, PluginActivation, ScriptActivation, + CompletionActivation, LockfileEntry, ModuleActivation, PluginActivation, ScriptActivation, }; use std::collections::BTreeMap; @@ -295,6 +311,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), } } @@ -343,6 +361,51 @@ mod tests { assert!(PendingLifecycle::load(root).unwrap().is_none()); } + #[test] + fn execute_refuses_active_completion_even_with_force() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let payload = root.join("packages/completions/owner/comp/1.0.0-abc12345"); + std::fs::create_dir_all(&payload).unwrap(); + let source = payload.join("git.nu"); + std::fs::write(&source, "# completions").unwrap(); + let mut lockfile = Lockfile::empty(); + let mut entry = base_entry(); + entry.package_type = "completion".into(); + entry.payload_path = "packages/completions/owner/comp/1.0.0-abc12345".into(); + entry.completion_activation = Some(CompletionActivation { + targets: Vec::new(), + vendor_autoload_dir: "/tmp/vendor".to_string(), + managed_file_path: "/tmp/vendor/numan.nu".to_string(), + nu_executable_sha256: "abc".to_string(), + nu_version: "0.95.0".to_string(), + activated_at: "0".to_string(), + }); + lockfile.packages.insert("owner/comp".into(), entry); + lockfile.save(root).unwrap(); + + let err = execute_with_tty( + &RemoveArgs { + package: "owner/comp".into(), + yes: true, + force: true, + }, + root, + false, + ) + .unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("active as a completion"), "got: {msg}"); + assert!(msg.contains("deactivate"), "got: {msg}"); + + // The lockfile entry and payload must be untouched. + assert!(source.is_file()); + assert!(Lockfile::load(root) + .unwrap() + .packages + .contains_key("owner/comp")); + } + fn plugin_activation() -> PluginActivation { PluginActivation { plugin_registry_path: "/tmp/plugins.nu".to_string(), @@ -374,6 +437,12 @@ mod tests { Run `numan deactivate {pkg_id}` first." ); } + if entry.completion_activation.is_some() { + bail!( + "Package '{pkg_id}' is currently active as a completion. \ + Run `numan deactivate {pkg_id}` first." + ); + } if !force && entry.module_activation.is_some() { bail!( "Package '{pkg_id}' is currently active. \ @@ -543,6 +612,25 @@ mod tests { assert!(err.to_string().contains("deactivate")); } + #[test] + fn refuse_active_completion_even_with_force() { + let entry = LockfileEntry { + package_type: "completion".to_string(), + completion_activation: Some(CompletionActivation { + targets: Vec::new(), + vendor_autoload_dir: "/tmp/vendor".to_string(), + managed_file_path: "/tmp/vendor/numan.nu".to_string(), + nu_executable_sha256: "abc".to_string(), + nu_version: "0.95.0".to_string(), + activated_at: "0".to_string(), + }), + ..base_entry() + }; + let err = ensure_removable(&entry, "owner/comp", true).unwrap_err(); + assert!(err.to_string().contains("active as a completion")); + assert!(err.to_string().contains("deactivate")); + } + #[test] fn execute_refuses_non_tty_without_yes() { // Force non-TTY via the injectable seam so the guard is deterministic diff --git a/src/cmd/snapshot.rs b/src/cmd/snapshot.rs index 23f1e4f..1212cc6 100644 --- a/src/cmd/snapshot.rs +++ b/src/cmd/snapshot.rs @@ -378,6 +378,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: Default::default(), } } diff --git a/src/cmd/try_cmd.rs b/src/cmd/try_cmd.rs index efff5bd..49af44c 100644 --- a/src/cmd/try_cmd.rs +++ b/src/cmd/try_cmd.rs @@ -345,10 +345,19 @@ where bail!("Install reported no changes for '{package_id}'"); } - let install_only = matches!( - package.package_type, - PackageType::Script | PackageType::Completion - ); + // Scripts stay install-only under `try` (existing behaviour). + // Completions are install-only only when the installed entry declares + // no targets; a completion package with declared targets activates + // through the normal path below (issue #15). + let install_only = match package.package_type { + PackageType::Script => true, + PackageType::Completion => !Lockfile::load(root)? + .packages + .get(package_id) + .map(|entry| entry.is_completion_activatable()) + .unwrap_or(false), + _ => false, + }; let hook_package = Lockfile::load(root)? .packages .get(package_id) @@ -372,6 +381,13 @@ where return Ok(()); } + // A completion package that declares targets is about to be activated: + // disclose the targets (and the command-backed execution behaviour) + // before activating, matching the `numan info` disclosure (issue #15). + if package.package_type == PackageType::Completion { + print_completion_activation_disclosure(root, package_id)?; + } + // Keep the install mutation lock through activation so a concurrent // remove/update cannot replace the just-installed payload before activate // reloads the lockfile. `activate::execute_under_lock` does not re-acquire. @@ -393,6 +409,35 @@ where Ok(()) } +/// Disclosure printed before `numan try` activates a completion package: +/// the declared targets it will wire into the managed autoload file, plus +/// the command-backed execution note, matching `numan info` (issue #15). +fn print_completion_activation_disclosure(root: &Path, package_id: &str) -> Result<()> { + let lockfile = Lockfile::load(root)?; + let Some(entry) = lockfile.packages.get(package_id) else { + return Ok(()); + }; + let Some(targets) = entry.completion_targets.as_ref() else { + return Ok(()); + }; + if targets.is_empty() { + return Ok(()); + } + println!("Activating completion targets for '{package_id}':"); + for target in targets { + println!( + " completion: {} -> {} [{}]", + target.command, target.path, target.kind + ); + if target.kind == crate::core::package::CompletionKind::CommandBacked { + println!( + " note: this completion provider can execute code during completion (only when Nushell requests a completion; never at install or activation)" + ); + } + } + Ok(()) +} + /// 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 { @@ -778,6 +823,147 @@ mod tests { assert!(activated.get()); } + fn completion_lock_entry( + targets: Option>, + locked_dependencies: BTreeMap, + ) -> crate::state::lockfile::LockfileEntry { + crate::state::lockfile::LockfileEntry { + hook_activation: None, + hook_bindings: None, + version: "1.0.0".to_string(), + package_type: "completion".to_string(), + source: "registry".to_string(), + target: None, + artifact_url: None, + artifact_sha256: None, + executable_path: None, + archive_root: None, + include: None, + entry: None, + 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: "packages/completions/test/completions/1.0.0-deadbeef".to_string(), + revision_id: None, + payload_sha256: None, + executable_sha256: None, + selection_reason: None, + origin: None, + module_activation: None, + module_import_mode: None, + overlay: None, + script_activation: None, + script_entry_points: None, + completion_activation: None, + completion_targets: targets, + locked_dependencies, + } + } + + fn completion_target() -> crate::core::package::CompletionTarget { + crate::core::package::CompletionTarget { + command: "git".to_string(), + path: "git.nu".to_string(), + kind: crate::core::package::CompletionKind::CommandBacked, + } + } + + #[test] + fn try_package_completion_with_targets_activates() { + let root = tempfile::tempdir().unwrap(); + let package = pkg("test/completions", "*", PackageType::Completion); + let platform = windows_platform(); + let nu = NuVersion::parse("0.114.1").unwrap(); + let args = TryArgs { + package: Some("test/completions".to_string()), + no_activate: false, + }; + + let activated = Cell::new(false); + let result = try_package( + &args, + root.path(), + &package, + "test/completions", + "test/completions", + None, + &platform, + &nu, + |_, _, options| { + let mut lock = Lockfile::empty(); + lock.packages.insert( + "test/completions".to_string(), + completion_lock_entry(Some(vec![completion_target()]), BTreeMap::new()), + ); + lock.save(options.root)?; + Ok(install_result("test/completions", "1.0.0", options.root)) + }, + |args, _, _lock| { + activated.set(true); + assert_eq!(args.packages, vec!["test/completions".to_string()]); + Ok(()) + }, + ); + assert!(result.is_ok(), "{result:?}"); + assert!( + activated.get(), + "a completion entry with declared targets must activate under try" + ); + } + + #[test] + fn try_package_completion_with_locked_dependencies_stays_install_only() { + let root = tempfile::tempdir().unwrap(); + let package = pkg("test/completions", "*", PackageType::Completion); + let platform = windows_platform(); + let nu = NuVersion::parse("0.114.1").unwrap(); + let args = TryArgs { + package: Some("test/completions".to_string()), + no_activate: false, + }; + + let activated = Cell::new(false); + let result = try_package( + &args, + root.path(), + &package, + "test/completions", + "test/completions", + None, + &platform, + &nu, + |_, _, options| { + let mut deps = BTreeMap::new(); + deps.insert("owner/dep".to_string(), "1.0.0".to_string()); + let mut lock = Lockfile::empty(); + lock.packages.insert( + "test/completions".to_string(), + completion_lock_entry(Some(vec![completion_target()]), deps), + ); + lock.save(options.root)?; + Ok(install_result("test/completions", "1.0.0", options.root)) + }, + |_, _, _lock| { + activated.set(true); + Ok(()) + }, + ); + assert!(result.is_ok(), "{result:?}"); + assert!( + !activated.get(), + "a completion entry with locked dependencies must stay install-only under try" + ); + } + #[test] fn try_package_no_activate_skips_activation() { let root = tempfile::tempdir().unwrap(); @@ -959,6 +1145,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), } } diff --git a/src/cmd/update.rs b/src/cmd/update.rs index 7b8ba30..2b2e752 100644 --- a/src/cmd/update.rs +++ b/src/cmd/update.rs @@ -225,7 +225,10 @@ pub fn execute_reviewed( None => { // Fail closed: without cached NuPaths we cannot verify activation // identity, so refuse active plugins/modules instead of Plain. - if current.module_activation.is_some() || current.script_activation.is_some() { + if current.module_activation.is_some() + || current.script_activation.is_some() + || current.completion_activation.is_some() + { let e = anyhow::anyhow!( "Package '{}' is currently active. \ Run `numan deactivate {}` first, then run update again.", @@ -646,6 +649,14 @@ fn plan_active_update( pkg_id ); } + if entry.completion_activation.is_some() { + bail!( + "Package '{}' is currently active as a completion. \ + Run `numan deactivate {}` first, then run update again.", + pkg_id, + pkg_id + ); + } if entry.activation.is_some() { let matching = entry.is_active_for( &nu_paths.nu_executable_hash, @@ -680,7 +691,8 @@ mod tests { use crate::state::active_plugin_mutation::EnvOverrideGuard; use crate::state::lifecycle_journal::PendingLifecycle; use crate::state::lockfile::{ - Lockfile, LockfileEntry, ModuleActivation, PluginActivation, ScriptActivation, + CompletionActivation, Lockfile, LockfileEntry, ModuleActivation, PluginActivation, + ScriptActivation, }; use crate::state::plugin_deactivate_journal::PendingPluginDeactivate; use std::collections::BTreeMap; @@ -762,6 +774,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), } } @@ -914,6 +928,23 @@ mod tests { assert!(err.to_string().contains("active as a script")); } + #[test] + fn plan_active_update_rejects_completion_activation() { + let entry = LockfileEntry { + completion_activation: Some(CompletionActivation { + targets: Vec::new(), + vendor_autoload_dir: "/tmp/vendor".to_string(), + managed_file_path: "/tmp/vendor/numan.nu".to_string(), + nu_executable_sha256: "abc".to_string(), + nu_version: "0.95.0".to_string(), + activated_at: "0".to_string(), + }), + ..base_entry() + }; + let err = plan_active_update(&entry, "owner/comp", &matching_nu_paths()).unwrap_err(); + assert!(err.to_string().contains("active as a completion")); + } + struct ActiveUpdateEnv { _dir: TempDir, root: PathBuf, @@ -995,6 +1026,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); diff --git a/src/cmd/use_cmd.rs b/src/cmd/use_cmd.rs index 0cbcd6e..0b091a4 100644 --- a/src/cmd/use_cmd.rs +++ b/src/cmd/use_cmd.rs @@ -671,6 +671,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); diff --git a/src/core/package.rs b/src/core/package.rs index 988f382..2c97903 100644 --- a/src/core/package.rs +++ b/src/core/package.rs @@ -111,8 +111,8 @@ pub struct VersionEntry { pub source: Option, #[serde(default)] pub dependencies: BTreeMap, - /// Optional activation metadata for module packages. - /// `None` for plugins, scripts, and completions. + /// Optional activation metadata (module, overlay, hook, script, or + /// completion packages). `None` for plugins and install-only packages. #[serde(default)] pub activation: Option, /// Optional provenance marker, e.g. `"commit-snapshot"` for versions @@ -238,8 +238,8 @@ impl fmt::Display for ModuleImportMode { /// Each activation kind is handled only by its explicit capability command. #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct RegistryActivationSpec { - /// Activation kind. `"nu-module"`, `"nu-overlay"`, `"nu-hook"`, or - /// `"nu-script"`. + /// Activation kind. `"nu-module"`, `"nu-overlay"`, `"nu-hook"`, + /// `"nu-script"`, or `"nu-completion"`. pub kind: String, /// Import mode for the module. Defaults to `ModuleImportMode::Module`. /// Ignored for `nu-overlay` and `nu-hook`. @@ -261,6 +261,9 @@ pub struct RegistryActivationSpec { /// Declared script entry points. Only meaningful for `"nu-script"`. #[serde(default)] pub entry_points: Vec, + /// Declared completion targets. Only meaningful for `"nu-completion"`. + #[serde(default)] + pub targets: Vec, } /// One public command contributed by a script package (issue #14). @@ -389,6 +392,101 @@ pub fn validate_script_entry_points(entry_points: &[ScriptEntryPoint]) -> Result Ok(()) } +/// Declared completion kind for a [`CompletionTarget`] (issue #15). +/// +/// The kind is declared by the package, never inferred from the source: +/// `Static` means completions are literal values in the source file; +/// `CommandBacked` means the source defines completer functions that +/// Nushell executes when it requests a completion. Activation and install +/// never invoke a completer either way. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum CompletionKind { + #[serde(rename = "static")] + Static, + #[serde(rename = "command-backed")] + CommandBacked, +} + +impl fmt::Display for CompletionKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + CompletionKind::Static => write!(f, "static"), + CompletionKind::CommandBacked => write!(f, "command-backed"), + } + } +} + +/// One command surface contributed by a completion package (issue #15). +/// +/// `command` is the external command the package augments (a single +/// command name, e.g. `git`); `path` is the package-relative `.nu` source +/// file contributing that command's completions; `kind` is the declared +/// completion kind. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CompletionTarget { + pub command: String, + pub path: String, + pub kind: CompletionKind, +} + +/// Validate a package's declared completion targets. +/// +/// Mirrors [`validate_script_entry_points`]: rejects an empty declaration, +/// invalid or duplicate command names (command names use the script +/// command-name grammar and must not be Nushell reserved keywords), +/// duplicate targets, and targets that are not safe package-relative +/// `.nu` paths. File existence is checked later, at activation time, +/// against the installed payload. +pub fn validate_completion_targets(targets: &[CompletionTarget]) -> Result<()> { + if targets.is_empty() { + bail!("Completion activation requires at least one target"); + } + let mut commands = std::collections::BTreeSet::new(); + let mut paths = std::collections::BTreeSet::new(); + for target in targets { + if !is_valid_script_command_name(&target.command) { + bail!( + "Invalid completion command name '{}': must start with a letter or '_' \ + and contain only letters, digits, '_' or '-'", + target.command + ); + } + if is_nu_reserved_keyword(&target.command) { + bail!( + "Invalid completion command name '{}': reserved Nushell keyword", + target.command + ); + } + if !commands.insert(&target.command) { + bail!("Duplicate completion command '{}'", target.command); + } + let rel = std::path::Path::new(&target.path); + if !crate::util::fs_safety::is_safe_relative_path(rel) { + bail!( + "Completion target '{}': path '{}' is not a safe package-relative path", + target.command, + target.path + ); + } + match rel.extension() { + Some(ext) if ext == "nu" => {} + _ => bail!( + "Completion target '{}': path '{}' does not have a '.nu' extension", + target.command, + target.path + ), + } + let normalized: std::path::PathBuf = rel + .components() + .filter(|c| !matches!(c, std::path::Component::CurDir)) + .collect(); + if !paths.insert(normalized) { + bail!("Duplicate completion target path '{}'", target.path); + } + } + Ok(()) +} + /// A single hook binding declared by a package (issue #16). #[derive(Debug, Clone, Serialize, Deserialize)] pub struct HookBinding { @@ -470,6 +568,77 @@ mod tests { .contains("unsupported Nushell hook event")); } + #[test] + fn completion_target_kind_serializes_kebab() { + let t: CompletionTarget = + serde_json::from_str(r#"{"command":"git","path":"git.nu","kind":"command-backed"}"#) + .unwrap(); + assert_eq!(t.kind, CompletionKind::CommandBacked); + let t: CompletionTarget = + serde_json::from_str(r#"{"command":"git","path":"git.nu","kind":"static"}"#).unwrap(); + assert_eq!(t.kind, CompletionKind::Static); + assert!(serde_json::from_str::( + r#"{"command":"git","path":"git.nu","kind":"inferred"}"# + ) + .is_err()); + } + + #[test] + fn validate_completion_targets_rules() { + let ok = vec![ + CompletionTarget { + command: "git".into(), + path: "git.nu".into(), + kind: CompletionKind::CommandBacked, + }, + CompletionTarget { + command: "cargo".into(), + path: "cargo.nu".into(), + kind: CompletionKind::Static, + }, + ]; + assert!(validate_completion_targets(&ok).is_ok()); + assert!(validate_completion_targets(&[]).is_err()); + let one = |command: &str, path: &str| { + vec![CompletionTarget { + command: command.into(), + path: path.into(), + kind: CompletionKind::Static, + }] + }; + assert!(validate_completion_targets(&one("9git", "git.nu")).is_err()); + assert!(validate_completion_targets(&one("has space", "git.nu")).is_err()); + assert!(validate_completion_targets(&one("if", "git.nu")).is_err()); + assert!(validate_completion_targets(&one("git", "../escape.nu")).is_err()); + assert!(validate_completion_targets(&one("git", "git.txt")).is_err()); + let dup_cmd = vec![ + CompletionTarget { + command: "git".into(), + path: "a.nu".into(), + kind: CompletionKind::Static, + }, + CompletionTarget { + command: "git".into(), + path: "b.nu".into(), + kind: CompletionKind::Static, + }, + ]; + assert!(validate_completion_targets(&dup_cmd).is_err()); + let dup_path = vec![ + CompletionTarget { + command: "git".into(), + path: "./git.nu".into(), + kind: CompletionKind::Static, + }, + CompletionTarget { + command: "cargo".into(), + path: "git.nu".into(), + kind: CompletionKind::Static, + }, + ]; + assert!(validate_completion_targets(&dup_path).is_err()); + } + #[test] fn parse_scoped_id() { let id = ScopedId::parse("fdncred/file").unwrap(); diff --git a/src/install/transaction.rs b/src/install/transaction.rs index a426405..c92d88d 100644 --- a/src/install/transaction.rs +++ b/src/install/transaction.rs @@ -179,6 +179,24 @@ pub fn install_package( let mut lockfile = Lockfile::load(options.root)?; let lock_key = id.to_string(); + // A reinstall that would replace the entry must not silently clear an + // autoload activation record: the fresh entry starts inactive while the + // managed `numan.nu` still carries the package's lines, orphaning them + // and dropping any completion target-ownership claims (issue #15 + // review). Refuse like update/remove do. A non-force reinstall of the + // same version with an intact payload early-returns below and keeps the + // record, so it stays allowed. + if let Some(existing) = lockfile.packages.get(&lock_key) { + // A missing payload also falls through to the fresh-install path even + // for a same-version non-force reinstall, so treat it as a replacement. + let replacing = options.force + || existing.version != version_str + || !options.root.join(existing.payload_path()).exists(); + if replacing { + ensure_reinstall_preserves_activation(existing, &lock_key)?; + } + } + if !options.force { if let Some(entry) = lockfile.packages.get_mut(&lock_key) { if entry.version == version_str { @@ -417,6 +435,22 @@ pub fn install_package( _ => None, }; + // Validate declared completion targets before promotion too (issue #15), + // mirroring the script validation above. + let completion_targets = match resolved.activation.as_ref() { + Some(spec) if spec.kind == "nu-completion" => { + if pkg.package_type != crate::core::package::PackageType::Completion { + bail!( + "Package {id} declares activation kind 'nu-completion' but is not a completion package" + ); + } + crate::core::package::validate_completion_targets(&spec.targets) + .with_context(|| format!("Invalid completion activation declaration for {id}"))?; + Some(spec.targets.clone()) + } + _ => None, + }; + // 9. Snapshot lockfile before mutation (after download/extraction preflight succeeds). // Skip when the lockfile is empty — there is no prior state worth restoring. if !Lockfile::load(options.root)?.packages.is_empty() { @@ -518,6 +552,8 @@ pub fn install_package( overlay, script_activation: None, script_entry_points, + completion_activation: None, + completion_targets, locked_dependencies, }; @@ -562,6 +598,39 @@ fn format_timestamp() -> String { format!("{secs:016}") } +/// Refuse to replace the lockfile entry of a package that is currently +/// active through the managed autoload file (module, script, or +/// completion): the replacement entry would start with no activation +/// record while `numan.nu` still carries the package's contribution. +/// Mirrors the guards in `update` and `remove`. +fn ensure_reinstall_preserves_activation(entry: &LockfileEntry, package_id: &str) -> Result<()> { + if entry.module_activation.is_some() { + bail!( + "Package '{}' is currently active as a module. \ + Run `numan deactivate {}` first.", + package_id, + package_id + ); + } + if entry.script_activation.is_some() { + bail!( + "Package '{}' is currently active as a script. \ + Run `numan deactivate {}` first.", + package_id, + package_id + ); + } + if entry.completion_activation.is_some() { + bail!( + "Package '{}' is currently active as a completion. \ + Run `numan deactivate {}` first.", + package_id, + package_id + ); + } + Ok(()) +} + fn persist_hook_bindings( activation: Option<&RegistryActivationSpec>, package_id: &str, diff --git a/src/nu/autoload.rs b/src/nu/autoload.rs index a33e559..a1456e3 100644 --- a/src/nu/autoload.rs +++ b/src/nu/autoload.rs @@ -251,7 +251,7 @@ pub fn render_use_statement(path: &Path, mode: &ModuleImportMode) -> Result Result { - generate_managed_content(entries, &[]) + generate_managed_content(entries, &[], &[]) } /// A fully-validated script command ready for managed-file generation @@ -314,13 +314,61 @@ pub fn render_script_wrapper(command: &ResolvedScriptCommand) -> Result )) } -/// Generate the full content of `numan.nu` from module entries and script -/// commands. Modules render first (sorted by scoped ID), then script -/// wrappers (sorted by scoped ID, then command name), under the -/// ownership header. +/// A fully-validated completion target ready for managed-file generation +/// (issue #15). Constructed by the activate lane from a package's declared +/// targets after containment checks via [`resolve_entry`]. +#[derive(Debug, Clone)] +pub struct ResolvedCompletionTarget { + /// Canonical scoped package ID (e.g. `"owner/name"`), used for ordering. + pub scoped_id: String, + /// External command name this target augments, as declared. + pub command: String, + /// Absolute canonical path to the `.nu` completion source file. + pub target_path: PathBuf, +} + +/// Render one completion target's contribution to the managed file: +/// +/// ```nu +/// # numan:completion +/// use "" * +/// ``` +/// +/// The `use ... *` form matches how Nushell custom-completion files are +/// consumed (they export `extern` definitions plus completer functions). +/// Rendering only references the file: no completer runs at activation +/// time; Nushell executes command-backed providers only when it requests +/// a completion in a live session. +pub fn render_completion_use(target: &ResolvedCompletionTarget) -> Result { + if target.scoped_id.chars().any(|c| c.is_control()) { + bail!( + "Completion package ID '{}' contains control characters", + target.scoped_id.escape_debug() + ); + } + if !crate::core::package::is_valid_script_command_name(&target.command) { + bail!( + "Invalid completion command name '{}'", + target.command.escape_debug() + ); + } + let stmt = render_use_statement(&target.target_path, &ModuleImportMode::All)?; + Ok(format!( + "# numan:completion {scoped} {command}\n{stmt}", + scoped = target.scoped_id, + command = target.command, + )) +} + +/// Generate the full content of `numan.nu` from module entries, script +/// commands, and completion targets. Modules render first (sorted by +/// scoped ID), then script wrappers (sorted by scoped ID, then command +/// name), then completion targets (sorted by scoped ID, then command), +/// under the ownership header. pub fn generate_managed_content( entries: &[ResolvedEntry], script_commands: &[ResolvedScriptCommand], + completion_targets: &[ResolvedCompletionTarget], ) -> Result { let mut sorted: Vec<&ResolvedEntry> = entries.iter().collect(); sorted.sort_by(|a, b| a.scoped_id.cmp(&b.scoped_id)); @@ -328,6 +376,14 @@ pub fn generate_managed_content( let mut sorted_scripts: Vec<&ResolvedScriptCommand> = script_commands.iter().collect(); sorted_scripts.sort_by(|a, b| a.scoped_id.cmp(&b.scoped_id).then(a.name.cmp(&b.name))); + let mut sorted_completions: Vec<&ResolvedCompletionTarget> = + completion_targets.iter().collect(); + sorted_completions.sort_by(|a, b| { + a.scoped_id + .cmp(&b.scoped_id) + .then(a.command.cmp(&b.command)) + }); + let mut out = String::new(); out.push_str(OWNERSHIP_MARKER); @@ -344,6 +400,12 @@ pub fn generate_managed_content( out.push('\n'); } + for target in &sorted_completions { + out.push('\n'); + out.push_str(&render_completion_use(target)?); + out.push('\n'); + } + Ok(out) } @@ -911,6 +973,63 @@ mod tests { ); } + // ── completion targets (issue #15) ───────────────────────────────────── + + #[test] + fn render_completion_use_line() { + let target = ResolvedCompletionTarget { + scoped_id: "nushell/git-completions".to_string(), + command: "git".to_string(), + target_path: PathBuf::from("/root/packages/completions/nushell/git/1.0.0-aaa/git.nu"), + }; + let rendered = render_completion_use(&target).unwrap(); + assert_eq!( + rendered, + "# numan:completion nushell/git-completions git\n\ + use \"/root/packages/completions/nushell/git/1.0.0-aaa/git.nu\" *" + ); + } + + #[test] + fn render_completion_rejects_bad_command() { + let target = ResolvedCompletionTarget { + scoped_id: "owner/x".to_string(), + command: "has space".to_string(), + target_path: PathBuf::from("/root/x.nu"), + }; + assert!(render_completion_use(&target).is_err()); + } + + #[test] + fn managed_content_renders_completions_after_scripts_sorted() { + let script = ResolvedScriptCommand { + scoped_id: "acme/weather".to_string(), + name: "wttr".to_string(), + script_path: PathBuf::from("/root/wttr.nu"), + nu_executable: "/usr/bin/nu".to_string(), + }; + let completions = vec![ + ResolvedCompletionTarget { + scoped_id: "nushell/git-completions".to_string(), + command: "git".to_string(), + target_path: PathBuf::from("/root/git.nu"), + }, + ResolvedCompletionTarget { + scoped_id: "acme/tools".to_string(), + command: "cargo".to_string(), + target_path: PathBuf::from("/root/cargo.nu"), + }, + ]; + let content = generate_managed_content(&[], &[script], &completions).unwrap(); + let script_pos = content.find("# numan:script").unwrap(); + let acme_pos = content.find("# numan:completion acme/tools cargo").unwrap(); + let git_pos = content + .find("# numan:completion nushell/git-completions git") + .unwrap(); + assert!(script_pos < acme_pos, "{content}"); + assert!(acme_pos < git_pos, "{content}"); + } + // ── resolve_entry ──────────────────────────────────────────────────────── #[test] diff --git a/src/nu/bootstrap.rs b/src/nu/bootstrap.rs index 6d89f03..3943b9b 100644 --- a/src/nu/bootstrap.rs +++ b/src/nu/bootstrap.rs @@ -446,6 +446,8 @@ fn discover_bundled_plugins(root: &Path, version_dir: &Path, nu_version: &str) - overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: std::collections::BTreeMap::new(), }; discovered.push((package_id, lockfile_entry)); @@ -1760,6 +1762,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: std::collections::BTreeMap::new(), }, ); @@ -1855,6 +1859,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: std::collections::BTreeMap::new(), }, ); diff --git a/src/nupm_compat/import.rs b/src/nupm_compat/import.rs index acc831c..4051e1c 100644 --- a/src/nupm_compat/import.rs +++ b/src/nupm_compat/import.rs @@ -598,6 +598,8 @@ fn commit_imports( overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }; lockfile.packages.insert(resolved.package_id.clone(), entry); @@ -983,6 +985,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), } } diff --git a/src/state/autoload_recovery.rs b/src/state/autoload_recovery.rs index b1275b2..b8b6f56 100644 --- a/src/state/autoload_recovery.rs +++ b/src/state/autoload_recovery.rs @@ -140,6 +140,45 @@ fn apply_replaced_transition( }); continue; } + if entry.package_type == "completion" { + // Completion activation record (issue #15): rebuild + // unconditionally from the persisted declaration, mirroring the + // script branch above. + let declared = entry.completion_targets.clone().unwrap_or_default(); + if declared.is_empty() { + bail!( + "Cannot recover completion '{package_id}': lockfile has no persisted targets" + ); + } + crate::core::package::validate_completion_targets(&declared).map_err(|e| { + anyhow!("Cannot recover completion '{package_id}': invalid persisted targets: {e}") + })?; + let mut targets = Vec::new(); + for ct in &declared { + let resolved = crate::nu::autoload::resolve_entry( + root, + &entry.payload_path, + &ct.path, + ModuleImportMode::Module, + package_id, + )?; + targets.push(crate::state::lockfile::CompletionActivationTarget { + command: ct.command.clone(), + path: ct.path.clone(), + target_path: resolved.absolute_path.to_string_lossy().into_owned(), + kind: ct.kind, + }); + } + entry.completion_activation = Some(crate::state::lockfile::CompletionActivation { + targets, + vendor_autoload_dir: journal.vendor_autoload_dir.clone(), + managed_file_path: journal.managed_file_path.clone(), + nu_executable_sha256: nu_paths.nu_executable_hash.clone(), + nu_version: nu_paths.nu_version.clone(), + activated_at: activated_at.clone(), + }); + continue; + } if entry.package_type != "module" { bail!( "Cannot recover module '{}': lockfile type is '{}'", @@ -190,6 +229,15 @@ fn apply_replaced_transition( if should_clear_script { entry.script_activation = None; } + let should_clear_completion = entry.completion_activation.as_ref().is_some_and(|a| { + a.nu_executable_sha256 == journal.nu_executable_sha256 + && a.nu_version == journal.nu_version + && a.vendor_autoload_dir == journal.vendor_autoload_dir + && a.managed_file_path == journal.managed_file_path + }); + if should_clear_completion { + entry.completion_activation = None; + } } if journal.operation == AutoloadOperation::Deactivate { @@ -205,6 +253,7 @@ fn apply_replaced_transition( if let Some(entry) = lockfile.packages.get_mut(package_id) { entry.module_activation = None; entry.script_activation = None; + entry.completion_activation = None; } } } diff --git a/src/state/autoload_state.rs b/src/state/autoload_state.rs index 63dbf95..cf8c375 100644 --- a/src/state/autoload_state.rs +++ b/src/state/autoload_state.rs @@ -109,9 +109,10 @@ impl AutoloadState { /// Sorted IDs of every package contributing to the managed autoload /// file for this Nu identity: active modules plus active scripts - /// (issue #14). The state file's `active_module_ids` field stores this - /// combined projection; the name predates script activation and is - /// kept for schema compatibility. + /// (issue #14) and active completions (issue #15). The state file's + /// `active_module_ids` field stores this combined projection; the + /// name predates script activation and is kept for schema + /// compatibility. pub fn active_autoload_ids_from_lockfile( lockfile: &Lockfile, nu_executable_sha256: &str, @@ -127,12 +128,17 @@ impl AutoloadState { managed_file_path, ); for (id, entry) in &lockfile.packages { - if entry.is_script_active_for( + if (entry.is_script_active_for( nu_executable_sha256, nu_version, vendor_autoload_dir, managed_file_path, - ) && !ids.contains(id) + ) || entry.is_completion_active_for( + nu_executable_sha256, + nu_version, + vendor_autoload_dir, + managed_file_path, + )) && !ids.contains(id) { ids.push(id.clone()); } @@ -288,6 +294,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); diff --git a/src/state/lockfile.rs b/src/state/lockfile.rs index 3beaeac..ee2607b 100644 --- a/src/state/lockfile.rs +++ b/src/state/lockfile.rs @@ -81,6 +81,39 @@ pub struct ScriptActivation { pub activated_at: String, } +/// One activated completion target: declared command plus resolved source. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct CompletionActivationTarget { + /// External command name this target augments, as declared. + pub command: String, + /// Package-relative `.nu` source path as declared. + pub path: String, + /// Absolute canonical path to the source within the installed payload. + pub target_path: String, + /// Declared completion kind (`static` / `command-backed`). + pub kind: crate::core::package::CompletionKind, +} + +/// Per-Nu-identity activation record stored on a completion lockfile entry +/// (issue #15). Mirrors [`ScriptActivation`]'s identity fields: the record +/// is current only when Nu hash, Nu version, vendor-autoload directory, +/// and managed file path all match the cached `NuPaths`. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct CompletionActivation { + /// Targets this package contributes while active. + pub targets: Vec, + /// Absolute path to the selected vendor-autoload directory. + pub vendor_autoload_dir: String, + /// Absolute path to the Numan-managed autoload file (`numan.nu`). + pub managed_file_path: String, + /// SHA-256 of the Nu executable used when this activation was recorded. + pub nu_executable_sha256: String, + /// Version string of the Nu executable used when recorded. + pub nu_version: String, + /// Timestamp when this activation record was written. + pub activated_at: String, +} + /// Registry metadata persisted for a package that can be activated as a Nu overlay. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct OverlayPackageSpec { @@ -236,6 +269,19 @@ pub struct LockfileEntry { #[serde(default)] pub script_entry_points: Option>, + /// Activation record for this completion package under a specific Nu + /// identity and vendor-autoload target. `None` means not yet activated + /// (issue #15). + #[serde(default)] + pub completion_activation: Option, + + /// Completion targets persisted from the registry at install time + /// (`activation.kind = "nu-completion"`). `None` for non-completion + /// packages and for completion packages installed without a + /// declaration, which stay install-only. + #[serde(default)] + pub completion_targets: Option>, + /// Registry dependency map captured at install time. /// Activation requires this to be empty in Phase 4. #[serde(default)] @@ -325,6 +371,38 @@ impl LockfileEntry { .unwrap_or(false) && self.locked_dependencies.is_empty() } + + /// Returns `true` if this completion entry is active for the given Nu + /// identity and vendor-autoload target. All four identity fields must + /// match, mirroring [`Self::is_script_active_for`]. + pub fn is_completion_active_for( + &self, + nu_executable_sha256: &str, + nu_version: &str, + vendor_autoload_dir: &str, + managed_file_path: &str, + ) -> bool { + match &self.completion_activation { + Some(a) => { + a.nu_executable_sha256 == nu_executable_sha256 + && a.nu_version == nu_version + && a.vendor_autoload_dir == vendor_autoload_dir + && a.managed_file_path == managed_file_path + } + None => false, + } + } + + /// Returns `true` when this completion package can be activated + /// (issue #15): declared targets were persisted at install time and no + /// locked dependencies need resolution. + pub fn is_completion_activatable(&self) -> bool { + self.completion_targets + .as_ref() + .map(|targets| !targets.is_empty()) + .unwrap_or(false) + && self.locked_dependencies.is_empty() + } } impl Lockfile { @@ -510,6 +588,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), } } @@ -570,6 +650,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); @@ -650,6 +732,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), } } @@ -792,6 +876,8 @@ mod tests { let no_points = LockfileEntry { package_type: "script".to_string(), script_entry_points: None, + completion_activation: None, + completion_targets: None, ..make_base_entry() }; assert!(!no_points.is_script_activatable()); @@ -799,6 +885,8 @@ mod tests { let empty_points = LockfileEntry { package_type: "script".to_string(), script_entry_points: Some(Vec::new()), + completion_activation: None, + completion_targets: None, ..make_base_entry() }; assert!(!empty_points.is_script_activatable()); @@ -819,6 +907,97 @@ mod tests { assert!(!with_deps.is_script_activatable()); } + #[test] + fn is_completion_active_for_matches_correctly() { + let entry = LockfileEntry { + package_type: "completion".to_string(), + completion_activation: Some(CompletionActivation { + targets: Vec::new(), + vendor_autoload_dir: "/nu/vendor/autoload".to_string(), + managed_file_path: "/nu/vendor/autoload/numan.nu".to_string(), + nu_executable_sha256: "exe-hash".to_string(), + nu_version: "0.113.1".to_string(), + activated_at: "0".to_string(), + }), + ..make_base_entry() + }; + + assert!(entry.is_completion_active_for( + "exe-hash", + "0.113.1", + "/nu/vendor/autoload", + "/nu/vendor/autoload/numan.nu" + )); + assert!(!entry.is_completion_active_for( + "wrong-hash", + "0.113.1", + "/nu/vendor/autoload", + "/nu/vendor/autoload/numan.nu" + )); + assert!(!entry.is_completion_active_for( + "exe-hash", + "0.114.0", + "/nu/vendor/autoload", + "/nu/vendor/autoload/numan.nu" + )); + assert!(!entry.is_completion_active_for( + "exe-hash", + "0.113.1", + "/other/vendor/autoload", + "/nu/vendor/autoload/numan.nu" + )); + assert!(!entry.is_completion_active_for( + "exe-hash", + "0.113.1", + "/nu/vendor/autoload", + "/other/path/numan.nu" + )); + } + + #[test] + fn is_completion_activatable_requires_targets_and_no_deps() { + use crate::core::package::{CompletionKind, CompletionTarget}; + + let target = CompletionTarget { + command: "git".to_string(), + path: "git.nu".to_string(), + kind: CompletionKind::CommandBacked, + }; + let ready = LockfileEntry { + package_type: "completion".to_string(), + completion_targets: Some(vec![target.clone()]), + locked_dependencies: BTreeMap::new(), + ..make_base_entry() + }; + assert!(ready.is_completion_activatable()); + + let no_targets = LockfileEntry { + package_type: "completion".to_string(), + completion_targets: None, + ..make_base_entry() + }; + assert!(!no_targets.is_completion_activatable()); + + let empty_targets = LockfileEntry { + package_type: "completion".to_string(), + completion_targets: Some(Vec::new()), + ..make_base_entry() + }; + assert!(!empty_targets.is_completion_activatable()); + + let with_deps = LockfileEntry { + package_type: "completion".to_string(), + completion_targets: Some(vec![target]), + locked_dependencies: { + let mut m = BTreeMap::new(); + m.insert("owner/dep".to_string(), "^1.0.0".to_string()); + m + }, + ..make_base_entry() + }; + assert!(!with_deps.is_completion_activatable()); + } + #[test] fn is_module_activatable_requires_mode_entry_and_no_deps() { use crate::core::package::ModuleImportMode; @@ -833,6 +1012,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), ..make_base_entry() }; @@ -870,6 +1051,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: { let mut m = BTreeMap::new(); m.insert("owner/dep".to_string(), "^1.0.0".to_string()); @@ -907,6 +1090,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: deps, module_activation: Some(ModuleActivation { entry_path: "/root/packages/modules/owner/mymod/1.0.0-aaa/mod.nu".to_string(), diff --git a/src/state/rollback.rs b/src/state/rollback.rs index f97473c..7ad3a73 100644 --- a/src/state/rollback.rs +++ b/src/state/rollback.rs @@ -545,6 +545,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), } } diff --git a/src/state/snapshot.rs b/src/state/snapshot.rs index 1629b71..c3bca35 100644 --- a/src/state/snapshot.rs +++ b/src/state/snapshot.rs @@ -920,6 +920,8 @@ mod tests { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), } } diff --git a/src/tui/app.rs b/src/tui/app.rs index d7a5b8d..41a1641 100644 --- a/src/tui/app.rs +++ b/src/tui/app.rs @@ -229,7 +229,7 @@ impl App { .map(|v| v.version.to_string()) .collect::>() .join(", "); - let install_only = operations::install_only_note(&item.package.package_type) + let install_only = operations::install_only_note_for_package(&item.package) .map(|note| format!("\n\n{note}")) .unwrap_or_default(); rows.push(Row { id, version, kind: item.package.package_type.to_string(), state: state.into(), registry: item.registry.clone(), @@ -471,7 +471,8 @@ impl App { }; let active = entry.activation.is_some() || entry.module_activation.is_some() - || entry.script_activation.is_some(); + || entry.script_activation.is_some() + || entry.completion_activation.is_some(); let allowed = !active || std::env::var("NUMAN_ENABLE_ACTIVE_PLUGIN_MUTATION").as_deref() == Ok("1"); if allowed && (!active || entry.package_type == "plugin") { @@ -484,11 +485,13 @@ impl App { Some(entry) if entry.activation.is_none() && entry.module_activation.is_none() - && entry.script_activation.is_none() => + && entry.script_activation.is_none() + && entry.completion_activation.is_none() => { let mut actions = vec![Action::Remove]; if matches!(row.kind.as_str(), "plugin" | "module") || (row.kind.as_str() == "script" && entry.is_script_activatable()) + || (row.kind.as_str() == "completion" && entry.is_completion_activatable()) { actions.insert(0, Action::Activate); } @@ -498,7 +501,9 @@ impl App { if row.state != "active" && (matches!(row.kind.as_str(), "plugin" | "module") || (row.kind.as_str() == "script" - && entry.is_script_activatable())) => + && entry.is_script_activatable()) + || (row.kind.as_str() == "completion" + && entry.is_completion_activatable())) => { vec![Action::Activate] } diff --git a/src/tui/operations.rs b/src/tui/operations.rs index cfe9ea4..e24ede1 100644 --- a/src/tui/operations.rs +++ b/src/tui/operations.rs @@ -8,7 +8,7 @@ use crate::cmd::{activate, doctor, plugin_lifecycle, remove, update}; use crate::config::Config; use crate::core::{ nu_version::NuVersion, - package::{Package, PackageType}, + package::{CompletionKind, Package, PackageType}, platform::Platform, registry::RegistryManager, resolve::Resolver, @@ -111,7 +111,10 @@ pub fn load(root: &Path) -> Result { } pub fn activation(entry: &LockfileEntry, nu: Option<&NuPaths>) -> &'static str { - if !matches!(entry.package_type.as_str(), "plugin" | "module" | "script") { + if !matches!( + entry.package_type.as_str(), + "plugin" | "module" | "script" | "completion" + ) { return "unsupported"; } let Some(nu) = nu else { @@ -133,6 +136,12 @@ pub fn activation(entry: &LockfileEntry, nu: Option<&NuPaths>) -> &'static str { dir, &path, ) + || entry.is_completion_active_for( + &nu.nu_executable_hash, + &nu.nu_version, + dir, + &path, + ) }) }) }; @@ -141,6 +150,7 @@ pub fn activation(entry: &LockfileEntry, nu: Option<&NuPaths>) -> &'static str { } else if entry.activation.is_some() || entry.module_activation.is_some() || entry.script_activation.is_some() + || entry.completion_activation.is_some() { "other Nu" } else { @@ -157,8 +167,9 @@ pub fn install_only_note(package_type: &PackageType) -> Option<&'static str> { Install only downloads the payload; run or source it from its payload path afterwards.", ), PackageType::Completion => Some( - "Install-only: completions cannot be activated. Install only downloads the \ - payload; source it from your shell configuration afterwards.", + "Install-only: this completion package declares no targets, so it cannot be \ + activated. Install only downloads the payload; source it from your shell \ + configuration afterwards.", ), PackageType::Plugin | PackageType::Module => None, } @@ -166,7 +177,9 @@ pub fn install_only_note(package_type: &PackageType) -> Option<&'static str> { /// Install-only note for a concrete catalog package: script packages /// that declare `nu-script` entry points can be activated (issue #14), -/// so the note applies only when no version declares entry points. +/// and completion packages that declare `nu-completion` targets can be +/// activated (issue #15), so the note applies only when no version +/// declares the relevant activation surface. pub fn install_only_note_for_package( package: &crate::core::package::Package, ) -> Option<&'static str> { @@ -181,6 +194,17 @@ pub fn install_only_note_for_package( return None; } } + if package.package_type == PackageType::Completion { + let declared = package.versions.iter().any(|v| { + v.activation + .as_ref() + .map(|spec| spec.kind == "nu-completion" && !spec.targets.is_empty()) + .unwrap_or(false) + }); + if declared { + return None; + } + } install_only_note(&package.package_type) } @@ -404,12 +428,18 @@ pub fn prepare( } Action::Activate => { let entry = entry.context("Package is not installed.")?; - if !matches!(entry.package_type.as_str(), "plugin" | "module" | "script") { + if !matches!( + entry.package_type.as_str(), + "plugin" | "module" | "script" | "completion" + ) { bail!("Activation is unsupported for this package type."); } if entry.package_type == "script" && !entry.is_script_activatable() { bail!("Script package has no declared entry points and cannot be activated."); } + if entry.package_type == "completion" && !entry.is_completion_activatable() { + bail!("Completion package has no declared targets and cannot be activated."); + } let nu = nu.context("Initialize Nu context before modifying packages.")?; if activation(entry, Some(nu)) == "active" { bail!("Already active for the current Nu."); @@ -428,6 +458,22 @@ pub fn prepare( Path::new(target).join("numan.nu").display() )); } + if entry.package_type == "completion" { + if let Some(targets) = entry.completion_targets.as_ref() { + for target in targets { + effects.push(format!( + "Completion: {} -> {} [{}]", + target.command, target.path, target.kind + )); + if target.kind == CompletionKind::CommandBacked { + effects.push( + "note: this completion provider can execute code during completion (only when Nushell requests a completion; never at install or activation)" + .into(), + ); + } + } + } + } effects.push( "Invokes the selected Nu; affects future Nu sessions, not the running shell." .into(), @@ -439,6 +485,7 @@ pub fn prepare( if entry.activation.is_some() || entry.module_activation.is_some() || entry.script_activation.is_some() + || entry.completion_activation.is_some() { bail!("Deactivate this package via CLI before removal. Force is CLI only."); } diff --git a/src/tui/tests.rs b/src/tui/tests.rs index 02ba70a..1240ebd 100644 --- a/src/tui/tests.rs +++ b/src/tui/tests.rs @@ -162,6 +162,7 @@ impl Fixture { exports: None, bindings: None, entry_points: Vec::new(), + targets: Vec::new(), }), provenance: None, evidence_tier: None, diff --git a/tests/activate_test.rs b/tests/activate_test.rs index fa15f4b..5e5b08b 100644 --- a/tests/activate_test.rs +++ b/tests/activate_test.rs @@ -130,6 +130,8 @@ impl TestEnv { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: std::collections::BTreeMap::new(), } } @@ -346,6 +348,8 @@ fn test_activate_path_with_spaces() { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: std::collections::BTreeMap::new(), }, ); diff --git a/tests/completion_activation_test.rs b/tests/completion_activation_test.rs new file mode 100644 index 0000000..616a8d7 --- /dev/null +++ b/tests/completion_activation_test.rs @@ -0,0 +1,876 @@ +/// Integration tests for first-class completion package activation (issue #15). +/// +/// Uses the injectable candidate-runner seam from the script activation +/// tests so no real Nu binary is required, plus an `#[ignore]`d real-Nu +/// acceptance test. Direct completion querying is not exposed by Nu in a +/// non-interactive invocation, so the real-Nu test proves the honest +/// equivalent: a fresh session that loads the managed file has the +/// contributed `extern` defined with its parameter bound to the declared +/// completer (the signature records the completion), and after +/// deactivation the managed contribution is gone. +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +use numan_cli::cmd::activate::{execute_with_candidate_runner, ActivateArgs}; +use numan_cli::cmd::deactivate::{ + execute_with_candidate_runner as deactivate_with_runner, DeactivateArgs, +}; +use numan_cli::core::integrity; +use numan_cli::core::package::{validate_completion_targets, CompletionKind, CompletionTarget}; +use numan_cli::nu::autoload::FakeCandidateRunner; +use numan_cli::nu::paths::NuPaths; +use numan_cli::state::autoload_journal::{ + AutoloadOperation, AutoloadStage, PendingAutoload, SCHEMA_VERSION as AUTOLOAD_SCHEMA_VERSION, +}; +use numan_cli::state::autoload_recovery::{reconcile_pending_autoload, AutoloadRecoveryOutcome}; +use numan_cli::state::autoload_state::AutoloadState; +use numan_cli::state::lockfile::{Lockfile, LockfileEntry}; +use numan_cli::util::fs_safety::{acquire_mutation_lock, OWNERSHIP_MARKER}; + +use tempfile::TempDir; + +struct CompletionTestEnv { + dir: TempDir, + nu_hash: String, + nu_exe: PathBuf, + vendor_dir: PathBuf, +} + +impl CompletionTestEnv { + fn new() -> Self { + let dir = TempDir::new().unwrap(); + let nu_exe = dir.path().join("fake_nu"); + std::fs::write(&nu_exe, b"fake nu binary").unwrap(); + let nu_hash = integrity::compute_sha256(b"fake nu binary"); + let vendor_dir = dir.path().join("vendor").join("autoload"); + std::fs::create_dir_all(&vendor_dir).unwrap(); + Self { + dir, + nu_hash, + nu_exe, + vendor_dir, + } + } + + fn root(&self) -> &Path { + self.dir.path() + } + + fn vendor_dir_str(&self) -> String { + self.vendor_dir.to_string_lossy().into_owned() + } + + fn managed_file_path(&self) -> PathBuf { + self.vendor_dir.join("numan.nu") + } + + fn write_nu_paths(&self) { + NuPaths { + nu_executable: self.nu_exe.to_string_lossy().into_owned(), + nu_version: "0.116.1".to_string(), + plugin_registry_path: self + .root() + .join("plugins.msgpackz") + .to_string_lossy() + .into_owned(), + nu_executable_hash: self.nu_hash.clone(), + platform: "x86_64-pc-windows-msvc".to_string(), + data_dir: Some(self.root().join("data").to_string_lossy().into_owned()), + vendor_autoload_dirs: vec![self.vendor_dir_str()], + vendor_autoload_dir: Some(self.vendor_dir_str()), + } + .save(self.root()) + .unwrap(); + } + + fn create_completion( + &self, + owner: &str, + name: &str, + files: &[(&str, &str)], + targets: Vec, + ) -> (String, LockfileEntry) { + let pkg_id = format!("{owner}/{name}"); + let payload_rel = format!("packages/completions/{owner}/{name}/1.0.0-aabbccdd"); + let payload_abs = self.root().join(&payload_rel); + std::fs::create_dir_all(&payload_abs).unwrap(); + for (rel, contents) in files { + let p = payload_abs.join(rel); + if let Some(parent) = p.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(&p, contents).unwrap(); + } + let entry = LockfileEntry { + hook_activation: None, + hook_bindings: None, + version: "1.0.0".to_string(), + package_type: "completion".to_string(), + source: "archive".to_string(), + target: None, + artifact_url: None, + artifact_sha256: None, + executable_path: None, + archive_root: None, + include: None, + entry: None, + installed_at: "0000000000000001".to_string(), + nu_version_at_install: Some("0.116.1".to_string()), + 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_rel, + revision_id: None, + payload_sha256: None, + executable_sha256: None, + selection_reason: None, + origin: None, + module_activation: None, + module_import_mode: None, + overlay: None, + script_activation: None, + script_entry_points: None, + completion_activation: None, + completion_targets: Some(targets), + locked_dependencies: BTreeMap::new(), + }; + (pkg_id, entry) + } + + fn write_lockfile(&self, entries: Vec<(String, LockfileEntry)>) { + Lockfile { + version: 2, + generated_at: "0000000000000001".to_string(), + nu_version: "0.116.1".to_string(), + platform: "x86_64-pc-windows-msvc".to_string(), + packages: entries.into_iter().collect(), + } + .save(self.root()) + .unwrap(); + } + + fn activate_args(&self, packages: &[&str]) -> ActivateArgs { + ActivateArgs { + packages: packages.iter().map(|s| s.to_string()).collect(), + verbose: false, + list: false, + check: false, + } + } + + fn deactivate_args(&self, packages: &[&str]) -> DeactivateArgs { + DeactivateArgs { + packages: packages.iter().map(|s| s.to_string()).collect(), + verbose: false, + } + } + + fn ok_registrar() -> impl Fn(&str, &str, &str) -> anyhow::Result<()> { + |_nu, _bin, _cfg| Ok(()) + } +} + +fn ct(command: &str, path: &str, kind: CompletionKind) -> CompletionTarget { + CompletionTarget { + command: command.to_string(), + path: path.to_string(), + kind, + } +} + +#[test] +fn target_validation_rejects_bad_names_and_duplicates() { + assert!(validate_completion_targets(&[ct("git", "git.nu", CompletionKind::Static)]).is_ok()); + assert!(validate_completion_targets(&[ + ct("git", "git.nu", CompletionKind::CommandBacked), + ct("cargo", "cargo.nu", CompletionKind::Static), + ]) + .is_ok()); + assert!(validate_completion_targets(&[]).is_err()); + assert!(validate_completion_targets(&[ct("9bad", "x.nu", CompletionKind::Static)]).is_err()); + assert!( + validate_completion_targets(&[ct("has space", "x.nu", CompletionKind::Static)]).is_err() + ); + assert!( + validate_completion_targets(&[ct("git", "../escape.nu", CompletionKind::Static)]).is_err() + ); + assert!(validate_completion_targets(&[ct("git", "x.txt", CompletionKind::Static)]).is_err()); + assert!(validate_completion_targets(&[ct("if", "x.nu", CompletionKind::Static)]).is_err()); + assert!(validate_completion_targets(&[ct("use", "x.nu", CompletionKind::Static)]).is_err()); + // Duplicate command, different files. + assert!(validate_completion_targets(&[ + ct("git", "a.nu", CompletionKind::Static), + ct("git", "b.nu", CompletionKind::Static), + ]) + .is_err()); + // Duplicate normalized path, different commands. + assert!(validate_completion_targets(&[ + ct("git", "x.nu", CompletionKind::Static), + ct("cargo", "./x.nu", CompletionKind::Static), + ]) + .is_err()); +} + +#[test] +fn activation_writes_completion_use_lines_and_lockfile_record() { + let env = CompletionTestEnv::new(); + env.write_nu_paths(); + let (id, entry) = env.create_completion( + "acme", + "dev-completions", + &[ + ("git.nu", "# git completions\n"), + ("cargo.nu", "# cargo completions\n"), + ], + vec![ + ct("git", "git.nu", CompletionKind::CommandBacked), + ct("cargo", "cargo.nu", CompletionKind::Static), + ], + ); + env.write_lockfile(vec![(id.clone(), entry)]); + + let runner = FakeCandidateRunner::success(); + execute_with_candidate_runner( + &env.activate_args(&[&id]), + env.root(), + &CompletionTestEnv::ok_registrar(), + &runner, + ) + .unwrap(); + + let content = std::fs::read_to_string(env.managed_file_path()).unwrap(); + assert!(content.starts_with(OWNERSHIP_MARKER), "{content}"); + assert!( + content.contains("# numan:completion acme/dev-completions git"), + "{content}" + ); + assert!( + content.contains("# numan:completion acme/dev-completions cargo"), + "{content}" + ); + // Targets render as `use "" *`, sorted by scoped ID then command: + // cargo's block precedes git's. + let cargo_pos = content + .find("# numan:completion acme/dev-completions cargo") + .unwrap(); + let git_pos = content + .find("# numan:completion acme/dev-completions git") + .unwrap(); + assert!(cargo_pos < git_pos, "{content}"); + let git_line = content + .lines() + .skip_while(|l| !l.contains("# numan:completion acme/dev-completions git")) + .nth(1) + .unwrap(); + assert!(git_line.starts_with("use \""), "{git_line}"); + assert!(git_line.ends_with("\" *"), "{git_line}"); + assert!(git_line.contains("git.nu"), "{git_line}"); + + let lockfile = Lockfile::load(env.root()).unwrap(); + let saved = &lockfile.packages[&id]; + let record = saved + .completion_activation + .as_ref() + .expect("completion record"); + assert_eq!(record.targets.len(), 2); + let git = record.targets.iter().find(|t| t.command == "git").unwrap(); + assert_eq!(git.kind, CompletionKind::CommandBacked); + assert!(git.target_path.ends_with("git.nu")); + // The production activation record stores the managed path built as + // `format!("{vendor_dir}/numan.nu")`, so build the expected value the + // same way; `PathBuf::join` would use a backslash on Windows. + let managed_path_str = format!("{}/numan.nu", env.vendor_dir_str()); + assert!(saved.is_completion_active_for( + &env.nu_hash, + "0.116.1", + &env.vendor_dir_str(), + &managed_path_str, + )); +} + +#[test] +fn already_active_completion_is_skipped_on_reactivation() { + let env = CompletionTestEnv::new(); + env.write_nu_paths(); + let (id, entry) = env.create_completion( + "acme", + "git-completions", + &[("git.nu", "# git completions\n")], + vec![ct("git", "git.nu", CompletionKind::CommandBacked)], + ); + env.write_lockfile(vec![(id.clone(), entry)]); + + let runner = FakeCandidateRunner::success(); + execute_with_candidate_runner( + &env.activate_args(&[&id]), + env.root(), + &CompletionTestEnv::ok_registrar(), + &runner, + ) + .unwrap(); + let before = std::fs::read_to_string(env.managed_file_path()).unwrap(); + + // Second activation: the package is already active for this Nu identity, + // so it is skipped and the managed file is unchanged. + let runner = FakeCandidateRunner::success(); + execute_with_candidate_runner( + &env.activate_args(&[&id]), + env.root(), + &CompletionTestEnv::ok_registrar(), + &runner, + ) + .unwrap(); + let after = std::fs::read_to_string(env.managed_file_path()).unwrap(); + assert_eq!(before, after); +} + +#[test] +fn activation_rejects_cross_package_target_conflict() { + let env = CompletionTestEnv::new(); + env.write_nu_paths(); + let (a, ea) = env.create_completion( + "acme", + "one", + &[("a.nu", "# a\n")], + vec![ct("git", "a.nu", CompletionKind::Static)], + ); + let (b, eb) = env.create_completion( + "acme", + "two", + &[("b.nu", "# b\n")], + vec![ct("git", "b.nu", CompletionKind::CommandBacked)], + ); + env.write_lockfile(vec![(a.clone(), ea), (b.clone(), eb)]); + + let runner = FakeCandidateRunner::success(); + execute_with_candidate_runner( + &env.activate_args(&[&a]), + env.root(), + &CompletionTestEnv::ok_registrar(), + &runner, + ) + .unwrap(); + + // Target ownership is exclusive: `git` already belongs to acme/one. + let runner = FakeCandidateRunner::success(); + let err = execute_with_candidate_runner( + &env.activate_args(&[&b]), + env.root(), + &CompletionTestEnv::ok_registrar(), + &runner, + ) + .unwrap_err(); + assert!(err.to_string().contains("git"), "{err}"); + assert!(err.to_string().contains("acme/one"), "{err}"); + + let lockfile = Lockfile::load(env.root()).unwrap(); + assert!(lockfile.packages[&b].completion_activation.is_none()); +} + +#[test] +fn activation_rejects_missing_target_and_traversal_declaration() { + // Declared target file does not exist in the payload. + let env = CompletionTestEnv::new(); + env.write_nu_paths(); + let (id, entry) = env.create_completion( + "acme", + "broken", + &[("a.nu", "# a\n")], + vec![ct("git", "missing.nu", CompletionKind::Static)], + ); + env.write_lockfile(vec![(id.clone(), entry)]); + let runner = FakeCandidateRunner::success(); + let err = execute_with_candidate_runner( + &env.activate_args(&[&id]), + env.root(), + &CompletionTestEnv::ok_registrar(), + &runner, + ) + .unwrap_err(); + assert!(err.to_string().contains("acme/broken"), "{err}"); + assert!(!env.managed_file_path().exists()); + + // Traversal declaration persisted directly in the lockfile (bypassing + // install-time validation) must still fail at activation time. + let env2 = CompletionTestEnv::new(); + env2.write_nu_paths(); + let (id2, entry2) = env2.create_completion( + "acme", + "evil", + &[("a.nu", "# a\n")], + vec![ct("git", "../escape.nu", CompletionKind::Static)], + ); + env2.write_lockfile(vec![(id2.clone(), entry2)]); + let runner = FakeCandidateRunner::success(); + let err = execute_with_candidate_runner( + &env2.activate_args(&[&id2]), + env2.root(), + &CompletionTestEnv::ok_registrar(), + &runner, + ) + .unwrap_err(); + assert!(err.to_string().contains("acme/evil"), "{err}"); + assert!(!env2.managed_file_path().exists()); +} + +#[test] +fn deactivation_removes_only_that_packages_lines() { + let env = CompletionTestEnv::new(); + env.write_nu_paths(); + let (a, ea) = env.create_completion( + "acme", + "one", + &[("a.nu", "# a\n")], + vec![ct("git", "a.nu", CompletionKind::Static)], + ); + let (b, eb) = env.create_completion( + "acme", + "two", + &[("b.nu", "# b\n")], + vec![ct("cargo", "b.nu", CompletionKind::CommandBacked)], + ); + env.write_lockfile(vec![(a.clone(), ea), (b.clone(), eb)]); + for id in [&a, &b] { + let runner = FakeCandidateRunner::success(); + execute_with_candidate_runner( + &env.activate_args(&[id]), + env.root(), + &CompletionTestEnv::ok_registrar(), + &runner, + ) + .unwrap(); + } + let runner = FakeCandidateRunner::success(); + deactivate_with_runner(&env.deactivate_args(&[&a]), env.root(), &runner).unwrap(); + + let content = std::fs::read_to_string(env.managed_file_path()).unwrap(); + assert!(!content.contains("acme/one"), "{content}"); + assert!(!content.contains("a.nu"), "{content}"); + assert!( + content.contains("# numan:completion acme/two cargo"), + "{content}" + ); + assert!(content.contains("b.nu"), "{content}"); + let lockfile = Lockfile::load(env.root()).unwrap(); + assert!(lockfile.packages[&a].completion_activation.is_none()); + assert!(lockfile.packages[&b].completion_activation.is_some()); +} + +/// Real-Nu acceptance: after activation, a fresh Nu session that loads the +/// managed file receives the contributed completion (the `fakecmd` extern +/// is defined and its positional parameter is bound to the declared +/// completer in the command signature); after deactivation it no longer +/// does. User config is never touched. +/// +/// Nu does not expose reedline's completion machinery to non-interactive +/// invocations, so this asserts the signature-level binding rather than +/// simulating a TAB keypress. +#[test] +#[ignore] +fn real_nu_completion_received_after_activation_and_gone_after_deactivation() { + use numan_cli::nu::autoload::NuCandidateRunner; + + // Resolve the Nu executable to a real path once: CI runs this test with + // `nu` only on PATH and no NU_BINARY, and bare names resolve against the + // working directory for `std::fs` calls, not PATH. + let nu: PathBuf = match std::env::var("NU_BINARY") { + Ok(p) if !p.is_empty() => PathBuf::from(p), + _ => { + let exe_name = format!("nu{}", std::env::consts::EXE_SUFFIX); + std::env::var_os("PATH") + .and_then(|paths| { + std::env::split_paths(&paths) + .map(|dir| dir.join(&exe_name)) + .find(|candidate| candidate.is_file()) + }) + .expect("nu executable not found on PATH (set NU_BINARY to its path)") + } + }; + let nu_version_out = std::process::Command::new(&nu) + .arg("--version") + .output() + .unwrap(); + let nu_version = String::from_utf8_lossy(&nu_version_out.stdout) + .trim() + .to_string(); + let nu_bytes = std::fs::read(&nu).unwrap(); + let nu_hash = integrity::compute_sha256(&nu_bytes); + let nu_abs = std::fs::canonicalize(&nu) + .unwrap() + .to_string_lossy() + .into_owned(); + + let dir = TempDir::new().unwrap(); + let root = dir.path(); + let vendor_dir = root.join("vendor").join("autoload"); + std::fs::create_dir_all(&vendor_dir).unwrap(); + let vendor_str = vendor_dir.to_string_lossy().into_owned(); + let managed_path = vendor_dir.join("numan.nu"); + + NuPaths { + nu_executable: nu_abs.clone(), + nu_version: nu_version.clone(), + plugin_registry_path: root.join("plugins.msgpackz").to_string_lossy().into_owned(), + nu_executable_hash: nu_hash, + platform: "x86_64-unknown-linux-gnu".to_string(), + data_dir: Some(root.join("data").to_string_lossy().into_owned()), + vendor_autoload_dirs: vec![vendor_str.clone()], + vendor_autoload_dir: Some(vendor_str), + } + .save(root) + .unwrap(); + + // A stand-in for user-authored Nu configuration: activation and + // deactivation must never modify it. + let user_config = root.join("config.nu"); + let user_config_content = "# my config\nalias ll = ls -l\n"; + std::fs::write(&user_config, user_config_content).unwrap(); + + // Fixture completion package: a completer plus an extern that binds it. + let payload_rel = "packages/completions/acme/fake-completions/1.0.0-aabbccdd"; + let payload_abs = root.join(payload_rel); + std::fs::create_dir_all(&payload_abs).unwrap(); + std::fs::write( + payload_abs.join("fakecmd-completions.nu"), + "def \"nu-complete fakecmd branches\" [] { [\"main\", \"dev\", \"feature-x\"] }\n\ + \n\ + export extern \"fakecmd\" [\n\ + \x20 branch?: string@\"nu-complete fakecmd branches\"\n\ + \x20 --flag\n\ + ]\n", + ) + .unwrap(); + + let pkg_id = "acme/fake-completions".to_string(); + let entry = LockfileEntry { + hook_activation: None, + hook_bindings: None, + version: "1.0.0".to_string(), + package_type: "completion".to_string(), + source: "archive".to_string(), + target: None, + artifact_url: None, + artifact_sha256: None, + executable_path: None, + archive_root: None, + include: None, + entry: None, + installed_at: "0000000000000001".to_string(), + nu_version_at_install: Some(nu_version.clone()), + 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_rel.to_string(), + revision_id: None, + payload_sha256: None, + executable_sha256: None, + selection_reason: None, + origin: None, + module_activation: None, + module_import_mode: None, + overlay: None, + script_activation: None, + script_entry_points: None, + completion_activation: None, + completion_targets: Some(vec![ct( + "fakecmd", + "fakecmd-completions.nu", + CompletionKind::CommandBacked, + )]), + locked_dependencies: BTreeMap::new(), + }; + Lockfile { + version: 2, + generated_at: "0000000000000001".to_string(), + nu_version: nu_version.clone(), + platform: "x86_64-unknown-linux-gnu".to_string(), + packages: vec![(pkg_id.clone(), entry)].into_iter().collect(), + } + .save(root) + .unwrap(); + + let registrar = |_nu: &str, _bin: &str, _cfg: &str| Ok(()); + let runner = NuCandidateRunner::new(&nu_abs); + execute_with_candidate_runner( + &ActivateArgs { + packages: vec![pkg_id.clone()], + verbose: false, + list: false, + check: false, + }, + root, + ®istrar, + &runner, + ) + .unwrap(); + assert!(managed_path.exists()); + + // Fresh session: prepend the managed file (as a vendor-autoload load + // would) and assert the extern + completer binding are received. + let managed = std::fs::read_to_string(&managed_path).unwrap(); + let session = root.join("session.nu"); + std::fs::write( + &session, + format!( + "{managed}\n\ + if (\"fakecmd\" not-in (scope commands | get name)) {{ exit 11 }}\n\ + let sig = (scope commands | where name == \"fakecmd\" | first | get signatures | values | first)\n\ + if (\"nu-complete fakecmd branches\" not-in ($sig | get completion)) {{ exit 12 }}\n\ + print \"completion received\"\n" + ), + ) + .unwrap(); + let out = std::process::Command::new(&nu_abs) + .arg("--no-config-file") + .arg(&session) + .output() + .unwrap(); + assert!( + out.status.success(), + "fresh session did not receive the completion: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&out.stdout).trim(), + "completion received" + ); + + // Deactivate and prove the contribution is gone from a fresh session. + let runner = NuCandidateRunner::new(&nu_abs); + deactivate_with_runner( + &DeactivateArgs { + packages: vec![pkg_id.clone()], + verbose: false, + }, + root, + &runner, + ) + .unwrap(); + + let mut session2_content = String::new(); + if managed_path.exists() { + session2_content = std::fs::read_to_string(&managed_path).unwrap(); + } + let session2 = root.join("session2.nu"); + std::fs::write( + &session2, + format!( + "{session2_content}\n\ + if (\"fakecmd\" in (scope commands | get name)) {{ exit 13 }}\n\ + print \"completion gone\"\n" + ), + ) + .unwrap(); + let out = std::process::Command::new(&nu_abs) + .arg("--no-config-file") + .arg(&session2) + .output() + .unwrap(); + assert!( + out.status.success(), + "fresh session still receives the completion after deactivation: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&out.stdout).trim(), + "completion gone" + ); + + assert_eq!( + std::fs::read_to_string(&user_config).unwrap(), + user_config_content, + "user config must be untouched" + ); +} + +#[test] +fn activate_recovers_interrupted_completion_deactivation() { + // Mirror of the module recovery case: a deactivation replaced (deleted) + // the managed file and journaled the transition, then crashed before the + // lockfile record was cleared. The next activation must complete the + // interrupted deactivation (old completion's record cleared, not + // resurrected) and activate the new package. + let env = CompletionTestEnv::new(); + env.write_nu_paths(); + let (old_id, old_entry) = env.create_completion( + "acme", + "old-completions", + &[("git.nu", "# git completions\n")], + vec![ct("git", "git.nu", CompletionKind::Static)], + ); + let (next_id, next_entry) = env.create_completion( + "acme", + "next-completions", + &[("cargo.nu", "# cargo completions\n")], + vec![ct("cargo", "cargo.nu", CompletionKind::Static)], + ); + env.write_lockfile(vec![ + (old_id.clone(), old_entry), + (next_id.clone(), next_entry), + ]); + + let runner = FakeCandidateRunner::success(); + execute_with_candidate_runner( + &env.activate_args(&[&old_id]), + env.root(), + &CompletionTestEnv::ok_registrar(), + &runner, + ) + .unwrap(); + let activation = Lockfile::load(env.root()).unwrap().packages[&old_id] + .completion_activation + .clone() + .unwrap(); + let previous_sha = integrity::compute_sha256(&std::fs::read(env.managed_file_path()).unwrap()); + std::fs::remove_file(env.managed_file_path()).unwrap(); + + std::fs::create_dir_all(env.root().join("state")).unwrap(); + PendingAutoload { + schema_version: AUTOLOAD_SCHEMA_VERSION, + operation: AutoloadOperation::Deactivate, + stage: AutoloadStage::Replaced, + nu_executable_sha256: env.nu_hash.clone(), + nu_version: "0.116.1".to_string(), + vendor_autoload_dir: activation.vendor_autoload_dir, + managed_file_path: activation.managed_file_path, + previous_file_exists: true, + previous_file_sha256: Some(previous_sha), + desired_file_exists: false, + candidate_sha256: None, + previous_active_module_ids: vec![old_id.clone()], + desired_active_module_ids: vec![], + targeted_module_ids: vec![old_id.clone()], + created_at: "0000000000000002".to_string(), + pre_mutation_snapshot_id: None, + } + .save(env.root()) + .unwrap(); + + execute_with_candidate_runner( + &env.activate_args(&[&next_id]), + env.root(), + &CompletionTestEnv::ok_registrar(), + &runner, + ) + .unwrap(); + + let recovered = Lockfile::load(env.root()).unwrap(); + assert!( + recovered.packages[&old_id].completion_activation.is_none(), + "interrupted deactivation must clear the old completion record" + ); + assert!(recovered.packages[&next_id].completion_activation.is_some()); + assert_eq!( + AutoloadState::load(env.root()) + .unwrap() + .unwrap() + .active_module_ids, + vec![next_id.clone()] + ); + assert!(PendingAutoload::load(env.root()).unwrap().is_none()); + let content = std::fs::read_to_string(env.managed_file_path()).unwrap(); + assert!(content.contains("# numan:completion acme/next-completions cargo")); + assert!(!content.contains("old-completions")); +} + +#[test] +fn recovery_rebuilds_completion_record_from_replaced_activate_journal() { + // The rebuild branch of `apply_replaced_transition` is distinct from the + // deactivation-clearing case above: here an interrupted *activation* + // replaced the managed file and journaled the transition, but crashed + // before writing the completion_activation record and autoload state. + // Recovery must rebuild the record from the persisted declaration. + let env = CompletionTestEnv::new(); + env.write_nu_paths(); + let (id, entry) = env.create_completion( + "acme", + "git-completions", + &[("git.nu", "# git completions\n")], + vec![ct("git", "git.nu", CompletionKind::CommandBacked)], + ); + env.write_lockfile(vec![(id.clone(), entry)]); + + let runner = FakeCandidateRunner::success(); + execute_with_candidate_runner( + &env.activate_args(&[&id]), + env.root(), + &CompletionTestEnv::ok_registrar(), + &runner, + ) + .unwrap(); + + // Simulate the crash window: the managed file is in place, but the + // lockfile record and derived autoload state were never written. + let managed_content = std::fs::read(env.managed_file_path()).unwrap(); + let candidate_sha = integrity::compute_sha256(&managed_content); + AutoloadState::delete(env.root()).unwrap(); + let mut lockfile = Lockfile::load(env.root()).unwrap(); + lockfile + .packages + .get_mut(&id) + .unwrap() + .completion_activation = None; + lockfile.save(env.root()).unwrap(); + + std::fs::create_dir_all(env.root().join("state")).unwrap(); + PendingAutoload { + schema_version: AUTOLOAD_SCHEMA_VERSION, + operation: AutoloadOperation::Activate, + stage: AutoloadStage::Replaced, + nu_executable_sha256: env.nu_hash.clone(), + nu_version: "0.116.1".to_string(), + vendor_autoload_dir: env.vendor_dir_str(), + managed_file_path: format!("{}/numan.nu", env.vendor_dir_str()), + previous_file_exists: false, + previous_file_sha256: None, + desired_file_exists: true, + candidate_sha256: Some(candidate_sha), + previous_active_module_ids: vec![], + desired_active_module_ids: vec![id.clone()], + targeted_module_ids: vec![id.clone()], + created_at: "0000000000000003".to_string(), + pre_mutation_snapshot_id: None, + } + .save(env.root()) + .unwrap(); + + let _guard = acquire_mutation_lock(env.root()).unwrap(); + let nu_paths = NuPaths::load(env.root()).unwrap(); + let mut lockfile = Lockfile::load(env.root()).unwrap(); + assert_eq!( + reconcile_pending_autoload(env.root(), &nu_paths, &mut lockfile).unwrap(), + AutoloadRecoveryOutcome::ReplacedCompleted + ); + + let recovered = Lockfile::load(env.root()).unwrap(); + let record = recovered.packages[&id] + .completion_activation + .as_ref() + .expect("recovery must rebuild the completion record"); + assert_eq!(record.targets.len(), 1); + assert_eq!(record.targets[0].command, "git"); + assert_eq!(record.targets[0].kind, CompletionKind::CommandBacked); + assert!(record.targets[0].target_path.ends_with("git.nu")); + assert_eq!( + AutoloadState::load(env.root()) + .unwrap() + .unwrap() + .active_module_ids, + vec![id.clone()] + ); + assert!(PendingAutoload::load(env.root()).unwrap().is_none()); +} diff --git a/tests/init_test.rs b/tests/init_test.rs index 8709795..f2d0888 100644 --- a/tests/init_test.rs +++ b/tests/init_test.rs @@ -12,7 +12,8 @@ use numan_cli::nu::autoload::{CandidateRunner, FakeCandidateRunner}; use numan_cli::nu::paths::NuPaths; use numan_cli::state::autoload_state::AutoloadState; use numan_cli::state::lockfile::{ - Lockfile, LockfileEntry, ModuleActivation, ScriptActivation, ScriptActivationCommand, + CompletionActivation, CompletionActivationTarget, Lockfile, LockfileEntry, ModuleActivation, + ScriptActivation, ScriptActivationCommand, }; use numan_cli::util::fs_safety::OWNERSHIP_MARKER; use tempfile::TempDir; @@ -136,6 +137,8 @@ impl InitRefreshEnv { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }, ); @@ -193,6 +196,69 @@ impl InitRefreshEnv { ); lockfile.save(self.root()).unwrap(); } + + fn write_active_completion_lockfile(&self, nu_exe: &Path) { + let payload_rel = "packages/completions/owner/completions/1.0.0-aabbccdd"; + let payload_abs = self.root().join(payload_rel); + std::fs::create_dir_all(&payload_abs).unwrap(); + std::fs::write(payload_abs.join("git.nu"), b"# git completions\n").unwrap(); + + let mut lockfile = Lockfile::empty(); + lockfile.packages.insert( + "owner/completions".to_string(), + LockfileEntry { + package_type: "completion".to_string(), + payload_path: payload_rel.to_string(), + completion_targets: Some(vec![numan_cli::core::package::CompletionTarget { + command: "git".to_string(), + path: "git.nu".to_string(), + kind: numan_cli::core::package::CompletionKind::CommandBacked, + }]), + completion_activation: Some(CompletionActivation { + targets: vec![CompletionActivationTarget { + command: "git".to_string(), + path: "git.nu".to_string(), + target_path: payload_abs.join("git.nu").to_string_lossy().into_owned(), + kind: numan_cli::core::package::CompletionKind::CommandBacked, + }], + vendor_autoload_dir: self.vendor_dir_str(), + managed_file_path: self.managed_file.to_string_lossy().into_owned(), + nu_executable_sha256: integrity::compute_sha256( + &std::fs::read(nu_exe).unwrap(), + ), + nu_version: "0.113.1".to_string(), + activated_at: "now".to_string(), + }), + ..empty_lockfile_entry() + }, + ); + lockfile.save(self.root()).unwrap(); + } + + fn write_completion_managed_file(&self) -> String { + let target = self + .root() + .join("packages/completions/owner/completions/1.0.0-aabbccdd/git.nu"); + let content = format!( + "{OWNERSHIP_MARKER}\n# numan:completion owner/completions git\nuse \"{}\" *\n", + nu_string_literal(target.to_string_lossy().as_ref()) + ); + std::fs::write(&self.managed_file, &content).unwrap(); + content + } + + fn write_completion_autoload_state(&self, nu_hash: &str, content: &str) { + let state = AutoloadState::new( + self.vendor_dir_str(), + self.managed_file.to_string_lossy().into_owned(), + nu_hash.to_string(), + "0.113.1".to_string(), + integrity::compute_sha256(content.as_bytes()), + vec!["owner/completions".to_string()], + "now".to_string(), + ); + state.save(self.root()).unwrap(); + } } fn empty_lockfile_entry() -> LockfileEntry { @@ -232,6 +298,8 @@ fn empty_lockfile_entry() -> LockfileEntry { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), } } @@ -350,6 +418,125 @@ fn refresh_regenerates_script_wrappers_for_new_nu_executable() { ); } +#[test] +fn refresh_with_active_completion_keeps_identity_when_nu_path_unchanged() { + let env = InitRefreshEnv::new(); + let vendor = env.vendor_dir_str(); + let paths_v1 = env.paths(&env.nu_v1, &vendor); + paths_v1.save(env.root()).unwrap(); + env.write_active_completion_lockfile(&env.nu_v1); + let managed = env.write_completion_managed_file(); + env.write_completion_autoload_state(&env.hash_v1, &managed); + + let detect = { + let paths = paths_v1.clone(); + move || Ok(paths.clone()) + }; + execute_with_runner( + &InitArgs { refresh: true }, + env.root(), + detect, + fake_runner_factory, + ) + .unwrap(); + + // Managed file and activation identity are untouched. + let content = std::fs::read_to_string(&env.managed_file).unwrap(); + assert_eq!(content, managed); + let lockfile = Lockfile::load(env.root()).unwrap(); + let activation = lockfile.packages["owner/completions"] + .completion_activation + .as_ref() + .unwrap(); + assert_eq!(activation.nu_executable_sha256, env.hash_v1); +} + +#[test] +fn refresh_with_active_completion_regenerates_for_new_nu_executable() { + let env = InitRefreshEnv::new(); + let vendor = env.vendor_dir_str(); + let paths_v1 = env.paths(&env.nu_v1, &vendor); + paths_v1.save(env.root()).unwrap(); + env.write_active_completion_lockfile(&env.nu_v1); + let managed = env.write_completion_managed_file(); + env.write_completion_autoload_state(&env.hash_v1, &managed); + + let paths_v2 = env.paths(&env.nu_v2, &vendor); + let detect = { + let paths = paths_v2.clone(); + move || Ok(paths.clone()) + }; + execute_with_runner( + &InitArgs { refresh: true }, + env.root(), + detect, + fake_runner_factory, + ) + .unwrap(); + + let content = std::fs::read_to_string(&env.managed_file).unwrap(); + assert!( + content.contains("# numan:completion owner/completions git"), + "{content}" + ); + assert!(content.contains("git.nu"), "{content}"); + + let lockfile = Lockfile::load(env.root()).unwrap(); + let activation = lockfile.packages["owner/completions"] + .completion_activation + .as_ref() + .unwrap(); + assert_eq!(activation.nu_executable_sha256, env.hash_v2); + + let state = AutoloadState::load(env.root()).unwrap().unwrap(); + assert_eq!( + state.generated_file_sha256, + integrity::compute_sha256(content.as_bytes()) + ); +} + +#[test] +fn refresh_validation_failure_keeps_live_managed_file_with_active_completion() { + let env = InitRefreshEnv::new(); + let vendor = env.vendor_dir_str(); + let paths_v1 = env.paths(&env.nu_v1, &vendor); + paths_v1.save(env.root()).unwrap(); + env.write_active_completion_lockfile(&env.nu_v1); + let managed = env.write_completion_managed_file(); + + let paths_v2 = env.paths(&env.nu_v2, &vendor); + let detect = { + let paths = paths_v2.clone(); + move || Ok(paths.clone()) + }; + let err = execute_with_runner( + &InitArgs { refresh: true }, + env.root(), + detect, + failing_runner_factory, + ) + .unwrap_err(); + assert!( + err.to_string().contains("candidate rejected"), + "expected validation failure, got: {err}" + ); + + // The live managed file must survive a failed refresh validation: + // validation runs against a copy, never the live file. + assert!(env.managed_file.is_file()); + assert_eq!(std::fs::read_to_string(&env.managed_file).unwrap(), managed); + + let lockfile = Lockfile::load(env.root()).unwrap(); + let activation = lockfile.packages["owner/completions"] + .completion_activation + .as_ref() + .unwrap(); + assert_eq!( + activation.nu_executable_sha256, env.hash_v1, + "refresh must not commit lockfile when validation fails" + ); +} + #[test] fn refresh_rejects_vendor_target_change_with_active_modules() { let env = InitRefreshEnv::new(); diff --git a/tests/install_test.rs b/tests/install_test.rs index 2538db1..9c7173f 100644 --- a/tests/install_test.rs +++ b/tests/install_test.rs @@ -780,3 +780,149 @@ fn integration_install_self_heals_corrupted_cached_artifact() { "cache entry must be replaced with the verified artifact" ); } + +#[test] +fn integration_force_reinstall_refuses_active_completion() { + use numan_cli::core::package::{CompletionKind, CompletionTarget, RegistryActivationSpec}; + use numan_cli::state::lockfile::CompletionActivation; + + let tmp = TempDir::new().unwrap(); + let root = tmp.path().to_path_buf(); + + let signing_key = setup_trusted_key(&root, "test"); + + let artifacts_dir = root.join("artifacts"); + std::fs::create_dir_all(&artifacts_dir).unwrap(); + let (zip_url, zip_sha) = create_plugin_zip(&artifacts_dir, "git.nu", b"# git completions\n"); + + let package = Package { + id: ScopedId::new("test", "completions"), + description: "Test completion package for integration test".to_string(), + repo: "https://github.com/test/completions".to_string(), + package_type: PackageType::Completion, + tags: vec!["test".to_string()], + versions: vec![VersionEntry { + version: semver::Version::new(1, 0, 0), + nu_version: "*".to_string(), + verified_with: vec![], + artifact: Artifact { + kind: "archive".to_string(), + url: Some(zip_url.clone()), + sha256: Some(zip_sha.clone()), + targets: HashMap::new(), + archive_root: None, + include: None, + entry: None, + }, + source: None, + dependencies: BTreeMap::new(), + activation: Some(RegistryActivationSpec { + kind: "nu-completion".to_string(), + targets: vec![CompletionTarget { + command: "git".to_string(), + path: "git.nu".to_string(), + kind: CompletionKind::Static, + }], + ..Default::default() + }), + provenance: None, + evidence_tier: None, + deferral_reason: None, + }], + }; + create_signed_registry(&root, "test", vec![package], &signing_key); + + std::fs::write( + root.join("config.toml"), + "[general]\ndefault_registry = \"test\"\n", + ) + .unwrap(); + + let platform = Platform { + os: Os::Windows, + arch: Arch::X86_64, + env: Env::Msvc, + triple: "x86_64-pc-windows-msvc".to_string(), + }; + let nu_version = NuVersion::parse("0.113.1").unwrap(); + + let options = InstallOptions { + root: &root, + platform: &platform, + nu_version: &nu_version, + force: false, + verbose: false, + registry_name: None, + snapshot_trigger: SnapshotTrigger::Install, + }; + + let result = transaction::install_package("test/completions", None, &options).unwrap(); + assert!(result.installed); + + // Mark the package active, as `numan activate` would record it. + let mut lockfile = Lockfile::load(&root).unwrap(); + { + let entry = lockfile.packages.get_mut("test/completions").unwrap(); + assert!( + entry.completion_targets.is_some(), + "install must persist the declared completion targets" + ); + entry.completion_activation = Some(CompletionActivation { + targets: Vec::new(), + vendor_autoload_dir: "/tmp/vendor".to_string(), + managed_file_path: "/tmp/vendor/numan.nu".to_string(), + nu_executable_sha256: "abc".to_string(), + nu_version: "0.113.1".to_string(), + activated_at: "0".to_string(), + }); + } + lockfile.save(&root).unwrap(); + + // A non-force reinstall of the same version is a no-op and must keep + // the activation record. + let again = transaction::install_package("test/completions", None, &options).unwrap(); + assert!(again.already_existed); + + // A force reinstall would replace the entry with a fresh inactive one + // while numan.nu keeps the package's lines: it must refuse and tell + // the user to deactivate first. + let force_options = InstallOptions { + root: &root, + platform: &platform, + nu_version: &nu_version, + force: true, + verbose: false, + registry_name: None, + snapshot_trigger: SnapshotTrigger::Install, + }; + let err = transaction::install_package("test/completions", None, &force_options).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("active as a completion"), "got: {msg}"); + assert!(msg.contains("deactivate"), "got: {msg}"); + + // The activation record survives the refused reinstall. + let lockfile = Lockfile::load(&root).unwrap(); + assert!( + lockfile.packages["test/completions"] + .completion_activation + .is_some(), + "force reinstall must not clear the completion activation record" + ); + + // A same-version non-force reinstall also replaces the entry when the + // installed payload directory is missing, so it must refuse too rather + // than silently orphaning the managed autoload lines. + let payload_dir = root.join(lockfile.packages["test/completions"].payload_path()); + std::fs::remove_dir_all(&payload_dir).unwrap(); + let err = transaction::install_package("test/completions", None, &options).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("active as a completion"), "got: {msg}"); + assert!(msg.contains("deactivate"), "got: {msg}"); + let lockfile = Lockfile::load(&root).unwrap(); + assert!( + lockfile.packages["test/completions"] + .completion_activation + .is_some(), + "missing-payload reinstall must not clear the completion activation record" + ); +} diff --git a/tests/module_autoload_test.rs b/tests/module_autoload_test.rs index c4c1d7d..1b480ce 100644 --- a/tests/module_autoload_test.rs +++ b/tests/module_autoload_test.rs @@ -161,6 +161,8 @@ impl ModuleTestEnv { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }; diff --git a/tests/nupm_compat_test.rs b/tests/nupm_compat_test.rs index d157da9..b4ae18a 100644 --- a/tests/nupm_compat_test.rs +++ b/tests/nupm_compat_test.rs @@ -294,6 +294,8 @@ fn t18_registry_collision_fails_without_lockfile_change() { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: Default::default(), }, ); diff --git a/tests/script_activation_test.rs b/tests/script_activation_test.rs index 1c3d1fe..20382b4 100644 --- a/tests/script_activation_test.rs +++ b/tests/script_activation_test.rs @@ -127,6 +127,8 @@ impl ScriptTestEnv { overlay: None, script_activation: None, script_entry_points: Some(entry_points), + completion_activation: None, + completion_targets: None, locked_dependencies: BTreeMap::new(), }; (pkg_id, entry) @@ -336,7 +338,8 @@ fn real_nu_script_command_runs_and_forwards_exit_status() { nu_executable: nu.clone(), }; let rendered = - numan_cli::nu::autoload::generate_managed_content(&[], std::slice::from_ref(&cmd)).unwrap(); + numan_cli::nu::autoload::generate_managed_content(&[], std::slice::from_ref(&cmd), &[]) + .unwrap(); std::fs::write(&wrapper, format!("{rendered}\nhello world\n")).unwrap(); let out = std::process::Command::new(&nu) .arg("--no-config-file") diff --git a/tests/setup_nu_test.rs b/tests/setup_nu_test.rs index 3b64218..cd4b077 100644 --- a/tests/setup_nu_test.rs +++ b/tests/setup_nu_test.rs @@ -732,6 +732,8 @@ fn list_shows_bundled_with_nu_tag() { overlay: None, script_activation: None, script_entry_points: None, + completion_activation: None, + completion_targets: None, locked_dependencies: Default::default(), }, );