This PR refactors package manager detection for Node.js and Python to eliminate code duplication and provide better traceability of how package managers are detected. (#50)

* Enhance package manager detection and prioritize package.json field over lock files

* Refactor package manager detection and improve test coverage

* Enhance package manager detection and add go vet command; improve test coverage and documentation

* Fix logging for invalid JSON in package.json and update comment for Windows taskkill

* Refactor package manager detection to consolidate logic and improve source tracking; enhance health command tests for clarity and reliability

* Refactor package manager detection to include pipenv support and remove unused readFileContent function

Author: jongio

cli/magefile.go

@@ -267,6 +267,16 @@ func Fmt() error {
 	return nil
 }
 
+// Vet runs go vet to check for suspicious constructs.
+func Vet() error {
+	fmt.Println("Running go vet...")
+	if err := sh.RunV("go", "vet", "./..."); err != nil {
+		return fmt.Errorf("go vet found issues: %w", err)
+	}
+	fmt.Println("✅ go vet passed!")
+	return nil
+}
+
 // Clean removes build artifacts and coverage reports.
 func Clean() error {
 	fmt.Println("Cleaning build artifacts...")
@@ -355,6 +365,7 @@ func Preflight() error {
 		{"Building and linting dashboard", DashboardBuild},
 		{"Running dashboard tests", DashboardTest},
 		{"Building Go binary", Build},
+		{"Running go vet", Vet},
 		{"Running standard linting", Lint},
 		{"Running quick security scan", runQuickSecurity},
 		{"Running all tests with coverage", TestCoverage},

cli/src/cmd/app/commands/generate.go

@@ -257,158 +257,60 @@ func hasGit(dir string) bool {
 
 // Tool detection functions
 func detectNode(_ string) DetectedRequirement {
-	req := DetectedRequirement{
-		Name:   "node",
-		Source: "package.json",
-	}
-
-	installedVersion, err := getToolVersion("node")
-	if err != nil {
-		return req
-	}
-
-	req.InstalledVersion = installedVersion
-	req.MinVersion = normalizeVersion(installedVersion, "node")
-	return req
+	return detectToolWithSource("node", "package.json", false)
 }
 
 func detectNodePackageManager(projectDir string) DetectedRequirement {
-	// Priority: pnpm > yarn > npm
-	if fileExists(projectDir, "pnpm-lock.yaml") || fileExists(projectDir, "pnpm-workspace.yaml") {
-		return detectTool("pnpm", "pnpm-lock.yaml")
-	}
-	if fileExists(projectDir, "yarn.lock") {
-		return detectTool("yarn", "yarn.lock")
-	}
-	if fileExists(projectDir, "package-lock.json") {
-		return detectTool("npm", "package-lock.json")
-	}
-	// Default to npm
-	return detectTool("npm", "package.json")
+	// Use detector to get both package manager and source in one call
+	info := detector.DetectNodePackageManagerWithSource(projectDir)
+	return detectTool(info.Name, info.Source)
 }
 
 func detectPython(_ string) DetectedRequirement {
-	req := DetectedRequirement{
-		Name:   "python",
-		Source: "requirements.txt or pyproject.toml",
-	}
-
-	installedVersion, err := getToolVersion("python")
-	if err != nil {
-		return req
-	}
-
-	req.InstalledVersion = installedVersion
-	req.MinVersion = normalizeVersion(installedVersion, "python")
-	return req
+	return detectToolWithSource("python", "requirements.txt or pyproject.toml", false)
 }
 
 func detectPythonPackageManager(projectDir string) DetectedRequirement {
-	// Priority: uv > poetry > pipenv > pip
-	if fileExists(projectDir, "uv.lock") {
-		return detectTool("uv", "uv.lock")
-	}
-
-	if fileExists(projectDir, "pyproject.toml") {
-		content := readFileContent(filepath.Join(projectDir, "pyproject.toml"))
-		if strings.Contains(content, "[tool.uv]") {
-			return detectTool("uv", "pyproject.toml")
-		}
-		if strings.Contains(content, "[tool.poetry]") {
-			return detectTool("poetry", "pyproject.toml")
-		}
-	}
-
-	if fileExists(projectDir, "poetry.lock") {
-		return detectTool("poetry", "poetry.lock")
-	}
-
-	if fileExists(projectDir, "Pipfile") || fileExists(projectDir, "Pipfile.lock") {
-		return detectTool("pipenv", "Pipfile")
-	}
-
-	// Default to pip
-	return detectTool("pip", "requirements.txt")
+	// Use detector to get both package manager and source in one call
+	info := detector.DetectPythonPackageManagerWithSource(projectDir)
+	return detectTool(info.Name, info.Source)
 }
 
 func detectDotnet(_ string) DetectedRequirement {
-	req := DetectedRequirement{
-		Name:   "dotnet",
-		Source: ".csproj or .sln",
-	}
-
-	installedVersion, err := getToolVersion("dotnet")
-	if err != nil {
-		return req
-	}
-
-	req.InstalledVersion = installedVersion
-	req.MinVersion = normalizeVersion(installedVersion, "dotnet")
-	return req
+	return detectToolWithSource("dotnet", ".csproj or .sln", false)
 }
 
 func detectAspire(_ string) DetectedRequirement {
-	req := DetectedRequirement{
-		Name:   "aspire",
-		Source: "AppHost.cs",
-	}
-
-	installedVersion, err := getToolVersion("aspire")
-	if err != nil {
-		return req
-	}
-
-	req.InstalledVersion = installedVersion
-	req.MinVersion = normalizeVersion(installedVersion, "aspire")
-	return req
+	return detectToolWithSource("aspire", "AppHost.cs", false)
 }
 
 func detectDocker(_ string) DetectedRequirement {
-	req := DetectedRequirement{
-		Name:         "docker",
-		Source:       "Dockerfile or docker-compose.yml",
-		CheckRunning: true,
-	}
-
-	installedVersion, err := getToolVersion("docker")
-	if err != nil {
-		return req
-	}
-
-	req.InstalledVersion = installedVersion
-	req.MinVersion = normalizeVersion(installedVersion, "docker")
-	return req
+	return detectToolWithSource("docker", "Dockerfile or docker-compose.yml", true)
 }
 
 func detectAzd(_ string) DetectedRequirement {
-	req := DetectedRequirement{
-		Name:   "azd",
-		Source: "azure.yaml",
-	}
-
-	installedVersion, err := getToolVersion("azd")
-	if err != nil {
-		return req
-	}
-
-	req.InstalledVersion = installedVersion
-	req.MinVersion = normalizeVersion(installedVersion, "azd")
-	return req
+	return detectToolWithSource("azd", "azure.yaml", false)
 }
 
 func detectGit(_ string) DetectedRequirement {
+	return detectToolWithSource("git", ".git directory", false)
+}
+
+// detectToolWithSource is a helper for detecting tools with specified source and checkRunning flag.
+func detectToolWithSource(toolName, source string, checkRunning bool) DetectedRequirement {
 	req := DetectedRequirement{
-		Name:   "git",
-		Source: ".git directory",
+		Name:         toolName,
+		Source:       source,
+		CheckRunning: checkRunning,
 	}
 
-	installedVersion, err := getToolVersion("git")
+	installedVersion, err := getToolVersion(toolName)
 	if err != nil {
 		return req
 	}
 
 	req.InstalledVersion = installedVersion
-	req.MinVersion = normalizeVersion(installedVersion, "git")
+	req.MinVersion = normalizeVersion(installedVersion, toolName)
 	return req
 }
 
@@ -533,18 +435,6 @@ func fileExists(dir, filename string) bool {
 	return err == nil
 }
 
-func readFileContent(path string) string {
-	if err := security.ValidatePath(path); err != nil {
-		return ""
-	}
-	// #nosec G304 -- Path validated by security.ValidatePath
-	data, err := os.ReadFile(path)
-	if err != nil {
-		return ""
-	}
-	return string(data)
-}
-
 // Display functions
 func displayDetectedDependencies(requirements []DetectedRequirement) {
 	sources := make(map[string]bool)

cli/src/cmd/app/commands/generate_test.go

@@ -850,6 +850,45 @@ func TestDetectNodePackageManager(t *testing.T) {
 		expectedID     string
 		expectedSource string
 	}{
+		{
+			name: "packageManager field takes priority over lock files - pnpm",
+			setup: func(dir string) error {
+				pkgJSON := `{"name": "test", "packageManager": "pnpm@8.15.0"}`
+				if err := os.WriteFile(filepath.Join(dir, "package.json"), []byte(pkgJSON), 0600); err != nil {
+					return err
+				}
+				// Create yarn.lock to test priority
+				return os.WriteFile(filepath.Join(dir, "yarn.lock"), []byte(""), 0600)
+			},
+			expectedID:     "pnpm",
+			expectedSource: "package.json (packageManager field)",
+		},
+		{
+			name: "packageManager field takes priority over lock files - yarn",
+			setup: func(dir string) error {
+				pkgJSON := `{"name": "test", "packageManager": "yarn@4.1.0"}`
+				if err := os.WriteFile(filepath.Join(dir, "package.json"), []byte(pkgJSON), 0600); err != nil {
+					return err
+				}
+				// Create pnpm-lock.yaml to test priority
+				return os.WriteFile(filepath.Join(dir, "pnpm-lock.yaml"), []byte(""), 0600)
+			},
+			expectedID:     "yarn",
+			expectedSource: "package.json (packageManager field)",
+		},
+		{
+			name: "packageManager field takes priority over lock files - npm",
+			setup: func(dir string) error {
+				pkgJSON := `{"name": "test", "packageManager": "npm@10.5.0"}`
+				if err := os.WriteFile(filepath.Join(dir, "package.json"), []byte(pkgJSON), 0600); err != nil {
+					return err
+				}
+				// Create yarn.lock to test priority
+				return os.WriteFile(filepath.Join(dir, "yarn.lock"), []byte(""), 0600)
+			},
+			expectedID:     "npm",
+			expectedSource: "package.json (packageManager field)",
+		},
 		{
 			name: "detects pnpm from lock file",
 			setup: func(dir string) error {
@@ -864,7 +903,7 @@ func TestDetectNodePackageManager(t *testing.T) {
 				return os.WriteFile(filepath.Join(dir, "pnpm-workspace.yaml"), []byte(""), 0600)
 			},
 			expectedID:     "pnpm",
-			expectedSource: "pnpm-lock.yaml",
+			expectedSource: "pnpm-workspace.yaml",
 		},
 		{
 			name: "detects yarn from lock file",
@@ -888,6 +927,30 @@ func TestDetectNodePackageManager(t *testing.T) {
 			expectedID:     "npm",
 			expectedSource: "package.json",
 		},
+		{
+			name: "unsupported package manager in packageManager field falls back to lock files",
+			setup: func(dir string) error {
+				// Set packageManager to an unsupported manager (e.g., "bun")
+				pkgJSON := `{"name": "test", "packageManager": "bun@1.0.0"}`
+				if err := os.WriteFile(filepath.Join(dir, "package.json"), []byte(pkgJSON), 0600); err != nil {
+					return err
+				}
+				// Create pnpm-lock.yaml - should fall back to this
+				return os.WriteFile(filepath.Join(dir, "pnpm-lock.yaml"), []byte(""), 0600)
+			},
+			expectedID:     "pnpm",
+			expectedSource: "pnpm-lock.yaml",
+		},
+		{
+			name: "unsupported package manager with no lock files defaults to npm",
+			setup: func(dir string) error {
+				// Set packageManager to an unsupported manager with no lock files
+				pkgJSON := `{"name": "test", "packageManager": "bun@1.0.0"}`
+				return os.WriteFile(filepath.Join(dir, "package.json"), []byte(pkgJSON), 0600)
+			},
+			expectedID:     "npm",
+			expectedSource: "package.json",
+		},
 	}
 
 	for _, tt := range tests {