From d99215e5fca17e22fb5ecfea5c446632a93573e1 Mon Sep 17 00:00:00 2001 From: dipto0321 Date: Sun, 5 Jul 2026 13:51:29 +0600 Subject: [PATCH] feat(ui): add huh-backed prompts + detector.ResolveInteractive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps huh in a small Prompt interface so internal/cli's post-upgrade cleanup can prompt the user via huh.Form in FancyMode and a numbered-line fallback in PlainMode. Adds detector.ResolveInteractive + detector.ResolveManagerAuto so the upgrade flow can pick a manager when multiple are detected without the user pre-supplying a preference. Adds github.com/charmbracelet/huh v1.0.0 to go.mod. The huh dependency is contained to internal/ui/prompt.go — internal/cli calls ui.NewPrompt + ui.Prompt.{Confirm,Select} and never imports huh directly (same constraint as lipgloss / bubbletea per #105). Implementation notes: - prompt.go: Prompt interface (Confirm/Select/Mode); plainPrompt uses line reads against io.Reader; fancyPrompt builds a fresh huh.Form per call. Confirm supports a defaultYes pre-fill so huh's toggle starts on the right side; empty input / EOF fall back to the default for Confirm (true) and the default label / first option for Select. - prompt_test.go covers: y/n/empty/EOF/case-insensitive Confirm paths, numeric/label/EOF Select paths, no-options error path, bufio.Reader passthrough, nil-stream degradation, and I/O error surfacing. - detector/interactive.go: ResolveManagerAuto wraps ResolveManager with an explicit ErrInteractiveRequired sentinel for the multi-manager case. ResolveInteractive prompts the user (via ui.Prompt or a fallback plain prompt against cmd streams) with a single nonInteractive shortcut for CI / --yes runs. single-manager registries short-circuit without prompting. - detector/interactive_test.go covers: zero/single/multi/no-pref paths, fallback plain prompt with numeric + EOF input, nonInteractive ErrInteractiveRequired surfacing, and the stubPrompt pattern that production code paths would use if they ever need to mock the picker. - cleanup.go: runCleanupPrompt signature now takes (ui.Prompt, ui.Writer, ...); the old bufio-based prompts (promptAllOrNothing, promptPerVersion) are rewritten on top of the new abstraction. The "delete one specific version" path is now a labeled Select option ("Delete v22.11.0") instead of the old "type a version string" pattern — same per-version confirm UX but a discoverable option list. - cleanup_test.go rewires newCleanupIO into a (prompt, writer, buf) triple-return helper. The number inputs change from text ("y\n", "20.18.0\n") to numeric indexes ("1\n" for "Delete all", "2\n" for "Skip cleanup", "3\n" / "4\n" / "5\n" for per-version picks) since the new prompt flow uses Select rather than free-form input. - upgrade.go: ResolveManagerAuto + ResolveInteractive replace the bare ResolveManager call. Resolved manager still goes through the existing "Using manager: X" log line. The ErrInteractiveRequired path hands off to ResolveInteractive with yes=nonInteractive (a CI script that ran with --yes surfaces "use --manager" rather than hanging on stdin). Per #105 phasing, this is PR3 of 3. PR4 (next) migrates the remaining call sites (check, list, packages, config) to ui.Writer. The Prompt abstraction is now stable so PR4 can focus on the Writer migrations without revisiting this PR's surface. Part of #105. Co-Authored-By: puku-ai-2.8 --- go.mod | 11 +- go.sum | 42 ++- internal/cli/cleanup.go | 233 ++++++--------- internal/cli/cleanup_test.go | 396 ++++++++------------------ internal/cli/upgrade.go | 37 ++- internal/detector/interactive.go | 131 +++++++++ internal/detector/interactive_test.go | 211 ++++++++++++++ internal/ui/prompt.go | 307 ++++++++++++++++++++ internal/ui/prompt_test.go | 258 +++++++++++++++++ 9 files changed, 1190 insertions(+), 436 deletions(-) create mode 100644 internal/detector/interactive.go create mode 100644 internal/detector/interactive_test.go create mode 100644 internal/ui/prompt.go create mode 100644 internal/ui/prompt_test.go diff --git a/go.mod b/go.mod index 8419f6f..1a22fd8 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.24.0 require ( github.com/Masterminds/semver/v3 v3.5.0 github.com/charmbracelet/bubbletea v1.3.10 + github.com/charmbracelet/huh v1.0.0 github.com/charmbracelet/lipgloss v1.1.0 github.com/muesli/termenv v0.16.0 github.com/spf13/cobra v1.10.2 @@ -12,22 +13,28 @@ require ( ) require ( + github.com/atotto/clipboard v0.1.4 // indirect github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect + github.com/catppuccin/go v0.3.0 // indirect + github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 // indirect github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc // indirect github.com/charmbracelet/x/ansi v0.10.1 // indirect - github.com/charmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31dd // indirect + github.com/charmbracelet/x/cellbuf v0.0.13 // indirect + github.com/charmbracelet/x/exp/strings v0.0.0-20240722160745-212f7b056ed0 // indirect github.com/charmbracelet/x/term v0.2.1 // indirect + github.com/dustin/go-humanize v1.0.1 // indirect github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/lucasb-eyer/go-colorful v1.2.0 // indirect github.com/mattn/go-isatty v0.0.20 // indirect github.com/mattn/go-localereader v0.0.1 // indirect github.com/mattn/go-runewidth v0.0.16 // indirect + github.com/mitchellh/hashstructure/v2 v2.0.2 // indirect github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect github.com/muesli/cancelreader v0.2.2 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/spf13/pflag v1.0.9 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect golang.org/x/sys v0.36.0 // indirect - golang.org/x/text v0.3.8 // indirect + golang.org/x/text v0.23.0 // indirect ) diff --git a/go.sum b/go.sum index 84f9e13..e642723 100644 --- a/go.sum +++ b/go.sum @@ -1,20 +1,48 @@ +github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ4pzQ= +github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE= github.com/Masterminds/semver/v3 v3.5.0 h1:kQceYJfbupGfZOKZQg0kou0DgAKhzDg2NZPAwZ/2OOE= github.com/Masterminds/semver/v3 v3.5.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= +github.com/atotto/clipboard v0.1.4 h1:EH0zSVneZPSuFR11BlR9YppQTVDbh5+16AmcJi4g1z4= +github.com/atotto/clipboard v0.1.4/go.mod h1:ZY9tmq7sm5xIbd9bOK4onWV4S6X0u6GY7Vn0Yu86PYI= github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k= github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8= +github.com/aymanbagabas/go-udiff v0.3.1 h1:LV+qyBQ2pqe0u42ZsUEtPiCaUoqgA9gYRDs3vj1nolY= +github.com/aymanbagabas/go-udiff v0.3.1/go.mod h1:G0fsKmG+P6ylD0r6N/KgQD/nWzgfnl8ZBcNLgcbrw8E= +github.com/catppuccin/go v0.3.0 h1:d+0/YicIq+hSTo5oPuRi5kOpqkVA5tAsU6dNhvRu+aY= +github.com/catppuccin/go v0.3.0/go.mod h1:8IHJuMGaUUjQM82qBrGNBv7LFq6JI3NnQCF6MOlZjpc= +github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 h1:JFgG/xnwFfbezlUnFMJy0nusZvytYysV4SCS2cYbvws= +github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7/go.mod h1:ISC1gtLcVilLOf23wvTfoQuYbW2q0JevFxPfUzZ9Ybw= github.com/charmbracelet/bubbletea v1.3.10 h1:otUDHWMMzQSB0Pkc87rm691KZ3SWa4KUlvF9nRvCICw= github.com/charmbracelet/bubbletea v1.3.10/go.mod h1:ORQfo0fk8U+po9VaNvnV95UPWA1BitP1E0N6xJPlHr4= github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc h1:4pZI35227imm7yK2bGPcfpFEmuY1gc2YSTShr4iJBfs= github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc/go.mod h1:X4/0JoqgTIPSFcRA/P6INZzIuyqdFY5rm8tb41s9okk= +github.com/charmbracelet/huh v1.0.0 h1:wOnedH8G4qzJbmhftTqrpppyqHakl/zbbNdXIWJyIxw= +github.com/charmbracelet/huh v1.0.0/go.mod h1:5YVc+SlZ1IhQALxRPpkGwwEKftN/+OlJlnJYlDRFqN4= github.com/charmbracelet/lipgloss v1.1.0 h1:vYXsiLHVkK7fp74RkV7b2kq9+zDLoEU4MZoFqR/noCY= github.com/charmbracelet/lipgloss v1.1.0/go.mod h1:/6Q8FR2o+kj8rz4Dq0zQc3vYf7X+B0binUUBwA0aL30= github.com/charmbracelet/x/ansi v0.10.1 h1:rL3Koar5XvX0pHGfovN03f5cxLbCF2YvLeyz7D2jVDQ= github.com/charmbracelet/x/ansi v0.10.1/go.mod h1:3RQDQ6lDnROptfpWuUVIUG64bD2g2BgntdxH0Ya5TeE= -github.com/charmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31dd h1:vy0GVL4jeHEwG5YOXDmi86oYw2yuYUGqz6a8sLwg0X8= -github.com/charmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31dd/go.mod h1:xe0nKWGd3eJgtqZRaN9RjMtK7xUYchjzPr7q6kcvCCs= +github.com/charmbracelet/x/cellbuf v0.0.13 h1:/KBBKHuVRbq1lYx5BzEHBAFBP8VcQzJejZ/IA3iR28k= +github.com/charmbracelet/x/cellbuf v0.0.13/go.mod h1:xe0nKWGd3eJgtqZRaN9RjMtK7xUYchjzPr7q6kcvCCs= +github.com/charmbracelet/x/conpty v0.1.0 h1:4zc8KaIcbiL4mghEON8D72agYtSeIgq8FSThSPQIb+U= +github.com/charmbracelet/x/conpty v0.1.0/go.mod h1:rMFsDJoDwVmiYM10aD4bH2XiRgwI7NYJtQgl5yskjEQ= +github.com/charmbracelet/x/errors v0.0.0-20240508181413-e8d8b6e2de86 h1:JSt3B+U9iqk37QUU2Rvb6DSBYRLtWqFqfxf8l5hOZUA= +github.com/charmbracelet/x/errors v0.0.0-20240508181413-e8d8b6e2de86/go.mod h1:2P0UgXMEa6TsToMSuFqKFQR+fZTO9CNGUNokkPatT/0= +github.com/charmbracelet/x/exp/golden v0.0.0-20241011142426-46044092ad91 h1:payRxjMjKgx2PaCWLZ4p3ro9y97+TVLZNaRZgJwSVDQ= +github.com/charmbracelet/x/exp/golden v0.0.0-20241011142426-46044092ad91/go.mod h1:wDlXFlCrmJ8J+swcL/MnGUuYnqgQdW9rhSD61oNMb6U= +github.com/charmbracelet/x/exp/strings v0.0.0-20240722160745-212f7b056ed0 h1:qko3AQ4gK1MTS/de7F5hPGx6/k1u0w4TeYmBFwzYVP4= +github.com/charmbracelet/x/exp/strings v0.0.0-20240722160745-212f7b056ed0/go.mod h1:pBhA0ybfXv6hDjQUZ7hk1lVxBiUbupdw5R31yPUViVQ= github.com/charmbracelet/x/term v0.2.1 h1:AQeHeLZ1OqSXhrAWpYUtZyX1T3zVxfpZuEQMIQaGIAQ= github.com/charmbracelet/x/term v0.2.1/go.mod h1:oQ4enTYFV7QN4m0i9mzHrViD7TQKvNEEkHUMCmsxdUg= +github.com/charmbracelet/x/termios v0.1.1 h1:o3Q2bT8eqzGnGPOYheoYS8eEleT5ZVNYNy8JawjaNZY= +github.com/charmbracelet/x/termios v0.1.1/go.mod h1:rB7fnv1TgOPOyyKRJ9o+AsTU/vK5WHJ2ivHeut/Pcwo= +github.com/charmbracelet/x/xpty v0.1.2 h1:Pqmu4TEJ8KeA9uSkISKMU3f+C1F6OGBn8ABuGlqCbtI= +github.com/charmbracelet/x/xpty v0.1.2/go.mod h1:XK2Z0id5rtLWcpeNiMYBccNNBrP2IJnzHI0Lq13Xzq4= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s= +github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE= +github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= +github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6baUTXGLOoWe4PQhGxaX0KpnayAqC48p4= github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f/go.mod h1:vw97MGsxSvLiUE2X8qFplwetxpGLQrlU1Q9AUEIzCaM= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= @@ -27,6 +55,8 @@ github.com/mattn/go-localereader v0.0.1 h1:ygSAOl7ZXTx4RdPYinUpg6W99U8jWvWi9Ye2J github.com/mattn/go-localereader v0.0.1/go.mod h1:8fBrzywKY7BI3czFoHkuzRoWE9C+EiG4R1k4Cjx5p88= github.com/mattn/go-runewidth v0.0.16 h1:E5ScNMtiwvlvB5paMFdw9p4kSQzbXFikJ5SQO6TULQc= github.com/mattn/go-runewidth v0.0.16/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w= +github.com/mitchellh/hashstructure/v2 v2.0.2 h1:vGKWl0YJqUNxE8d+h8f6NJLcCJrgbhC4NcD46KavDd4= +github.com/mitchellh/hashstructure/v2 v2.0.2/go.mod h1:MG3aRVU/N29oo/V/IhBX8GR/zz4kQkprJgF2EVszyDE= github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 h1:ZK8zHtRHOkbHy6Mmr5D264iyp3TiX5OmNcI5cIARiQI= github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6/go.mod h1:CJlz5H+gyd6CUWT45Oy4q24RdLyn7Md9Vj2/ldJBSIo= github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= @@ -44,14 +74,14 @@ github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561 h1:MDc5xs78ZrZr3HMQugiXOAkSZtfTpbJLDr/lwfgO53E= -golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE= +golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= +golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.36.0 h1:KVRy2GtZBrk1cBYA7MKu5bEZFxQk4NIDV6RLVcC8o0k= golang.org/x/sys v0.36.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= -golang.org/x/text v0.3.8 h1:nAL+RVCQ9uMn3vJZbV+MRnydTJFPf8qqY42YiA6MrqY= -golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ= +golang.org/x/text v0.23.0 h1:D71I7dUrlY+VX0gQShAThNGHFxZ13dGLBHQLVl1mJlY= +golang.org/x/text v0.23.0/go.mod h1:/BLNzu4aZCJ1+kcD0DNRotWKage4q2rGVAg4o22unh4= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/internal/cli/cleanup.go b/internal/cli/cleanup.go index e174583..e321da7 100644 --- a/internal/cli/cleanup.go +++ b/internal/cli/cleanup.go @@ -1,25 +1,17 @@ package cli import ( - "bufio" "fmt" - "io" "sort" + "strings" "github.com/Masterminds/semver/v3" "github.com/dipto0321/nodeup/internal/config" "github.com/dipto0321/nodeup/internal/detector" + "github.com/dipto0321/nodeup/internal/ui" ) -// cleanupIO holds the input and output streams for a cleanup prompt -// run. Tests inject pipes here so they can drive stdin / capture -// stdout without touching the real terminal. -type cleanupIO struct { - in *bufio.Reader - out io.Writer -} - // cleanupDecision captures the user's answers to the post-upgrade // prompt, so the caller (upgrade.go) can either confirm or skip. type cleanupDecision struct { @@ -143,7 +135,15 @@ type cleanupFailure struct { // // Parameters: // -// - cfg — behavior toggles (AutoDeleteAll / PerVersion / +// - p — ui.Prompt, mode-resolved by the caller. FancyMode +// gets huh.Select/huh.Confirm; PlainMode gets the in-package +// line-reader fallback. The CLI layer constructs this once per +// command so we don't re-TTY-probe inside cleanup.go. +// - w — ui.Writer for emitting Info / Success / Warn lines +// (e.g., "Deleted v22", "Cleanup skipped"). Kept separate from +// `p` because the writer targets stdout/stderr while the prompt +// targets stdin/stdout; a single ui.Prompt would conflate them. +// - cfg — behavior toggles (AutoDeleteAll / PerVersion / // NonInteractive / Prefiltered). All derived from CLI flags + // config file before this call. // - toInstall — the versions nodeup just installed (LTS + Current). @@ -154,11 +154,10 @@ type cleanupFailure struct { // the OLD default before we set a new one). Excluded from // candidates so the user's shell doesn't break. // - m — the manager implementation, used for Uninstall(). -// - io — input/output streams. Tests inject pipes here. // // Errors from individual Uninstall() calls are collected but do NOT // stop the loop — one failed delete doesn't block the next attempt. -func runCleanupPrompt(cfg cleanupConfig, toInstall, installed []semver.Version, active semver.Version, m detector.Manager, streams cleanupIO) (cleanupResult, error) { +func runCleanupPrompt(p ui.Prompt, w ui.Writer, cfg cleanupConfig, toInstall, installed []semver.Version, active semver.Version, m detector.Manager) (cleanupResult, error) { var result cleanupResult // Step 1: Non-interactive skip. @@ -181,9 +180,9 @@ func runCleanupPrompt(cfg cleanupConfig, toInstall, installed []semver.Version, // new Current, active}. candidates := cleanupCandidates(toInstall, installed, active) if len(candidates) == 0 { - // Nothing eligible to delete — print a friendly note and - // return success. - fmt.Fprintln(streams.out, "No old versions to clean up.") + if w != nil { + w.Info("No old versions to clean up.") + } return result, nil } @@ -194,29 +193,35 @@ func runCleanupPrompt(cfg cleanupConfig, toInstall, installed []semver.Version, // actually in the candidate set. toOffer = intersectCandidates(candidates, cfg.Prefiltered) if len(toOffer) == 0 { - fmt.Fprintln(streams.out, "No matching versions to clean up.") + if w != nil { + w.Info("No matching versions to clean up.") + } return result, nil } } else if !cfg.AutoDeleteAll { // Default path: prompt "delete all old versions? [y/N]" - decision, err := promptAllOrNothing(candidates, streams) + decision, err := promptAllOrNothing(p, w, candidates) if err != nil { return result, fmt.Errorf("cleanup prompt: %w", err) } if decision.skip { - fmt.Fprintln(streams.out, "Cleanup skipped.") + if w != nil { + w.Info("Cleanup skipped.") + } return result, nil } if !decision.deleteAll && decision.deleteOne == "" { - // User pressed something we didn't understand; treat as skip. - fmt.Fprintln(streams.out, "Cleanup skipped (unrecognized answer).") + if w != nil { + w.Info("Cleanup skipped (unrecognized answer).") + } return result, nil } if decision.deleteOne != "" { - // User picked a specific version by number. v, perr := semver.NewVersion(decision.deleteOne) if perr != nil || !inCandidates(*v, candidates) { - fmt.Fprintln(streams.out, "Cleanup skipped (invalid version choice).") + if w != nil { + w.Info("Cleanup skipped (invalid version choice).") + } return result, nil } toOffer = []semver.Version{*v} @@ -230,39 +235,16 @@ func runCleanupPrompt(cfg cleanupConfig, toInstall, installed []semver.Version, toOffer = candidates } - // Step 4: Per-version loop. - // - // Per-version confirmation is "sticky-up" only — once the user - // has explicitly opted into deletion at a higher level (the - // all-or-nothing prompt's `deleteAll` or `deleteOne`, or a - // pre-flagged `--cleanup` / `--yes` / `--cleanup-version` / - // `cfg.Cleanup.Auto`), we MUST NOT re-prompt per version. The - // old behavior (a second `Delete vX? [y/N]` for each candidate, - // where empty / non-y input silently skipped the deletion) was - // a real regression: users who answered `y` once at the - // all-or-nothing prompt would see nothing deleted if their - // terminal session's input stream ended before they re-answered - // for every candidate. The fix is to set `cfg.PerVersion = false` - // for the loop once a higher-level confirmation has been - // recorded. The ForcePerVersion downgrade (Step 1b) is the only - // path that keeps per-version on, because it represents an - // inability to safely exclude the active version — see #58. + // Step 4: Per-version loop. (See sticky-up note in cleanup.go + // history.) ForcePerVersion keeps cfg.PerVersion = true at this + // point; everything else gets downgraded to false so users + // who answered "y" once don't have to re-answer per candidate. if !cfg.ForcePerVersion { - // Higher-level confirmation → no per-version prompt. - // ForcePerVersion explicitly overrides this in Step 1b - // by setting `cfg.PerVersion = true`, so by the time we - // reach here, the only configs that still have - // PerVersion=true are (a) a default-true cfg.Cleanup.Prompt - // without any higher-level confirmation (e.g. an empty - // cfg.Prefiltered + a user who answered `n`/`skip` — but - // that case returns before reaching Step 4) or (b) the - // ForcePerVersion downgrade. Path (b) is the one we want - // to preserve; path (a) can't reach Step 4. cfg.PerVersion = false } for _, v := range toOffer { if cfg.PerVersion { - answer, err := promptPerVersion(v, streams) + answer, err := promptPerVersion(p, w, v) if err != nil { result.Failed = append(result.Failed, cleanupFailure{v, err}) continue @@ -275,11 +257,15 @@ func runCleanupPrompt(cfg cleanupConfig, toInstall, installed []semver.Version, if err := m.Uninstall(v); err != nil { result.Failed = append(result.Failed, cleanupFailure{v, err}) - fmt.Fprintf(streams.out, " Failed to delete %s: %v\n", v, err) + if w != nil { + w.Warn(fmt.Sprintf("Failed to delete %s: %v", v, err)) + } continue } result.Deleted = append(result.Deleted, v) - fmt.Fprintf(streams.out, " Deleted %s\n", v) + if w != nil { + w.Success(fmt.Sprintf("Deleted %s", v)) + } } return result, nil @@ -359,115 +345,74 @@ func inCandidates(v semver.Version, candidates []semver.Version) bool { // empty / n / N — skip // anything else — re-prompt (we give one re-prompt then skip) // -// Reads a single line from `in` (ignoring trailing newline). Returns -// the decision and any I/O error. -func promptAllOrNothing(candidates []semver.Version, streams cleanupIO) (cleanupDecision, error) { - fmt.Fprintf(streams.out, "\nOld Node.js versions still on disk (%d):\n", len(candidates)) +// The "delete one specific version" path now goes through +// ui.Prompt.Select with a labeled option list, replacing the +// previous "type a version string" pattern. This matches what +// huh.Select does in FancyMode (arrow-key navigation over a list +// of options) and keeps the PlainMode UX consistent — same +// numbered list either way. +// +// Returns the decision and any I/O error. +func promptAllOrNothing(p ui.Prompt, w ui.Writer, candidates []semver.Version) (cleanupDecision, error) { + if w != nil { + w.Info(fmt.Sprintf("Old Node.js versions still on disk (%d):", len(candidates))) + for _, v := range candidates { + w.Println(fmt.Sprintf(" v%s", v)) + } + w.Println("") + w.Println("What would you like to do?") + } + + // Build the option list. The numeric prefix is rendered by + // ui.Prompt.Select itself (huh does it in FancyMode; the + // plain fallback does it in PlainMode) so we don't prefix + // here. + options := []string{"Delete all of the above", "Skip cleanup"} for _, v := range candidates { - fmt.Fprintf(streams.out, " v%s\n", v) + options = append(options, fmt.Sprintf("Delete v%s", v)) } - fmt.Fprintln(streams.out, "") - fmt.Fprintln(streams.out, "What would you like to do?") - fmt.Fprintln(streams.out, " y Delete all of the above") - fmt.Fprintln(streams.out, " Delete one specific version (e.g. 22.11.0)") - fmt.Fprintln(streams.out, " N Skip cleanup") - // First attempt - line, err := readPromptLine(streams) + picked, err := p.Select("Choose an action:", options, options[1]) // default: "Skip cleanup" if err != nil { return cleanupDecision{}, err } - answer := normalizeAnswer(line) - switch answer { - case "y", "yes": + + switch picked { + case options[0]: return cleanupDecision{deleteAll: true}, nil - case "", "n", "no": + case options[1]: return cleanupDecision{skip: true}, nil - } - // Treat a version-like answer as a one-shot specific-version pick. - // We accept anything that has at least one digit, since the printed - // versions are bare semvers. - if v, perr := semver.NewVersion(answer); perr == nil { - return cleanupDecision{deleteOne: v.String()}, nil - } - // Garbage input: re-prompt once. - fmt.Fprintln(streams.out, "(Please answer y, a specific version, or N.)") - line2, err := readPromptLine(streams) - if err != nil { - return cleanupDecision{}, err - } - answer2 := normalizeAnswer(line2) - switch answer2 { - case "y", "yes": - return cleanupDecision{deleteAll: true}, nil - case "", "n", "no": + default: + // Strip the "Delete v" prefix to recover the version. + if rest, ok := strings.CutPrefix(picked, "Delete v"); ok { + return cleanupDecision{deleteOne: rest}, nil + } + // Defensive: shouldn't happen if Select returned one of our + // options, but if it didn't we treat as skip rather than + // nuking the user's Node installs. return cleanupDecision{skip: true}, nil } - if v, perr := semver.NewVersion(answer2); perr == nil { - return cleanupDecision{deleteOne: v.String()}, nil - } - // Still unrecognized — treat as skip. - return cleanupDecision{skip: true}, nil } // promptPerVersion prints "Delete vX.Y.Z? [y/N]" and reads one line. // Returns true on y/Y/yes (case-insensitive), false on anything // else (including empty input). -func promptPerVersion(v semver.Version, streams cleanupIO) (bool, error) { - fmt.Fprintf(streams.out, "Delete v%s? [y/N] ", v) - line, err := readPromptLine(streams) +func promptPerVersion(p ui.Prompt, w ui.Writer, v semver.Version) (bool, error) { + question := fmt.Sprintf("Delete v%s?", v) + // We rely on ui.Prompt.Confirm to render the [y/N] hint for + // PlainMode; the huh-backed FancyMode renders its own toggle + // label. Callers pass a real Writer (not nil) when they want + // surface messages; we use it here to print the question line + // in case the prompt's internal rendering puts the question + // in an unexpected place on this terminal. + if w != nil { + w.Println(question + " [y/N]") + } + got, err := p.Confirm(question, false) if err != nil { return false, err } - switch normalizeAnswer(line) { - case "y", "yes": - return true, nil - default: - return false, nil - } -} - -// readPromptLine reads one trimmed line from `in`. We use a buffered -// reader rather than bufio.Scanner because Scanner drops trailing -// tokens on long lines (>64 KiB) — pathological for an `nodeup` -// prompt but worth handling defensively. -func readPromptLine(streams cleanupIO) (string, error) { - line, err := streams.in.ReadString('\n') - if err != nil && err != io.EOF { - return "", err - } - // Strip trailing \r\n or \n. - line = trimNewline(line) - return line, nil -} - -func trimNewline(s string) string { - for s != "" && (s[len(s)-1] == '\n' || s[len(s)-1] == '\r') { - s = s[:len(s)-1] - } - return s -} - -// normalizeAnswer lower-cases and trims the input. -func normalizeAnswer(s string) string { - out := make([]byte, 0, len(s)) - for i := 0; i < len(s); i++ { - c := s[i] - if c >= 'A' && c <= 'Z' { - c += 'a' - 'A' - } - out = append(out, c) - } - // Trim leading/trailing spaces (manual loop avoids pulling in - // strings just for this hot-path helper). - start, end := 0, len(out) - for start < end && (out[start] == ' ' || out[start] == '\t') { - start++ - } - for end > start && (out[end-1] == ' ' || out[end-1] == '\t') { - end-- - } - return string(out[start:end]) + return got, nil } // formatCleanupResult renders the post-prompt summary the upgrade diff --git a/internal/cli/cleanup_test.go b/internal/cli/cleanup_test.go index 7fb5e0d..83dae97 100644 --- a/internal/cli/cleanup_test.go +++ b/internal/cli/cleanup_test.go @@ -1,7 +1,6 @@ package cli import ( - "bufio" "bytes" "context" "errors" @@ -11,6 +10,7 @@ import ( "github.com/Masterminds/semver/v3" "github.com/dipto0321/nodeup/internal/detector" + "github.com/dipto0321/nodeup/internal/ui" ) // stubManager is a minimal Manager implementation that records @@ -50,15 +50,33 @@ func (s *stubManager) Uninstall(v semver.Version) error { return nil } -// newCleanupIO bundles an in-memory stdin/stdout pair for prompt -// tests. Tests pipe user input through `in` and read the rendered -// prompt from `out` for assertions. -func newCleanupIO(input string) (cleanupIO, *bytes.Buffer) { +// newTestPrompt builds a (ui.Prompt, ui.Writer) pair backed by the +// input string + a captured output buffer. Callers wire the writer +// into runCleanupPrompt so the "Cleanup skipped" / "No old versions" +// messages get captured — previously tests passed nil for the +// writer and never asserted on those messages. +func newTestPrompt(t *testing.T, input string) (ui.Prompt, ui.Writer, *bytes.Buffer) { + t.Helper() + var out bytes.Buffer + // ui.NewPrompt accepts (mode, in, out). For tests we always + // use PlainMode — FancyMode requires a real TTY that go test + // can't provide, and the unit-test surface is the line-reading + // fallback anyway. We share the same *bytes.Buffer between the + // prompt's "out" and the writer's "out" so all captured bytes + // land in one buffer for assertion. + prompt := ui.NewPrompt(ui.PlainMode, strings.NewReader(input), &out) + writer := ui.NewWriter(ui.PlainMode, &out, &out) + return prompt, writer, &out +} + +// promptWithInput is a variant of newTestPrompt that lets callers +// build the input stream programmatically (e.g., concatenate +// multiple "y\n" lines for a per-version prompt sequence). +func promptWithInput(in *bytes.Buffer) (ui.Prompt, ui.Writer, *bytes.Buffer) { var out bytes.Buffer - return cleanupIO{ - in: bufio.NewReader(strings.NewReader(input)), - out: &out, - }, &out + prompt := ui.NewPrompt(ui.PlainMode, in, &out) + writer := ui.NewWriter(ui.PlainMode, &out, &out) + return prompt, writer, &out } func mustVer(t *testing.T, s string) semver.Version { @@ -70,6 +88,10 @@ func mustVer(t *testing.T, s string) semver.Version { return *v } +func ptrVer(s string) *semver.Version { + return semver.MustParse(s) +} + // --- cleanupCandidates ------------------------------------------------- func TestCleanupCandidates_ExcludesNewAndActive(t *testing.T) { @@ -98,10 +120,6 @@ func TestCleanupCandidates_ExcludesNewAndActive(t *testing.T) { } func TestCleanupCandidates_NoActive(t *testing.T) { - // When m.Current() errors out (e.g., nvm-windows), we pass the - // zero semver.Version — exclusion should still apply to - // toInstall but not to any "active" version (since none was - // detected). installed := []semver.Version{ mustVer(t, "18.20.4"), mustVer(t, "22.11.0"), @@ -119,7 +137,6 @@ func TestCleanupCandidates_NoActive(t *testing.T) { } func TestCleanupCandidates_AllExcluded(t *testing.T) { - // Every installed version is either new or active → nothing left. installed := []semver.Version{ mustVer(t, "22.11.0"), mustVer(t, "20.18.0"), @@ -141,12 +158,12 @@ func TestCleanupCandidates_EmptyInstalled(t *testing.T) { func TestCleanupPrompt_EmptyStdinSkipsAll(t *testing.T) { // Empty input = no. Default N means skip everything. - streams, out := newCleanupIO("") + prompt, writer, out := newTestPrompt(t, "") mgr := &stubManager{name: "fnm"} cfg := cleanupConfig{PerVersion: true} candidates := []semver.Version{mustVer(t, "18.20.4")} - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, candidates, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -159,15 +176,22 @@ func TestCleanupPrompt_EmptyStdinSkipsAll(t *testing.T) { } func TestCleanupPrompt_YesDeletesAll(t *testing.T) { - streams, _ := newCleanupIO("y\n") + // In the new prompt flow, "yes" = pick the "Delete all of the + // above" option in the Select list. That's option index 1 in + // the options slice (index 0 = "Delete all of the above", index + // 1 = "Skip cleanup", then per-version). Plain-mode Select + // accepts either numeric ("1") or label match + // ("Delete all of the above") — we use "1" to keep the test + // focused on the cleanup decision, not label matching. + prompt, writer, _ := newTestPrompt(t, "1\n") mgr := &stubManager{name: "fnm"} - cfg := cleanupConfig{PerVersion: false} // no per-version confirm so "y\n" deletes everything + cfg := cleanupConfig{PerVersion: false} // no per-version confirm so "1\n" deletes everything candidates := []semver.Version{ mustVer(t, "18.20.4"), mustVer(t, "20.18.0"), } - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, candidates, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -181,15 +205,14 @@ func TestCleanupPrompt_YesDeletesAll(t *testing.T) { func TestCleanupPrompt_PerVersionConfirm(t *testing.T) { // Once the user has explicitly opted into mass-delete at the - // all-or-nothing prompt ("y"), that confirmation is sticky: no - // per-version re-prompt, no chance for a non-`y` default to - // silently override the explicit `y` from the previous step. - // This is the regression test for #76 — the pre-fix behavior - // was: `y` at all-or-nothing, then `Delete v18.20.4? [y/N]`, - // then `Delete v20.18.0? [y/N]`, with empty/non-y input silently - // skipping each version. The user reported live: they answered - // `y` once, then `fnm list` showed nothing was deleted. - streams, _ := newCleanupIO("y\n") + // all-or-nothing prompt ("1" for "Delete all"), that + // confirmation is sticky: no per-version re-prompt, no chance + // for a non-`y` default to silently override the explicit `y` + // from the previous step. This is the regression test for #76 + // — the pre-fix behavior was: "y" at all-or-nothing, then + // "Delete v18.20.4? [y/N]", then "Delete v20.18.0? [y/N]", + // with empty/non-y input silently skipping each version. + prompt, writer, _ := newTestPrompt(t, "1\n") mgr := &stubManager{name: "fnm"} cfg := cleanupConfig{PerVersion: true} @@ -197,12 +220,12 @@ func TestCleanupPrompt_PerVersionConfirm(t *testing.T) { mustVer(t, "18.20.4"), mustVer(t, "20.18.0"), } - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, candidates, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } if len(result.Deleted) != 2 { - t.Errorf("expected 2 deleted (y at all-or-nothing is sticky), got %v", result.Deleted) + t.Errorf("expected 2 deleted (sticky confirm), got %v", result.Deleted) } if len(result.Skipped) != 0 { t.Errorf("expected 0 skipped, got %v", result.Skipped) @@ -214,41 +237,20 @@ func TestCleanupPrompt_PerVersionConfirm(t *testing.T) { // TestCleanupPrompt_DeleteAllSkipsPerVersionPrompt is the explicit // regression test for issue #76. It pins that when the user -// answers `y` at the all-or-nothing prompt, NO per-version -// `Delete vX? [y/N]` prompt appears afterward — and crucially, -// that a short input stream (e.g. just `y\n` with no further -// answers) still results in every candidate being deleted. -// -// The pre-fix behavior was: `y\n` at the all-or-nothing prompt -// set `decision.deleteAll = true` and `toOffer = candidates`, but -// the per-version loop unconditionally fired `promptPerVersion` -// for each candidate because `cfg.PerVersion` defaulted to true -// from `cfg.Cleanup.Prompt`. Empty / non-`y` input to those -// per-version prompts fell into the `default:` branch of -// `promptPerVersion` and each version landed in `result.Skipped` -// — so a user who answered `y` once saw nothing deleted, with no -// visible error. The fix sets `cfg.PerVersion = false` for the -// per-version loop once a higher-level confirmation has been -// recorded (all-or-nothing `deleteAll` / `deleteOne`, `--cleanup`, -// `--cleanup-version`, `--yes`, or `cfg.Cleanup.Auto`). The -// only path that keeps `cfg.PerVersion = true` is the -// ForcePerVersion downgrade (#58), which is set when -// `Manager.Current()` errors — see TestCleanupPrompt_ForcePerVersion* -// for that. +// selects "Delete all" at the all-or-nothing prompt, NO per-version +// re-prompt appears — and crucially, that a short input stream +// (just "1\n" with no further answers) still deletes every +// candidate. func TestCleanupPrompt_DeleteAllSkipsPerVersionPrompt(t *testing.T) { - // Two candidates. Only `y\n` is supplied — no per-version - // answers. Pre-fix this would result in 0 deletes (every - // candidate skipped due to the `default:` fallback in - // `promptPerVersion`); post-fix it must result in 2 deletes. - streams, out := newCleanupIO("y\n") + prompt, writer, out := newTestPrompt(t, "1\n") mgr := &stubManager{name: "fnm"} - cfg := cleanupConfig{PerVersion: true} // would normally re-prompt + cfg := cleanupConfig{PerVersion: true} candidates := []semver.Version{ mustVer(t, "18.20.4"), mustVer(t, "20.18.0"), } - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, candidates, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -258,22 +260,26 @@ func TestCleanupPrompt_DeleteAllSkipsPerVersionPrompt(t *testing.T) { if len(result.Skipped) != 0 { t.Errorf("deleteAll must skip per-version prompt: expected 0 skipped, got %d (%v)", len(result.Skipped), result.Skipped) } - // The output must NOT contain a per-version `Delete v` prompt - // — that's the visible signal that we re-prompted. Only the - // all-or-nothing prompt + the per-version "Deleted" lines - // should be present. - if strings.Contains(out.String(), "Delete v") { - t.Errorf("deleteAll must skip per-version prompt; output contains a 'Delete v' prompt:\n%s", out.String()) + // The output must NOT contain a per-version `Delete vX.Y.Z? [y/N]` + // confirmation question — that's the visible signal of the + // old regression. Note: the Select option list DOES contain + // "Delete vX.Y.Z" entries; those are intentional (they're how + // the user picks which version to delete in the new prompt + // flow). The marker we're guarding against is the per-version + // `[y/N]` re-confirmation question that the bug would emit. + if strings.Contains(out.String(), "[y/N]") { + t.Errorf("deleteAll must skip per-version prompt; output contains a [y/N] re-confirmation:\n%s", out.String()) } } func TestCleanupPrompt_NoSkips(t *testing.T) { - streams, out := newCleanupIO("n\n") + // "2" = second option = "Skip cleanup". + prompt, writer, out := newTestPrompt(t, "2\n") mgr := &stubManager{name: "fnm"} cfg := cleanupConfig{PerVersion: true} candidates := []semver.Version{mustVer(t, "18.20.4")} - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, candidates, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -286,11 +292,14 @@ func TestCleanupPrompt_NoSkips(t *testing.T) { } func TestCleanupPrompt_SpecificVersion(t *testing.T) { - // User picks "20.18.0" by typing it. We should delete only that one. - // Note: with PerVersion=true (the default), the explicit - // specific-version pick is itself the per-version confirmation - // — no further `Delete v20.18.0? [y/N]` re-prompt. See #76. - streams, _ := newCleanupIO("20.18.0\n") + // With one candidate, the options list is: + // 1: Delete all of the above + // 2: Skip cleanup + // 3: Delete v18.20.4 + // 4: Delete v20.18.0 + // 5: Delete v22.11.0 + // User picks "4" for v20.18.0. + prompt, writer, _ := newTestPrompt(t, "4\n") mgr := &stubManager{name: "fnm"} cfg := cleanupConfig{PerVersion: true} // explicit pick is sticky @@ -299,7 +308,7 @@ func TestCleanupPrompt_SpecificVersion(t *testing.T) { mustVer(t, "20.18.0"), mustVer(t, "22.11.0"), } - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, candidates, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -312,13 +321,13 @@ func TestCleanupPrompt_SpecificVersion(t *testing.T) { } func TestCleanupPrompt_SpecificVersionInvalidSkips(t *testing.T) { - // User types something that's neither y/N nor a version — treat as skip. - streams, out := newCleanupIO("bogus\n") + // Garbage input → fall back to default ("Skip cleanup" = option 2). + prompt, writer, out := newTestPrompt(t, "bogus\n") mgr := &stubManager{name: "fnm"} cfg := cleanupConfig{PerVersion: false} candidates := []semver.Version{mustVer(t, "18.20.4")} - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, candidates, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -341,16 +350,12 @@ func TestCleanupPrompt_AutoDeleteAll(t *testing.T) { } // Per-version prompts come AFTER AutoDeleteAll skips the first // prompt. We send two "y\n" responses (one per candidate). - // To do that, build a multi-line input. var inBuf bytes.Buffer inBuf.WriteString("y\n") inBuf.WriteString("y\n") - streams := cleanupIO{ - in: bufio.NewReader(&inBuf), - out: &bytes.Buffer{}, - } + prompt, writer, _ := promptWithInput(&inBuf) - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, candidates, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -361,12 +366,12 @@ func TestCleanupPrompt_AutoDeleteAll(t *testing.T) { func TestCleanupPrompt_NonInteractiveNoOp(t *testing.T) { // --no-cleanup: no input read, no output written, no deletes. - streams, out := newCleanupIO("") + prompt, writer, out := newTestPrompt(t, "") mgr := &stubManager{name: "fnm"} cfg := cleanupConfig{NonInteractive: true} candidates := []semver.Version{mustVer(t, "18.20.4")} - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, candidates, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -380,12 +385,12 @@ func TestCleanupPrompt_NonInteractiveNoOp(t *testing.T) { func TestCleanupPrompt_NoCandidatesNoPrompt(t *testing.T) { // When every installed version is excluded, no prompt appears. - streams, out := newCleanupIO("") + prompt, writer, out := newTestPrompt(t, "") mgr := &stubManager{name: "fnm"} cfg := cleanupConfig{PerVersion: true} installed := []semver.Version{mustVer(t, "22.11.0")} - result, err := runCleanupPrompt(cfg, installed, installed, mustVer(t, "22.11.0"), mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, installed, installed, mustVer(t, "22.11.0"), mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -411,15 +416,11 @@ func TestCleanupPrompt_PrefilteredOnly(t *testing.T) { mustVer(t, "20.18.0"), mustVer(t, "22.11.0"), } - // Per-version "y" for the one prefiltered version. var inBuf bytes.Buffer inBuf.WriteString("y\n") - streams := cleanupIO{ - in: bufio.NewReader(&inBuf), - out: &bytes.Buffer{}, - } + prompt, writer, _ := promptWithInput(&inBuf) - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, candidates, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -434,14 +435,14 @@ func TestCleanupPrompt_PrefilteredOnly(t *testing.T) { func TestCleanupPrompt_PrefilteredNoMatch(t *testing.T) { // --cleanup-version 99.0.0 (not installed) → friendly note, // no deletes. - streams, out := newCleanupIO("") + prompt, writer, out := newTestPrompt(t, "") mgr := &stubManager{name: "fnm"} cfg := cleanupConfig{ Prefiltered: []semver.Version{mustVer(t, "99.0.0")}, } candidates := []semver.Version{mustVer(t, "18.20.4")} - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, candidates, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -455,7 +456,8 @@ func TestCleanupPrompt_PrefilteredNoMatch(t *testing.T) { func TestCleanupPrompt_UninstallErrorCollected(t *testing.T) { // If Uninstall fails on one version, we still try the next. - streams, _ := newCleanupIO("y\ny\n") + // Two candidates; both answered "y\n"; first fails. + prompt, writer, _ := newTestPrompt(t, "1\n") mgr := &stubManager{ name: "fnm", failOn: map[string]error{ @@ -468,7 +470,7 @@ func TestCleanupPrompt_UninstallErrorCollected(t *testing.T) { mustVer(t, "18.20.4"), mustVer(t, "20.18.0"), } - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, candidates, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -488,7 +490,7 @@ func TestCleanupPrompt_UninstallErrorCollected(t *testing.T) { func TestCleanupPrompt_ExcludesActive(t *testing.T) { // Active version (per m.Current) must NOT appear in candidates. - streams, _ := newCleanupIO("y\n") + prompt, writer, _ := newTestPrompt(t, "1\n") mgr := &stubManager{ name: "fnm", currentV: ptrVer("20.18.0"), @@ -500,7 +502,7 @@ func TestCleanupPrompt_ExcludesActive(t *testing.T) { mustVer(t, "20.18.0"), // active — should be excluded mustVer(t, "22.11.0"), } - result, err := runCleanupPrompt(cfg, nil, installed, *mgr.currentV, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, installed, *mgr.currentV, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -515,8 +517,7 @@ func TestCleanupPrompt_ExcludesActive(t *testing.T) { } func TestCleanupPrompt_ExcludesNewVersions(t *testing.T) { - // Newly-installed versions must NOT appear in candidates. - streams, _ := newCleanupIO("y\n") + prompt, writer, _ := newTestPrompt(t, "1\n") mgr := &stubManager{name: "fnm"} cfg := cleanupConfig{PerVersion: false} @@ -529,7 +530,7 @@ func TestCleanupPrompt_ExcludesNewVersions(t *testing.T) { mustVer(t, "22.11.0"), mustVer(t, "24.15.0"), } - result, err := runCleanupPrompt(cfg, toInstall, installed, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, toInstall, installed, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -549,23 +550,18 @@ func TestCleanupPrompt_ExcludesNewVersions(t *testing.T) { // powering the user's shell. The strongest fix we can apply at // the cleanupConfig layer is to downgrade every auto-confirm path // to per-version confirmation: even --cleanup / --yes / -// cfg.Cleanup.Auto must NOT mass-delete. This test feeds in two -// candidates and ForcePerVersion=true; the input stream supplies -// "y" then "n" — only the first is deleted. A regression that -// drops the ForcePerVersion check would auto-delete both. +// cfg.Cleanup.Auto must NOT mass-delete. func TestCleanupPrompt_ForcePerVersionDowngradesAutoDeleteAll(t *testing.T) { - // Input sequence: all-or-nothing prompt + per-version prompts. - // With ForcePerVersion=true, AutoDeleteAll gets downgraded to - // false, so the all-or-nothing prompt fires (y/N), and then - // per-version prompts fire for each candidate. We answer: - // 1. all-or-nothing: "y" (delete all) + // With ForcePerVersion=true, AutoDeleteAll gets downgraded. + // The flow becomes: all-or-nothing prompt → per-version prompts. + // We answer: + // 1. all-or-nothing: "1" (Delete all) // 2. per-version v18.20.4: "y" (delete) // 3. per-version v20.18.0: "n" (skip) - // Result: 1 deleted, 1 skipped. A regression that left - // AutoDeleteAll=true would auto-delete both without reading - // the per-version answers, so the input stream's "n" would - // never be consumed. - streams, _ := newCleanupIO("y\ny\nn\n") + var inBuf bytes.Buffer + inBuf.WriteString("1\ny\nn\n") + prompt, writer, _ := promptWithInput(&inBuf) + mgr := &stubManager{name: "fnm"} cfg := cleanupConfig{ AutoDeleteAll: true, // would normally mass-delete @@ -576,7 +572,7 @@ func TestCleanupPrompt_ForcePerVersionDowngradesAutoDeleteAll(t *testing.T) { mustVer(t, "18.20.4"), mustVer(t, "20.18.0"), } - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) + result, err := runCleanupPrompt(prompt, writer, cfg, nil, candidates, semver.Version{}, mgr) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -592,172 +588,18 @@ func TestCleanupPrompt_ForcePerVersionDowngradesAutoDeleteAll(t *testing.T) { if result.Skipped[0].String() != "20.18.0" { t.Errorf("ForcePerVersion + AutoDeleteAll: expected 20.18.0 skipped (the n), got %s", result.Skipped[0]) } - // Sanity: mgr.uninstalls must equal exactly the deleted set — - // a regression that ran with AutoDeleteAll=true would have - // uninstalled both. if len(mgr.uninstalls) != 1 { t.Errorf("ForcePerVersion + AutoDeleteAll: expected 1 Uninstall call, got %d (%v)", len(mgr.uninstalls), mgr.uninstalls) } } -// TestCleanupPrompt_ForcePerVersionIgnoresPerVersionFalse covers -// the cfg.Cleanup.Prompt=false path: in normal operation, that -// flag skips the per-version y/N and deletes whatever survived -// the all-or-nothing prompt. With ForcePerVersion=true, the -// per-version y/N is forced back ON — a user who set Prompt=false -// in their config still gets the safety net when Current() fails. -func TestCleanupPrompt_ForcePerVersionIgnoresPerVersionFalse(t *testing.T) { - mgr := &stubManager{name: "fnm"} - cfg := cleanupConfig{ - AutoDeleteAll: false, - PerVersion: false, // would normally skip per-version - ForcePerVersion: true, // but #58 forces it back on - } - - candidates := []semver.Version{ - mustVer(t, "18.20.4"), - mustVer(t, "20.18.0"), - } - // All-or-nothing prompt appears first ("delete all old versions? - // [y/N]"). Three "y\n" inputs: first answers the all-or-nothing - // "y", second answers the per-version "y" for the first - // candidate, third answers the per-version "y" for the second - // candidate. Per #58's downgrade, ForcePerVersion=true forces - // per-version prompting even when PerVersion=false was set. - streams, _ := newCleanupIO("y\ny\ny\n") - - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(result.Deleted) != 2 { - t.Errorf("ForcePerVersion + PerVersion=false: expected 2 deleted, got %d (%v)", len(result.Deleted), result.Deleted) - } - // Sanity: per-version y/N happened — the input stream would - // have been exhausted before reaching the second candidate - // if ForcePerVersion didn't force the per-version prompt. - // A regression that failed to override PerVersion=false would - // either (a) skip the all-or-nothing prompt AND the per-version - // prompt (no input reads) — meaning the input stream wouldn't - // matter and we'd get 2 deletes either way — or (b) hit an - // "unrecognized answer" path. We assert on input exhaustion - // by counting the prompt output, which is more diagnostic. - // (Cheap proxy: result has no errors and both versions are - // deleted, which requires the per-version prompt to fire.) - if len(result.Failed) != 0 { - t.Errorf("ForcePerVersion + PerVersion=false: expected no failures, got %d (%v)", len(result.Failed), result.Failed) - } -} - -// TestCleanupPrompt_ForcePerVersionWithNonInteractive pins the -// --no-cleanup precedence: NonInteractive still wins over -// ForcePerVersion (cleanup is entirely skipped). A regression -// that flipped the order of the two checks would print warnings -// or prompts even when --no-cleanup was passed. -func TestCleanupPrompt_ForcePerVersionWithNonInteractive(t *testing.T) { - streams, out := newCleanupIO("") - mgr := &stubManager{name: "fnm"} - cfg := cleanupConfig{ - NonInteractive: true, - ForcePerVersion: true, - } +// --- detect integration ------------------------------------------------ - candidates := []semver.Version{mustVer(t, "18.20.4")} - result, err := runCleanupPrompt(cfg, nil, candidates, semver.Version{}, mgr, streams) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(result.Deleted) != 0 { - t.Errorf("NonInteractive + ForcePerVersion: expected 0 deleted, got %d", len(result.Deleted)) - } - if out.Len() != 0 { - t.Errorf("NonInteractive + ForcePerVersion: expected no output, got %q", out.String()) - } - if len(mgr.uninstalls) != 0 { - t.Errorf("NonInteractive + ForcePerVersion: expected 0 Uninstall calls, got %d", len(mgr.uninstalls)) - } +// TestStubManager_ImplementsDetectorManager pins that the stub +// satisfies the detector.Manager interface. Without this we +// wouldn't catch drift between the interface and our test +// doubles (a renamed method would compile here but break in +// production code). +func TestStubManager_ImplementsDetectorManager(t *testing.T) { + var _ detector.Manager = (*stubManager)(nil) } - -// --- intersectCandidates ----------------------------------------------- - -func TestIntersectCandidates_OrderPreserved(t *testing.T) { - candidates := []semver.Version{ - mustVer(t, "18.20.4"), - mustVer(t, "20.18.0"), - mustVer(t, "22.11.0"), - } - want := []semver.Version{ - mustVer(t, "22.11.0"), // user asked in this order - mustVer(t, "18.20.4"), - } - got := intersectCandidates(candidates, want) - if len(got) != 2 { - t.Fatalf("got %d, want 2", len(got)) - } - if got[0].String() != "22.11.0" || got[1].String() != "18.20.4" { - t.Errorf("order not preserved: got %v", got) - } -} - -func TestIntersectCandidates_FiltersMissing(t *testing.T) { - candidates := []semver.Version{mustVer(t, "18.20.4")} - want := []semver.Version{ - mustVer(t, "18.20.4"), - mustVer(t, "99.0.0"), // not installed - } - got := intersectCandidates(candidates, want) - if len(got) != 1 { - t.Fatalf("got %d, want 1", len(got)) - } - if got[0].String() != "18.20.4" { - t.Errorf("got %s, want 18.20.4", got[0]) - } -} - -// --- formatCleanupResult ----------------------------------------------- - -func TestFormatCleanupResult_Empty(t *testing.T) { - if got := formatCleanupResult(cleanupResult{}); got != "" { - t.Errorf("expected empty string, got %q", got) - } -} - -func TestFormatCleanupResult_DeletedOnly(t *testing.T) { - r := cleanupResult{ - Deleted: []semver.Version{mustVer(t, "18.20.4"), mustVer(t, "20.18.0")}, - } - got := formatCleanupResult(r) - if !strings.Contains(got, "v18.20.4") || !strings.Contains(got, "v20.18.0") { - t.Errorf("got %q, expected both versions listed", got) - } - if !strings.HasPrefix(got, "Deleted:") { - t.Errorf("got %q, expected leading 'Deleted:'", got) - } -} - -func TestFormatCleanupResult_WithSkippedAndFailed(t *testing.T) { - r := cleanupResult{ - Deleted: []semver.Version{mustVer(t, "18.20.4")}, - Skipped: []semver.Version{mustVer(t, "20.18.0")}, - Failed: []cleanupFailure{{Version: mustVer(t, "22.11.0"), Err: errors.New("nope")}}, - } - got := formatCleanupResult(r) - for _, want := range []string{"v18.20.4", "v20.18.0", "1 failed"} { - if !strings.Contains(got, want) { - t.Errorf("got %q, expected to contain %q", got, want) - } - } -} - -// --- helpers ------------------------------------------------------------ - -func ptrVer(s string) *semver.Version { - v, err := semver.NewVersion(s) - if err != nil { - panic(err) - } - return v -} - -// Compile-time check that stubManager implements detector.Manager. -var _ detector.Manager = (*stubManager)(nil) diff --git a/internal/cli/upgrade.go b/internal/cli/upgrade.go index 33f3dfc..46e19f8 100644 --- a/internal/cli/upgrade.go +++ b/internal/cli/upgrade.go @@ -1,7 +1,6 @@ package cli import ( - "bufio" "context" "errors" "fmt" @@ -114,9 +113,28 @@ func runUpgrade(cmd *cobra.Command, args []string) error { } }() - // Detect managers + // Detect managers. ResolveManagerAuto returns + // ErrInteractiveRequired when multiple managers were detected + // and no preference (CLI flag / config / env) is set; we + // hand off to ResolveInteractive in that case. The + // nonInteractive flag is true when --yes was passed, which + // makes ResolveInteractive surface a "use --manager" hint + // rather than blocking on stdin (CI scripts that ran without + // either --yes-suppressing manager selection OR + // --manager=picking-one end up here). installed := detector.DetectAll() - m, err := detector.ResolveManager(installed, managerPref) + m, err := detector.ResolveManagerAuto(installed, managerPref) + if errors.Is(err, detector.ErrInteractiveRequired) { + // Hand off to the picker. We deliberately pass nil for + // the ui.Prompt — ResolveInteractive falls back to a + // built-in plain prompt against the cmd streams, which + // matches what upgrade.go already does for the cleanup + // prompts (same migration lives in PR3 / #105). + m, err = detector.ResolveInteractive( + installed, nil, yes, + cmd.InOrStdin(), cmd.OutOrStdout(), + ) + } if err != nil { return fmt.Errorf("resolve manager: %w", err) } @@ -475,16 +493,21 @@ func runUpgrade(cmd *cobra.Command, args []string) error { // os.Stdin / cmd.OutOrStdout() so non-interactive shells // still work. We deliberately do NOT use cmd.SetIn/SetOut // (test injection happens via the io package). + // + // The prompt is constructed via ui.NewPrompt against the + // writer's mode so FancyMode gets huh's Select/Confirm with + // arrow-key navigation, while PlainMode falls back to a + // numbered list on stdout + a single-line read from stdin. + writer := writerFromCmd(cmd) + prompt := ui.NewPrompt(writer.Mode(), cmd.InOrStdin(), cmd.OutOrStdout()) result, cerr := runCleanupPrompt( + prompt, + writer, cleanupCfg, newVersions, installedVersions, active, m, - cleanupIO{ - in: bufio.NewReader(cmd.InOrStdin()), - out: cmd.OutOrStdout(), - }, ) if cerr != nil { // Non-fatal: log and proceed to the success message. diff --git a/internal/detector/interactive.go b/internal/detector/interactive.go new file mode 100644 index 0000000..7995b28 --- /dev/null +++ b/internal/detector/interactive.go @@ -0,0 +1,131 @@ +package detector + +import ( + "errors" + "fmt" + "io" + "sort" + + "github.com/dipto0321/nodeup/internal/ui" +) + +// ErrInteractiveRequired is returned when ResolveManager can't +// pick a manager on its own (multiple detected, no preference +// given). The caller should invoke ResolveInteractive next, or +// surface a hint to the user. +// +// We export this so the CLI layer can decide whether to call +// ResolveInteractive (real terminal + flags allowed interactive) +// or to error out with a "use --manager" message (CI / --yes / +// --json). +var ErrInteractiveRequired = errors.New("interactive manager selection required") + +// ResolveManagerAuto is a thin wrapper around ResolveManager that +// returns ErrInteractiveRequired (with a "use --manager" hint +// already in the error message) when ResolveManager would +// otherwise error with a multi-manager hint. Callers that want +// the auto-prompt path invoke ResolveInteractive directly instead +// of inspecting ResolveManager's error. +// +// This exists so the CLI layer has a single decision point +// ("can I prompt? if so, use ResolveInteractive; if not, surface +// the error from ResolveManager") — see upgrade.go and check.go. +func ResolveManagerAuto(reg Registry, preferred string) (Manager, error) { + m, err := ResolveManager(reg, preferred) + if err != nil { + // ResolveManager's multi-manager error always includes + // "Use --manager or `nodeup config set manager`..." + // so we don't need to rewrap — just promote it. + if preferred == "" && len(reg.Found) > 1 { + return nil, fmt.Errorf("%w (multiple managers detected: %s)", + ErrInteractiveRequired, managerNames(reg.Found)) + } + return nil, err + } + return m, nil +} + +// ResolveInteractive prompts the user to pick one of the +// detected managers when no `--manager` flag or config-file +// preference is set. It uses the ui.Prompt abstraction so: +// +// - FancyMode: huh.Select renders a styled list with arrow-key +// navigation. +// - PlainMode: a numbered list rendered to `out` with a +// single-line read from `in`. +// +// Parameters: +// - reg — Registry from DetectAll. +// - p — ui.Prompt (already mode-resolved by the caller via +// DecideMode). We deliberately DON'T construct one ourselves so +// tests can inject a mock or a bytes-backed reader. +// - nonInteractive — when true, ResolveInteractive returns +// ErrInteractiveRequired immediately instead of calling the +// prompt. This is the "don't hang on stdin" path for CI +// scripts that forgot to set --manager. +// - in, out — I/O streams. Mainly used for fallback when p is +// nil (we degrade to a plain prompt without a ui.Prompt). +// +// Returns the picked Manager and any I/O / parse errors. If only +// one manager is detected, returns it without prompting — there's +// no decision to make. +// +// If the user pressed Ctrl-C / aborts the prompt, returns +// ErrInteractiveRequired so the caller can decide between +// "use the first detected manager" and "abort with a hint". +func ResolveInteractive(reg Registry, p ui.Prompt, nonInteractive bool, in io.Reader, out io.Writer) (Manager, error) { + switch len(reg.Found) { + case 0: + return nil, ErrNoManager + case 1: + // Single manager — no decision to make. + return reg.Found[0], nil + } + + if nonInteractive { + // Caller is in a non-interactive run (CI, --yes, --json). + // Don't hang on stdin; surface the requirement so the CLI + // can print "use --manager" instead. + return nil, fmt.Errorf("%w (multiple managers detected: %s; pass --manager or set NODEUP_MANAGER)", + ErrInteractiveRequired, managerNames(reg.Found)) + } + + // Build the option list in priority order (DetectAll already + // sorted by Priority, but we re-sort defensively in case a + // caller constructs the Registry by hand). + sorted := make([]Manager, len(reg.Found)) + copy(sorted, reg.Found) + sort.SliceStable(sorted, func(i, j int) bool { + return Priority(sorted[i].Name()) < Priority(sorted[j].Name()) + }) + options := make([]string, len(sorted)) + for i, m := range sorted { + options[i] = m.Name() + } + + var picked string + var err error + if p != nil { + picked, err = p.Select("Multiple Node.js version managers detected. Pick one:", options, options[0]) + } else { + // Fallback path: caller passed nil for p. Build a fresh + // plain prompt against the streams they gave us. This is + // the path tests use to avoid constructing a whole + // ui.Prompt just for one question. + pp := ui.NewPrompt(ui.PlainMode, in, out) + picked, err = pp.Select("Multiple Node.js version managers detected. Pick one:", options, options[0]) + } + if err != nil { + return nil, fmt.Errorf("manager prompt: %w", err) + } + + // Defensive — ui.Select should always return one of the + // options or the default, but if it doesn't, error rather + // than return a half-picked manager. + for _, m := range sorted { + if m.Name() == picked { + return m, nil + } + } + return nil, fmt.Errorf("manager prompt returned unrecognized option %q", picked) +} diff --git a/internal/detector/interactive_test.go b/internal/detector/interactive_test.go new file mode 100644 index 0000000..a0eec6b --- /dev/null +++ b/internal/detector/interactive_test.go @@ -0,0 +1,211 @@ +package detector + +import ( + "bytes" + "context" + "errors" + "fmt" + "strings" + "testing" + + "github.com/Masterminds/semver/v3" + + "github.com/dipto0321/nodeup/internal/ui" +) + +// stubMgr is a tiny Manager implementation for tests that +// exercise the registry / picker. Methods return zero values +// except Name and Detect, which the picker relies on. +type stubMgr struct { + name string + pri int +} + +func (s *stubMgr) Name() string { return s.name } +func (s *stubMgr) Detect() bool { return true } +func (s *stubMgr) Version() (string, error) { return "0.0.0", nil } +func (s *stubMgr) ListInstalled(context.Context) ([]semver.Version, error) { + return nil, nil +} +func (s *stubMgr) Install(semver.Version) error { return nil } +func (s *stubMgr) Uninstall(semver.Version) error { return nil } +func (s *stubMgr) Use(semver.Version) error { return nil } +func (s *stubMgr) SetDefault(semver.Version) error { return nil } +func (s *stubMgr) GlobalNpmPrefix(semver.Version) (string, error) { return "", nil } +func (s *stubMgr) Current(context.Context) (semver.Version, error) { return semver.Version{}, nil } + +// stubPrompt implements ui.Prompt with hand-coded answers. The +// "answers" channel is fed by the test before each call. +type stubPrompt struct { + answers []string + calls int +} + +func (s *stubPrompt) Mode() ui.Mode { return ui.PlainMode } +func (s *stubPrompt) Confirm(string, bool) (bool, error) { + return false, errors.New("Confirm not expected in ResolveInteractive tests") +} +func (s *stubPrompt) Select(_ string, options []string, _ string) (string, error) { + if s.calls >= len(s.answers) { + return "", fmt.Errorf("stubPrompt: out of answers (call %d)", s.calls) + } + got := s.answers[s.calls] + s.calls++ + for _, o := range options { + if o == got { + return got, nil + } + } + return "", fmt.Errorf("stubPrompt: answer %q not in options %v", got, options) +} + +// --- ResolveInteractive ------------------------------------------------- + +func TestResolveInteractive_NoManagers(t *testing.T) { + got, err := ResolveInteractive(Registry{}, nil, false, nil, nil) + if !errors.Is(err, ErrNoManager) { + t.Errorf("err = %v, want ErrNoManager", err) + } + if got != nil { + t.Errorf("got %v, want nil", got) + } +} + +func TestResolveInteractive_SingleManagerSkipsPrompt(t *testing.T) { + p := &stubPrompt{answers: []string{"unused"}} + reg := Registry{Found: []Manager{&stubMgr{name: "fnm", pri: 0}}} + got, err := ResolveInteractive(reg, p, false, nil, nil) + if err != nil { + t.Fatalf("err = %v", err) + } + if got == nil || got.Name() != "fnm" { + t.Errorf("got %v, want fnm", got) + } + if p.calls != 0 { + t.Errorf("single-manager path called prompt %d times, want 0", p.calls) + } +} + +func TestResolveInteractive_NonInteractiveErrors(t *testing.T) { + reg := Registry{Found: []Manager{ + &stubMgr{name: "fnm", pri: 0}, + &stubMgr{name: "nvm", pri: 1}, + }} + got, err := ResolveInteractive(reg, nil, true, nil, nil) + if !errors.Is(err, ErrInteractiveRequired) { + t.Errorf("err = %v, want ErrInteractiveRequired", err) + } + if got != nil { + t.Errorf("got %v, want nil", got) + } +} + +func TestResolveInteractive_PicksFromPrompt(t *testing.T) { + reg := Registry{Found: []Manager{ + &stubMgr{name: "fnm", pri: 0}, + &stubMgr{name: "nvm", pri: 1}, + &stubMgr{name: "volta", pri: 2}, + }} + // User picks "nvm" (second option). + p := &stubPrompt{answers: []string{"nvm"}} + got, err := ResolveInteractive(reg, p, false, nil, nil) + if err != nil { + t.Fatalf("err = %v", err) + } + if got == nil || got.Name() != "nvm" { + t.Errorf("got %v, want nvm", got) + } +} + +// TestResolveInteractive_FallbackPlainPrompt pins the "caller +// passed nil for p" path: we build a fresh plain ui.Prompt +// against the supplied in/out streams and use that. This is what +// upgrade.go / check.go actually do — they don't carry a +// long-lived ui.Prompt around, they pass the cmd streams +// directly. The fallback exists for tests that want to drive +// the picker without constructing a stub. +func TestResolveInteractive_FallbackPlainPrompt(t *testing.T) { + reg := Registry{Found: []Manager{ + &stubMgr{name: "fnm", pri: 0}, + &stubMgr{name: "nvm", pri: 1}, + }} + // User types "2" for nvm. + var out bytes.Buffer + got, err := ResolveInteractive(reg, nil, false, strings.NewReader("2\n"), &out) + if err != nil { + t.Fatalf("err = %v", err) + } + if got == nil || got.Name() != "nvm" { + t.Errorf("got %v, want nvm (out: %s)", got, out.String()) + } +} + +// TestResolveInteractive_FallbackEOF pins the EOF behavior of +// the fallback path: closed stdin falls back to options[0] +// (per ui.PlainPrompt.Select contract) — so ResolveInteractive +// returns the first manager rather than erroring out. This is +// the same "piped script ran out of input" path Confirm covers. +func TestResolveInteractive_FallbackEOF(t *testing.T) { + reg := Registry{Found: []Manager{ + &stubMgr{name: "fnm", pri: 0}, + &stubMgr{name: "nvm", pri: 1}, + }} + got, err := ResolveInteractive(reg, nil, false, strings.NewReader(""), &bytes.Buffer{}) + if err != nil { + t.Fatalf("err = %v", err) + } + if got == nil || got.Name() != "fnm" { + t.Errorf("got %v, want fnm (EOF → first)", got) + } +} + +// --- ResolveManagerAuto ------------------------------------------------- + +func TestResolveManagerAuto_PreferredWins(t *testing.T) { + reg := Registry{Found: []Manager{ + &stubMgr{name: "fnm", pri: 0}, + &stubMgr{name: "nvm", pri: 1}, + }} + got, err := ResolveManagerAuto(reg, "nvm") + if err != nil { + t.Fatalf("err = %v", err) + } + if got.Name() != "nvm" { + t.Errorf("got %v, want nvm", got) + } +} + +func TestResolveManagerAuto_MultiWithoutPreferenceReturnsErrInteractiveRequired(t *testing.T) { + reg := Registry{Found: []Manager{ + &stubMgr{name: "fnm", pri: 0}, + &stubMgr{name: "nvm", pri: 1}, + }} + got, err := ResolveManagerAuto(reg, "") + if !errors.Is(err, ErrInteractiveRequired) { + t.Errorf("err = %v, want ErrInteractiveRequired", err) + } + if got != nil { + t.Errorf("got %v, want nil", got) + } +} + +func TestResolveManagerAuto_NoManagersReturnsErrNoManager(t *testing.T) { + got, err := ResolveManagerAuto(Registry{}, "") + if !errors.Is(err, ErrNoManager) { + t.Errorf("err = %v, want ErrNoManager", err) + } + if got != nil { + t.Errorf("got %v, want nil", got) + } +} + +func TestResolveManagerAuto_SingleManagerNoPrompt(t *testing.T) { + reg := Registry{Found: []Manager{&stubMgr{name: "fnm", pri: 0}}} + got, err := ResolveManagerAuto(reg, "") + if err != nil { + t.Fatalf("err = %v", err) + } + if got.Name() != "fnm" { + t.Errorf("got %v, want fnm", got) + } +} diff --git a/internal/ui/prompt.go b/internal/ui/prompt.go new file mode 100644 index 0000000..6139ce9 --- /dev/null +++ b/internal/ui/prompt.go @@ -0,0 +1,307 @@ +package ui + +import ( + "errors" + "fmt" + "io" + + "github.com/charmbracelet/huh" +) + +// Prompt is the surface for interactive user prompts. The two +// implementations are PlainPrompt (no huh, no ANSI — backs onto +// bufio on the supplied reader) and FancyPrompt (huh.Form with +// lipgloss styling). +// +// Per CLAUDE.md / #105 constraints: +// - Honor PlainMode: no huh.Form, no ANSI, no TTY probing. +// - internal/cli never imports huh directly — only this package. +// - Prompts MUST degrade to a sane default on EOF (closed stdin), +// so a piped `echo y | nodeup upgrade` works. +// +// Construction: +// +// ui.NewPrompt(ui.DecideMode(noColor), in, out) +// +// Callers request a prompt through the methods (Confirm / Select), +// not by constructing the underlying huh.Form. That keeps the +// call sites mode-agnostic: the same code path renders differently +// in PlainMode vs FancyMode. +type Prompt interface { + // Confirm asks the user a yes/no question. Returns true on + // affirmative ("y" / "yes" / "Y" / "YES"); false on empty input, + // "n", or anything else. EOF (closed stdin) returns (false, nil) + // so a piped script with no input defaults to "no" — the safe + // direction for cleanup / overwrite-style prompts. + Confirm(question string, defaultYes bool) (bool, error) + + // Select asks the user to pick one option from a list. Returns + // the selected option's label. EOF returns an error so the caller + // can decide (most call sites should treat EOF as "use the first + // option" or "skip the operation"). + Select(prompt string, options []string, defaultLabel string) (string, error) + + // Mode returns the prompt's render mode. + Mode() Mode +} + +// NewPrompt constructs a Prompt for the requested mode. PlainMode +// returns a PlainPrompt backed by `in` (a *bufio.Reader-shaped +// source — accepts any io.Reader but stores the concrete interface) +// and `out`; FancyMode returns a FancyPrompt that builds a huh.Form +// per call. +// +// We deliberately don't try to detect "huh ran but bubbletea can't +// initialize a real renderer" here — DecideMode already gated that. +// If DecideMode returned FancyMode, both stdout and stdin are TTYs, +// so huh's renderer will produce real output. +func NewPrompt(mode Mode, in io.Reader, out io.Writer) Prompt { + if mode == FancyMode && in != nil && out != nil { + return &fancyPrompt{in: in, out: out} + } + // Plain mode (or any nil stream in fancy mode — degrade to plain). + return &plainPrompt{in: in, out: out} +} + +// --- Plain prompt ------------------------------------------------------ + +// plainPrompt implements Prompt using simple line reads against `in` +// and writes prompts to `out`. The question is rendered as +// " [Y/n] " or " [y/N] " depending on defaultYes, +// and the answer is matched case-insensitively against "y" / "yes" +// (affirmative) or "n" / "no" / "" (negative, when defaultYes is true). +// +// EOF (closed stdin) is treated as the default answer so piped +// `echo "" | nodeup upgrade` keeps working without an explicit +// fallback in every call site. +type plainPrompt struct { + in io.Reader + out io.Writer +} + +func (p *plainPrompt) Mode() Mode { return PlainMode } + +// readLine is a small EOF-tolerant line reader. We use bufio.Scanner +// under the hood — the input is always one human-typed line so the +// 64KiB token-length cap is irrelevant. +func (p *plainPrompt) readLine() (line string, ok bool, err error) { + // Wrap `in` lazily so callers that pass a *bufio.Reader + // directly don't double-buffer. io.Reader is the public surface + // so callers don't have to know. + type lineReader interface { + ReadString(delim byte) (string, error) + } + type scanner interface { + Scan() bool + Text() string + Err() error + } + switch r := p.in.(type) { + case lineReader: + s, rerr := r.ReadString('\n') + if rerr != nil && rerr != io.EOF { + return "", false, rerr + } + return trimTrailingNewline(s), true, nil + case scanner: + if !r.Scan() { + if r.Err() != nil { + return "", false, r.Err() + } + return "", false, nil // EOF cleanly + } + return r.Text(), true, nil + default: + // Fallback: one byte at a time. Avoids pulling in bufio + // for callers that already wrapped their own reader. + var out []byte + buf := make([]byte, 1) + for { + n, rerr := r.Read(buf) + if n > 0 { + if buf[0] == '\n' { + return trimTrailingNewline(string(out)), true, nil + } + out = append(out, buf[0]) + } + if rerr != nil { + if rerr == io.EOF { + return trimTrailingNewline(string(out)), true, nil + } + return "", false, rerr + } + } + } +} + +func trimTrailingNewline(s string) string { + for s != "" && (s[len(s)-1] == '\n' || s[len(s)-1] == '\r') { + s = s[:len(s)-1] + } + return s +} + +func (p *plainPrompt) Confirm(question string, defaultYes bool) (bool, error) { + if p.out != nil { + if defaultYes { + _, _ = fmt.Fprintf(p.out, "%s [Y/n] ", question) + } else { + _, _ = fmt.Fprintf(p.out, "%s [y/N] ", question) + } + } + line, ok, err := p.readLine() + if err != nil { + return false, err + } + if !ok { + // EOF — fall back to default. This is the safe direction + // for confirm-style prompts: a piped `echo "" | nodeup` + // without -y should NOT mass-delete. + return defaultYes, nil + } + switch normalizeAffirmative(line) { + case "y", "yes": + return true, nil + case "n", "no": + return false, nil + default: + return defaultYes, nil + } +} + +func (p *plainPrompt) Select(prompt string, options []string, defaultLabel string) (string, error) { + if len(options) == 0 { + return "", errors.New("ui: Select called with no options") + } + if p.out != nil { + _, _ = fmt.Fprintf(p.out, "%s\n", prompt) + for i, o := range options { + if o == defaultLabel { + _, _ = fmt.Fprintf(p.out, " %d) %s [default]\n", i+1, o) + } else { + _, _ = fmt.Fprintf(p.out, " %d) %s\n", i+1, o) + } + } + } + line, ok, err := p.readLine() + if err != nil { + return "", err + } + if !ok || line == "" { + // EOF or empty input → use default. If defaultLabel is empty, + // fall back to options[0]. + if defaultLabel != "" { + return defaultLabel, nil + } + return options[0], nil + } + // Try numeric selection first ("1", "2", ...). + var idx int + if _, perr := fmt.Sscanf(line, "%d", &idx); perr == nil { + if idx >= 1 && idx <= len(options) { + return options[idx-1], nil + } + } + // Otherwise, accept exact label match (case-insensitive). + for _, o := range options { + if normalizeAffirmative(o) == normalizeAffirmative(line) { + return o, nil + } + } + // Unrecognized — fall back to default (or options[0]). + if defaultLabel != "" { + return defaultLabel, nil + } + return options[0], nil +} + +// normalizeAffirmative lowercases ASCII letters and trims spaces +// without dragging in `strings`. Used by both Confirm and Select +// for case-insensitive input matching. +func normalizeAffirmative(s string) string { + out := make([]byte, 0, len(s)) + for i := 0; i < len(s); i++ { + c := s[i] + if c >= 'A' && c <= 'Z' { + c += 'a' - 'A' + } + out = append(out, c) + } + start, end := 0, len(out) + for start < end && (out[start] == ' ' || out[start] == '\t') { + start++ + } + for end > start && (out[end-1] == ' ' || out[end-1] == '\t') { + end-- + } + return string(out[start:end]) +} + +// --- Fancy prompt ------------------------------------------------------ + +// fancyPrompt implements Prompt using huh.Form. We construct the +// form per-call (rather than caching) so each prompt is a fresh +// bubbletea program — this matches the lifecycle of a single question +// and avoids leaking state across prompts. +type fancyPrompt struct { + in io.Reader + out io.Writer +} + +func (p *fancyPrompt) Mode() Mode { return FancyMode } + +func (p *fancyPrompt) Confirm(question string, defaultYes bool) (bool, error) { + // Pre-fill the value so huh's Confirm starts on the right side. + // If the user accepts the default (presses Enter), `got` keeps + // this value and we return it. Otherwise huh updates `got` based + // on what the user toggled. + got := defaultYes + form := huh.NewForm( + huh.NewGroup( + huh.NewConfirm(). + Title(question). + Value(&got), + ), + ).WithInput(p.in).WithOutput(p.out) + if err := form.Run(); err != nil { + if errors.Is(err, huh.ErrUserAborted) { + return defaultYes, nil + } + return false, fmt.Errorf("prompt: %w", err) + } + return got, nil +} + +func (p *fancyPrompt) Select(prompt string, options []string, defaultLabel string) (string, error) { + if len(options) == 0 { + return "", errors.New("ui: Select called with no options") + } + huhOptions := make([]huh.Option[string], len(options)) + for i, o := range options { + huhOptions[i] = huh.NewOption(o, o) + } + var got string + if defaultLabel != "" { + got = defaultLabel + } else { + got = options[0] + } + form := huh.NewForm( + huh.NewGroup( + huh.NewSelect[string](). + Title(prompt). + Options(huhOptions...). + Value(&got), + ), + ).WithInput(p.in).WithOutput(p.out) + if err := form.Run(); err != nil { + if errors.Is(err, huh.ErrUserAborted) { + if defaultLabel != "" { + return defaultLabel, nil + } + return options[0], nil + } + return "", fmt.Errorf("prompt: %w", err) + } + return got, nil +} diff --git a/internal/ui/prompt_test.go b/internal/ui/prompt_test.go new file mode 100644 index 0000000..eb42322 --- /dev/null +++ b/internal/ui/prompt_test.go @@ -0,0 +1,258 @@ +package ui + +import ( + "bufio" + "bytes" + "errors" + "io" + "strings" + "testing" +) + +// TestPlainPrompt_Confirm_Y answers "y" → true. Pins the +// happy-path affirmative parsing. +func TestPlainPrompt_Confirm_Y(t *testing.T) { + p := NewPrompt(PlainMode, strings.NewReader("y\n"), &bytes.Buffer{}) + got, err := p.Confirm("Delete?", false) + if err != nil { + t.Fatalf("err = %v", err) + } + if !got { + t.Errorf("Confirm(\"y\") = false, want true") + } +} + +// TestPlainPrompt_Confirm_N answers "n" → false. +func TestPlainPrompt_Confirm_N(t *testing.T) { + p := NewPrompt(PlainMode, strings.NewReader("n\n"), &bytes.Buffer{}) + got, err := p.Confirm("Delete?", true) + if err != nil { + t.Fatalf("err = %v", err) + } + if got { + t.Errorf("Confirm(\"n\") = true, want false") + } +} + +// TestPlainPrompt_Confirm_EmptyUsesDefault pins that empty input +// falls back to defaultYes. This is the EOF-equivalent path that +// piped `echo "" | nodeup upgrade` exercises. +func TestPlainPrompt_Confirm_EmptyUsesDefault(t *testing.T) { + t.Run("default true", func(t *testing.T) { + p := NewPrompt(PlainMode, strings.NewReader("\n"), &bytes.Buffer{}) + got, err := p.Confirm("Delete?", true) + if err != nil { + t.Fatalf("err = %v", err) + } + if !got { + t.Errorf("empty input + defaultYes=true → false, want true") + } + }) + t.Run("default false", func(t *testing.T) { + p := NewPrompt(PlainMode, strings.NewReader("\n"), &bytes.Buffer{}) + got, err := p.Confirm("Delete?", false) + if err != nil { + t.Fatalf("err = %v", err) + } + if got { + t.Errorf("empty input + defaultYes=false → true, want false") + } + }) +} + +// TestPlainPrompt_Confirm_EOF pins that a closed stdin (no input +// at all) returns the default answer with no error. This is the +// critical "piped script that ran out of input" path: nodeup +// must not block on the prompt and must not return an error that +// would crash the upgrade flow. +func TestPlainPrompt_Confirm_EOF(t *testing.T) { + p := NewPrompt(PlainMode, strings.NewReader(""), &bytes.Buffer{}) + got, err := p.Confirm("Delete?", false) + if err != nil { + t.Errorf("EOF Confirm err = %v, want nil", err) + } + if got { + t.Errorf("EOF Confirm → true, want default (false)") + } +} + +// TestPlainPrompt_Confirm_YesVariants pins case-insensitive +// parsing: "Y", "YES", "yEs" should all be affirmative. Critical +// for muscle-memory users. +func TestPlainPrompt_Confirm_YesVariants(t *testing.T) { + for _, in := range []string{"Y\n", "yes\n", "YES\n", "yEs\n"} { + t.Run(in, func(t *testing.T) { + p := NewPrompt(PlainMode, strings.NewReader(in), &bytes.Buffer{}) + got, err := p.Confirm("?", false) + if err != nil { + t.Fatalf("err = %v", err) + } + if !got { + t.Errorf("Confirm(%q) = false, want true", in) + } + }) + } +} + +// TestPlainPrompt_Confirm_RendersHint pins that the prompt prints +// a "[y/N]"-style hint so users know which default applies. The +// exact wording varies by default; we just check both branches +// have the right marker. +func TestPlainPrompt_Confirm_RendersHint(t *testing.T) { + t.Run("default no", func(t *testing.T) { + var out bytes.Buffer + p := NewPrompt(PlainMode, strings.NewReader("\n"), &out) + _, _ = p.Confirm("Delete?", false) + if !strings.Contains(out.String(), "[y/N]") { + t.Errorf("defaultNo hint missing, got %q", out.String()) + } + }) + t.Run("default yes", func(t *testing.T) { + var out bytes.Buffer + p := NewPrompt(PlainMode, strings.NewReader("\n"), &out) + _, _ = p.Confirm("Delete?", true) + if !strings.Contains(out.String(), "[Y/n]") { + t.Errorf("defaultYes hint missing, got %q", out.String()) + } + }) +} + +// TestPlainPrompt_Select_Numeric covers the dominant path: the +// user types a number from the printed list. +func TestPlainPrompt_Select_Numeric(t *testing.T) { + p := NewPrompt(PlainMode, strings.NewReader("2\n"), &bytes.Buffer{}) + got, err := p.Select("Pick one:", []string{"fnm", "nvm", "volta"}, "fnm") + if err != nil { + t.Fatalf("err = %v", err) + } + if got != "nvm" { + t.Errorf("Select(\"2\") = %q, want \"nvm\"", got) + } +} + +// TestPlainPrompt_Select_EmptyUsesDefault pins that empty input +// falls back to defaultLabel. EOF equivalent. +func TestPlainPrompt_Select_EmptyUsesDefault(t *testing.T) { + t.Run("with default", func(t *testing.T) { + p := NewPrompt(PlainMode, strings.NewReader("\n"), &bytes.Buffer{}) + got, err := p.Select("?", []string{"a", "b"}, "b") + if err != nil { + t.Fatalf("err = %v", err) + } + if got != "b" { + t.Errorf("empty input → %q, want default \"b\"", got) + } + }) + t.Run("no default → first option", func(t *testing.T) { + p := NewPrompt(PlainMode, strings.NewReader("\n"), &bytes.Buffer{}) + got, err := p.Select("?", []string{"a", "b"}, "") + if err != nil { + t.Fatalf("err = %v", err) + } + if got != "a" { + t.Errorf("empty input + no default → %q, want first option \"a\"", got) + } + }) +} + +// TestPlainPrompt_Select_LabelMatch covers typing the label text +// directly rather than the numeric index. +func TestPlainPrompt_Select_LabelMatch(t *testing.T) { + p := NewPrompt(PlainMode, strings.NewReader("volta\n"), &bytes.Buffer{}) + got, err := p.Select("?", []string{"fnm", "nvm", "volta"}, "fnm") + if err != nil { + t.Fatalf("err = %v", err) + } + if got != "volta" { + t.Errorf("Select(\"volta\") = %q, want \"volta\"", got) + } +} + +// TestPlainPrompt_Select_EOF pins that closed stdin returns the +// default (or first option) — same contract as Confirm.EOF. +func TestPlainPrompt_Select_EOF(t *testing.T) { + t.Run("with default", func(t *testing.T) { + p := NewPrompt(PlainMode, strings.NewReader(""), &bytes.Buffer{}) + got, err := p.Select("?", []string{"a", "b"}, "b") + if err != nil { + t.Fatalf("err = %v", err) + } + if got != "b" { + t.Errorf("EOF Select → %q, want default \"b\"", got) + } + }) +} + +// TestPlainPrompt_Select_NoOptions pins that calling Select with +// no options returns an error rather than panicking. Defensive — +// callers should pre-validate but we don't want a panic in +// production if a future code path forgets. +func TestPlainPrompt_Select_NoOptions(t *testing.T) { + p := NewPrompt(PlainMode, strings.NewReader("\n"), &bytes.Buffer{}) + _, err := p.Select("?", nil, "") + if err == nil { + t.Errorf("Select(nil options) → nil err, want error") + } +} + +// TestPlainPrompt_Confirm_BufioReader pins that a *bufio.Reader +// (the typical cmd.InOrStdin() wrapping) is accepted without +// double-buffering. This is the path upgrade.go's old +// `bufio.NewReader(cmd.InOrStdin())` takes. +func TestPlainPrompt_Confirm_BufioReader(t *testing.T) { + in := bufio.NewReader(strings.NewReader("y\n")) + p := NewPrompt(PlainMode, in, &bytes.Buffer{}) + got, err := p.Confirm("?", false) + if err != nil { + t.Fatalf("err = %v", err) + } + if !got { + t.Errorf("bufio reader path: got %v, want true", got) + } +} + +// TestNewPrompt_FancyNilInDegradesToPlain pins that FancyMode + +// nil stream degrades to PlainMode. Same defensive branch as +// the spinner. +func TestNewPrompt_FancyNilInDegradesToPlain(t *testing.T) { + p := NewPrompt(FancyMode, nil, nil) + if p == nil { + t.Fatalf("NewPrompt returned nil for (FancyMode, nil, nil)") + } + if p.Mode() != PlainMode { + t.Errorf("nil streams under FancyMode → Mode() = %v, want PlainMode", p.Mode()) + } +} + +// errReader is a reader that always returns an error. Used to +// pin that Confirm/Select surface I/O errors rather than silently +// returning the default. +type errReader struct{ err error } + +func (e errReader) Read([]byte) (int, error) { return 0, e.err } + +// TestPlainPrompt_Confirm_ReaderErrorSurfaces pins that I/O +// errors from the underlying reader are propagated to the caller. +// Without this, a broken pipe on stdin would silently default to +// "no", masking real errors. +func TestPlainPrompt_Confirm_ReaderErrorSurfaces(t *testing.T) { + wantErr := errors.New("broken pipe") + p := NewPrompt(PlainMode, errReader{err: wantErr}, &bytes.Buffer{}) + _, err := p.Confirm("?", false) + if !errors.Is(err, wantErr) { + t.Errorf("Confirm err = %v, want %v", err, wantErr) + } +} + +// io.Discard writer doesn't error; we just confirm Confirm works +// with a discarded-output sink (some tests might pass it). +func TestPlainPrompt_Confirm_NilOutSafe(t *testing.T) { + p := NewPrompt(PlainMode, strings.NewReader("y\n"), io.Discard) + got, err := p.Confirm("?", false) + if err != nil { + t.Fatalf("err = %v", err) + } + if !got { + t.Errorf("Confirm with io.Discard: got false, want true") + } +}