fix(shim): discover install-time shims from disk, not static provider (#270)

Author: CalvinAllen

src/internal/shim/manager.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"sort"
 
 	"github.com/CodingWithCalvin/dtvem.cli/src/internal/config"
 	"github.com/CodingWithCalvin/dtvem.cli/src/internal/constants"
@@ -282,33 +283,16 @@ func (m *Manager) RehashWithCallback(callback RehashCallback) (*RehashResult, er
 			version := versionEntry.Name()
 			versionDir := filepath.Join(runtimeVersionsDir, version)
 
-			// Scan the directories where runtime and package executables
-			// live. Anything found is recorded as a shim provided by this
-			// version. We deliberately do NOT pre-populate the provider's
-			// declared core shims (e.g. python3, pip3) without checking
-			// the filesystem: an embeddable Windows install may ship only
-			// python.exe, and asserting that 3.8.9 "provides pip" when it
-			// doesn't would corrupt the version-coverage data the shim
-			// uses to give users an informed error.
-			if execs, err := findExecutables(filepath.Join(versionDir, "bin")); err == nil {
-				for _, exec := range execs {
-					recordShim(exec, version)
-				}
-			}
-
-			// On Windows, also check the root version directory for .cmd/.bat files
-			// and Scripts directory for Python packages
-			if runtime.GOOS == constants.OSWindows {
-				if execs, err := findExecutables(versionDir); err == nil {
-					for _, exec := range execs {
-						recordShim(exec, version)
-					}
-				}
-				if execs, err := findExecutables(filepath.Join(versionDir, "Scripts")); err == nil {
-					for _, exec := range execs {
-						recordShim(exec, version)
-					}
-				}
+			// Anything found on disk is recorded as a shim provided by
+			// this version. We deliberately do NOT pre-populate the
+			// provider's declared core shims (e.g. python3, pip3)
+			// without checking the filesystem: an embeddable Windows
+			// install may ship only python.exe, and asserting that
+			// 3.8.9 "provides pip" when it doesn't would corrupt the
+			// version-coverage data the shim uses to give users an
+			// informed error.
+			for _, exec := range DiscoverShimsForVersion(versionDir) {
+				recordShim(exec, version)
 			}
 		}
 	}
@@ -391,6 +375,50 @@ func RuntimeShims(runtimeName string) []string {
 	return provider.Shims()
 }
 
+// DiscoverShimsForVersion scans an installed runtime version's directory tree
+// and returns the executable base names that should be shimmed.
+//
+// versionDir is the absolute path to a single version's install directory
+// (e.g., ~/.dtvem/versions/python/3.14.2). The same directories `Rehash`
+// walks are scanned here: `bin/` always, plus the root and `Scripts/` on
+// Windows. Missing directories are not an error — they just contribute no
+// names — so callers can use this on any layout without pre-checking.
+//
+// The intersection-with-provider-declarations is deliberately omitted:
+// reshim registers every executable it finds (pip-installed tools, npm-
+// installed binaries, version-suffixed names like pip3.14), and install
+// should match that policy so reshim doesn't surprise the user by adding
+// or removing shims that the install command didn't.
+//
+// Returns names sorted alphabetically so callers get stable output across
+// runs (filesystem iteration order is not guaranteed).
+func DiscoverShimsForVersion(versionDir string) []string {
+	seen := make(map[string]struct{})
+	scan := func(dir string) {
+		execs, err := findExecutables(dir)
+		if err != nil {
+			return
+		}
+		for _, e := range execs {
+			seen[e] = struct{}{}
+		}
+	}
+
+	scan(filepath.Join(versionDir, "bin"))
+
+	if runtime.GOOS == constants.OSWindows {
+		scan(versionDir)
+		scan(filepath.Join(versionDir, "Scripts"))
+	}
+
+	out := make([]string, 0, len(seen))
+	for name := range seen {
+		out = append(out, name)
+	}
+	sort.Strings(out)
+	return out
+}
+
 // findExecutables scans a directory for executable files and returns their base names
 func findExecutables(dir string) ([]string, error) {
 	entries, err := os.ReadDir(dir)

src/internal/shim/manager_test.go

@@ -438,6 +438,147 @@ func TestListShims_SkipsCmdFiles(t *testing.T) {
 	}
 }
 
+// writeExecutable writes a file with the executable bit set on Unix. The
+// content is irrelevant — findExecutables only looks at extension (Windows)
+// or the exec bit (Unix) — but a non-zero body keeps file readers happy.
+func writeExecutable(t *testing.T, path string) {
+	t.Helper()
+	if err := os.WriteFile(path, []byte("x"), 0755); err != nil {
+		t.Fatalf("Failed to write %s: %v", path, err)
+	}
+}
+
+// platformExeName returns name with the OS-appropriate executable extension.
+// On Windows the .exe suffix is required for findExecutables to recognize
+// the file; on Unix the bare name is used and the exec bit drives detection.
+func platformExeName(name string) string {
+	if runtime.GOOS == constants.OSWindows {
+		return name + constants.ExtExe
+	}
+	return name
+}
+
+func TestDiscoverShimsForVersion_EmptyDir(t *testing.T) {
+	versionDir := t.TempDir()
+
+	got := DiscoverShimsForVersion(versionDir)
+	if len(got) != 0 {
+		t.Errorf("DiscoverShimsForVersion(empty) = %v, want []", got)
+	}
+}
+
+func TestDiscoverShimsForVersion_MissingVersionDir(t *testing.T) {
+	// Caller may pass a path that doesn't exist (e.g., when called before
+	// the install has moved files into place). The helper must not panic
+	// or surface an error — it just returns no names.
+	got := DiscoverShimsForVersion(filepath.Join(t.TempDir(), "does-not-exist"))
+	if len(got) != 0 {
+		t.Errorf("DiscoverShimsForVersion(missing) = %v, want []", got)
+	}
+}
+
+func TestDiscoverShimsForVersion_BinDir(t *testing.T) {
+	versionDir := t.TempDir()
+	binDir := filepath.Join(versionDir, "bin")
+	if err := os.MkdirAll(binDir, 0755); err != nil {
+		t.Fatalf("mkdir bin: %v", err)
+	}
+
+	writeExecutable(t, filepath.Join(binDir, platformExeName("node")))
+	writeExecutable(t, filepath.Join(binDir, platformExeName("npm")))
+
+	got := DiscoverShimsForVersion(versionDir)
+	want := []string{"node", "npm"}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("DiscoverShimsForVersion(bin) = %v, want %v", got, want)
+	}
+}
+
+// TestDiscoverShimsForVersion_PythonWindowsOnlyRoot models the python-build-
+// standalone Windows tarball before ensurepip runs: python.exe and
+// pythonw.exe live in the version root and Scripts/ is either absent or
+// only contains the upstream .empty placeholder. The discover helper must
+// not invent pip / python3 entries that don't exist on disk — that was
+// the root cause of issue #269 where install-time shim creation used a
+// static provider declaration instead of disk truth.
+func TestDiscoverShimsForVersion_PythonWindowsOnlyRoot(t *testing.T) {
+	if runtime.GOOS != constants.OSWindows {
+		t.Skip("Windows-specific layout (root .exe files)")
+	}
+
+	versionDir := t.TempDir()
+	writeExecutable(t, filepath.Join(versionDir, "python.exe"))
+	writeExecutable(t, filepath.Join(versionDir, "pythonw.exe"))
+	// Upstream ships an empty Scripts/.empty placeholder; recreate it
+	// here so the test reflects the exact layout users see and proves
+	// the helper does not surface the placeholder as a shim.
+	if err := os.MkdirAll(filepath.Join(versionDir, "Scripts"), 0755); err != nil {
+		t.Fatalf("mkdir Scripts: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(versionDir, "Scripts", ".empty"), nil, 0644); err != nil {
+		t.Fatalf("write .empty: %v", err)
+	}
+
+	got := DiscoverShimsForVersion(versionDir)
+	want := []string{"python", "pythonw"}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+// TestDiscoverShimsForVersion_PythonWindowsAfterEnsurepip models the
+// post-ensurepip state: root has python.exe/pythonw.exe, Scripts/ has
+// pip.exe, pip3.exe, and the versioned pip3.14.exe. All five should be
+// discovered and the result should be sorted+deduplicated.
+func TestDiscoverShimsForVersion_PythonWindowsAfterEnsurepip(t *testing.T) {
+	if runtime.GOOS != constants.OSWindows {
+		t.Skip("Windows-specific layout (Scripts/*.exe)")
+	}
+
+	versionDir := t.TempDir()
+	writeExecutable(t, filepath.Join(versionDir, "python.exe"))
+	writeExecutable(t, filepath.Join(versionDir, "pythonw.exe"))
+
+	scriptsDir := filepath.Join(versionDir, "Scripts")
+	if err := os.MkdirAll(scriptsDir, 0755); err != nil {
+		t.Fatalf("mkdir Scripts: %v", err)
+	}
+	writeExecutable(t, filepath.Join(scriptsDir, "pip.exe"))
+	writeExecutable(t, filepath.Join(scriptsDir, "pip3.exe"))
+	writeExecutable(t, filepath.Join(scriptsDir, "pip3.14.exe"))
+
+	got := DiscoverShimsForVersion(versionDir)
+	want := []string{"pip", "pip3", "pip3.14", "python", "pythonw"}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+// TestDiscoverShimsForVersion_DedupesAcrossDirs proves the helper unions
+// names across bin/ root/ and Scripts/ rather than double-counting. This
+// matters because some runtimes (e.g., Ruby on Windows) place the same
+// command name in multiple search locations.
+func TestDiscoverShimsForVersion_DedupesAcrossDirs(t *testing.T) {
+	if runtime.GOOS != constants.OSWindows {
+		t.Skip("Multi-dir Windows scan (root + Scripts)")
+	}
+
+	versionDir := t.TempDir()
+	writeExecutable(t, filepath.Join(versionDir, "tool.exe"))
+
+	scriptsDir := filepath.Join(versionDir, "Scripts")
+	if err := os.MkdirAll(scriptsDir, 0755); err != nil {
+		t.Fatalf("mkdir Scripts: %v", err)
+	}
+	writeExecutable(t, filepath.Join(scriptsDir, "tool.exe"))
+
+	got := DiscoverShimsForVersion(versionDir)
+	want := []string{"tool"}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
 func TestRuntimeShims_AllKnownRuntimes(t *testing.T) {
 	// Verify all known runtimes have shim mappings
 	knownRuntimes := []string{"python", "node", "ruby", "go"}

src/runtimes/node/provider_full.go

@@ -124,13 +124,21 @@ func (p *Provider) getDownloadURL(version string) (string, string, error) {
 // than falling back to the provider registry. The version is recorded in the
 // cache so the shim can detect when an active runtime version is one that
 // does not provide a given executable.
+//
+// The shim list is derived from disk (the same scan reshim uses), not from
+// the provider's static Shims() declaration, so install and reshim stay in
+// sync and only register executables that actually exist for this version.
 func (p *Provider) createShims(version string) error {
 	manager, err := shim.NewManager()
 	if err != nil {
 		return err
 	}
 
-	shimNames := shim.RuntimeShims("node")
+	versionDir := config.RuntimeVersionPath("node", version)
+	shimNames := shim.DiscoverShimsForVersion(versionDir)
+	if len(shimNames) == 0 {
+		return fmt.Errorf("no executables found in %s", versionDir)
+	}
 
 	return manager.CreateShimsForRuntime("node", version, shimNames)
 }

src/runtimes/python/provider_full.go

@@ -181,13 +181,26 @@ func (p *Provider) getDownloadURL(version string) (string, string, error) {
 // than falling back to the provider registry. The version is recorded in the
 // cache so the shim can detect when an active runtime version is one that
 // does not provide a given executable.
+//
+// The shim list is derived from disk (the same scan reshim uses), not from
+// the provider's static Shims() declaration. That keeps install and reshim
+// honest about which executables actually exist: on Windows the upstream
+// python-build-standalone tarball ships only python.exe / pythonw.exe — no
+// python3.exe, no pip.exe — so the static list would create phantom shims
+// (python3, pip3) that error at invocation time. Pip executables only
+// appear in Scripts/ after installPipIfNeeded succeeds, and scanning here
+// ensures they're picked up when present and silently skipped when not.
 func (p *Provider) createShims(version string) error {
 	manager, err := shim.NewManager()
 	if err != nil {
 		return err
 	}
 
-	shimNames := shim.RuntimeShims("python")
+	versionDir := config.RuntimeVersionPath("python", version)
+	shimNames := shim.DiscoverShimsForVersion(versionDir)
+	if len(shimNames) == 0 {
+		return fmt.Errorf("no executables found in %s", versionDir)
+	}
 
 	return manager.CreateShimsForRuntime("python", version, shimNames)
 }

src/runtimes/ruby/provider_full.go

@@ -197,13 +197,21 @@ func (p *Provider) getDownloadURL(version string) (string, string, error) {
 // than falling back to the provider registry. The version is recorded in the
 // cache so the shim can detect when an active runtime version is one that
 // does not provide a given executable.
+//
+// The shim list is derived from disk (the same scan reshim uses), not from
+// the provider's static Shims() declaration, so install and reshim stay in
+// sync and only register executables that actually exist for this version.
 func (p *Provider) createShims(version string) error {
 	manager, err := shim.NewManager()
 	if err != nil {
 		return err
 	}
 
-	shimNames := shim.RuntimeShims("ruby")
+	versionDir := config.RuntimeVersionPath("ruby", version)
+	shimNames := shim.DiscoverShimsForVersion(versionDir)
+	if len(shimNames) == 0 {
+		return fmt.Errorf("no executables found in %s", versionDir)
+	}
 
 	return manager.CreateShimsForRuntime("ruby", version, shimNames)
 }