fix(doctor): stop removing other dtvem installs' shims from PATH

src/internal/doctor/check_stale_shims.go

@@ -3,6 +3,7 @@ package doctor
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	goruntime "runtime"
 	"strings"
 
@@ -51,19 +52,31 @@ func (staleShimsCheck) Name() string { return "stale-shims-path" }
 
 func (c *staleShimsCheck) Run() Finding {
 	currentShims := c.currentShimsDir()
-	pathEntries := path.SplitPath(c.pathEnv())
-	stale := path.FindStaleShimsEntries(pathEntries, currentShims)
+	entries := path.ClassifyShimsEntries(path.SplitPath(c.pathEnv()), currentShims)
+	// The same directory often appears twice, from the System and the User
+	// PATH; report each once.
+	stale, others := uniqueDirs(entries.Stale), uniqueDirs(entries.OtherRoots)
 
-	if len(stale) == 0 {
+	if len(stale) == 0 && len(others) == 0 {
 		return Finding{OK: true, Title: "No stale dtvem shims entries in PATH"}
 	}
 
 	details := []Detail{{Key: "Expected", Value: currentShims}}
 	for _, s := range stale {
 		details = append(details, Detail{Key: "Found", Value: s})
 	}
+	for _, o := range others {
+		details = append(details, Detail{Key: "Other install", Value: o})
+	}
+
+	if len(stale) == 0 {
+		return otherRootsFinding(others, details)
+	}
 
 	resolution, fix := c.fixOrInstructions(stale, currentShims)
+	if len(others) > 0 {
+		resolution += "\nShims directories of other dtvem installs (\"Other install\") still exist and are left in place."
+	}
 	return Finding{
 		Severity:   SeverityError,
 		Title:      fmt.Sprintf("Stale dtvem shims entr%s in PATH", plural(len(stale), "y", "ies")),
@@ -73,6 +86,42 @@ func (c *staleShimsCheck) Run() Finding {
 	}
 }
 
+// uniqueDirs returns dirs without repeats, keeping the first spelling of
+// each. Comparison is case-insensitive on Windows.
+func uniqueDirs(dirs []string) []string {
+	seen := make(map[string]bool, len(dirs))
+	var unique []string
+	for _, d := range dirs {
+		key := filepath.Clean(strings.TrimSpace(d))
+		if goruntime.GOOS == constants.OSWindows {
+			key = strings.ToLower(key)
+		}
+		if !seen[key] {
+			seen[key] = true
+			unique = append(unique, d)
+		}
+	}
+	return unique
+}
+
+// otherRootsFinding reports PATH entries for another dtvem root's shims
+// directory that still exists: a second install, or the default install
+// while DTVEM_ROOT points elsewhere. That root may still be in use, so
+// doctor never removes these; it only says how to, if the install is gone.
+func otherRootsFinding(others []string, details []Detail) Finding {
+	return Finding{
+		Severity: SeverityWarning,
+		Title:    fmt.Sprintf("Another dtvem install's shims director%s in PATH", plural(len(others), "y is", "ies are")),
+		Details:  details,
+		Resolution: strings.Join([]string{
+			"These shims belong to a different dtvem root than the one this dtvem uses (\"Expected\").",
+			"If you use more than one dtvem root on purpose, nothing needs to change.",
+			"If that install is no longer used, remove its entry from your PATH (on Windows: your User or",
+			"System PATH; elsewhere: the export in your shell config), then restart your terminal.",
+		}, "\n"),
+	}
+}
+
 // fixOrInstructions returns the resolution text and (optionally) a Fix
 // closure for the current platform. On both platforms we always return
 // instructions, but only return a Fix when we have a safe in-place

src/internal/doctor/check_stale_shims_test.go

@@ -242,3 +242,92 @@ func TestStaleShimsCheck_FixPropagatesImplError(t *testing.T) {
 		t.Errorf("Fix err = %v, want chain containing %v", err, wantErr)
 	}
 }
+
+// existingOtherRootShims creates another dtvem root's shims directory, as a
+// second install or the default install would have.
+func existingOtherRootShims(t *testing.T) string {
+	t.Helper()
+	dir := filepath.Join(t.TempDir(), "other", ".dtvem", "shims")
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatalf("creating %s: %v", dir, err)
+	}
+	return dir
+}
+
+func TestStaleShimsCheck_OtherRootIsAWarningWithoutFix(t *testing.T) {
+	_, shims := withDtvemRoot(t)
+	other := existingOtherRootShims(t)
+	withPathEnv(t, []string{shims, other, "/usr/local/bin"})
+
+	got := newStaleShimsCheck().Run()
+	if got.OK {
+		t.Fatal("expected a finding for another install's shims directory, got OK")
+	}
+	if got.Severity != SeverityWarning {
+		t.Errorf("Severity = %v, want warning: another root is not an error", got.Severity)
+	}
+	if got.Fixable() {
+		t.Error("finding is fixable; another root's shims must never be removed automatically")
+	}
+
+	var gotOther bool
+	for _, d := range got.Details {
+		if d.Key == "Other install" && d.Value == other {
+			gotOther = true
+		}
+		if d.Key == "Found" {
+			t.Errorf("Details report %s as stale (Found); it still exists", d.Value)
+		}
+	}
+	if !gotOther {
+		t.Errorf("Details = %v, want an \"Other install\" entry for %s", got.Details, other)
+	}
+	if !strings.Contains(got.Resolution, "no longer used") {
+		t.Errorf("Resolution %q should explain when to remove the entry", got.Resolution)
+	}
+}
+
+func TestStaleShimsCheck_RepeatedEntryReportedOnce(t *testing.T) {
+	_, shims := withDtvemRoot(t)
+	other := existingOtherRootShims(t)
+	// Windows puts the System PATH before the User PATH, so a directory
+	// added to both appears twice.
+	withPathEnv(t, []string{other, shims, other})
+
+	got := newStaleShimsCheck().Run()
+	count := 0
+	for _, d := range got.Details {
+		if d.Key == "Other install" {
+			count++
+		}
+	}
+	if count != 1 {
+		t.Errorf("Details list the other install %d times, want once: %v", count, got.Details)
+	}
+}
+
+func TestStaleShimsCheck_StaleAndOtherRoot(t *testing.T) {
+	_, shims := withDtvemRoot(t)
+	other := existingOtherRootShims(t)
+	missing := filepath.Join(t.TempDir(), "gone", ".dtvem", "shims")
+	withPathEnv(t, []string{missing, shims, other})
+
+	got := newStaleShimsCheck().Run()
+	if got.OK || got.Severity != SeverityError || !got.Fixable() {
+		t.Fatalf("Run() OK=%v severity=%v fixable=%v, want a fixable error for the missing directory", got.OK, got.Severity, got.Fixable())
+	}
+
+	found := map[string]string{}
+	for _, d := range got.Details {
+		found[d.Value] = d.Key
+	}
+	if found[missing] != "Found" {
+		t.Errorf("missing directory reported as %q, want \"Found\"", found[missing])
+	}
+	if found[other] != "Other install" {
+		t.Errorf("existing other root reported as %q, want \"Other install\"", found[other])
+	}
+	if !strings.Contains(got.Resolution, "left in place") {
+		t.Errorf("Resolution %q should say other installs' shims are left in place", got.Resolution)
+	}
+}

