Merge pull request #834 from MotorwaySouth9/fix/windows-lsp-doctor-and-detection

fix(lsp): improve Windows server detection and avoid unix 'which' in doctor

src/cli/doctor/checks/gh.test.ts

@@ -3,15 +3,60 @@ import * as gh from "./gh"
 
 describe("gh cli check", () => {
   describe("getGhCliInfo", () => {
-    it("returns gh cli info structure", async () => {
-      // #given
-      // #when checking gh cli info
-      const info = await gh.getGhCliInfo()
+    function createProc(opts: { stdout?: string; stderr?: string; exitCode?: number }) {
+      const stdoutText = opts.stdout ?? ""
+      const stderrText = opts.stderr ?? ""
+      const exitCode = opts.exitCode ?? 0
+      const encoder = new TextEncoder()
 
-      // #then should return valid info structure
-      expect(typeof info.installed).toBe("boolean")
-      expect(info.authenticated === true || info.authenticated === false).toBe(true)
-      expect(Array.isArray(info.scopes)).toBe(true)
+      return {
+        stdout: new ReadableStream({
+          start(controller) {
+            if (stdoutText) controller.enqueue(encoder.encode(stdoutText))
+            controller.close()
+          },
+        }),
+        stderr: new ReadableStream({
+          start(controller) {
+            if (stderrText) controller.enqueue(encoder.encode(stderrText))
+            controller.close()
+          },
+        }),
+        exited: Promise.resolve(exitCode),
+        exitCode,
+      } as unknown as ReturnType<typeof Bun.spawn>
+    }
+
+    it("returns gh cli info structure", async () => {
+      const spawnSpy = spyOn(Bun, "spawn").mockImplementation((cmd) => {
+        if (Array.isArray(cmd) && cmd[0] === "which" && cmd[1] === "gh") {
+          return createProc({ stdout: "/usr/bin/gh\n" })
+        }
+
+        if (Array.isArray(cmd) && cmd[0] === "gh" && cmd[1] === "--version") {
+          return createProc({ stdout: "gh version 2.40.0\n" })
+        }
+
+        if (Array.isArray(cmd) && cmd[0] === "gh" && cmd[1] === "auth" && cmd[2] === "status") {
+          return createProc({
+            exitCode: 0,
+            stderr: "Logged in to github.com account octocat (keyring)\nToken scopes: 'repo', 'read:org'\n",
+          })
+        }
+
+        throw new Error(`Unexpected Bun.spawn call: ${Array.isArray(cmd) ? cmd.join(" ") : String(cmd)}`)
+      })
+
+      try {
+        const info = await gh.getGhCliInfo()
+
+        expect(info.installed).toBe(true)
+        expect(info.version).toBe("2.40.0")
+        expect(typeof info.authenticated).toBe("boolean")
+        expect(Array.isArray(info.scopes)).toBe(true)
+      } finally {
+        spawnSpy.mockRestore()
+      }
     })
   })
 

src/cli/doctor/checks/lsp.test.ts

@@ -17,6 +17,23 @@ describe("lsp check", () => {
         expect(Array.isArray(s.extensions)).toBe(true)
       })
     })
+
+    it("does not spawn 'which' command (windows compatibility)", async () => {
+      // #given
+      const spawnSpy = spyOn(Bun, "spawn")
+
+      try {
+        // #when getting servers info
+        await lsp.getLspServersInfo()
+
+        // #then should not spawn which
+        const calls = spawnSpy.mock.calls
+        const whichCalls = calls.filter((c) => Array.isArray(c) && Array.isArray(c[0]) && c[0][0] === "which")
+        expect(whichCalls.length).toBe(0)
+      } finally {
+        spawnSpy.mockRestore()
+      }
+    })
   })
 
   describe("getLspServerStats", () => {

src/cli/doctor/checks/lsp.ts

@@ -12,21 +12,13 @@ const DEFAULT_LSP_SERVERS: Array<{
   { id: "gopls", binary: "gopls", extensions: [".go"] },
 ]
 
-async function checkBinaryExists(binary: string): Promise<boolean> {
-  try {
-    const proc = Bun.spawn(["which", binary], { stdout: "pipe", stderr: "pipe" })
-    await proc.exited
-    return proc.exitCode === 0
-  } catch {
-    return false
-  }
-}
+import { isServerInstalled } from "../../../tools/lsp/config"
 
 export async function getLspServersInfo(): Promise<LspServerInfo[]> {
   const servers: LspServerInfo[] = []
 
   for (const server of DEFAULT_LSP_SERVERS) {
-    const installed = await checkBinaryExists(server.binary)
+    const installed = isServerInstalled([server.binary])
     servers.push({
       id: server.id,
       installed,

src/tools/lsp/config.test.ts

@@ -0,0 +1,130 @@
+import { describe, test, expect, beforeEach, afterEach } from "bun:test"
+import { isServerInstalled } from "./config"
+import { mkdtempSync, rmSync, writeFileSync } from "fs"
+import { join } from "path"
+import { tmpdir } from "os"
+
+describe("isServerInstalled", () => {
+  let tempDir: string
+  let savedEnv: { [key: string]: string | undefined }
+
+  beforeEach(() => {
+    tempDir = mkdtempSync(join(tmpdir(), "lsp-config-test-"))
+    savedEnv = {
+      PATH: process.env.PATH,
+      Path: process.env.Path,
+      PATHEXT: process.env.PATHEXT,
+    }
+  })
+
+  afterEach(() => {
+    try {
+      rmSync(tempDir, { recursive: true, force: true })
+    } catch (e) {
+      console.error(`Failed to clean up temp dir: ${e}`)
+    }
+
+    if (process.platform === "win32") {
+      const pathVal = savedEnv.PATH ?? savedEnv.Path
+      if (pathVal === undefined) {
+        delete process.env.PATH
+        delete process.env.Path
+      } else {
+        process.env.PATH = pathVal
+        process.env.Path = pathVal
+      }
+    } else {
+      if (savedEnv.PATH === undefined) {
+        delete process.env.PATH
+      } else {
+        process.env.PATH = savedEnv.PATH
+      }
+
+      if (savedEnv.Path === undefined) {
+        delete process.env.Path
+      } else {
+        process.env.Path = savedEnv.Path
+      }
+    }
+
+    const pathextVal = savedEnv.PATHEXT
+    if (pathextVal === undefined) {
+      delete process.env.PATHEXT
+    } else {
+      process.env.PATHEXT = pathextVal
+    }
+  })
+
+  test("detects executable in PATH", () => {
+    const binName = "test-lsp-server"
+    const ext = process.platform === "win32" ? ".cmd" : ""
+    const binPath = join(tempDir, binName + ext)
+    
+    writeFileSync(binPath, "echo hello")
+    
+    const pathSep = process.platform === "win32" ? ";" : ":"
+    process.env.PATH = `${tempDir}${pathSep}${process.env.PATH || ""}`
+
+    expect(isServerInstalled([binName])).toBe(true)
+  })
+
+  test("returns false for missing executable", () => {
+    expect(isServerInstalled(["non-existent-server"])).toBe(false)
+  })
+
+  if (process.platform === "win32") {
+    test("Windows: detects executable with Path env var", () => {
+       const binName = "test-lsp-server-case"
+       const binPath = join(tempDir, binName + ".cmd")
+       writeFileSync(binPath, "echo hello")
+
+       delete process.env.PATH
+       process.env.Path = tempDir
+
+       expect(isServerInstalled([binName])).toBe(true)
+    })
+
+    test("Windows: respects PATHEXT", () => {
+       const binName = "test-lsp-server-custom"
+       const binPath = join(tempDir, binName + ".COM")
+       writeFileSync(binPath, "echo hello")
+
+       process.env.PATH = tempDir
+       process.env.PATHEXT = ".COM;.EXE"
+
+       expect(isServerInstalled([binName])).toBe(true)
+    })
+    
+    test("Windows: ensures default extensions are checked even if PATHEXT is missing", () => {
+       const binName = "test-lsp-server-default"
+       const binPath = join(tempDir, binName + ".bat")
+       writeFileSync(binPath, "echo hello")
+
+       process.env.PATH = tempDir
+       delete process.env.PATHEXT
+
+       expect(isServerInstalled([binName])).toBe(true)
+    })
+
+    test("Windows: ensures default extensions are checked even if PATHEXT does not include them", () => {
+        const binName = "test-lsp-server-ps1"
+        const binPath = join(tempDir, binName + ".ps1")
+        writeFileSync(binPath, "echo hello")
+ 
+        process.env.PATH = tempDir
+        process.env.PATHEXT = ".COM"
+ 
+        expect(isServerInstalled([binName])).toBe(true)
+     })
+  } else {
+      test("Non-Windows: does not use windows extensions", () => {
+          const binName = "test-lsp-server-win"
+          const binPath = join(tempDir, binName + ".cmd")
+          writeFileSync(binPath, "echo hello")
+          
+          process.env.PATH = tempDir
+          
+          expect(isServerInstalled([binName])).toBe(false)
+      })
+  }
+})

src/tools/lsp/config.ts

@@ -170,31 +170,46 @@ export function isServerInstalled(command: string[]): boolean {
   }
 
   const isWindows = process.platform === "win32"
-  const ext = isWindows ? ".exe" : ""
+  
+  let exts = [""]
+  if (isWindows) {
+    const pathExt = process.env.PATHEXT || ""
+    if (pathExt) {
+       const systemExts = pathExt.split(";").filter(Boolean)
+       exts = [...new Set([...exts, ...systemExts, ".exe", ".cmd", ".bat", ".ps1"])]
+    } else {
+       exts = ["", ".exe", ".cmd", ".bat", ".ps1"]
+    }
+  }
 
-  const pathEnv = process.env.PATH || ""
+  let pathEnv = process.env.PATH || ""
+  if (isWindows && !pathEnv) {
+    pathEnv = process.env.Path || ""
+  }
+  
   const pathSeparator = isWindows ? ";" : ":"
   const paths = pathEnv.split(pathSeparator)
 
   for (const p of paths) {
-    if (existsSync(join(p, cmd)) || existsSync(join(p, cmd + ext))) {
-      return true
+    for (const suffix of exts) {
+      if (existsSync(join(p, cmd + suffix))) {
+        return true
+      }
     }
   }
 
   const cwd = process.cwd()
-  const additionalPaths = [
-    join(cwd, "node_modules", ".bin", cmd),
-    join(cwd, "node_modules", ".bin", cmd + ext),
-    join(homedir(), ".config", "opencode", "bin", cmd),
-    join(homedir(), ".config", "opencode", "bin", cmd + ext),
-    join(homedir(), ".config", "opencode", "node_modules", ".bin", cmd),
-    join(homedir(), ".config", "opencode", "node_modules", ".bin", cmd + ext),
+  const additionalBases = [
+    join(cwd, "node_modules", ".bin"),
+    join(homedir(), ".config", "opencode", "bin"),
+    join(homedir(), ".config", "opencode", "node_modules", ".bin"),
   ]
 
-  for (const p of additionalPaths) {
-    if (existsSync(p)) {
-      return true
+  for (const base of additionalBases) {
+    for (const suffix of exts) {
+      if (existsSync(join(base, cmd + suffix))) {
+        return true
+      }
     }
   }