diff --git a/src/cli/doctor/checks/gh.test.ts b/src/cli/doctor/checks/gh.test.ts index 95a9bf1576..8411b649e0 100644 --- a/src/cli/doctor/checks/gh.test.ts +++ b/src/cli/doctor/checks/gh.test.ts @@ -3,15 +3,60 @@ import * as gh from "./gh" describe("gh cli check", () => { describe("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() + + 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 + } + it("returns gh cli info structure", async () => { - // #given - // #when checking gh cli info - const info = await gh.getGhCliInfo() + 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() - // #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) + 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() + } }) }) diff --git a/src/cli/doctor/checks/lsp.test.ts b/src/cli/doctor/checks/lsp.test.ts index b266cc0a6e..259456faa4 100644 --- a/src/cli/doctor/checks/lsp.test.ts +++ b/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", () => { diff --git a/src/cli/doctor/checks/lsp.ts b/src/cli/doctor/checks/lsp.ts index 70350edd3e..254e3d6730 100644 --- a/src/cli/doctor/checks/lsp.ts +++ b/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 { - 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 { 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, diff --git a/src/tools/lsp/config.test.ts b/src/tools/lsp/config.test.ts new file mode 100644 index 0000000000..da65e67ee0 --- /dev/null +++ b/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) + }) + } +}) diff --git a/src/tools/lsp/config.ts b/src/tools/lsp/config.ts index b29f9ac63d..10a6febcfc 100644 --- a/src/tools/lsp/config.ts +++ b/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 + } } }