fix(shim): prune orphan shims on reshim

src/cmd/reshim.go

@@ -19,6 +19,10 @@ var reshimCmd = &cobra.Command{
 	Long: `Regenerate shim binaries for all installed runtime versions.
 
 This command scans all installed runtimes and creates shims for their executables.
+It also removes orphan shims — leftover shims whose executable is no longer
+provided by any installed version, such as a pip or npm package that has since
+been uninstalled.
+
 Run this command after installing new versions or if shims become corrupted.
 
 Example:
@@ -91,6 +95,18 @@ Example:
 		fmt.Println(table.Render())
 		fmt.Println()
 		ui.Success("Created %d shims for %d runtime(s)", result.TotalShims, len(result.ShimsByRuntime))
+
+		if len(result.RemovedShims) > 0 {
+			ui.Info("Removed %d orphan shim(s): %s",
+				len(result.RemovedShims), strings.Join(result.RemovedShims, ", "))
+		}
+
+		// A shim that is running right now can't be deleted on Windows.
+		// Name each one so the user knows what to retry rather than
+		// seeing doctor keep reporting drift with no explanation.
+		for _, failure := range result.PruneFailures {
+			ui.Warning("Could not remove orphan shim '%s': %v", failure.Name, failure.Err)
+		}
 	},
 }
 

src/cmd/shim/main.go

@@ -40,7 +40,11 @@ func runShim() error {
 	ui.Debug("Arguments: %v", os.Args[1:])
 
 	// Determine which runtime this shim belongs to
-	runtimeName := mapShimToRuntime(shimName)
+	runtimeName, known := mapShimToRuntime(shimName)
+	if !known {
+		ui.Debug("No runtime claims shim: %s", shimName)
+		return handleUnknownShim(shimName)
+	}
 	ui.Debug("Mapped to runtime: %s", runtimeName)
 
 	// Get the runtime provider (using ShimProvider interface for minimal dependencies)
@@ -203,15 +207,24 @@ func shimNameFromPath(shimPath string) string {
 	return shimPath
 }
 
-// mapShimToRuntime maps a shim name to its runtime
-// For example: python3 -> python, pip -> python, npm -> node, tsc -> node
+// mapShimToRuntime maps a shim name to the runtime that owns it.
+// For example: python3 -> python, pip -> python, npm -> node, tsc -> node.
 // First checks the shim map cache (generated by reshim), then falls back
 // to querying registered providers.
-func mapShimToRuntime(shimName string) string {
+//
+// The boolean reports whether any runtime actually claims the name. An
+// unclaimed shim is a stale file — a package executable that was shimmed
+// and later uninstalled — and callers must not fall back to treating the
+// shim name as a runtime name: that produced the baffling
+// "runtime provider not found: runtime provider 'ruff' not found" that
+// users saw when invoking such a shim.
+func mapShimToRuntime(shimName string) (string, bool) {
 	// First, try the shim map cache for O(1) lookup
 	// This handles both core shims and dynamically installed packages (tsc, eslint, black, etc.)
-	if runtimeName, ok := shim.LookupRuntime(shimName); ok {
-		return runtimeName
+	// A cache entry with no runtime recorded is treated as a miss so the
+	// provider scan below still gets a chance to claim the shim.
+	if runtimeName, ok := shim.LookupRuntime(shimName); ok && runtimeName != "" {
+		return runtimeName, true
 	}
 
 	// Fall back to provider-based lookup if cache is missing or doesn't have the shim
@@ -222,7 +235,7 @@ func mapShimToRuntime(shimName string) string {
 	for _, provider := range providers {
 		for _, s := range provider.Shims() {
 			if s == shimName {
-				return provider.Name()
+				return provider.Name(), true
 			}
 		}
 	}
@@ -231,13 +244,43 @@ func mapShimToRuntime(shimName string) string {
 	for _, provider := range providers {
 		for _, s := range provider.Shims() {
 			if strings.HasPrefix(shimName, s) {
-				return provider.Name()
+				return provider.Name(), true
 			}
 		}
 	}
 
-	// Default: use shim name as runtime name
-	return shimName
+	return "", false
+}
+
+// handleUnknownShim handles a shim binary that no runtime claims.
+//
+// This happens when a shim outlives the executable it was created for —
+// a pip- or npm-installed tool that was later uninstalled, or a runtime
+// version that was removed — and `dtvem reshim` has not run since. (As of
+// the orphan-pruning change in reshim, running it clears these out; the
+// window is between the uninstall and the next reshim.)
+//
+// Handing off to the real command deeper in PATH keeps the user working:
+// the stale shim sits ahead of the system install and would otherwise
+// shadow it. The warning goes to stderr so stdout stays clean for
+// piping, and it names the fix so the state is visible rather than
+// silently permanent.
+func handleUnknownShim(shimName string) error {
+	systemPath := path.LookPathExcludingShims(shimName)
+	if systemPath == "" {
+		ui.Error("'%s' is not managed by dtvem and was not found in PATH", shimName)
+		ui.Info("This is a leftover dtvem shim. Run 'dtvem reshim' to remove it.")
+		return fmt.Errorf("no runtime provides %s", shimName)
+	}
+
+	ui.Warning("'%s' is a leftover dtvem shim; running %s instead", shimName, systemPath)
+	ui.Info("Run 'dtvem reshim' to remove it.")
+	fmt.Fprintln(os.Stderr) // Empty line for spacing
+
+	if err := executeCommand(systemPath, os.Args[1:], nil); err != nil {
+		return fmt.Errorf("failed to execute %s: %w", systemPath, err)
+	}
+	return nil
 }
 
 // secondaryExecutableError formats a user-facing error explaining that a

src/cmd/shim/main_test.go

@@ -1,6 +1,11 @@
 package main
 
-import "testing"
+import (
+	"testing"
+
+	"github.com/CodingWithCalvin/dtvem.cli/src/internal/config"
+	"github.com/CodingWithCalvin/dtvem.cli/src/internal/shim"
+)
 
 func TestShimNameFromPath(t *testing.T) {
 	tests := []struct {
@@ -54,3 +59,116 @@ func TestShimNameFromPath(t *testing.T) {
 		})
 	}
 }
+
+// isolateShimEnv points dtvem at an empty temp root so shim resolution
+// runs against the registered providers rather than whatever shim-map
+// cache happens to exist on the machine running the tests. Both the
+// paths and the shim map memoize on first use, so each is dropped going
+// in and coming out.
+func isolateShimEnv(t *testing.T) {
+	t.Helper()
+
+	tmpRoot := t.TempDir()
+	t.Setenv("HOME", tmpRoot)
+	t.Setenv("USERPROFILE", tmpRoot)
+	t.Setenv("DTVEM_ROOT", tmpRoot)
+
+	config.ResetPathsCache()
+	t.Cleanup(config.ResetPathsCache)
+	shim.ResetShimMapCache()
+	t.Cleanup(shim.ResetShimMapCache)
+}
+
+func TestMapShimToRuntime(t *testing.T) {
+	isolateShimEnv(t)
+
+	tests := []struct {
+		name        string
+		shimName    string
+		wantRuntime string
+		wantKnown   bool
+	}{
+		{
+			name:        "runtime name itself",
+			shimName:    "python",
+			wantRuntime: "python",
+			wantKnown:   true,
+		},
+		{
+			name:        "declared secondary shim",
+			shimName:    "pip",
+			wantRuntime: "python",
+			wantKnown:   true,
+		},
+		{
+			name:        "node package manager",
+			shimName:    "npm",
+			wantRuntime: "node",
+			wantKnown:   true,
+		},
+		{
+			name:        "ruby bundler",
+			shimName:    "bundle",
+			wantRuntime: "ruby",
+			wantKnown:   true,
+		},
+		{
+			name:        "version-suffixed name resolves by prefix",
+			shimName:    "python3.13",
+			wantRuntime: "python",
+			wantKnown:   true,
+		},
+		// #273: these shims outlived the pip packages that created
+		// them. Treating the shim name as a runtime name produced the
+		// baffling "runtime provider 'ruff' not found" on invocation;
+		// reporting them as unclaimed lets the caller fall through to
+		// the real command on PATH.
+		{
+			name:        "orphaned pip console script",
+			shimName:    "ruff",
+			wantRuntime: "",
+			wantKnown:   false,
+		},
+		{
+			name:        "orphaned test runner",
+			shimName:    "pytest",
+			wantRuntime: "",
+			wantKnown:   false,
+		},
+		{
+			name:        "unknown command",
+			shimName:    "definitely-not-a-runtime",
+			wantRuntime: "",
+			wantKnown:   false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotRuntime, gotKnown := mapShimToRuntime(tt.shimName)
+			if gotRuntime != tt.wantRuntime || gotKnown != tt.wantKnown {
+				t.Errorf("mapShimToRuntime(%q) = (%q, %v), want (%q, %v)",
+					tt.shimName, gotRuntime, gotKnown, tt.wantRuntime, tt.wantKnown)
+			}
+		})
+	}
+}
+
+// TestMapShimToRuntime_CacheClaimsPackageShims covers the healthy path
+// for a package executable: no provider's static Shims() list mentions
+// `ruff`, so only the shim-map cache written by reshim can route it.
+func TestMapShimToRuntime_CacheClaimsPackageShims(t *testing.T) {
+	isolateShimEnv(t)
+
+	if err := shim.SaveShimMap(shim.ShimMap{
+		"ruff": {Runtime: "python", Versions: []string{"3.13.11"}},
+	}); err != nil {
+		t.Fatalf("SaveShimMap() error: %v", err)
+	}
+	shim.ResetShimMapCache()
+
+	gotRuntime, gotKnown := mapShimToRuntime("ruff")
+	if gotRuntime != "python" || !gotKnown {
+		t.Errorf(`mapShimToRuntime("ruff") = (%q, %v), want ("python", true)`, gotRuntime, gotKnown)
+	}
+}

src/internal/shim/manager.go

@@ -196,6 +196,21 @@ type RehashResult struct {
 	ShimsByRuntime map[string][]string
 	// TotalShims is the total number of shims created
 	TotalShims int
+	// RemovedShims lists the orphan shim files deleted during the rehash,
+	// sorted alphabetically. An orphan is a shim binary in shims/ whose
+	// name no installed runtime version provides any more.
+	RemovedShims []string
+	// PruneFailures lists orphan shims that could not be deleted. These
+	// are reported rather than fatal — see pruneOrphanShims.
+	PruneFailures []PruneFailure
+}
+
+// PruneFailure records an orphan shim file that could not be deleted.
+type PruneFailure struct {
+	// Name is the bare shim name (no .exe / .cmd suffix).
+	Name string
+	// Err is the error returned by the removal attempt.
+	Err error
 }
 
 // RuntimeShimInfo contains shim information for a single runtime
@@ -313,12 +328,89 @@ func (m *Manager) RehashWithCallback(callback RehashCallback) (*RehashResult, er
 		}
 	}
 
+	// Delete shim files the scan above did not produce. This is only
+	// reached when the scan found at least one executable: the
+	// len(shimMap) == 0 check above returns early, so an empty scan —
+	// which is the signal that something is wrong rather than that
+	// everything was uninstalled — never wipes the shims directory.
+	removed, pruneFailures := m.pruneOrphanShims(shimMap)
+
 	return &RehashResult{
 		ShimsByRuntime: shimsByRuntime,
 		TotalShims:     len(shimMap),
+		RemovedShims:   removed,
+		PruneFailures:  pruneFailures,
 	}, nil
 }
 
+// protectedShimNames are files that can legitimately live in the shims
+// directory without being runtime shims, and so must never be pruned.
+// Installs prior to 0.4 placed dtvem-shim itself in shims/ rather than
+// bin/; deleting it would break every remaining shim on the machine.
+var protectedShimNames = map[string]bool{
+	"dtvem":      true,
+	"dtvem-shim": true,
+}
+
+// pruneOrphanShims deletes shim files whose names are absent from the
+// freshly-scanned shim map, returning the names removed and any that
+// could not be.
+//
+// Reshim rebuilds the shim-map cache from the executables found in the
+// installed runtime versions, but it used to only ever *add* shim files.
+// A shim whose backing executable disappeared — a pip- or npm-installed
+// tool that was later uninstalled, a runtime version that was removed,
+// or a phantom shim created by an older dtvem that trusted the provider's
+// static Shims() list over what shipped on disk — stayed in shims/
+// forever. That left two user-visible failures: `dtvem doctor` reporting
+// cache/disk drift that `dtvem reshim` could never resolve, and dead
+// shims sitting early on PATH shadowing perfectly good system tools.
+//
+// Removal failures are collected rather than returned as an error. On
+// Windows a shim that is currently executing cannot be deleted, and
+// failing an entire reshim over one locked file would be a worse outcome
+// than reporting it; doctor's shim-map check re-reports whatever drift
+// remains.
+func (m *Manager) pruneOrphanShims(keep ShimMap) (removed []string, failures []PruneFailure) {
+	existing, err := m.ListShims()
+	if err != nil {
+		// The shims directory is unreadable. Rehash has already
+		// recreated the shims it knows about, so leave the rest alone
+		// rather than guessing at what is safe to delete.
+		return nil, nil
+	}
+
+	for _, name := range existing {
+		if _, wanted := keep[name]; wanted || protectedShimNames[name] {
+			continue
+		}
+
+		// Only delete a file that is actually a shim binary, so an
+		// unrelated file dropped into shims/ survives. On Windows that
+		// means an .exe at the shim path: ListShims derives bare names
+		// by trimming a trailing extension, so notes.txt surfaces as
+		// "notes" and no notes.exe exists. On Unix, require the
+		// executable bit — the same test findExecutables uses to decide
+		// what is shimmable in the first place.
+		info, statErr := os.Stat(config.ShimPath(name))
+		if statErr != nil || info.IsDir() {
+			continue
+		}
+		if runtime.GOOS != constants.OSWindows && info.Mode()&0111 == 0 {
+			continue
+		}
+
+		if err := m.RemoveShim(name); err != nil {
+			failures = append(failures, PruneFailure{Name: name, Err: err})
+			continue
+		}
+		removed = append(removed, name)
+	}
+
+	sort.Strings(removed)
+	return removed, failures
+}
+
 // Rehash regenerates all shims by scanning installed versions (no progress callback)
 func (m *Manager) Rehash() (*RehashResult, error) {
 	return m.RehashWithCallback(nil)