src/internal/path/path.go

@@ -85,70 +85,101 @@ func IsDtvemShimsPath(path string) bool {
 	return leafEq(parent, "dtvem") || leafEq(parent, ".dtvem")
 }
 
-// FindStaleShimsEntries scans pathEntries for entries that look like dtvem
-// shims directories but do not match currentShimsDir. The returned slice
-// preserves the order of appearance in pathEntries and has the original
-// (un-cleaned) entry strings, so callers can match them against registry
-// or config-file content.
-//
-// Comparison against currentShimsDir is case-insensitive on Windows.
+// FindStaleShimsEntries scans pathEntries for dtvem shims directories that
+// are not currentShimsDir and no longer exist. See ClassifyShimsEntries.
+// The returned slice preserves order and has the original (un-cleaned)
+// entry strings, so callers can match them against registry or
+// config-file content.
 func FindStaleShimsEntries(pathEntries []string, currentShimsDir string) []string {
-	_, stale := PartitionStaleShimsEntries(pathEntries, currentShimsDir)
-	return stale
+	return ClassifyShimsEntries(pathEntries, currentShimsDir).Stale
+}
+
+// FindOtherRootShimsEntries scans pathEntries for dtvem shims directories
+// that are not currentShimsDir but still exist: another dtvem root, such
+// as a second install or the default install while DTVEM_ROOT points
+// elsewhere. Such entries are reported, never removed. The returned slice
+// preserves order and has the original entry strings.
+func FindOtherRootShimsEntries(pathEntries []string, currentShimsDir string) []string {
+	return ClassifyShimsEntries(pathEntries, currentShimsDir).OtherRoots
 }
 
 // PartitionStaleShimsEntries splits pathEntries into the entries to keep
