refactor(detector): add context.Context to Manager.ListInstalled and Current

CHANGELOG.md

@@ -745,6 +745,38 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   action tag would silently pull unreviewed code into
   `release.yml` (which runs with `contents: write` and
   handles publish secrets). Closes #67.
+- `internal/detector` `Manager` interface now takes a
+  `context.Context` as the first parameter on `ListInstalled`
+  and `Current`. Both methods were defined without `ctx`
+  when the interface was first laid out, and the omission
+  became user-visible when the post-upgrade cleanup feature
+  (PR #56 / issue #41) added `Current()` and shipped across
+  all 8 manager implementations: a Ctrl-C mid-`nodeup
+  upgrade` would still wait for the underlying `manager list`
+  / `fnm list` / `nvm current` subprocess to finish before
+  returning, because cancellation had no path into the read
+  paths. The 8 concrete implementations (`fnm`, `nvm`,
+  `volta`, `asdf`, `mise`, `n`, `nodenv`, `nvm-windows`)
+  plumb the new `ctx` into their existing `runShell` /
+  `runScript` calls (both already ctx-aware via
+  `internal/platform/shell.go`). The 6 production call-sites
+  in `internal/cli/{check,list,upgrade,packages}.go` now
+  pass `cmd.Context()` (or, for the new
+  `getCurrentVersion(ctx, m)` helper, the caller's ctx) so
+  the `contextcheck` linter is satisfied. The 3 mock/stub
+  Manager types (`fakeManager`, `listTestStubManager`,
+  `stubManager`) take a discarded `_ context.Context`
+  parameter; their bodies are unchanged since they don't do
+  real I/O. The 39 test call-sites across the per-manager
+  `*_test.go` files now pass `t.Context()` (Go 1.24 added
+  this; the project's `go.mod` is on `go 1.24`). Out of scope
+  (per the issue title): the remaining `Manager` methods
+  (`Install`, `Uninstall`, `Use`, `SetDefault`,
+  `GlobalNpmPrefix`, `Version`) keep their no-ctx signatures
+  for now — most have a pre-existing cancellation story (the
+  platform lock + interrupted-upgrade sentinel from #46)
+  and a follow-up can plumb `ctx` through them when the
+  call-sites warrant it. Closes #53.
 
 ## [0.0.0] - 2024-07-01
 

internal/cli/check.go

@@ -118,7 +118,7 @@ func outputCheckJSON(cmd *cobra.Command, lts, current *node.ManifestVersion, ins
 
 	installedVersions := make([]string, 0)
 	for _, m := range installed.Found {
-		versions, err := m.ListInstalled()
+		versions, err := m.ListInstalled(cmd.Context())
 		if err != nil {
 			continue
 		}
@@ -153,7 +153,7 @@ func outputCheckTable(cmd *cobra.Command, lts, current *node.ManifestVersion, in
 	} else {
 		cmd.Println("Installed versions:")
 		for _, m := range installed.Found {
-			versions, err := m.ListInstalled()
+			versions, err := m.ListInstalled(cmd.Context())
 			if err != nil {
 				cmd.Printf("  - %s: [error listing versions]\n", m.Name())
 				continue

internal/cli/cleanup_test.go

@@ -3,6 +3,7 @@ package cli
 import (
 	"bufio"
 	"bytes"
+	"context"
 	"errors"
 	"strings"
 	"testing"
@@ -24,15 +25,15 @@ type stubManager struct {
 	currentErr error
 }
 
-func (s *stubManager) Name() string                                   { return s.name }
-func (s *stubManager) Detect() bool                                   { return true }
-func (s *stubManager) Version() (string, error)                       { return "0.0.0-test", nil }
-func (s *stubManager) ListInstalled() ([]semver.Version, error)       { return nil, nil }
-func (s *stubManager) Install(semver.Version) error                   { return nil }
-func (s *stubManager) Use(semver.Version) error                       { return nil }
-func (s *stubManager) SetDefault(semver.Version) error                { return nil }
-func (s *stubManager) GlobalNpmPrefix(semver.Version) (string, error) { return "", nil }
-func (s *stubManager) Current() (semver.Version, error) {
+func (s *stubManager) Name() string                                            { return s.name }
+func (s *stubManager) Detect() bool                                            { return true }
+func (s *stubManager) Version() (string, error)                                { return "0.0.0-test", nil }
+func (s *stubManager) ListInstalled(context.Context) ([]semver.Version, error) { return nil, nil }
+func (s *stubManager) Install(semver.Version) error                            { return nil }
+func (s *stubManager) Use(semver.Version) error                                { return nil }
+func (s *stubManager) SetDefault(semver.Version) error                         { return nil }
+func (s *stubManager) GlobalNpmPrefix(semver.Version) (string, error)          { return "", nil }
+func (s *stubManager) Current(context.Context) (semver.Version, error) {
 	if s.currentErr != nil {
 		return semver.Version{}, s.currentErr
 	}

internal/cli/list.go

@@ -96,7 +96,7 @@ func runList(cmd *cobra.Command, args []string) error {
 	// (one active Node per machine — typically one manager owns it).
 	var active *string
 	for _, m := range selected {
-		versions, err := m.ListInstalled()
+		versions, err := m.ListInstalled(cmd.Context())
 		entry := installedEntryJSON{Manager: m.Name()}
 		if err != nil {
 			entry.Error = err.Error()
@@ -115,7 +115,7 @@ func runList(cmd *cobra.Command, args []string) error {
 		// answers. This is best-effort: managers that don't implement
 		// Current() return an error, which we silently drop.
 		if active == nil {
-			if cur, cerr := m.Current(); cerr == nil {
+			if cur, cerr := m.Current(cmd.Context()); cerr == nil {
 				s := cur.String()
 				active = &s
 			}

internal/cli/list_test.go

@@ -2,6 +2,7 @@ package cli
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"strings"
 	"testing"
@@ -50,14 +51,14 @@ func (s *listTestStubManager) SetDefault(semver.Version) error                {
 func (s *listTestStubManager) GlobalNpmPrefix(semver.Version) (string, error) { return "", nil }
 func (s *listTestStubManager) Uninstall(semver.Version) error                 { return nil }
 
-func (s *listTestStubManager) ListInstalled() ([]semver.Version, error) {
+func (s *listTestStubManager) ListInstalled(_ context.Context) ([]semver.Version, error) {
 	if s.installedErr != nil {
 		return nil, s.installedErr
 	}
 	return s.installed, nil
 }
 
-func (s *listTestStubManager) Current() (semver.Version, error) {
+func (s *listTestStubManager) Current(_ context.Context) (semver.Version, error) {
 	if s.currentErr != nil {
 		return semver.Version{}, s.currentErr
 	}

internal/cli/packages.go

@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"context"
 	"fmt"
 	"strings"
 
@@ -47,7 +48,7 @@ func runSnapshot(cmd *cobra.Command, args []string) error {
 	}
 
 	m := installed.Found[0]
-	version, err := getCurrentVersion(m)
+	version, err := getCurrentVersion(cmd.Context(), m)
 	if err != nil {
 		return fmt.Errorf("get current version: %w", err)
 	}
@@ -60,8 +61,8 @@ func runSnapshot(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
-func getCurrentVersion(m detector.Manager) (semver.Version, error) {
-	versions, err := m.ListInstalled()
+func getCurrentVersion(ctx context.Context, m detector.Manager) (semver.Version, error) {
+	versions, err := m.ListInstalled(ctx)
 	if err != nil {
 		return semver.Version{}, err
 	}

internal/cli/upgrade.go

@@ -166,7 +166,7 @@ func runUpgrade(cmd *cobra.Command, args []string) error {
 	}
 
 	// Get installed versions
-	installedVersions, err := m.ListInstalled()
+	installedVersions, err := m.ListInstalled(cmd.Context())
 	if err != nil {
 		return fmt.Errorf("list installed versions: %w", err)
 	}
@@ -331,7 +331,7 @@ func runUpgrade(cmd *cobra.Command, args []string) error {
 		// why the prompt is now per-version. See #58.
 		var active semver.Version
 		var currentErr error
-		if cur, cerr := m.Current(); cerr == nil {
+		if cur, cerr := m.Current(cmd.Context()); cerr == nil {
 			active = cur
 		} else {
 			currentErr = cerr

internal/detector/asdf.go

@@ -168,8 +168,8 @@ func parseASDFVersion(stdout string) (string, error) {
 //
 // ASDF does NOT have an nvm-style "system" sentinel — we don't filter
 // for one.
-func (a *ASDF) ListInstalled() ([]semver.Version, error) {
-	res, err := runShell(context.Background(), "asdf", "list", "nodejs")
+func (a *ASDF) ListInstalled(ctx context.Context) ([]semver.Version, error) {
+	res, err := runShell(ctx, "asdf", "list", "nodejs")
 	if err != nil {
 		return nil, fmt.Errorf("asdf list nodejs: %w", err)
 	}
@@ -333,8 +333,8 @@ func (a *ASDF) GlobalNpmPrefix(ver semver.Version) (string, error) {
 // We take the second token (the version column) and feed it to
 // semver.NewVersion. We treat "system" / "None" as errors so the
 // cleanup prompt doesn't try to exclude them.
-func (a *ASDF) Current() (semver.Version, error) {
-	res, err := runShell(context.Background(), "asdf", "current", "nodejs")
+func (a *ASDF) Current(ctx context.Context) (semver.Version, error) {
+	res, err := runShell(ctx, "asdf", "current", "nodejs")
 	if err != nil {
 		return semver.Version{}, fmt.Errorf("asdf current nodejs: %w", err)
 	}

internal/detector/asdf_test.go

@@ -264,7 +264,7 @@ func TestASDF_ListInstalled_Success(t *testing.T) {
 		},
 	)
 
-	got, err := NewASDF().ListInstalled()
+	got, err := NewASDF().ListInstalled(t.Context())
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -295,7 +295,7 @@ func TestASDF_ListInstalled_EmptyStdout(t *testing.T) {
 		return &platform.RunResult{Stdout: ""}, nil
 	})
 
-	got, err := NewASDF().ListInstalled()
+	got, err := NewASDF().ListInstalled(t.Context())
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -313,7 +313,7 @@ func TestASDF_ListInstalled_RunShellError(t *testing.T) {
 		return nil, wantErr
 	})
 
-	_, err := NewASDF().ListInstalled()
+	_, err := NewASDF().ListInstalled(t.Context())
 	if err == nil {
 		t.Fatal("expected error, got nil")
 	}
@@ -519,7 +519,7 @@ func TestASDFCurrent_InvokesShell(t *testing.T) {
 		},
 	)
 
-	got, err := NewASDF().Current()
+	got, err := NewASDF().Current(t.Context())
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}

internal/detector/detector.go

@@ -19,6 +19,7 @@
 package detector
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"sort"
@@ -47,8 +48,11 @@ type Manager interface {
 	Version() (string, error)
 
 	// ListInstalled returns every Node.js version the manager currently
-	// has installed, sorted ascending.
-	ListInstalled() ([]semver.Version, error)
+	// has installed, sorted ascending. Implementations should respect
+	// ctx cancellation so a Ctrl-C during `nodeup upgrade` propagates
+	// into the underlying `manager list` subprocess. The CLI layer
+	// passes cmd.Context().
+	ListInstalled(ctx context.Context) ([]semver.Version, error)
 
 	// Install downloads and installs the given Node.js version.
 	Install(v semver.Version) error
@@ -73,8 +77,9 @@ type Manager interface {
 	// manager has no way to query the active version (e.g.,
 	// nvm-windows — that one returns a sentinel "not implemented"
 	// error). Callers should treat errors as "active version unknown"
-	// and proceed without excluding it.
-	Current() (semver.Version, error)
+	// and proceed without excluding it. Implementations should respect
+	// ctx cancellation; the CLI layer passes cmd.Context().
+	Current(ctx context.Context) (semver.Version, error)
 }
 
 // Registry holds the list of managers nodeup knows about. It is built

internal/detector/fnm.go

@@ -101,8 +101,8 @@ func parseFNMVersion(stdout string) (string, error) {
 // it. The literal "system" line (which represents the system Node, not
 // an fnm-managed install) is excluded — nodeup only cares about versions
 // fnm actually manages.
-func (f *FNM) ListInstalled() ([]semver.Version, error) {
-	res, err := runShell(context.Background(), "fnm", "list")
+func (f *FNM) ListInstalled(ctx context.Context) ([]semver.Version, error) {
+	res, err := runShell(ctx, "fnm", "list")
 	if err != nil {
 		return nil, fmt.Errorf("fnm list: %w", err)
 	}
@@ -304,8 +304,8 @@ func fnmDir() string {
 // shells — i.e., `fnm current`. The output is a bare semver like
 // "v22.11.0" or "22.11.0"; fnm 1.39+ includes a "v" prefix, older
 // versions don't. We strip the prefix defensively.
-func (f *FNM) Current() (semver.Version, error) {
-	res, err := runShell(context.Background(), "fnm", "current")
+func (f *FNM) Current(ctx context.Context) (semver.Version, error) {
+	res, err := runShell(ctx, "fnm", "current")
 	if err != nil {
 		return semver.Version{}, fmt.Errorf("fnm current: %w", err)
 	}

internal/detector/fnm_test.go

@@ -217,7 +217,7 @@ func TestFNM_ListInstalled_Success(t *testing.T) {
 		},
 	)
 
-	got, err := NewFNM().ListInstalled()
+	got, err := NewFNM().ListInstalled(t.Context())
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -238,7 +238,7 @@ func TestFNM_ListInstalled_RunShellError(t *testing.T) {
 		return nil, wantErr
 	})
 
-	_, err := NewFNM().ListInstalled()
+	_, err := NewFNM().ListInstalled(t.Context())
 	if err == nil {
 		t.Fatal("expected error, got nil")
 	}
@@ -354,7 +354,7 @@ func TestFNMCurrent_InvokesShell(t *testing.T) {
 		},
 	)
 
-	got, err := NewFNM().Current()
+	got, err := NewFNM().Current(t.Context())
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}

internal/detector/mise.go

@@ -197,8 +197,8 @@ func parseMiseVersion(stdout string) (string, error) {
 //
 // Note: mise does not have an nvm-style "system" sentinel — we
 // don't filter for one.
-func (m *Mise) ListInstalled() ([]semver.Version, error) {
-	res, err := runShell(context.Background(), "mise", "ls", "--installed", "--json", "node")
+func (m *Mise) ListInstalled(ctx context.Context) ([]semver.Version, error) {
+	res, err := runShell(ctx, "mise", "ls", "--installed", "--json", "node")
 	if err != nil {
 		return nil, fmt.Errorf("mise ls --installed --json node: %w", err)
 	}
@@ -370,8 +370,8 @@ func miseDataDir() string {
 // Current returns the version Mise currently has active for the
 // user. Source: `mise current node`. The output is a bare semver
 // like "22.11.0" (with optional "v" prefix on some builds).
-func (m *Mise) Current() (semver.Version, error) {
-	res, err := runShell(context.Background(), "mise", "current", "node")
+func (m *Mise) Current(ctx context.Context) (semver.Version, error) {
+	res, err := runShell(ctx, "mise", "current", "node")
 	if err != nil {
 		return semver.Version{}, fmt.Errorf("mise current node: %w", err)
 	}

internal/detector/mise_test.go

@@ -368,7 +368,7 @@ func TestMise_ListInstalled_Success(t *testing.T) {
 		},
 	)
 
-	got, err := NewMise().ListInstalled()
+	got, err := NewMise().ListInstalled(t.Context())
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -399,7 +399,7 @@ func TestMise_ListInstalled_EmptyArray(t *testing.T) {
 		return &platform.RunResult{Stdout: "[]"}, nil
 	})
 
-	got, err := NewMise().ListInstalled()
+	got, err := NewMise().ListInstalled(t.Context())
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -418,7 +418,7 @@ func TestMise_ListInstalled_EmptyStdout(t *testing.T) {
 		return &platform.RunResult{Stdout: ""}, nil
 	})
 
-	got, err := NewMise().ListInstalled()
+	got, err := NewMise().ListInstalled(t.Context())
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -436,7 +436,7 @@ func TestMise_ListInstalled_RunShellError(t *testing.T) {
 		return nil, wantErr
 	})
 
-	_, err := NewMise().ListInstalled()
+	_, err := NewMise().ListInstalled(t.Context())
 	if err == nil {
 		t.Fatal("expected error, got nil")
 	}
@@ -451,7 +451,7 @@ func TestMise_ListInstalled_JSONParseError(t *testing.T) {
 		return &platform.RunResult{Stdout: "{not json"}, nil
 	})
 
-	_, err := NewMise().ListInstalled()
+	_, err := NewMise().ListInstalled(t.Context())
 	if err == nil {
 		t.Error("expected JSON parse error, got nil")
 	}
@@ -629,7 +629,7 @@ func TestMiseCurrent_InvokesShell(t *testing.T) {
 		},
 	)
 
-	got, err := NewMise().Current()
+	got, err := NewMise().Current(t.Context())
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}