Refactor setup commands and implement version management features - #71
Merged
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.
- Validate and normalize Nu versions - Propagate legacy migration errors - Handle invalid active markers gracefully - Avoid parent path panic - Update Nu setup guidance
…ompilation errors
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
…master for setup.rs, snapshot.rs, cli.rs, bootstrap.rs; keep full use_cmd.rs implementation
wip: puts numan use + confirm-gate UX + migration journal + reconcile into a single baseline commit so the pr-migrate-legacy-installs split can branch off a known starting point. This commit does not represent either final PR; the next step surgically extracts migration into its own branch and reverts the migration hunks on feature/numan-use.
- Pull 251 lines of migration fs (Legacy*Detector type alias, LegacyPostCreateHook type alias, detect_legacy_version, migrate_legacy_install, migrate_legacy_install_with_detector, parse_nu_version_from_output) plus 8 migrate_legacy_* tests + create_legacy_binary helper + production_detector_prefers_version_metadata_file + test_parse_nu_version_from_output into a new crate::nu::migrate_legacy module. - Add 'pub(crate)' to write_active_marker (now consumed by crate::nu::migrate_legacy). - Retarget use_cmd.rs to call crate::nu::migrate_legacy::migrate_legacy_install. - Re-register the module in src/nu/mod.rs. This is the pr-migrate-legacy-installs half of the split: phase 1 cleanup + journaled transaction for legacy single-binary -> versioned installation with self-healing and numan doctor --fix reconciliation. Co-authored-by: Codebuff <noreply@codebuff.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Threads addressed (changed in code):
* chatgpt PR69 S1A src/cmd/doctor.rs
doctor --fix auto-repair branch reacquires the root mutation lock
before invoking migration_journal::reconcile, so a concurrent numan use
cannot race the same Prepared -> Renamed transition.
* copilot PR69 VwSra src/state/migration_journal.rs
PendingMigration::load now hard-fails on unknown schema_version
(no longer silently coerces); a future journal variant cannot be
misinterpreted as the current one.
* cubic PR69 UzV src/cmd/use_cmd.rs
numan use latest preserves an existing off-tree binary_path when the
selected version matches the existing marker (the previous write
dropped binary_path, breaking resolution of 'setup nu use <path>'
picks).
* chatgpt PR69 S08 src/cmd/setup.rs
Both setup nu use <path> callsites now write_active_version_with_binary
after register_existing_nu succeeds, so numan use list reports the
registered off-tree binary as the active selection.
* chatgpt PR69 S09 src/cmd/setup.rs
remove_managed_nu and remove_managed_nu_if_present clear the
active-version marker at the top of the function, before deleting the
managed tree, so the marker cannot dangle at a binary just removed.
* cubic PR69 UzU src/nu/migrate_legacy.rs
parse_nu_version_from_output delegates to core::nu_version::NuVersion
::parse, which already strips build-hash suffixes ('0.113.1 (abc123)').
Manually-placed legacy binaries whose --version includes a build hash
migrate cleanly; bare '0.113.1' falls through to the semver-only path.
* cubic PR69 UzM src/nu/paths.rs
find_nu_executable_with_root now propagates read_active_version errors
instead of silently treating them as 'no marker'. A present-but-malformed
active-version marker surfaces loudly so numan init / setup loader
cannot silently fall back to PATH Nu.
* cubic PR69 UzG src/nu/migrate_legacy.rs
migrate_legacy_install_with_detector refuses to scan / mutate under a
symlinked managed directory; the rename or filesystem-truth cleanup
cannot redirect outside $NUMAN_ROOT.
Build / clippy / fmt / lib tests (cargo test --lib -> 468 passed; 0
failed).
Deferred (kept as Discussion for the user):
* qodo S26 — anyhow -> thiserror migration in public APIs (large refactor)
* chatgpt S05 — refresh cached NuPaths when switching versions (architectural)
* chatgpt S06 — preserve ~/.local/bin/nu symlink through migration (UX)
* cubic UzO — HOME-mutating test seam isolation (test infra refactor)
* chatgpt VwS04 — install pinned releases to versioned layout (overlaps
PR67 VpJrb on the other branch; left for cross-PR consolidation)
* outdated=True threads: VwSrT, VwSrk, VwS1C (skip per skill rules)
Per user instruction: push to PR branch only; do NOT merge the PR.
🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
…ate cfg(test) read_active_version After the rebase of pr-migrate-legacy-installs onto current master, the detection function was missing its closing bracket (rustc reported an 'unclosed delimiter' at detect_legacy_version that reached EOF), and cargo clippy -- -D warnings surfaced three warnings on lib build: - src/nu/bootstrap.rs:5 — std::io::IsTerminal imported but unused now that the IsTerminal check moved to src/util/confirm.rs. - src/nu/migrate_legacy.rs:19 — read_active_version imported at module scope but only used inside #[cfg(test)]. Restrict to #[cfg(test)] so the lib build is clean and tests still reach it via a sibling import inside the tests module. - src/cmd/doctor.rs:1342 — value assigned to lock was never read; the reacquire from the chatgpt PR69 S1A path is the canonical call. Drop the redundant 'lock = Some(...)' line. Also re-runs cargo fmt to absorb any drift carried in by the rebase. Validation: cargo build OK; cargo clippy -- -D warnings clean; cargo test --lib 468 passed / 0 failed on pr-migrate-legacy-installs.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Ignore active_nu_binary errors when resolving the already-installed binary for latest setup so a stale marker cannot block fallback to the newest on-tree install. Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Resolve conflicts between the setup-subcommand-lock branch and master: - setup.rs: keep branch's versioned-layout use/remove logic (force gate, version-before-mutation, error-context clears) and add master's pre-mutation snapshots. - bootstrap.rs: keep branch's flow-aware already-installed short-circuit (dangling-marker fallback) and add master's snapshot; adopt master's is_tty seam for the non-TTY guard; keep both test sets. - doctor.rs: adopt master's redesigned DoctorArgs (scan/json) and map the branch's migration-journal tests onto it; keep master's fail-closed off-path repair (no --yes). - confirm.rs/hints.rs/mod.rs: auto-merged. - model.rs: deactivate drops --yes (master removed the flag); remove keeps --yes (branch's fail-closed removal gate). - setup_nu_test.rs: keep branch's --force gate tests and master's audit-text golden test; doctor_test.rs: adapt use_existing calls to 3-arg signature.
Derive the concrete release tag from the on-tree install path after `numan setup nu` (no --version) and persist active-version.json so `numan use list` and active_nu_binary stay consistent with pinned installs.
execute_nu_command_wraps_installer planted a legacy tools/nushell/nu
binary; after writing active-version for latest installs, that path's
parent ("nushell") failed normalize_version. Plant a versioned fixture
and prefer versioned installs over leftover legacy binaries when
resolving the active marker.
10 tasks
cursor Bot
pushed a commit
that referenced
this pull request
Aug 3, 2026
Make destructive-guard tests inject non-TTY status, reconcile only after successful managed-Nu deletion, refuse migrating into nonempty version dirs, and propagate active-marker/list errors instead of swallowing them. Co-authored-by: Cursor <cursoragent@cursor.com>
cursor Bot
pushed a commit
that referenced
this pull request
Aug 3, 2026
Serialize paths.rs PATH-mutating tests through PathRestoreGuard, and clear the active-version marker with context immediately before managed tree deletion after confirmation. Co-authored-by: Cursor <cursoragent@cursor.com>
cursor Bot
pushed a commit
that referenced
this pull request
Aug 3, 2026
Resolve conflicts between landed #69/#71 on master and PR83 review hardening. Prefer PR83 safety gates (layout containment, exact latest dest, off-tree marker preserve, list read-only) while adopting master's DoctorArgs --scan API, confirm alias, and snapshot-before-setup wiring. Co-authored-by: Anthony Thompson <github@trackdub.com>
tonythethompson
added a commit
that referenced
this pull request
Aug 4, 2026
* Fix PR #71 review: TTY seams, migration safety, marker order Make destructive-guard tests inject non-TTY status, reconcile only after successful managed-Nu deletion, refuse migrating into nonempty version dirs, and propagate active-marker/list errors instead of swallowing them. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix PR #71 review: PATH test mutex and clear-before-delete Serialize paths.rs PATH-mutating tests through PathRestoreGuard, and clear the active-version marker with context immediately before managed tree deletion after confirmation. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(pr71): hoist migration symlink guard and cover use snapshots Compute managed_dir once and assert_not_symlink before any reconcile stage recovery, including Active. Add regression coverage for a symlinked tools/nushell tree, and assert that `numan use list` creates no snapshot while switch/latest create a PreMutation snapshot. * docs(roadmap): audit each Post-1.0 claim against shipped code Catches the next stale roadmap claim before it misleads downstream planning in numan-plugins or numan-registry. Each bullet in the "Side-by-side Nu version management (numan use)" section is now either backed by src/ code with a file:line pointer or visibly tagged as "Vision only — not yet shipped." Specific corrections (cross-repo claim → shipped behavior): * "numan use list ... + per-version plugin counts" → drop the inaccurate half; execute_list (src/cmd/use_cmd.rs) currently prints version + (active) only. Counts are forward-looking. * "PATH/shim: Numan does not manage a shim." → CONTRADICTION. persist_user_path_unix (src/nu/bootstrap.rs:561) creates a ~/.local/bin/nu symlink via std::os::unix::fs::symlink on Unix; on Windows it appends the binary's parent to the user PATH. The bullet is rewritten into three accurate bullets: PATH (Unix) / PATH (process-only) / Active marker ownership, each citing the concrete call site. * "Lockfile plugin_activation becomes keyed by Nu version" → partial. Each PluginActivation already carries nu_version: String (src/state/lockfile.rs:44); the "Switching activates/deactivates automatically" companion claim is aspirational. Split into one SHIPPED bullet and three Vision-only aspirational bullets so the shipped structure stays separated from the future behavior. * All three "Numan-level aliases (optional)" bullets → Vision only. No numan alias command exists in src/cli.rs; tag each. * Catalog implication + "Use this to drive backfill waves once numan use ships" → Vision-tag the forward-looking clauses; keep the verifiable cross-repo fact about numan-plugins/docs/backlog.json schema v1 (verified outside this repo). The check-roadmap-drift.py script still passes on the post-audit roadmap (0 errors, 1 warning for the absent repo-local roadmap, expected for numan). A re-run of the negative PR67 contradiction injection still exits 1 with four forbidden-phrase matches, so the guardrail survives the audit. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com> * feat(roadmap-contract): freeze the cross-repo guardrail at numan-roadmap-contract/v1 The roadmap-drift CI workflow in numan, numan-plugins, and numan-registry used to fetch the consolidated roadmap + drift script from 'raw.githubusercontent.com/.../numan/master/...'. A push to master could silently change the guardrail; sibling roadmap files drifted independently (PR 67 had to do a 'cross-repo audit' pass to undo 'numan use >= stub' claims after the feature shipped). This commit freezes the cross-repo guardrail at a versioned tag. - docs/contracts/roadmap-v1.md -- the freeze doc explaining what v1 freezes, the sentinel rules, and the bump procedure. - scripts/bump-contract.sh -- the only sanctioned way to bump to vN>1. Validates drift locally, refreshes the pinned SHA in three workflow yml files, pushes the new tag, opens three coordinated PRs. - .github/workflows/ci.yml, cross-repo-mirror/{numan-plugins, numan-registry}/.github/workflows/roadmap-drift.yml -- pinned at CONTRACT_TAG=numan-roadmap-contract/v1 / CONTRACT_SHA=<this commit>. Adds a 'Verify pinned SHA still resolves to tag' step that fails closed if a force-pushed tag moves. - cross-repo-mirror/README.md -- updated to instruct contributors to pin at the contract SHA, not @master. Initial v1 (--init) was published from a pre-commit dry-run; that tag pointed at SHA 99aa695 without the freeze infra and has been deleted. v1 will be recreated at this commit so the tag SHA matches the freeze infra. * fix(roadmap-contract): align CONTRACT_SHA with the v1 tag's resolved SHA The numan-roadmap-contract/v1 tag was created at freeze commit f220940, but the workflow yml files on the same commit pinned CONTRACT_SHA to 99aa695 (the pre-freeze SHA from feature/numan-use HEAD), because that SHA was the only known-stable value at freeze time. The verify-SHA step in every CI job therefore compares the tag (resolved f220940) against the env (99aa695) and fails closed. This follow-up commit rewrites CONTRACT_SHA to f220940 across the four workflow yml files, the cross-repo-mirror README, and the contract doc. The change is in lockstep with a tag force-move so the v1 contract is internally consistent: the tag points to commit f220940, and every workflow yml now names f220940 as its CONTRACT_SHA. URL fetches via $CONTRACT_SHA resolve to f220940's content, which contains the contract doc + drift script + consolidated-multi-repo-roadmap.md that v1 freezes. Validation: $CONTRACT_SHA in ci.yml, numan-plugins, numan-registry, and the README's copy-paste example all match $git rev-parse numan-roadmap-contract/v1^{commit} = f220940. * fix(ci): drop CONTRACT_SHA verify in favor of tag-existence check Earlier freeze revisions pinned both CONTRACT_TAG and a literal CONTRACT_SHA in the yml env. The intent was 'tag-resolved SHA == yml-pinned SHA' to detect force-pushes. But the freeze commit can't self-reference its own SHA without a chicken-and-egg followup commit (ad infinitum); and the literal pin would always be one tag-move behind the frozen tag. Trading the comparison for a tag-existence verification loses force-push detection but gains a single-source-of-truth pattern: the tag itself is the contract. The bump script (scripts/bump-contract.sh) is still the only sanctioned way to create or move a tag, and it explicitly refuses to clobber. So the tag is effectively immutable from the maintainer side. CI fails if the tag disappears; CI succeeds if it points anywhere, because the content is versioned in the tag history. * fix(roadmap-contract): correct bump tag math and mirror v1 SHA pin Align sibling CONTRACT_SHA with the live v1 tag, fix INIT/version tag computation and fail-closed drift preflight in bump-contract.sh, and harden mirror dry-run / install docs without diverging pinned contract artifacts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Anthony Thompson <github@trackdub.com> * fix(roadmap-contract): address Cubic/CodeRabbit PR 72 findings Pop deferred headings correctly in check-roadmap-drift, pin numan CI to an immutable CONTRACT_SHA (peeling annotated tags), rewrite bump-contract to commit pins, materialize sibling branches, and keep mirror docs/smoke-tests aligned with the SHA pin. Co-authored-by: Anthony Thompson <github@trackdub.com> * chore: retarget roadmap contract v1 pin to 60a015f Point CONTRACT_SHA and sibling blob links at the content freeze that includes the drift-script and catalog markdown fixes. Co-authored-by: Anthony Thompson <github@trackdub.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * ci(roadmap-drift): authenticate GitHub API verify step Pass GITHUB_TOKEN to curl and the annotated-tag peel so shared runners avoid unauthenticated rate limits on the roadmap pin check. Co-authored-by: Cursor <cursoragent@cursor.com> * ci: tolerate missing roadmap contract tag during bootstrap The Verify pinned SHA step now captures the GitHub API HTTP status. If the contract tag has not been pushed yet (bootstrap/bump PR), a 404 is treated as a warning and the step succeeds, removing the catch-22 where the required roadmap-drift check blocks merge before the tag can be published. Real tag/SHA drift or non-404 API errors still fail closed. * fix: address remaining PR 72 roadmap-contract review findings Replace Python tag peels with bash+jq, pin-diff the contract doc, gate bump-contract mutations behind DRY_RUN with a content-then-pin commit flow, restrict drift preamble checks to pre-H2 text, and correct mirror README docs. Co-authored-by: Anthony Thompson <github@trackdub.com> * chore: pin roadmap contract SHA to content freeze c98a2f3 Separate pin rewrite so the annotated tag can point at the content commit without self-referencing this pin commit. Move numan-roadmap-contract/v1 to c98a2f3 after merge. Co-authored-by: Anthony Thompson <github@trackdub.com> * fix: drop leftover conflict markers in setup nu remove Rebase left empty-side conflict markers around the master clear-before-delete path; keep snapshot_before_setup_mutation and remove the markers. Co-authored-by: Anthony Thompson <github@trackdub.com> * fix(ci): soft-fail missing contract tag in sibling mirrors Match numan CI: warn and continue only on HTTP 404 for the contract-tag ref lookup; still fail on other non-200 responses and SHA mismatches. Co-authored-by: Anthony Thompson <github@trackdub.com> * fix(roadmap-contract): harden bump script and document pin protocol - Reject a dirty index and fail staging before freezing CONTENT_SHA - Fail closed when origin OLD_TAG cannot be fetched or pin SHAs drift - Resume when NEW_TAG already points at CONTENT_SHA; clearer recovery text - Clarify local .bak vs published vN-deleted rollback (dual TAG/SHA) - Document API-resolve-then-SHA-fetch pin protocol and partial-bump recovery * chore: pin roadmap contract v1 SHA to content freeze 2829230 Retarget CONTRACT_SHA after pin-protocol/rollback doc freeze. Tag numan-roadmap-contract/v1 will be force-moved to this freeze commit. * fix: define _self_test before module entry point * Revert "fix: define _self_test before module entry point" This reverts commit 81dd0d5. * fix(roadmap-contract): address CodeRabbit pin/rollback/freeze reviews Limit artifact-diff claims to the canonical workflow; spell out rollback publication order (merge restore, tag vN-deleted, then pin rewrite); reject unstaged freeze-artifact edits before CONTENT_SHA is derived. * chore: retarget roadmap contract v1 pin to f242dea Retarget CONTRACT_SHA after contract-doc/rollback-order/freeze-check review fixes. Tag numan-roadmap-contract/v1 will be force-moved to the content freeze commit f242dea. * fix: restore frozen roadmap-v1.md; contract wording needs v2 bump Restore docs/contracts/roadmap-v1.md and CONTRACT_SHA pins to freeze 2829230. Wording clarifications require a coordinated scripts/bump-contract.sh v2 bump, not an in-place v1 edit or pin retarget. Keep bump-contract.sh unstaged freeze-artifact guard. * fix(roadmap-contract): invoke drift checker with python3 Use python3 in CI, sibling mirror workflows, smoke docs, and the check-roadmap-drift.py docstring so invocations match the shebang and the rest of the repo's Python tooling. * chore: retarget roadmap contract v1 pin to 2cd6f5a Freeze includes python3 docstring update in check-roadmap-drift.py. * Create check-roadmap-drift.cpython-311.pyc * fix(ci): expand CONTRACT_* in workflow step names via env context GitHub Actions does not expand shell-style $VAR in step name fields. Use ${{ env.CONTRACT_SHA }} / ${{ env.CONTRACT_TAG }} so the UI shows the resolved pin. Leave $CONTRACT_* untouched inside run: blocks. Co-authored-by: Anthony Thompson <github@trackdub.com> * fix(ci): harden roadmap-drift checkout; drop stray pyc Set persist-credentials: false on roadmap-drift checkouts (workflow already scopes contents: read). Clarify sibling-pointer step points at cross-repo-mirror install, not cross-repo dispatch. Remove accidental __pycache__ artifact and ignore Python bytecode. Co-authored-by: Anthony Thompson <github@trackdub.com> * chore: ignore Python bytecode caches Prevent accidental __pycache__ / .pyc commits after the stray check-roadmap-drift.cpython-311.pyc removal. Co-authored-by: Anthony Thompson <github@trackdub.com> * fix(ci): fail sibling drift jobs on empty validator download Add test -s after curling check-roadmap-drift.py in both cross-repo-mirror roadmap-drift workflows, matching the existing non-empty checks for the roadmap and contract doc fetches. Co-authored-by: Anthony Thompson <github@trackdub.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Codebuff <noreply@codebuff.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Devin <devin@devin.ai>
tonythethompson
added a commit
that referenced
this pull request
Aug 4, 2026
* refactor(setup nu): replace action flags with subcommands + shared confirm 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 * fix: 3 findings — Prevent PATH subcommand from deleting active managed N - Prevent PATH subcommand from deleting active managed Nu - Guard loader overwrites with ownership verification - Reject incompatible legacy Nu setup flags * fix: address PR #66 review feedback 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. * fix: clarify PATH messaging in Nu setup hints Change 'your PATH Nu is not touched' to 'your existing Nu is not replaced' since setup nu does modify PATH by default. * Add reserved `numan use` CLI stub 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. * fix: apply CodeRabbit auto-fixes Fixed 4 file(s) based on 5 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> * feat: implement numan use for side-by-side Nu version management - 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. * fix: 5 findings — Validate and normalize Nu versions; Propagate legacy m - Validate and normalize Nu versions - Propagate legacy migration errors - Handle invalid active markers gracefully - Avoid parent path panic - Update Nu setup guidance * fix: address PR review — validate before delete, fix help text, fix compilation errors * chore: remove temp files * Update src/cmd/snapshot.rs Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * fix: PR 67 review - bootstrap yes flag, version check, mutation lock * Update .gitignore * wip(numan-use): integrate use + migration pieces (split pending) wip: puts numan use + confirm-gate UX + migration journal + reconcile into a single baseline commit so the pr-migrate-legacy-installs split can branch off a known starting point. This commit does not represent either final PR; the next step surgically extracts migration into its own branch and reverts the migration hunks on feature/numan-use. * feat(nu): extract legacy migrate fs + tests into src/nu/migrate_legacy - Pull 251 lines of migration fs (Legacy*Detector type alias, LegacyPostCreateHook type alias, detect_legacy_version, migrate_legacy_install, migrate_legacy_install_with_detector, parse_nu_version_from_output) plus 8 migrate_legacy_* tests + create_legacy_binary helper + production_detector_prefers_version_metadata_file + test_parse_nu_version_from_output into a new crate::nu::migrate_legacy module. - Add 'pub(crate)' to write_active_marker (now consumed by crate::nu::migrate_legacy). - Retarget use_cmd.rs to call crate::nu::migrate_legacy::migrate_legacy_install. - Re-register the module in src/nu/mod.rs. This is the pr-migrate-legacy-installs half of the split: phase 1 cleanup + journaled transaction for legacy single-binary -> versioned installation with self-healing and numan doctor --fix reconciliation. Co-authored-by: Codebuff <noreply@codebuff.com> * fix: Reconcile migration journal under lock * fix: Recover prepared migrations from filesystem state * fix: Remove redundant migration comment * fix: Keep use list read-only * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update src/cmd/setup.rs Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * Update src/nu/bootstrap.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update src/state/migration_journal.rs Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Update src/nu/version_manager.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update src/nu/migrate_legacy.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix(migrate): address PR 69 unresolved review threads (8 fixes) Threads addressed (changed in code): * chatgpt PR69 S1A src/cmd/doctor.rs doctor --fix auto-repair branch reacquires the root mutation lock before invoking migration_journal::reconcile, so a concurrent numan use cannot race the same Prepared -> Renamed transition. * copilot PR69 VwSra src/state/migration_journal.rs PendingMigration::load now hard-fails on unknown schema_version (no longer silently coerces); a future journal variant cannot be misinterpreted as the current one. * cubic PR69 UzV src/cmd/use_cmd.rs numan use latest preserves an existing off-tree binary_path when the selected version matches the existing marker (the previous write dropped binary_path, breaking resolution of 'setup nu use <path>' picks). * chatgpt PR69 S08 src/cmd/setup.rs Both setup nu use <path> callsites now write_active_version_with_binary after register_existing_nu succeeds, so numan use list reports the registered off-tree binary as the active selection. * chatgpt PR69 S09 src/cmd/setup.rs remove_managed_nu and remove_managed_nu_if_present clear the active-version marker at the top of the function, before deleting the managed tree, so the marker cannot dangle at a binary just removed. * cubic PR69 UzU src/nu/migrate_legacy.rs parse_nu_version_from_output delegates to core::nu_version::NuVersion ::parse, which already strips build-hash suffixes ('0.113.1 (abc123)'). Manually-placed legacy binaries whose --version includes a build hash migrate cleanly; bare '0.113.1' falls through to the semver-only path. * cubic PR69 UzM src/nu/paths.rs find_nu_executable_with_root now propagates read_active_version errors instead of silently treating them as 'no marker'. A present-but-malformed active-version marker surfaces loudly so numan init / setup loader cannot silently fall back to PATH Nu. * cubic PR69 UzG src/nu/migrate_legacy.rs migrate_legacy_install_with_detector refuses to scan / mutate under a symlinked managed directory; the rename or filesystem-truth cleanup cannot redirect outside $NUMAN_ROOT. Build / clippy / fmt / lib tests (cargo test --lib -> 468 passed; 0 failed). Deferred (kept as Discussion for the user): * qodo S26 — anyhow -> thiserror migration in public APIs (large refactor) * chatgpt S05 — refresh cached NuPaths when switching versions (architectural) * chatgpt S06 — preserve ~/.local/bin/nu symlink through migration (UX) * cubic UzO — HOME-mutating test seam isolation (test infra refactor) * chatgpt VwS04 — install pinned releases to versioned layout (overlaps PR67 VpJrb on the other branch; left for cross-PR consolidation) * outdated=True threads: VwSrT, VwSrk, VwS1C (skip per skill rules) Per user instruction: push to PR branch only; do NOT merge the PR. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com> * fix(migrate): rebase fallout — close stray fn, drop unused imports, gate cfg(test) read_active_version After the rebase of pr-migrate-legacy-installs onto current master, the detection function was missing its closing bracket (rustc reported an 'unclosed delimiter' at detect_legacy_version that reached EOF), and cargo clippy -- -D warnings surfaced three warnings on lib build: - src/nu/bootstrap.rs:5 — std::io::IsTerminal imported but unused now that the IsTerminal check moved to src/util/confirm.rs. - src/nu/migrate_legacy.rs:19 — read_active_version imported at module scope but only used inside #[cfg(test)]. Restrict to #[cfg(test)] so the lib build is clean and tests still reach it via a sibling import inside the tests module. - src/cmd/doctor.rs:1342 — value assigned to lock was never read; the reacquire from the chatgpt PR69 S1A path is the canonical call. Drop the redundant 'lock = Some(...)' line. Also re-runs cargo fmt to absorb any drift carried in by the rebase. Validation: cargo build OK; cargo clippy -- -D warnings clean; cargo test --lib 468 passed / 0 failed on pr-migrate-legacy-installs. * docs(agents): list nu/migrate_legacy.rs in the project structure PR69 cubic WCi (Quick win, Trivial): the 'nu/' module block under the project structure was missing the two new files this branch introduces. Add migrate_legacy.rs and version_manager.rs entries so contributors can locate these from the AGENTS.md index. fix(migrate): drop the unreachable remove_dir cleanup after migration PR69 cubic WDN (Quick win, Minor): the post-migration cleanup '`std::fs::remove_dir(parent)` where parent == tools/nushell/' can never succeed in practice — after the rename, tools/nushell/ contains the freshly-moved '<version>/' subtree. The result was always swallowed, so the cleanup was a misleading no-op. Replace the misleading line with a comment that names the authoritative post-migration location. Validation: cargo build OK, cargo clippy -- -D warnings clean, cargo fmt --check clean, cargo test --lib 468 passed / 0 failed. * fix(migrate): address PR 69 remaining review threads (7 fixes) Resolves 7 of the 22 unresolved PR 69 reviews, in the priority order the maintainer requested (maintainability batch first, security quartet second). Maintainability batch (WC/WD threads): - WDG + nu/nu.exe helper extraction: pull the duplicated 'if cfg!(windows) { "nu.exe" } else { "nu" }' expression out of 7+ sites in src/nu/migrate_legacy.rs into a single pub(crate) fn nu_binary_name() in src/nu/version_manager.rs. Also adds pub(crate) fn legacy_managed_binary_with_bin for the legacy single-binary install shape. - WDM: add .with_context(...) to the lone bare write_active_version(root, &version)? call in src/nu/migrate_legacy.rs; every neighboring call already had one. - WDR: failure-path tests now assert journal state explicitly so they cannot pass with the journal removed (propagate_detector_failure asserts no journal after detection; recovery tests assert Prepared during the recovery window and zero journal on completion). Security/hardening batch (the quartet): - UzW: src/state/migration_journal.rs gains a free function is_safe_version_component; save() refuses to persist a tampered version and reconcile() refuses to act on a tampered journal. The threat is path-traversal: the journal's version field is appended as a directory name under tools/nushell/<version>/; ../etc would otherwise escape the managed tree. - WDS: src/nu/paths.rs::find_nu_executable_with_root now distinguishes io::ErrorKind::NotFound from whatever the second error class is (JSON parse failure). The 'not found' is still silently tolerated; any other Err escalates with an audit-grade fix hint so a torn active-version.json cannot silently fall back to PATH Nu. - WDT: extract the per-test PathRestoreGuard into a shared src/util/test_paths.rs module (#[cfg(test)] gated). Registered via src/util/mod.rs. Any future test that mutates PATH imports the canonical helper instead of redefining a local guard. - WDY: src/nu/version_manager.rs::write_active_version_with_binary refuses any (ParentDir) component so a tampered relative path cannot anchor an open() outside the Numan root. Absolute paths remain accepted because off-tree Nu markers (numan setup nu use <external-path>) intentionally record the user's canonical external Nu at an absolute path. Validation: - cargo build OK - cargo fmt --check OK - cargo clippy -- -D warnings OK - cargo test --lib 468 passed / 0 failed on pr-migrate-legacy-installs * refactor(setup): route setup_subcommand_lock across destructive setup paths (WCr/WCk/WCq) * refactor(setup): require_tty_or_yes seam + apply across all destructive setup entries (WCt/WDr) * feat(setup): --force flag for `setup nu use <path>` opt-in to destructive swap * fix: 4 findings — Lock direct setup execution; Fix setup test arguments; - Lock direct setup execution - Fix setup test arguments - Reject undetectable external versions - Propagate marker clear failures * fix: Include legacy installs in version listing * fix(migrate): install into versioned layout (PR69 Srm) + versioned-layout tests * audit(destructive): require_tty_or_yes for remove and snapshot delete/rollback * Update migrate_legacy.rs * Fix PR #71 review: version-before-mutation and force gates Hold the doctor migration repair lock across reconcile, surface journal.migration_invalid in text reports, detect Nu versions before destructive setup steps, require --force for setup nu path, and harden migration journal/version listing edge cases called out in review. Co-authored-by: Anthony Thompson <github@trackdub.com> * Fix PR #71 review: TTY seams, migration safety, marker order Make destructive-guard tests inject non-TTY status, reconcile only after successful managed-Nu deletion, refuse migrating into nonempty version dirs, and propagate active-marker/list errors instead of swallowing them. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix rustfmt in migrate_legacy regression test Co-authored-by: Cursor <cursoragent@cursor.com> * fix(nu): return VersionManagerError from public version APIs Replace anyhow::Result on the library-facing version_manager surface with a thiserror::Error enum so callers can match concrete failure modes. Document the library vs application error split at the crate root and update find_nu_executable_with_root to match the typed marker errors. * Fix PR #71 review: docs, doctor hints, detect timeout Document numan use, clean migration_invalid/schema messages, bound legacy version probes, and share PathRestoreGuard via a PATH mutex. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix PR #71 review: PATH test mutex and clear-before-delete Serialize paths.rs PATH-mutating tests through PathRestoreGuard, and clear the active-version marker with context immediately before managed tree deletion after confirmation. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(pr71): hoist migration symlink guard and cover use snapshots Compute managed_dir once and assert_not_symlink before any reconcile stage recovery, including Active. Add regression coverage for a symlinked tools/nushell tree, and assert that `numan use list` creates no snapshot while switch/latest create a PreMutation snapshot. * fix: Validate migration journal versions on load * fix: Validate preserved off-tree binaries * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update src/state/migration_journal.rs Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Update src/cmd/remove.rs Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * fix: apply PR83 review fixes across bootstrap, doctor, migrate_legacy, migration_journal, version_manager, setup, use_cmd, and tests Co-authored-by: Anthony Thompson <github@trackdub.com> * fix: finish remaining PR83 review items Propagate active-marker errors with context, strengthen snapshot --yes bypass assertions, set CMD_USE to a runnable `numan use latest`, drop the unused require_tty_or_yes_with_tty alias, and restore PATH around the ignored setup-nu force acceptance test. Co-authored-by: Anthony Thompson <github@trackdub.com> * style: rustfmt remove confirm message formatting Co-authored-by: Anthony Thompson <github@trackdub.com> * fix: close remaining PR83 review gaps Guard migration/reconcile against symlinked tools ancestors and version dirs via assert_managed_nushell_layout + version_dir checks. Reconcile migration journals before off-PATH Nu repair in doctor --fix so one pass can clear Prepared orphans. Add use latest dangling off-tree self-heal coverage. Co-authored-by: Anthony Thompson <github@trackdub.com> * fix: macOS path containment, consent-before-network, empty managed tree assert_contained builds non-existent paths from canonical root so macOS /var vs /private/var no longer fails migrate_legacy. Unpinned setup nu requires TTY/--yes before GitHub fetch and reuses the release. setup nu path|use force-gates only when a real managed install exists so doctor --fix off-PATH works with empty/partial managed dirs. * fix: do not silently clear unreadable active-version marker on remove When setup nu remove finds no managed tree, treat read_active_version errors as diagnostic failures (leave the marker for numan doctor --fix) instead of silently deleting MalformedMarker/ReadMarker state. Propagate clear_active_version failures for legitimate stale-marker cleanup. --------- Co-authored-by: qodo-code-review[bot] <151058649+qodo-code-review[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai> Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Co-authored-by: Codebuff <noreply@codebuff.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
tonythethompson
added a commit
that referenced
this pull request
Aug 4, 2026
* refactor(setup nu): replace action flags with subcommands + shared confirm 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 * fix: 3 findings — Prevent PATH subcommand from deleting active managed N - Prevent PATH subcommand from deleting active managed Nu - Guard loader overwrites with ownership verification - Reject incompatible legacy Nu setup flags * fix: address PR #66 review feedback 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. * fix: clarify PATH messaging in Nu setup hints Change 'your PATH Nu is not touched' to 'your existing Nu is not replaced' since setup nu does modify PATH by default. * Add reserved `numan use` CLI stub 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. * fix: apply CodeRabbit auto-fixes Fixed 4 file(s) based on 5 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> * feat: implement numan use for side-by-side Nu version management - 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. * fix: 5 findings — Validate and normalize Nu versions; Propagate legacy m - Validate and normalize Nu versions - Propagate legacy migration errors - Handle invalid active markers gracefully - Avoid parent path panic - Update Nu setup guidance * fix: address PR review — validate before delete, fix help text, fix compilation errors * chore: remove temp files * Update src/cmd/snapshot.rs Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * fix: PR 67 review - bootstrap yes flag, version check, mutation lock * Update .gitignore * wip(numan-use): integrate use + migration pieces (split pending) wip: puts numan use + confirm-gate UX + migration journal + reconcile into a single baseline commit so the pr-migrate-legacy-installs split can branch off a known starting point. This commit does not represent either final PR; the next step surgically extracts migration into its own branch and reverts the migration hunks on feature/numan-use. * feat(nu): extract legacy migrate fs + tests into src/nu/migrate_legacy - Pull 251 lines of migration fs (Legacy*Detector type alias, LegacyPostCreateHook type alias, detect_legacy_version, migrate_legacy_install, migrate_legacy_install_with_detector, parse_nu_version_from_output) plus 8 migrate_legacy_* tests + create_legacy_binary helper + production_detector_prefers_version_metadata_file + test_parse_nu_version_from_output into a new crate::nu::migrate_legacy module. - Add 'pub(crate)' to write_active_marker (now consumed by crate::nu::migrate_legacy). - Retarget use_cmd.rs to call crate::nu::migrate_legacy::migrate_legacy_install. - Re-register the module in src/nu/mod.rs. This is the pr-migrate-legacy-installs half of the split: phase 1 cleanup + journaled transaction for legacy single-binary -> versioned installation with self-healing and numan doctor --fix reconciliation. Co-authored-by: Codebuff <noreply@codebuff.com> * fix: Reconcile migration journal under lock * fix: Recover prepared migrations from filesystem state * fix: Remove redundant migration comment * fix: Keep use list read-only * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update src/cmd/setup.rs Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * Update src/nu/bootstrap.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update src/state/migration_journal.rs Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Update src/nu/version_manager.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update src/nu/migrate_legacy.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix(migrate): address PR 69 unresolved review threads (8 fixes) Threads addressed (changed in code): * chatgpt PR69 S1A src/cmd/doctor.rs doctor --fix auto-repair branch reacquires the root mutation lock before invoking migration_journal::reconcile, so a concurrent numan use cannot race the same Prepared -> Renamed transition. * copilot PR69 VwSra src/state/migration_journal.rs PendingMigration::load now hard-fails on unknown schema_version (no longer silently coerces); a future journal variant cannot be misinterpreted as the current one. * cubic PR69 UzV src/cmd/use_cmd.rs numan use latest preserves an existing off-tree binary_path when the selected version matches the existing marker (the previous write dropped binary_path, breaking resolution of 'setup nu use <path>' picks). * chatgpt PR69 S08 src/cmd/setup.rs Both setup nu use <path> callsites now write_active_version_with_binary after register_existing_nu succeeds, so numan use list reports the registered off-tree binary as the active selection. * chatgpt PR69 S09 src/cmd/setup.rs remove_managed_nu and remove_managed_nu_if_present clear the active-version marker at the top of the function, before deleting the managed tree, so the marker cannot dangle at a binary just removed. * cubic PR69 UzU src/nu/migrate_legacy.rs parse_nu_version_from_output delegates to core::nu_version::NuVersion ::parse, which already strips build-hash suffixes ('0.113.1 (abc123)'). Manually-placed legacy binaries whose --version includes a build hash migrate cleanly; bare '0.113.1' falls through to the semver-only path. * cubic PR69 UzM src/nu/paths.rs find_nu_executable_with_root now propagates read_active_version errors instead of silently treating them as 'no marker'. A present-but-malformed active-version marker surfaces loudly so numan init / setup loader cannot silently fall back to PATH Nu. * cubic PR69 UzG src/nu/migrate_legacy.rs migrate_legacy_install_with_detector refuses to scan / mutate under a symlinked managed directory; the rename or filesystem-truth cleanup cannot redirect outside $NUMAN_ROOT. Build / clippy / fmt / lib tests (cargo test --lib -> 468 passed; 0 failed). Deferred (kept as Discussion for the user): * qodo S26 — anyhow -> thiserror migration in public APIs (large refactor) * chatgpt S05 — refresh cached NuPaths when switching versions (architectural) * chatgpt S06 — preserve ~/.local/bin/nu symlink through migration (UX) * cubic UzO — HOME-mutating test seam isolation (test infra refactor) * chatgpt VwS04 — install pinned releases to versioned layout (overlaps PR67 VpJrb on the other branch; left for cross-PR consolidation) * outdated=True threads: VwSrT, VwSrk, VwS1C (skip per skill rules) Per user instruction: push to PR branch only; do NOT merge the PR. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com> * fix(migrate): rebase fallout — close stray fn, drop unused imports, gate cfg(test) read_active_version After the rebase of pr-migrate-legacy-installs onto current master, the detection function was missing its closing bracket (rustc reported an 'unclosed delimiter' at detect_legacy_version that reached EOF), and cargo clippy -- -D warnings surfaced three warnings on lib build: - src/nu/bootstrap.rs:5 — std::io::IsTerminal imported but unused now that the IsTerminal check moved to src/util/confirm.rs. - src/nu/migrate_legacy.rs:19 — read_active_version imported at module scope but only used inside #[cfg(test)]. Restrict to #[cfg(test)] so the lib build is clean and tests still reach it via a sibling import inside the tests module. - src/cmd/doctor.rs:1342 — value assigned to lock was never read; the reacquire from the chatgpt PR69 S1A path is the canonical call. Drop the redundant 'lock = Some(...)' line. Also re-runs cargo fmt to absorb any drift carried in by the rebase. Validation: cargo build OK; cargo clippy -- -D warnings clean; cargo test --lib 468 passed / 0 failed on pr-migrate-legacy-installs. * docs(agents): list nu/migrate_legacy.rs in the project structure PR69 cubic WCi (Quick win, Trivial): the 'nu/' module block under the project structure was missing the two new files this branch introduces. Add migrate_legacy.rs and version_manager.rs entries so contributors can locate these from the AGENTS.md index. fix(migrate): drop the unreachable remove_dir cleanup after migration PR69 cubic WDN (Quick win, Minor): the post-migration cleanup '`std::fs::remove_dir(parent)` where parent == tools/nushell/' can never succeed in practice — after the rename, tools/nushell/ contains the freshly-moved '<version>/' subtree. The result was always swallowed, so the cleanup was a misleading no-op. Replace the misleading line with a comment that names the authoritative post-migration location. Validation: cargo build OK, cargo clippy -- -D warnings clean, cargo fmt --check clean, cargo test --lib 468 passed / 0 failed. * fix(migrate): address PR 69 remaining review threads (7 fixes) Resolves 7 of the 22 unresolved PR 69 reviews, in the priority order the maintainer requested (maintainability batch first, security quartet second). Maintainability batch (WC/WD threads): - WDG + nu/nu.exe helper extraction: pull the duplicated 'if cfg!(windows) { "nu.exe" } else { "nu" }' expression out of 7+ sites in src/nu/migrate_legacy.rs into a single pub(crate) fn nu_binary_name() in src/nu/version_manager.rs. Also adds pub(crate) fn legacy_managed_binary_with_bin for the legacy single-binary install shape. - WDM: add .with_context(...) to the lone bare write_active_version(root, &version)? call in src/nu/migrate_legacy.rs; every neighboring call already had one. - WDR: failure-path tests now assert journal state explicitly so they cannot pass with the journal removed (propagate_detector_failure asserts no journal after detection; recovery tests assert Prepared during the recovery window and zero journal on completion). Security/hardening batch (the quartet): - UzW: src/state/migration_journal.rs gains a free function is_safe_version_component; save() refuses to persist a tampered version and reconcile() refuses to act on a tampered journal. The threat is path-traversal: the journal's version field is appended as a directory name under tools/nushell/<version>/; ../etc would otherwise escape the managed tree. - WDS: src/nu/paths.rs::find_nu_executable_with_root now distinguishes io::ErrorKind::NotFound from whatever the second error class is (JSON parse failure). The 'not found' is still silently tolerated; any other Err escalates with an audit-grade fix hint so a torn active-version.json cannot silently fall back to PATH Nu. - WDT: extract the per-test PathRestoreGuard into a shared src/util/test_paths.rs module (#[cfg(test)] gated). Registered via src/util/mod.rs. Any future test that mutates PATH imports the canonical helper instead of redefining a local guard. - WDY: src/nu/version_manager.rs::write_active_version_with_binary refuses any (ParentDir) component so a tampered relative path cannot anchor an open() outside the Numan root. Absolute paths remain accepted because off-tree Nu markers (numan setup nu use <external-path>) intentionally record the user's canonical external Nu at an absolute path. Validation: - cargo build OK - cargo fmt --check OK - cargo clippy -- -D warnings OK - cargo test --lib 468 passed / 0 failed on pr-migrate-legacy-installs * refactor(setup): route setup_subcommand_lock across destructive setup paths (WCr/WCk/WCq) * refactor(setup): require_tty_or_yes seam + apply across all destructive setup entries (WCt/WDr) * feat(setup): --force flag for `setup nu use <path>` opt-in to destructive swap * fix: 4 findings — Lock direct setup execution; Fix setup test arguments; - Lock direct setup execution - Fix setup test arguments - Reject undetectable external versions - Propagate marker clear failures * fix: Include legacy installs in version listing * fix(migrate): install into versioned layout (PR69 Srm) + versioned-layout tests * audit(destructive): require_tty_or_yes for remove and snapshot delete/rollback * Update migrate_legacy.rs * Fix PR #71 review: version-before-mutation and force gates Hold the doctor migration repair lock across reconcile, surface journal.migration_invalid in text reports, detect Nu versions before destructive setup steps, require --force for setup nu path, and harden migration journal/version listing edge cases called out in review. Co-authored-by: Anthony Thompson <github@trackdub.com> * Fix PR #71 review: TTY seams, migration safety, marker order Make destructive-guard tests inject non-TTY status, reconcile only after successful managed-Nu deletion, refuse migrating into nonempty version dirs, and propagate active-marker/list errors instead of swallowing them. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix rustfmt in migrate_legacy regression test Co-authored-by: Cursor <cursoragent@cursor.com> * fix(nu): return VersionManagerError from public version APIs Replace anyhow::Result on the library-facing version_manager surface with a thiserror::Error enum so callers can match concrete failure modes. Document the library vs application error split at the crate root and update find_nu_executable_with_root to match the typed marker errors. * fix: Mark the freshly installed Nu version active * fix: Return concrete migration API errors * fix: Correct use command documentation * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update src/cmd/doctor.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update src/util/fs_safety.rs Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * fix: drop opaque anyhow thiserror wrappers on migration APIs LegacyMigrationError and MigrationJournalError were transparent newtypes over anyhow and not matchable. Return anyhow::Result directly; VersionManagerError remains the typed library surface. Co-authored-by: Anthony Thompson <github@trackdub.com> * fix: address still-valid PR 82 review findings Skip doctor off-path repair when a managed tree exists, confirm before snapshot locks, clarify managed-removal messaging, cover marker and legacy-list behavior, and harden ignored setup-nu preconditions. Co-authored-by: Anthony Thompson <github@trackdub.com> * fix: address PR 82 P1 bare-setup, remove confirm, doctor hint Bare numan setup nu no longer short-circuits when any version is installed; only pinned installs short-circuit on an exact match so latest can land a newer release without --force. Interactive numan remove now prompts after validation; --yes skips it. Doctor migration_pending fix hint includes the journaled version. * fix: rustfmt doctor test and update setup_nu short-circuit test Format doctor_reports_migration_journal_finding for cargo fmt --check. Rewrite execute_nu_command_wraps_installer to pin-only short-circuit without network, matching bare-setup latest always invoking install_latest. * fix: surface inconsistent migration journals as doctor errors Renamed journals with a missing versioned binary (and journals with unsafe version components) cannot be auto-reconciled. Report them as journal.migration_invalid Error/Manual so doctor --fix exits non-zero instead of masking corrupt migration state as a warning. * fix: preflight active-marker writability before PATH Nu switch Refuse execute_use_path / execute_use_existing before managed-tree deletion and PATH mutation when nu_state cannot be written, so a later active-version marker failure cannot leave partial selection state. * fix: write normalized Nu VERSION marker with I/O context Store the same normalized version used for tools/nushell/<version>/ in the VERSION file (strip leading v from tag_name) and attach path context on write failure. * style: rustfmt doctor, setup, and bootstrap after PR 82 fixes * fix: route setup loader/use through setup_subcommand_lock - Wrap numan setup loader public entry in setup_subcommand_lock - Route numan use list/switch through setup_subcommand_lock - Legacy --use-path/--use-existing never pass install --force * fix: pin migrate/paths/journal failure guards Add Unix symlink refuse test, preserve VersionManagerError source chain on active-marker errors, and test schema/unsafe-version guards. * fix: route legacy binary path via versioned_nu_dir * fix: normalize migration journal version before Renamed path probes Doctor and reconcile probed version_binary / version_install_dir with the raw journal string. A safe-but-prefixed value like v0.113.1 looked under tools/nushell/v0.113.1/ instead of the normalized layout migrate_legacy writes. Normalize before probing (and refuse non-normalizable versions in reconcile) so Auto repair stays consistent with doctor classification. Co-authored-by: Anthony Thompson <github@trackdub.com> * fix: address remaining PR 82 P2 review findings Skip already-fixed reconcile normalization. For migration_pending, hint doctor --fix / setup nu when the versioned binary is absent (Prepared), and numan use only when the binary is present. Take the remove mutation lock after interactive confirm. Guard managed-tree deletion with assert_not_symlink. Restore graceful skip in ignored setup-nu tests when Nu is off PATH. Co-authored-by: Anthony Thompson <github@trackdub.com> * fix: gate migration Auto on validate_reconcile; harden related UX Expose non-mutating validate_reconcile (normalize, symlink safety, Renamed presence, Prepared orphan emptiness) and use it for doctor pending vs invalid classification. Snapshot PATH/off-path setup before preflight; make `numan use list` read-only via VERSION discovery; assert pinned short-circuit preserves binary bytes; strip review/PR metadata from durable comments. Docs define pending by validator acceptance. Co-authored-by: Anthony Thompson <github@trackdub.com> * fix: address PR 82 review on doctor hints, marker order, reconcile Decouple migration invalid hints from error-string grepping, clear the active marker only after successful managed-dir deletion, and return the normalized version from validate_reconcile for callers. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: drop duplicate migrate_legacy test and rustfmt doctor Merge left two identical symlink-refusal unit tests (E0428) and an unformatted matches! in doctor, which broke Test/MSRV/Real-Nu and Format CI. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: qodo-code-review[bot] <151058649+qodo-code-review[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai> Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Co-authored-by: Codebuff <noreply@codebuff.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
4 tasks
This was referenced Aug 7, 2026
Merged
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary by Qodo
Add Nu version management (
numan use) and refactorsetup nuinto subcommands✨ Enhancement🐞 Bug fix🧪 Tests📝 Documentation🕐 40+ MinutesAI Description
Diagram
graph TD CLI["CLI: setup nu / use / doctor"] --> LOCK["setup_subcommand_lock"] --> VM["version_manager"] VM --> MARKER[("active-version.json")] CLI --> MIGRATE["migrate_legacy"] --> JOURNAL[("migration-journal.json")] MIGRATE --> VM DOCTOR["doctor --fix"] --> JOURNAL DOCTOR --> VM PATHS["paths::find_nu_executable"] --> VM subgraph Legend direction LR _svc([Service]) ~~~ _db[(State file)] endHigh-Level Assessment
1. Symlink-based active-version pointer instead of JSON marker
currentsymlink)binary_pathmetadata needed forsetup nu use2. Skip journaling and rely on idempotent retry of legacy migration
doctorto surface actionable guidance vs silent best-effort cleanupFiles changed (19)
+3256 / -62Enhancement (12)
+2891 / -48doctor.rs
Add migration-journal findings and repair via reconcile+199/-2setup.rs
Refactor 'setup nu' actions, add safety gates, and persist active marker+390/-18use_cmd.rs
Implement 'numan use' (list/latest/switch) with snapshots + legacy migration+228/-17bootstrap.rs
Add snapshotting + active marker persistence + hoisted-consent PATH prompt+117/-11migrate_legacy.rs
Introduce journaled legacy single-binary migration module+578/-0mod.rs
Export new Nu modules+2/-0version_manager.rs
Add active-version marker + installed-version listing utilities+606/-0migration_journal.rs
Add migration journal state machine and reconcile logic+599/-0mod.rs
Register migration_journal in state module+1/-0confirm.rs
Add require_tty_or_yes guard with injectable seam+78/-0fs_safety.rs
Add setup_subcommand_lock for audited mutation locking+87/-0hints.rs
Add 'CMD_USE' hint constant+6/-0Bug fix (1)
+169 / -2paths.rs
Resolve Nu binary via active-version marker hint table+169/-2Tests (4)
+188 / -6mod.rs
Add test-only module export for PATH guard+3/-0test_paths.rs
Add PathRestoreGuard RAII helper for tests+47/-0doctor_test.rs
Adjust doctor test for new NuAction::Use shape+2/-2setup_nu_test.rs
Expand setup nu tests for new force flag and options fields+136/-4Documentation (2)
+8 / -6AGENTS.md
Document migration journal and active-version marker+5/-0consolidated-multi-repo-roadmap.md
Update roadmap for implemented 'numan use' semantics+3/-6