You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
docs/managers.md describes an nvm-windows-specific suppression message that doesn't exist in the code. It states (around the manager command table): "For nvm-windows, the upgrade proceeds normally but the cleanup prompt is suppressed with a note: 'nvm-windows cleanup is not yet implemented — leave the old versions in place or remove them manually via nvm uninstall <v> from an elevated shell.'" Grepping internal/cli/cleanup.go and internal/cli/upgrade.go for any nvm-windows-specific branching or that message text turns up nothing. In actual code, nvm-windows's Current() returns the ErrNVMWindowsNotImplemented sentinel (so its active version just isn't excluded, same as any other Current() error — see the separate issue about that), and the cleanup prompt still runs normally offering nvm-windows candidates; each Uninstall() call then fails with the same sentinel error and lands in the generic " Failed to delete %s: %v\n" per-candidate message — not the single clear informational note the docs describe.
docs/managers.md's "Manager not on PATH" failure mode is inaccurate. It states: "Manager not on PATH — the cleanup step is skipped entirely; the upgrade itself still completes." In reality, detector.ResolveManager returns ErrNoManager when no manager is detected, and runUpgrade returns that error immediately — the entire upgrade command aborts, it never reaches install/migrate/cleanup at all. There's no scenario matching "the upgrade itself still completes" while only cleanup is skipped due to a missing manager.
README.md and docs/managers.md disagree about Phase 7's status in the same merge. README's Project-status table says "Phase 7 ✅" (done). docs/managers.md's own status banner says "Phase 7 in progress." Pick one and make both consistent — Phase 7 also covers distribution packaging (GoReleaser/brew/scoop/npm, tracked by issues chore(release): finalize GoReleaser config and npm wrapper #17/chore(release): prepare v1.0.0 release #18), which is not done, so "in progress" is probably the more accurate framing for README to adopt, or the cleanup-specific portion should be called out as its own sub-line.
Suggested fix
Fix the #40→#41 citation in CHANGELOG.md; correct or remove the fabricated nvm-windows suppression-note claim in docs/managers.md (describe the real per-candidate failure behavior instead, or implement the nicer single-note UX to match the docs — implementer's choice); correct the "Manager not on PATH" failure-mode description; reconcile the Phase 7 status wording between README.md and docs/managers.md.
Severity
Medium — none of these are code bugs, but they actively mislead anyone (including future AI assistants) reading the docs about what the shipped feature actually does.
Describe the bug
Several documentation inaccuracies were introduced by the post-upgrade cleanup feature (PR #56, closing issue #41):
CHANGELOG.md's "Native mutation commands..." bullet cites the wrong issue. It ends with "Closes chore(ci): gitignore the local bin/ and coverage.out #40", but issue chore(ci): gitignore the local bin/ and coverage.out #40 ischore(ci): gitignore the local bin/ and coverage.out— completely unrelated. This bullet is part of the same cleanup-feature work as the preceding bullet (which correctly cites "Closes feat(cleanup): post-upgrade cleanup prompt with version-by-version confirmation #41") and should reference feat(cleanup): post-upgrade cleanup prompt with version-by-version confirmation #41 instead (or carry no issue reference, since feat(cleanup): post-upgrade cleanup prompt with version-by-version confirmation #41 is already closed by the adjacent entry).docs/managers.mddescribes an nvm-windows-specific suppression message that doesn't exist in the code. It states (around the manager command table): "For nvm-windows, the upgrade proceeds normally but the cleanup prompt is suppressed with a note: 'nvm-windows cleanup is not yet implemented — leave the old versions in place or remove them manually vianvm uninstall <v>from an elevated shell.'" Greppinginternal/cli/cleanup.goandinternal/cli/upgrade.gofor any nvm-windows-specific branching or that message text turns up nothing. In actual code, nvm-windows'sCurrent()returns theErrNVMWindowsNotImplementedsentinel (so its active version just isn't excluded, same as any otherCurrent()error — see the separate issue about that), and the cleanup prompt still runs normally offering nvm-windows candidates; eachUninstall()call then fails with the same sentinel error and lands in the generic" Failed to delete %s: %v\n"per-candidate message — not the single clear informational note the docs describe.docs/managers.md's "Manager not on PATH" failure mode is inaccurate. It states: "Manager not on PATH — the cleanup step is skipped entirely; the upgrade itself still completes." In reality,detector.ResolveManagerreturnsErrNoManagerwhen no manager is detected, andrunUpgradereturns that error immediately — the entire upgrade command aborts, it never reaches install/migrate/cleanup at all. There's no scenario matching "the upgrade itself still completes" while only cleanup is skipped due to a missing manager.README.md and
docs/managers.mddisagree about Phase 7's status in the same merge. README's Project-status table says "Phase 7 ✅" (done).docs/managers.md's own status banner says "Phase 7 in progress." Pick one and make both consistent — Phase 7 also covers distribution packaging (GoReleaser/brew/scoop/npm, tracked by issues chore(release): finalize GoReleaser config and npm wrapper #17/chore(release): prepare v1.0.0 release #18), which is not done, so "in progress" is probably the more accurate framing for README to adopt, or the cleanup-specific portion should be called out as its own sub-line.Suggested fix
Fix the
#40→#41citation in CHANGELOG.md; correct or remove the fabricated nvm-windows suppression-note claim in docs/managers.md (describe the real per-candidate failure behavior instead, or implement the nicer single-note UX to match the docs — implementer's choice); correct the "Manager not on PATH" failure-mode description; reconcile the Phase 7 status wording between README.md and docs/managers.md.Severity
Medium — none of these are code bugs, but they actively mislead anyone (including future AI assistants) reading the docs about what the shipped feature actually does.