-// and the stale dtvem-shims entries to remove. Order is preserved in
-// both slices, and the strings are the original (un-cleaned) entries so
-// callers can match them against registry-stored values verbatim.
-//
-// An empty string entry is treated as "drop silently" — it lands in
-// neither slice, which matches what callers want when rewriting PATH
-// (we don't want to preserve stray empty entries that arose from
-// trailing separators).
-//
-// When currentShimsDir is empty the entire input is returned as kept,
-// since without a "current" reference we can't decide which dtvem-shims
-// entries are stale and which are correct.
+// and the stale dtvem-shims entries to remove. See ClassifyShimsEntries;
+// another root's existing shims directory is kept.
 func PartitionStaleShimsEntries(pathEntries []string, currentShimsDir string) (kept, stale []string) {
-	if currentShimsDir == "" {
-		kept = make([]string, 0, len(pathEntries))
-		for _, entry := range pathEntries {
-			if strings.TrimSpace(entry) == "" {
-				continue
-			}
-			kept = append(kept, entry)
-		}
-		return kept, nil
-	}
-	currentClean := filepath.Clean(currentShimsDir)
+	entries := ClassifyShimsEntries(pathEntries, currentShimsDir)
+	return entries.Kept, entries.Stale
+}
+
+// ShimsEntries sorts PATH entries by how they relate to the current dtvem
+// shims directory. Each slice preserves order and holds the original
+// (un-cleaned) entry strings, so callers can match them against
+// registry-stored values and config files verbatim.
+type ShimsEntries struct {
+	// Kept holds every entry to keep: anything that isn't a dtvem shims
+	// directory, the current shims directory, and OtherRoots.
+	Kept []string
+	// Stale holds dtvem shims directories, other than the current one,
+	// that no longer exist: leftovers of a removed or moved install, such
+	// as a pre-XDG root or the other side of a system/user switch.
+	Stale []string
+	// OtherRoots holds dtvem shims directories, other than the current
+	// one, that still exist. They belong to another dtvem root that may
+	// still be in use, so they are kept as well.
+	OtherRoots []string
+}
 
