feat: implement numan use for side-by-side Nu version management - #67
Conversation
…nfirm utility Breaking change: --remove, --use-path, --use-existing are replaced by subcommands (remove, path, use <path>). Hidden backward-compat flags emit deprecation warnings (remove in v0.3.0). - Add src/util/confirm.rs: confirm_or_auto / confirm_or_bail (non-TTY auto-confirms) - Replace 16 inline TTY-check sites across 8 files - Normalize all --yes help text to 'Skip confirmation prompts' - Redesign NuSetupArgs with Option<NuAction> subcommand + positional VERSION - Add NuSetupArgs constructors for internal callers (doctor, nu_pin_offer) - Add 8 CLI-parse tests + update all affected test assertions - Update README, CHANGELOG, docs/numan-doctor.md, AGENTS.md
- Prevent PATH subcommand from deleting active managed Nu - Guard loader overwrites with ownership verification - Reject incompatible legacy Nu setup flags
Fix remaining --version references in resolve.rs, tighten doctor_test assertion, add skip_path guard to legacy compat path, add negative tests for version+subcommand and legacy use_existing+skip_path.
Change 'your PATH Nu is not touched' to 'your existing Nu is not replaced' since setup nu does modify PATH by default.
Introduce a new `numan use <version>` command path (`cli`, `main`, and `cmd::use_cmd`) as a post-1.0 placeholder that currently fails with a clear guidance message to use `numan setup nu <version>`. Update roadmap/docs to reflect the consolidated plan filename, add post-1.0 side-by-side Nu management notes, and register the new command module in AGENTS metadata.
Fixed 4 file(s) based on 5 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
- Add src/nu/version_manager.rs for active version tracking - Active version stored in <root>/nu_state/active-version.json - Installed versions discovered from <root>/tools/nushell/<version>/ - Helpers: read/write active version, list installed, check if installed - Migration logic for legacy single-binary installs - Update bootstrap.rs to install to versioned subdirectories - install_from_archive() now writes to <root>/tools/nushell/<version>/ - managed_nu_binary() delegates to version_manager for active version - Automatically sets newly installed version as active - Implement numan use command - numan use <version> — switch to specific installed version - numan use latest — switch to newest installed version - numan use list — show all installed versions with active marker - Validates version is installed before switching - Provides helpful hints when version not found - Auto-migrates legacy single-binary installs on first run - Remove Commands::Use from root init exclusion (now needs root) All 425 tests pass, clippy clean, fmt applied.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @tonythethompson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds side-by-side Nu installations, active-version persistence, legacy migration, and ChangesNushell version management
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
PR Summary by QodoAdd
AI Description
Diagram
High-Level Assessment
Files changed (31)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Greptile SummaryThis PR implements
Confidence Score: 4/5Mostly safe to merge; one defect in the install fast-path leaves the active-version marker out of sync with the session PATH when a pinned version is already on disk. The already-installed early return in Files Needing Attention:
|
| Filename | Overview |
|---|---|
| src/nu/bootstrap.rs | Refactored to write versioned layout, persist active-version marker post-install, and hoist PATH-consent flag. Contains a P1 bug: the --yes already-installed early-return skips the active-marker write, leaving marker/PATH out of sync for the pinned-version case. |
| src/nu/version_manager.rs | New file implementing the side-by-side version management core: active-version JSON marker read/write/clear, versioned binary path helpers, normalized-version validation (path-traversal safe), and list/resolve APIs with off-tree fallback. Well-tested; note previously-flagged migration re-entry guard issue. |
| src/cmd/use_cmd.rs | New numan use command implementing list/latest/switch subcommands with mutation lock and PreMutation snapshot; previously-flagged hint text still references deprecated --version flag in error messages. |
| src/cmd/setup.rs | Adds consolidated destructive-step confirm gate (merged deletion + PATH prompt) for execute_use_path/execute_use_existing, require_tty_or_yes guard before managed-tree removal, ExecuteUseOpts DI seam for tests, and clears active-version marker before remove_dir_all. |
| src/util/confirm.rs | Adds require_tty_or_yes / require_tty_or_yes_with_tty hard-fail guard for destructive non-interactive sessions, and extracts TTY injection seam (confirm_or_auto_with_tty) for deterministic test coverage. |
| src/nu/paths.rs | Extends find_nu_executable_with_root with a 3-step active-marker hint table (on-tree versioned → off-tree recorded path → full version scan fallback), updates error hint to new subcommand syntax, and changes validate_nushell_binary to return the version string. |
| src/util/hints.rs | Adds POSIX shell_quote helper to make setup_nu_use_existing copy-pasteable for paths with spaces or shell metacharacters; well-tested. Note: single-quote wrapping is not valid on Windows cmd/PowerShell. |
Reviews (14): Last reviewed commit: "Merge branch 'master' into feature/numan..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Validate and normalize Nu versions - Propagate legacy migration errors - Handle invalid active markers gracefully - Avoid parent path panic - Update Nu setup guidance
Qodo Fixer✅ Committed (5) · ☑ Fixed (5) Commits pushed directly to this PR — no separate fix PR opened. Process — 5 fixed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cmd/setup.rs (1)
374-384: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire explicit confirmation for non-TTY configuration changes.
The shared confirmation path now auto-confirms when standard input is not a terminal. A piped or CI invocation can overwrite loader or configuration files without
--yes.src/cmd/snapshot.rsstill rejects non-TTY sessions, so the mutation contract is inconsistent.Keep the non-TTY guard for loader and configuration writes, or require an explicit
yesvalue.Also applies to: 421-426
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cmd/setup.rs` around lines 374 - 384, Update the overwrite-confirmation logic in the setup flow around assert_managed_file_owned and confirm_or_bail, including the corresponding configuration-write block, to reject non-TTY sessions unless args.yes is explicitly set. Preserve interactive confirmation for TTY sessions and the existing cancellation behavior, matching the non-TTY guard used by the snapshot command.Source: MCP tools
♻️ Duplicate comments (7)
src/cmd/use_cmd.rs (2)
1-33:⚠️ Potential issue | 🟠 MajorMutating Nushell entry points bypass the required lock boundary.
Both paths can rename, remove, register, or write state without an exclusive lock and pre-write snapshot.
src/cmd/use_cmd.rs#L1-L33: acquire the mutation lock and snapshot before migration and command dispatch.src/cmd/setup.rs#L161-L207: route doctor and normal setup calls through one lock-aware entry point without double-locking.As per coding guidelines: “Every new mutating code path must acquire the mutation lock and snapshot the lockfile before writes.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cmd/use_cmd.rs` around lines 1 - 33, The mutating use command currently performs migration and dispatch without the required exclusive lock and pre-write snapshot. Update use_cmd::execute to acquire the mutation lock and snapshot before migrate_legacy_install or any command dispatch. In src/cmd/setup.rs lines 161-207, route doctor and normal setup through a single lock-aware entry point and avoid acquiring the lock twice.Source: Coding guidelines
35-104:⚠️ Potential issue | 🟠 MajorRefresh cached Nu state after changing the active version.
execute_latestandexecute_switchwriteactive-version.jsonbut do not refreshNuPathsor activation records. Subsequent activation commands can still identify the previous Nu binary untilnuman init --refresh.Refresh after
write_active_versionsucceeds, using a lock-aware entry point.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cmd/use_cmd.rs` around lines 35 - 104, After each successful version_manager::write_active_version call in execute_latest and execute_switch, refresh the cached NuPaths and activation records through the existing lock-aware refresh entry point. Ensure the refresh runs only after the active-version write succeeds and preserve the current success messages and error propagation.Source: MCP tools
src/nu/version_manager.rs (3)
153-234:⚠️ Potential issue | 🟠 MajorDo not run an unbounded Nu probe during migration.
Command::output()waits for process exit and pipe closure. A hung legacy binary can blocknuman useindefinitely. Prefer the existingVERSIONmetadata. If a process probe remains necessary, make it explicit and enforce a wall-clock timeout.As per coding guidelines: “External-call hazards ... blocking calls without timeouts on request threads.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/nu/version_manager.rs` around lines 153 - 234, Update migrate_legacy_install to avoid the unbounded Command::output probe when determining the legacy binary version, preferring the existing VERSION metadata. If probing the binary remains necessary, execute it through an explicit timeout mechanism that enforces a wall-clock limit and handles timeout or termination before continuing migration.Source: Coding guidelines
261-341: 🧹 Nitpick | 🔵 TrivialAdd seam-based tests for
migrate_legacy_install.The tests cover marker persistence, discovery, and output parsing, but not migration rename, marker-write failure, skip behavior, or process failure. Inject the version detector and use a fake instead of spawning real
nu.As per coding guidelines: “Unit tests must use injectable seams ...; do not spawn real
nuin unit tests” and “Behavior changes require corresponding tests.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/nu/version_manager.rs` around lines 261 - 341, Add seam-based unit tests for migrate_legacy_install covering successful directory migration and marker persistence, skipping when migration is unnecessary, marker-write failures, and detector/process failures. Refactor the migration flow to accept or reuse an injectable version-detection seam, then use a fake detector in tests so no real nu process is spawned; assert each expected result and filesystem state.Source: Coding guidelines
1-58:⚠️ Potential issue | 🟠 MajorKeep broken active-version state visible.
The read path treats invalid markers as absent. The active-binary path also treats a selected version with a missing binary as absent. These states are not equivalent.
src/nu/bootstrap.rscan then select the legacy binary whilenu_state/active-version.jsonnames another version.Return
Noneonly when the marker file is missing. Return a contextual error for malformed or dangling state, and letnuman doctorreport it.Also applies to: 78-105
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/nu/version_manager.rs` around lines 1 - 58, Update read_active_version so it returns None only when active-version.json is missing; propagate malformed JSON as a contextual error instead of treating it as absent. In the active-binary resolution path, distinguish a selected version whose binary is missing from no active marker and return a contextual dangling-state error. Preserve these errors so numan doctor can report the broken active-version state rather than falling back to the legacy binary.Source: MCP tools
src/cmd/setup.rs (2)
242-253:⚠️ Potential issue | 🔴 CriticalComplete replacement validation before destructive removal.
resolved.is_file()checks only the file type. It does not validate executable permissions or Nushell format. Line 253 removes the managed installation beforeregister_existing_nuperforms full validation. A regular invalid file can delete the working installation and then return an error.Reuse the full Nushell validator before removal. Apply the same ordering to
execute_use_path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cmd/setup.rs` around lines 242 - 253, Update execute_use_existing and execute_use_path to run the full Nushell binary validation, including format and executable checks, before calling remove_managed_nu_if_present. Reuse the existing validator that register_existing_nu uses, while preserving the current path resolution and error context, so invalid binaries cannot trigger destructive removal.
242-253:⚠️ Potential issue | 🟠 MajorKeep active-version state consistent with managed-tree removal.
The removal helpers operate on the directory that now contains all side-by-side Nu versions. Deleting it removes every installed version but leaves
nu_state/active-version.jsonunchanged. Future resolution can then reference a missing binary and fall back to a different binary.Preserve the versioned tree for replacement flows, or clear the active marker atomically under the mutation lock and report the destructive scope.
Also applies to: 264-311
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cmd/setup.rs` around lines 242 - 253, Update execute_use_existing and the related removal flows so deleting the managed version tree cannot leave nu_state/active-version.json pointing to a missing binary. Preserve the versioned tree during replacement operations, or clear the active-version marker atomically while holding the mutation lock; ensure destructive removal reports that all installed versions are affected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/cmd/setup.rs`:
- Around line 374-384: Update the overwrite-confirmation logic in the setup flow
around assert_managed_file_owned and confirm_or_bail, including the
corresponding configuration-write block, to reject non-TTY sessions unless
args.yes is explicitly set. Preserve interactive confirmation for TTY sessions
and the existing cancellation behavior, matching the non-TTY guard used by the
snapshot command.
---
Duplicate comments:
In `@src/cmd/setup.rs`:
- Around line 242-253: Update execute_use_existing and execute_use_path to run
the full Nushell binary validation, including format and executable checks,
before calling remove_managed_nu_if_present. Reuse the existing validator that
register_existing_nu uses, while preserving the current path resolution and
error context, so invalid binaries cannot trigger destructive removal.
- Around line 242-253: Update execute_use_existing and the related removal flows
so deleting the managed version tree cannot leave nu_state/active-version.json
pointing to a missing binary. Preserve the versioned tree during replacement
operations, or clear the active-version marker atomically while holding the
mutation lock; ensure destructive removal reports that all installed versions
are affected.
In `@src/cmd/use_cmd.rs`:
- Around line 1-33: The mutating use command currently performs migration and
dispatch without the required exclusive lock and pre-write snapshot. Update
use_cmd::execute to acquire the mutation lock and snapshot before
migrate_legacy_install or any command dispatch. In src/cmd/setup.rs lines
161-207, route doctor and normal setup through a single lock-aware entry point
and avoid acquiring the lock twice.
- Around line 35-104: After each successful
version_manager::write_active_version call in execute_latest and execute_switch,
refresh the cached NuPaths and activation records through the existing
lock-aware refresh entry point. Ensure the refresh runs only after the
active-version write succeeds and preserve the current success messages and
error propagation.
In `@src/nu/version_manager.rs`:
- Around line 153-234: Update migrate_legacy_install to avoid the unbounded
Command::output probe when determining the legacy binary version, preferring the
existing VERSION metadata. If probing the binary remains necessary, execute it
through an explicit timeout mechanism that enforces a wall-clock limit and
handles timeout or termination before continuing migration.
- Around line 261-341: Add seam-based unit tests for migrate_legacy_install
covering successful directory migration and marker persistence, skipping when
migration is unnecessary, marker-write failures, and detector/process failures.
Refactor the migration flow to accept or reuse an injectable version-detection
seam, then use a fake detector in tests so no real nu process is spawned; assert
each expected result and filesystem state.
- Around line 1-58: Update read_active_version so it returns None only when
active-version.json is missing; propagate malformed JSON as a contextual error
instead of treating it as absent. In the active-binary resolution path,
distinguish a selected version whose binary is missing from no active marker and
return a contextual dangling-state error. Preserve these errors so numan doctor
can report the broken active-version state rather than falling back to the
legacy binary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 71831383-ada2-41e5-a354-326cf612e513
📒 Files selected for processing (5)
src/cli.rssrc/cmd/setup.rssrc/cmd/snapshot.rssrc/cmd/use_cmd.rssrc/nu/version_manager.rs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Trackdubllc/Trackdub(manual)tonythethompson/QuickShell(manual)tonythethompson/numan(manual)tonythethompson/dependency-chain-substrate(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Greptile Review
- GitHub Check: Analyze (rust)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use Rust 2021 edition conventions and match existing naming, module layout, and documentation level in edited files.
Useanyhow::Resultwith.context(...)in application code; usethiserrorfor library error types that callers match on.
Useclapderive macros for CLI definitions.
Useserdewithserde_jsonortomlfor serialization.
Function parameters must use&Path, not&PathBuf.
Error paths must returnanyhow::Resultwith context, and library code must not panic.
Unit tests must use injectable seams such asFakeCandidateRunnerand registrars; do not spawn realnuin unit tests.
Real-Nu acceptance tests should be marked#[ignore]and run separately when activation or nupm-import behavior changes.
Every new mutating code path must acquire the mutation lock and snapshot the lockfile before writes.
Mutating commands, includinginstall,remove,update,gc, andnupm import, must callacquire_mutation_lock(root).
Lockfiles, journals, and state files must be written atomically usingwrite_json_atomic.
numan installmust write only to$NUMAN_ROOTand must not invoke Nu or register plugins/autoloads.
Onlyactivateanddeactivatemay modify Nu integration state.
Treat the lockfile as the authoritative source of truth; derived projections such as autoload state are not authoritative.
Install payloads must use versioned, content-addressed paths and must never be overwritten in place.
Never overwrite foreign autoload files; respectOWNERSHIP_MARKER.
Pass plugin paths through environment variables only; do not use runtime interpolation in Nu program strings.
The nupm boundary must remain read-only towardNUPM_HOME, must not executebuild.nu, and must not perform bidirectional synchronization.
**/*.rs: Use Rust 2021 edition conventions; useanyhow::Resultfor application errors,thiserrorfor library errors, add error context with.context(...)or?, and never panic in library code.
Use&Pathrather tha...
Files:
src/cli.rssrc/cmd/snapshot.rssrc/nu/version_manager.rssrc/cmd/use_cmd.rssrc/cmd/setup.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{rs,toml}: All changes must pass formatting and linting:cargo fmtandcargo clippy -- -D warnings.
Behavior changes require corresponding tests, including relevant failure paths.
Files:
src/cli.rssrc/cmd/snapshot.rssrc/nu/version_manager.rssrc/cmd/use_cmd.rssrc/cmd/setup.rs
**/*.{rs,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update documentation when changing structure, conventions, or user-visible behavior, using
AGENTS.md,docs/, or command help as appropriate.
Files:
src/cli.rssrc/cmd/snapshot.rssrc/nu/version_manager.rssrc/cmd/use_cmd.rssrc/cmd/setup.rs
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Redact secrets when reporting relevant logs or lockfile excerpts in issues.
**/*: Run CI-equivalent validation:cargo test,cargo clippy -- -D warnings, andcargo fmt --check; ignored real-Nu acceptance tests require Nu 0.113 on PATH.
Do not force-push tomaster; use imperative commit messages under 72 characters, feature/fix branch naming, and squash merges for features.
Files:
src/cli.rssrc/cmd/snapshot.rssrc/nu/version_manager.rssrc/cmd/use_cmd.rssrc/cmd/setup.rs
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Pass Nu paths and names through environment variables (
NUMAN_PLUGIN_BINARY,NUMAN_PLUGIN_CONFIG,NUMAN_PLUGIN_NAME); keep the Nu program string compile-time constant with no runtime interpolation.
Files:
src/cli.rssrc/cmd/snapshot.rssrc/nu/version_manager.rssrc/cmd/use_cmd.rssrc/cmd/setup.rs
src/cli.rs
📄 CodeRabbit inference engine (AGENTS.md)
Define the CLI with Clap derive macros and use lowercase
numanfor the binary command name.
Files:
src/cli.rs
**/*.{js,jsx,ts,tsx,py,java,go,rs,rb,php,c,cpp,h,hpp,cs,swift,kotlin,scala}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx,py,java,go,rs,rb,php,c,cpp,h,hpp,cs,swift,kotlin,scala}: For code-file work, use Serena's semantic, symbol-aware tools as the primary tools. Use built-in Read, Glob, Grep, and Edit only when Serena has failed, the file is not parseable, a cross-file regex discovery is required, a small read is more appropriate, or the full file is absolutely necessary.
Before editing a code file, first runget_symbols_overviewunless it was already run in the session, then usefind_symbolwithinclude_body=truefor the symbols being changed, and edit with Serena symbol-aware operations such asreplace_symbol_body,insert_before_symbol,insert_after_symbol, orreplace_content.
Use Serena mappings for code navigation and editing:get_symbols_overviewfor structure,find_symbolfor symbol bodies or name searches,find_referencing_symbolsfor callers, declaration and implementation tools for definitions, symbol replacement/insertion tools for edits, and hierarchy tools for type relationships.
Write comments only when the rationale is non-obvious, such as a hidden constraint, workaround, or subtle invariant; do not narrate behavior or reference the current task or pull request.
Files:
src/cli.rssrc/cmd/snapshot.rssrc/nu/version_manager.rssrc/cmd/use_cmd.rssrc/cmd/setup.rs
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Understand the existing implementation before changing it, and make the smallest change that satisfies the request.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Do not add unrelated scope, speculative abstractions, unnecessary error handling, feature flags, or backward-compatibility shims unless requested.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Prefer editing existing files over creating new ones, and never create Markdown or README files unless explicitly requested.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: For exploratory questions, respond in two or three sentences with a recommendation and the main tradeoff; do not implement until the user agrees.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: For UI or frontend changes that cannot be tested in a browser, explicitly state that browser testing was not performed rather than claiming success.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Watch for security issues such as injection, XSS, SQL injection, path traversal, and secret leaks, and fix them when discovered.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Pause and confirm before destructive, hard-to-reverse, externally visible, or third-party-upload actions, including deleting files or branches, dropping tables, killing processes, force-pushing, modifying CI/CD, posting externally, or uploading content.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: When encountering an obstacle, investigate the root cause; do not bypass it with `--no-verify`, `--force`, or deletion of the blocking item.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Investigate unfamiliar files, branches, or configuration before deleting them because they may contain the user's in-progress work.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: A user's approval applies only to the requested action and does not grant perpetual approval for later actions.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Only commit when the user asks; never proactively create commits.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Never update Git configuration or skip hooks with `--no-verify` or `--no-gpg-sign` unless explicitly requested.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Prefer new commits over amending. If a pre-commit hook fails, fix the issue, re-stage, and create a new commit instead of amending.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Stage files by explicit name; do not use `git add -A` or `git add .` because they may include secrets or large binaries.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Use a HEREDOC for commit messages and include the trailer `Co-Authored-By: Claude Opus 4.7 (1M context) <noreplyanthropic.com>`.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Do not push unless asked. Never force-push to `main` or `master`; warn if explicitly asked.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: For pull requests, use `gh` via Bash and inspect the complete diff against the base branch before drafting the title and body.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Before the first tool call, state in one sentence what will be done; provide short updates at key moments, avoid narrating internal deliberation, and end with a one- or two-sentence summary.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Use GitHub-flavored Markdown, reference code locations as `path:line`, avoid emojis unless requested, and match response shape to task complexity.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: Run independent tool calls in parallel; run dependent calls sequentially after resolving their inputs without placeholders or guesses.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:12:22.839Z
Learning: When ambiguity materially changes the work, ask one focused question; otherwise choose a reasonable interpretation and state it.
🔍 Remote MCP DeepWiki, GitHub Copilot
Relevant review context
- Scope mismatch: The supplied summary describes implemented side-by-side version management, but PR
#66’s actual diff adds only a reservednuman usestub that always errors and suggestssetup nu; it does not add active-version persistence, version discovery, migration, or switching logic. [src/cmd/use_cmd.rs] - DeepWiki identifies Nu identity as cached in
NuPathsand activation state as stored in lockfile/autoload records. Switching binaries requiresnuman init --refreshto update those identities; stale journals, plugin activation records, and autoload state must be rejected or refreshed. - The PR changes confirmation behavior globally: non-TTY sessions now auto-confirm activation, deactivation, setup, loader configuration, and snapshot rollback instead of requiring
--yes. This is a significant behavioral/security change and is documented as breaking. - The
setup nu use <path>andsetup nu pathpaths remove the managed Nushell directory before registering the replacement. The specific-path flow performs removal before the replacement path is validated, so an invalid path can destroy the existing managed installation. - All reported PR checks passed, including Ubuntu/Windows/macOS tests, real-Nu acceptance, Clippy, MSRV, CodeQL, Rust analysis, and formatting.
🔇 Additional comments (5)
src/nu/version_manager.rs (2)
107-151: Verify that installed-version sorting uses a total order.If invalid directory names reach a comparator that mixes semantic-version order with lexical fallback, values such as
0.10.0,0.2.0, and0.15.0xcan produce a non-transitive order. Filter invalid names before sorting, or partition valid and invalid names. Add a regression test.
66-77: LGTM!src/cmd/snapshot.rs (1)
1-2: LGTM!Also applies to: 243-246
src/cli.rs (1)
64-65: LGTM!src/cmd/use_cmd.rs (1)
106-116: LGTM!
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/nu/bootstrap.rs (1)
699-713: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject non-interactive setup when
--yesis absent.Line 704 delegates to
confirm_or_bail. In non-TTY execution, that helper auto-confirms. The flow then downloads Nu and can callpersist_user_pathat Lines 718-719 without explicit consent.When
options.yesis false and stdin is not interactive, return the cancellation error. Preserve automatic execution only for callers that explicitly set--yes.Based on learnings: “Pause and confirm before destructive, hard-to-reverse, externally visible” actions. External tool context states that
confirm_or_bailauto-confirms in non-TTY execution.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/nu/bootstrap.rs` around lines 699 - 713, Update the setup flow around the final `confirm_or_bail` call so non-interactive execution returns the existing cancellation error when `options.yes` is false; only bypass confirmation for callers explicitly setting `options.yes`. Preserve the interactive confirmation behavior and the earlier reinstall prompt, while preventing download and subsequent `persist_user_path` execution without consent.Sources: Learnings, MCP tools
♻️ Duplicate comments (2)
src/cmd/use_cmd.rs (1)
23-31: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSnapshot the lockfile before migration writes.
Line 26 acquires the mutation lock, but Line 30 can rename the legacy binary and write active-version state before any lockfile snapshot occurs. Add the established snapshot step immediately after lock acquisition and before migration or dispatch.
As per coding guidelines: “Every new mutating code path must acquire the mutation lock and snapshot the lockfile before writes.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cmd/use_cmd.rs` around lines 23 - 31, Add the established lockfile snapshot operation immediately after acquiring the mutation lock in execute, before migrate_legacy_install or any subsequent dispatch. Preserve the existing mutation lock and migration error context while ensuring all writes occur after both protections are established.Source: Coding guidelines
src/nu/bootstrap.rs (1)
237-240: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not activate Nu during archive installation.
Line 240 writes the active-version marker as an archive extraction side effect. This silently selects a Nu version and leaves Nu identity-bound state stale until refresh.
Return the installed version to the caller. Run the activation lifecycle only after explicit user consent.
As per coding guidelines: “Only
activateanddeactivatemay modify Nu integration state.” Based on learnings: “Do not silently switch the user's Nushell version; managed Nu pinning must be offer-based and require user consent.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/nu/bootstrap.rs` around lines 237 - 240, Remove the write_active_version call from the archive installation flow in the Nu bootstrap function, and return the installed version after writing the VERSION file. Move activation to the explicit user-consent lifecycle, ensuring only activate/deactivate modify Nu integration state and archive extraction never silently selects a version.Sources: Coding guidelines, Learnings, MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/nu/bootstrap.rs`:
- Around line 699-713: Update the setup flow around the final `confirm_or_bail`
call so non-interactive execution returns the existing cancellation error when
`options.yes` is false; only bypass confirmation for callers explicitly setting
`options.yes`. Preserve the interactive confirmation behavior and the earlier
reinstall prompt, while preventing download and subsequent `persist_user_path`
execution without consent.
---
Duplicate comments:
In `@src/cmd/use_cmd.rs`:
- Around line 23-31: Add the established lockfile snapshot operation immediately
after acquiring the mutation lock in execute, before migrate_legacy_install or
any subsequent dispatch. Preserve the existing mutation lock and migration error
context while ensuring all writes occur after both protections are established.
In `@src/nu/bootstrap.rs`:
- Around line 237-240: Remove the write_active_version call from the archive
installation flow in the Nu bootstrap function, and return the installed version
after writing the VERSION file. Move activation to the explicit user-consent
lifecycle, ensuring only activate/deactivate modify Nu integration state and
archive extraction never silently selects a version.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 22ea51d3-a219-4069-8c2c-a2edf74dbc42
📒 Files selected for processing (2)
src/cmd/use_cmd.rssrc/nu/bootstrap.rs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Trackdubllc/Trackdub(manual)tonythethompson/QuickShell(manual)tonythethompson/numan(manual)tonythethompson/dependency-chain-substrate(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Greptile Review
- GitHub Check: Analyze (rust)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use Rust 2021 edition conventions and match existing naming, module layout, and documentation level in edited files.
Useanyhow::Resultwith.context(...)in application code; usethiserrorfor library error types that callers match on.
Useclapderive macros for CLI definitions.
Useserdewithserde_jsonortomlfor serialization.
Function parameters must use&Path, not&PathBuf.
Error paths must returnanyhow::Resultwith context, and library code must not panic.
Unit tests must use injectable seams such asFakeCandidateRunnerand registrars; do not spawn realnuin unit tests.
Real-Nu acceptance tests should be marked#[ignore]and run separately when activation or nupm-import behavior changes.
Every new mutating code path must acquire the mutation lock and snapshot the lockfile before writes.
Mutating commands, includinginstall,remove,update,gc, andnupm import, must callacquire_mutation_lock(root).
Lockfiles, journals, and state files must be written atomically usingwrite_json_atomic.
numan installmust write only to$NUMAN_ROOTand must not invoke Nu or register plugins/autoloads.
Onlyactivateanddeactivatemay modify Nu integration state.
Treat the lockfile as the authoritative source of truth; derived projections such as autoload state are not authoritative.
Install payloads must use versioned, content-addressed paths and must never be overwritten in place.
Never overwrite foreign autoload files; respectOWNERSHIP_MARKER.
Pass plugin paths through environment variables only; do not use runtime interpolation in Nu program strings.
The nupm boundary must remain read-only towardNUPM_HOME, must not executebuild.nu, and must not perform bidirectional synchronization.
**/*.rs: Use Rust 2021 edition and MSRV 1.88; ensure code builds with the stable toolchain.
Useanyhow::Resultfor application code andthiserrorfor library error types; add context with.context(...)or?, and nev...
Files:
src/cmd/use_cmd.rssrc/nu/bootstrap.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{rs,toml}: All changes must pass formatting and linting:cargo fmtandcargo clippy -- -D warnings.
Behavior changes require corresponding tests, including relevant failure paths.Use
cargo testas the required test gate; all tests must pass, with platform-specific behavior covered using mock platforms.
Files:
src/cmd/use_cmd.rssrc/nu/bootstrap.rs
**/*.{rs,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update documentation when changing structure, conventions, or user-visible behavior, using
AGENTS.md,docs/, or command help as appropriate.
Files:
src/cmd/use_cmd.rssrc/nu/bootstrap.rs
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Redact secrets when reporting relevant logs or lockfile excerpts in issues.
Files:
src/cmd/use_cmd.rssrc/nu/bootstrap.rs
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.rs: Use#[cfg(target_env)]and compile-time target information for platform detection, notstd::env::consts.
Useserde/serde_jsonfor JSON state andtomlfor configuration serialization.
All JSON state files, including the lockfile, journals, andnu_state/paths.json, must usewrite_json_atomicwith a same-directory temporary file and persist operation.
Serialize mutations withacquire_mutation_lock(root); the second acquisition for the same root must fail immediately rather than block.
Files:
src/cmd/use_cmd.rssrc/nu/bootstrap.rs
**/*.{js,jsx,ts,tsx,py,java,go,rs,rb,php,c,cpp,h,hpp,cs,swift,kt,kts,scala}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx,py,java,go,rs,rb,php,c,cpp,h,hpp,cs,swift,kt,kts,scala}: For code files, use Serena's semantic tools as the primary tools for structure inspection, symbol reading, searching, and editing; use built-in Read, Glob, Grep, and Edit only under the explicitly listed fallback conditions.
Before editing a code file, runget_symbols_overview, thenfind_symbolwithinclude_body=truefor the symbols being changed, and edit using Serena symbol-editing tools such asreplace_symbol_body,insert_before_symbol,insert_after_symbol, orreplace_content.
Add comments only when the reason is non-obvious, such as a hidden constraint, workaround, or subtle invariant; do not narrate behavior or reference the current task or PR.
Watch for and fix security issues when discovered, including injection, XSS, SQL injection, path traversal, and secret leaks.
Files:
src/cmd/use_cmd.rssrc/nu/bootstrap.rs
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:20.434Z
Learning: Source builds require explicit user consent before cloning or building, with a separate consent scope.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:20.434Z
Learning: Do not silently switch the user's Nushell version; managed Nu pinning must be offer-based and require user consent.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:20.434Z
Learning: Do not force-push to `master`; use imperative commit messages under 72 characters and branch names such as `feature/description` or `fix/description`.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:30.030Z
Learning: Understand existing code before changing it and make the smallest change that satisfies the request; do not add unrelated cleanup, speculative abstractions, unnecessary error handling, feature flags, or compatibility shims unless requested.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:30.030Z
Learning: Prefer editing existing files over creating new ones, and never create Markdown or README files unless explicitly requested.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:30.030Z
Learning: For exploratory questions, respond with a 2–3 sentence recommendation and its main tradeoff; do not implement until the user agrees.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:30.030Z
Learning: Pause and confirm before destructive, hard-to-reverse, externally visible, or third-party-upload actions, including deleting data, modifying CI/CD, pushing changes, posting messages, or uploading content.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:30.030Z
Learning: When encountering an obstacle, investigate the root cause; do not bypass it with `--no-verify`, `--force`, or deletion of the blocking item, and investigate unfamiliar files, branches, or configuration before deleting them.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:30.030Z
Learning: Only commit when the user asks; never update Git configuration; do not skip hooks; prefer new commits over amend; stage files by name; do not commit likely secrets; and do not push unless asked.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:30.030Z
Learning: If a pre-commit hook fails, treat the commit as unsuccessful, fix the issue, restage, and create a new commit rather than amending.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:30.030Z
Learning: Use a HEREDOC for commit messages and include the trailer `Co-Authored-By: Claude Opus 4.7 (1M context) <noreplyanthropic.com>`.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:30.030Z
Learning: Never force-push to `main` or `master`; warn if such an action is requested.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:30.030Z
Learning: For pull requests, use `gh` via Bash and inspect the full diff against the base branch before drafting the title and body.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:30.030Z
Learning: Before the first tool call, briefly state the intended action; provide short updates at key moments; avoid narrating internal deliberation; and end with a one- or two-sentence summary.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:30.030Z
Learning: When tool calls are independent, issue them in parallel; when dependent, run them sequentially after resolving required values.
Learnt from: CR
Repo: tonythethompson/numan
Timestamp: 2026-08-02T03:19:30.030Z
Learning: Ask one focused question when ambiguity materially changes the work; otherwise choose a reasonable interpretation and state it.
🔍 Remote MCP DeepWiki, GitHub Copilot
Relevant review context
- The PR’s actual implementation adds a reserved
numan usestub: it always returns an error and suggestsnuman setup nu [VERSION]; it does not implement active-version persistence, discovery, migration, or switching. [src/cmd/use_cmd.rs] - DeepWiki identifies
NuPaths, lockfile activation records, autoload state, and journals as identity-bound to the selected Nu executable. Switching binaries requires refreshingpaths.json, activation identities, autoload state, and stale journals vianuman init --refresh; merely adding ausecommand would not be sufficient. execute_use_existingremoves the managed Nu directory before validating the supplied replacement path. An invalid or non-runnable path can therefore destroy the existing managed installation. [src/cmd/setup.rs]- The same destructive ordering exists for
setup nu pathafter PATH discovery, although that path is validated before removal. [src/cmd/setup.rs] - Confirmation behavior changed globally: non-TTY invocations now auto-confirm activation, deactivation, snapshot rollback, setup, loader overwrite/configuration, and PATH changes. This is a broad security/behavioral change, not limited to Nu version management. [
src/util/confirm.rs, CHANGELOG.md`] - All reported CI checks passed, including Ubuntu, Windows, macOS, real-Nu acceptance, MSRV, Clippy, CodeQL, formatting, and static analysis.
🔀 Multi-repo context tonythethompson/QuickShell, Trackdubllc/Trackdub
Linked repositories findings
tonythethompson/QuickShell
- Nushell is launched generically via
nu -c '…'; nonumancommands or managed-version paths are referenced.[::tonythethompson/QuickShell::](docs/architecture/launch.md:130,QuickShell.Core/Services/TerminalLauncherArgs.cs:62-70)
Trackdubllc/Trackdub
- Only incidental documentation references to Nushell-related projects; no
numanCLI or setup API consumers found.[::Trackdubllc/Trackdub::](docs/plans/open-core-split-continuation.md:264,docs/plans/phase-3-validation-report.md:140)
tonythethompson/dependency-chain-substrate
- No references to
numan, Nushell setup commands, or version-management paths were found.
🔇 Additional comments (1)
src/nu/bootstrap.rs (1)
668-685: LGTM!Also applies to: 741-741
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
6f842c4
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| pub fn active_nu_binary(root: &Path) -> Result<Option<PathBuf>> { | ||
| let Some(active) = read_active_version(root)? else { | ||
| return Ok(None); | ||
| }; | ||
| let version = normalize_version(&active.version)?; | ||
|
|
||
| // Prefer the on-tree version-binary when present. This covers the | ||
| // common case where the off-tree marker was later matched by an | ||
| // on-tree install (e.g. the user ran `setup nu <version>` to give the | ||
| // off-tree selection a versioned home). | ||
| let on_tree = version_binary(root, &version); | ||
| if on_tree.exists() { | ||
| return Ok(Some(on_tree)); | ||
| } | ||
|
|
||
| // Fall back to the recorded off-tree path when one was stored. | ||
| if let Some(off_tree) = active.binary_path.as_ref() { | ||
| let off_tree_path = std::path::PathBuf::from(off_tree); | ||
| if off_tree_path.is_file() { | ||
| return Ok(Some(off_tree_path)); | ||
| } | ||
| } | ||
|
|
||
| // Build the message conditionally: skip the off-tree clause when no | ||
| // off-tree path is recorded. The literal "<none>" placeholder previously | ||
| // rendered here was clunky in `numan doctor` output. | ||
| match active.binary_path.as_ref() { | ||
| Some(off_tree) => Err(anyhow::anyhow!( | ||
| "Active Nu version '{}' is set but neither the on-tree binary at '{}' \ | ||
| nor the recorded off-tree path '{}' is present. \ | ||
| Run 'numan setup nu' to install the selected version or \ | ||
| 'numan use <version>' / 'numan use latest' to choose a different one.", | ||
| version, | ||
| on_tree.display(), | ||
| std::path::PathBuf::from(off_tree).display(), | ||
| )), | ||
| None => Err(anyhow::anyhow!( | ||
| // pre-migration `nu_state/active-version.json` markers have no off-tree field | ||
| "Active Nu version '{}' is set but the on-tree binary at '{}' is missing. \ | ||
| Run 'numan setup nu' to install the selected version or \ | ||
| 'numan use <version>' / 'numan use latest' to choose a different one.", | ||
| version, | ||
| on_tree.display(), | ||
| )), | ||
| } | ||
| } |
There was a problem hiding this comment.
🔍 Off-tree marker resolution is tolerant of stale paths in one lookup and fatal in another
find_nu_executable_with_root deliberately falls through to PATH when both the on-tree and recorded off-tree paths are stale (src/nu/paths.rs:316-333), while version_manager::active_nu_binary returns Err for the same state (src/nu/version_manager.rs:210-232). That divergence is documented, but active_nu_binary currently has no callers, so the "doctor surfaces dangling markers" behaviour the doc comments promise does not exist yet — nothing reports a marker pointing at a deleted binary.
Was this helpful? React with 👍 or 👎 to provide feedback.
23c0e31
993acea
The equire_tty_or_yes hard-fail guard introduced for destructive uman setup paths correctly blocks non-TTY, non---yes runs, but it also prevented the unit test that injects a decline-only confirm seam from exercising the merged prompt. Skip the TTY guard only when a confirm seam is provided, so the production fail-closed behavior is preserved while tests can still verify the decline path and the 'no undo' prompt text.
Resolve conflicts between journaled legacy migration (#69) and shipped numan use (#67): keep mutate-path migration under the use mutation guard, prefer off-tree-aware version resolution from master, and retain PR setup_subcommand_lock audit labeling for doctor repair. Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve conflicts from #67 (numan use) landing on master while this branch carried migration/hardening: - Keep PR71 migration journal, setup_subcommand_lock, --force, and require_tty_or_yes hardening. - Fold master additions: confirm_or_auto_with_tty, resolve_installed_version, execute_nu_repair alias, reject_skip_path helper, installed-version PATH fallback, and roadmap PATH docs. - Align setup unit fake-nu + confirm-seam tty gates so execute_use_existing tests pass after the merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Anthony Thompson <github@trackdub.com>
Summary
Implements
numan usefor side-by-side Nu version management. The legacy-migration story (single-binary → versioned layout, journaled transition, doctor reconcile) lives on its own PR (#PR_MIGRATE_ID) so this PR stays small and reviewable.Changes
Tracking:
src/nu/version_manager.rsActive version tracking + versioned-layout helpers for side-by-side Nu installs. The active Nu selector is stored at
<root>/nu_state/active-version.jsonand the per-version binaries live at<root>/tools/nushell/<v>/nu. Helpers include listing installed versions, normalizing version strings, resolving the active binary (on-tree or off-tree via the marker), and clearing the active marker (cleared atomically before removing the versioned tree so the marker cannot dangle at a missing binary).This file no longer carries the legacy single-binary migration fs in this PR — that work lands in #PR_MIGRATE_ID and the call site in
cmd/use_cmd.rsis dormant until then.Updated:
src/nu/bootstrap.rsinstall_from_archive()writes to<root>/tools/nushell/<version>/(immutable, one dir per version).managed_nu_binary()delegates toversion_managerfor active version lookup.numan setup nu <x.y.z>persists the freshly installed version as the active version atomically (active-marker write sequenced after archive extraction).numan setup nuis fail-closed without--yes, mirroring setup.rs; refuses to mutate state silently.caller_consented_destructiveflag onNuSetupOptionsso setup.rs and bootstrap.rs only prompt once for the combined wipe + PATH-add.New:
numan usecommand (src/cmd/use_cmd.rs)numan use <version>— switch to a specific installed version.numan use latest— switch to the newest installed version.numan use list— show installed versions + the active marker + per-version binary paths.setup nu <version>if it isn't. Does not auto-download.numan setup nuandnuman usecannot interleave) and creates a PreMutation snapshot before any marker write.numan setup nu use <path>switches to a user-supplied Nu outside the versioned layout, the marker records the resolved binary path so subsequentnuman use listandfind_nu_executable_with_rootcan resolve that version even if no on-tree install exists.New: destructive-confirm +
--yesgate insrc/cmd/setup.rsFor
numan setup nu use <path>/numan use existing, where the destructive step is wiping<root>/tools/nushell/(every managed Nu version + active-version marker) before adding the resolved Nu's parent to PATH:"there is no undo".!args.yes && std::io::stdin().is_terminal()— non-interactive sessions refuse to commit destructive mutations without explicit--yes. An audit-gradeeprintln!records whether the gate was an explicit--yesor an interactive confirmation.caller_consented_destructiveso setup.rs only prompts once for the combined wipe + PATH-add.New:
require_tty_or_yes(src/util/confirm.rs)Hard-fail audit helper used by destructive
numan setupflows. Refuses to run mutations in a non-TTY session without--yes; audit-gradeeprintln!records the gate outcome. Other destructive prompts (remove_managed_nu, the loader overwrite) can adopt the same rubric in a follow-up.docs/plans/consolidated-multi-repo-roadmap.md"reserved stub"/ "post-1.0" line fornuman use.numan use <version>errors (with a hint to runsetup nu <version>) if the version isn't installed — it never auto-downloads.<root>/nu_state/active-version.json, atomic clear-before-tree-remove, and that PATH persistence is owned bynuman setup nuwhilenuman useonly flips the active marker under the mutation lock with a PreMutation snapshot.Testing
cargo clippy -- -D warningsclean.cargo fmt --checkclean.src/cmd/setup.rs(DI seams viaExecuteUseOpts<'a>).Architecture Notes
This implements the non-auto-installing shape of
numan use: switching the active marker is fast and silent, but downloading a new Nu stays innuman setup nu --version <version>. If the requested version is not installed, the command errors with a hint to runsetup nufirst. This keepsnuman usepredictable for users (no surprise downloads) and matches the north-star product decision: Nu-compat onboarding flows throughsetup nu + tryrather thanuse.Future Work
numan aliasfor Numan-level aliases (post-1.0)