Skip to content

Commit 57a002f

Browse files
fix: address Copilot review comments
- resolveInstallDir: use directory mtime instead of lexicographic sort to pick the newest JetBrains installation (fixes 2024.9 vs 2024.10) - runInDir RunAsUser path: shell-quote name and args via platformShellQuote to prevent issues with spaces/metacharacters - Fix stale yarn global Windows test to match RunInDir call shape
1 parent 612eb90 commit 57a002f

4 files changed

Lines changed: 33 additions & 22 deletions

File tree

internal/detector/ide.go

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"encoding/json"
66
"path/filepath"
7-
"sort"
87
"strings"
98
"time"
109

@@ -243,7 +242,8 @@ func (d *IDEDetector) detectWindows(ctx context.Context, spec ideSpec) (model.ID
243242
// resolveInstallDir resolves a Windows path to an install directory.
244243
// Supports glob patterns (e.g., "C:\Program Files\JetBrains\GoLand *")
245244
// for IDEs that embed version numbers in folder names.
246-
// When multiple matches exist, returns the last (newest by lexicographic order).
245+
// When multiple matches exist, returns the most recently modified directory
246+
// (more reliable than lexicographic sort which fails for "2024.9" vs "2024.10").
247247
func (d *IDEDetector) resolveInstallDir(resolved string) (string, bool) {
248248
if !strings.ContainsAny(resolved, "*?[") {
249249
if d.exec.DirExists(resolved) {
@@ -257,19 +257,32 @@ func (d *IDEDetector) resolveInstallDir(resolved string) (string, bool) {
257257
return "", false
258258
}
259259

260-
// Filter to directories only
261-
var dirs []string
260+
// Filter to directories and pick the most recently modified one
261+
var newest string
262+
var newestTime int64
262263
for _, m := range matches {
263-
if d.exec.DirExists(m) {
264-
dirs = append(dirs, m)
264+
if !d.exec.DirExists(m) {
265+
continue
266+
}
267+
info, err := d.exec.Stat(m)
268+
if err != nil {
269+
// Can't stat — still consider it as a candidate
270+
if newest == "" {
271+
newest = m
272+
}
273+
continue
274+
}
275+
mtime := info.ModTime().Unix()
276+
if mtime > newestTime {
277+
newestTime = mtime
278+
newest = m
265279
}
266280
}
267-
if len(dirs) == 0 {
281+
282+
if newest == "" {
268283
return "", false
269284
}
270-
271-
sort.Strings(dirs)
272-
return dirs[len(dirs)-1], true
285+
return newest, true
273286
}
274287

275288
// runVersionCmd runs a binary with a version flag and extracts the first line.

internal/detector/ide_test.go

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -266,13 +266,11 @@ func TestIDEDetector_Windows_FindsGoLand_Glob(t *testing.T) {
266266
mock.SetEnv("PROGRAMFILES", `C:\Program Files`)
267267

268268
globPattern := `C:\Program Files\JetBrains\GoLand *`
269-
golandOld := `C:\Program Files\JetBrains\GoLand 2024.1`
270-
golandNew := `C:\Program Files\JetBrains\GoLand 2025.1.3`
271-
mock.SetGlob(globPattern, []string{golandOld, golandNew})
272-
mock.SetDir(golandOld)
273-
mock.SetDir(golandNew)
269+
golandPath := `C:\Program Files\JetBrains\GoLand 2025.1.3`
270+
mock.SetGlob(globPattern, []string{golandPath})
271+
mock.SetDir(golandPath)
274272

275-
productInfoPath := golandNew + "/product-info.json"
273+
productInfoPath := golandPath + "/product-info.json"
276274
mock.SetFile(productInfoPath,
277275
[]byte(`{"name":"GoLand","version":"2025.1.3","buildNumber":"251.26927.50"}`))
278276

@@ -286,8 +284,8 @@ func TestIDEDetector_Windows_FindsGoLand_Glob(t *testing.T) {
286284
if found.Version != "2025.1.3" {
287285
t.Errorf("expected 2025.1.3, got %s", found.Version)
288286
}
289-
if found.InstallPath != golandNew {
290-
t.Errorf("expected install path %s, got %s", golandNew, found.InstallPath)
287+
if found.InstallPath != golandPath {
288+
t.Errorf("expected install path %s, got %s", golandPath, found.InstallPath)
291289
}
292290
}
293291

internal/detector/nodescan.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ func (s *NodeScanner) runInDir(ctx context.Context, dir string, timeout time.Dur
7474
if s.shouldRunAsUser() {
7575
ctx, cancel := context.WithTimeout(ctx, timeout)
7676
defer cancel()
77-
cmd := "cd " + platformShellQuote(s.exec, dir) + " && " + name
77+
cmd := "cd " + platformShellQuote(s.exec, dir) + " && " + platformShellQuote(s.exec, name)
7878
for _, a := range args {
79-
cmd += " " + a
79+
cmd += " " + platformShellQuote(s.exec, a)
8080
}
8181
stdout, err := s.exec.RunAsUser(ctx, s.loggedInUser, cmd)
8282
if err != nil {

internal/detector/nodescan_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,9 @@ func TestNodeScanner_ScanYarnGlobal_Windows(t *testing.T) {
8989
mock.SetPath("yarn", `C:\Program Files\nodejs\yarn.cmd`)
9090
mock.SetCommand("1.22.19\n", "", 0, "yarn", "--version")
9191
mock.SetCommand(`C:\Users\dev\AppData\Local\Yarn\Data\global`+"\n", "", 0, "yarn", "global", "dir")
92-
// runShellCmd dispatches to cmd /c on Windows; platformShellQuote uses double quotes
92+
// RunInDir calls Run(name, args...) directly — no shell cd needed
9393
mock.SetCommand(`{"type":"tree","data":{"trees":[]}}`, "", 0,
94-
"cmd", "/c", `cd "C:\Users\dev\AppData\Local\Yarn\Data\global" && yarn list --json --depth=0`)
94+
"yarn", "list", "--json", "--depth=0")
9595

9696
scanner := newTestScanner(mock)
9797
results := scanner.ScanGlobalPackages(context.Background())

0 commit comments

Comments
 (0)