+// ClassifyShimsEntries sorts pathEntries against currentShimsDir. Entries
+// are compared with environment references expanded (%USERPROFILE% on
+// Windows, $HOME elsewhere), so a registry or shell-config spelling of
+// the current shims directory is recognized as current.
+//
+// Empty entries land in no slice, which is what callers want when
+// rewriting PATH (stray empties come from trailing separators). When
+// currentShimsDir is empty every entry is kept, since without a current
+// reference no dtvem shims entry can be judged stale.
+func ClassifyShimsEntries(pathEntries []string, currentShimsDir string) ShimsEntries {
+	var entries ShimsEntries
 	for _, entry := range pathEntries {
 		trimmed := strings.TrimSpace(entry)
-		if trimmed == "" {
-			continue
-		}
-		if !IsDtvemShimsPath(trimmed) {
-			kept = append(kept, entry)
-			continue
-		}
-		entryClean := filepath.Clean(trimmed)
-		matches := entryClean == currentClean
-		if runtime.GOOS == constants.OSWindows {
-			matches = strings.EqualFold(entryClean, currentClean)
-		}
-		if matches {
-			kept = append(kept, entry)
+		switch {
+		case trimmed == "":
 			continue
+		case currentShimsDir == "", !IsDtvemShimsPath(expandPathEntry(trimmed)), sameShimsDir(trimmed, currentShimsDir):
+			entries.Kept = append(entries.Kept, entry)
+		case shimsDirExists(trimmed):
+			entries.Kept = append(entries.Kept, entry)
+			entries.OtherRoots = append(entries.OtherRoots, entry)
+		default:
+			entries.Stale = append(entries.Stale, entry)
 		}
-		stale = append(stale, entry)
 	}
-	return kept, stale
+	return entries
+}
+
+// sameShimsDir reports whether two shims directory spellings name the same
+// directory once environment references are expanded. Comparison is
+// case-insensitive on Windows.
+func sameShimsDir(a, b string) bool {
+	a = filepath.Clean(expandPathEntry(a))
+	b = filepath.Clean(expandPathEntry(b))
+	if runtime.GOOS == constants.OSWindows {
+		return strings.EqualFold(a, b)
+	}
+	return a == b
+}
+
+// shimsDirExists reports whether a shims directory entry, with environment
+// references expanded, exists on disk.
+func shimsDirExists(entry string) bool {
+	info, err := os.Stat(expandPathEntry(entry))
+	return err == nil && info.IsDir()
 }
 
 // RemoveStaleShimsFromShellConfig removes "marker + export" blocks
-// from a single shell config file when the export references a dtvem
-// shims directory that doesn't match currentShimsDir. The file is
+// from a single shell config file when the export references a stale
+// dtvem shims directory: not currentShimsDir, and no longer on disk.
+// Another root's existing shims directory is left alone. The file is
 // rewritten in place; the returned slice lists the stale paths whose
 // blocks were dropped, in source order.
 //
@@ -186,8 +217,8 @@ func RemoveStaleShimsFromShellConfig(configFile, currentShimsDir string) ([]stri
 
 // removeDtvemMarkerBlocks scans configContent line by line, dropping
 // each pair of (marker comment, following export line) whose export
-// line references a dtvem shims path that doesn't match
-// currentShimsDir. Returns the rewritten content and the stale paths
+// line references a stale dtvem shims path (see
+// extractStaleShimsPath). Returns the rewritten content and the stale paths
 // that were dropped, in source order.
 //
 // Behavior intentionally errs on the side of keeping content:
@@ -227,8 +258,9 @@ func removeDtvemMarkerBlocks(configContent, currentShimsDir string) (string, []s
 
 // extractStaleShimsPath returns the dtvem shims path referenced in a
 // PATH-export line if and only if that path is non-empty, looks like a
-// dtvem shims dir, and is not currentShimsDir. Returns "" when the
-// line isn't a stale-referencing export.
+// dtvem shims dir, is not currentShimsDir, and no longer exists (see
+// ClassifyShimsEntries). Returns "" when the line isn't a
+// stale-referencing export.
 //
 // The matcher is permissive across bash/zsh ("export PATH=...") and
 // fish ("set -gx PATH ...") because the dtvem shims path appears
@@ -237,8 +269,6 @@ func removeDtvemMarkerBlocks(configContent, currentShimsDir string) (string, []s
 // bash export concatenates, and check the first segment against
 // IsDtvemShimsPath.
 func extractStaleShimsPath(line, currentShimsDir string) string {
-	currentClean := filepath.Clean(currentShimsDir)
-
 	for _, quoted := range quotedSubstrings(line) {
 		// Bash-style "<dir>:$PATH" — keep the part before the first
 		// colon. Fish-style "<dir>" — the whole quoted string is the
@@ -248,14 +278,18 @@ func extractStaleShimsPath(line, currentShimsDir string) string {
 			candidate = candidate[:idx]
 		}
 		candidate = strings.TrimSpace(candidate)
-		if !IsDtvemShimsPath(candidate) {
+		if !IsDtvemShimsPath(expandPathEntry(candidate)) {
 			continue
 		}
-		if filepath.Clean(candidate) == currentClean {
+		if sameShimsDir(candidate, currentShimsDir) {
 			// References the current shims dir — that's the active
 			// export we want to leave in place.
 			continue
 		}
+		if shimsDirExists(candidate) {
+			// Another dtvem root's shims, which may still be in use.
+			continue
+		}
 		return candidate
 	}
 	return ""