fix(mcp): normalize path separators in LSP/ast-grep cli candidate detection (fixes #4151)
On Windows, path.resolve() returns paths with backslash separators. The previous endsWith("dist/cli.js") check uses forward slashes and always returned false on Windows, causing both LSP and ast-grep MCPs to fall through to the bootstrap path even when the dist cli exists. Result: LSP MCP completely unusable on Windows with MODULE_NOT_FOUND for dist/packages/lsp-tools-mcp/dist/cli.js.
Fix: derive a platform-aware suffix at module load time by replacing forward slashes in DIST_CLI_REL / SOURCE_CLI_REL with path.sep, then use that suffix in the endsWith check.
Verification: all 4 src/mcp/lsp.test.ts cases pass on Windows (previously 2 failed); all 13 src/mcp/ast-grep.test.ts cases pass (previously 2 failed). Total: 17/17 src/mcp tests green. bun run typecheck clean.
This commit is contained in:
+5
-3
@@ -1,6 +1,7 @@
|
||||
import { existsSync } from "node:fs";
|
||||
import { dirname, resolve } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { hasCliSuffix } from "./cli-suffix";
|
||||
import type { LocalMcpConfig } from "./lsp";
|
||||
import { resolveRuntimeExecutable, type RuntimeExecutable, type RuntimeExecutableResolver } from "./runtime-executable";
|
||||
|
||||
@@ -96,11 +97,12 @@ function resolveAstGrepCommand(options: AstGrepMcpConfigOptions = {}): CommandCa
|
||||
const moduleDirectory = getModuleDirectory(options.moduleUrl ?? import.meta.url);
|
||||
if (moduleDirectory) addAncestorCommandCandidates(moduleDirectory, candidates, seenPaths, pathExists, resolveExecutable);
|
||||
|
||||
const distCandidate = candidates.find((candidate) => candidate.path.endsWith(DIST_CLI_REL) && candidate.exists);
|
||||
const distCandidate = candidates.find((candidate) => hasCliSuffix(candidate.path, DIST_CLI_REL) && candidate.exists);
|
||||
if (distCandidate) return distCandidate;
|
||||
const sourceCandidate = candidates.find((candidate) => candidate.path.endsWith(SOURCE_CLI_REL) && candidate.exists);
|
||||
const sourceCandidate = candidates.find((candidate) => hasCliSuffix(candidate.path, SOURCE_CLI_REL) && candidate.exists);
|
||||
if (sourceCandidate) return sourceCandidate;
|
||||
const fallbackCandidate = candidates.find((candidate) => candidate.path.endsWith(DIST_CLI_REL)) ?? createFallbackCandidate(resolveExecutable);
|
||||
const fallbackCandidate =
|
||||
candidates.find((candidate) => hasCliSuffix(candidate.path, DIST_CLI_REL)) ?? createFallbackCandidate(resolveExecutable);
|
||||
return { ...fallbackCandidate, exists: fallbackCandidate.runtimeAvailable };
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
import { describe, expect, it } from "bun:test"
|
||||
import { hasCliSuffix } from "./cli-suffix"
|
||||
|
||||
describe("hasCliSuffix", () => {
|
||||
it("matches cli suffixes across platform separators", () => {
|
||||
// given
|
||||
const suffix = "packages/lsp-tools-mcp/dist/cli.js"
|
||||
const candidatePaths = [
|
||||
"/home/user/project/packages/lsp-tools-mcp/dist/cli.js",
|
||||
"C:\\Users\\yeongyu\\project\\packages\\lsp-tools-mcp\\dist\\cli.js",
|
||||
"\\\\server\\share\\project\\packages\\lsp-tools-mcp\\dist\\cli.js",
|
||||
"C:/Users/yeongyu/project\\packages/lsp-tools-mcp\\dist/cli.js",
|
||||
]
|
||||
|
||||
// when
|
||||
const results = candidatePaths.map((candidatePath) => hasCliSuffix(candidatePath, suffix))
|
||||
|
||||
// then
|
||||
expect(results).toEqual([true, true, true, true])
|
||||
})
|
||||
|
||||
it("does not match unrelated cli suffixes", () => {
|
||||
// given
|
||||
const candidatePath = "C:\\Users\\yeongyu\\project\\packages\\other-mcp\\dist\\cli.js"
|
||||
|
||||
// when
|
||||
const result = hasCliSuffix(candidatePath, "packages/lsp-tools-mcp/dist/cli.js")
|
||||
|
||||
// then
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,7 @@
|
||||
function normalizeCliPath(path: string): string {
|
||||
return path.replaceAll("\\", "/")
|
||||
}
|
||||
|
||||
export function hasCliSuffix(candidatePath: string, suffix: string): boolean {
|
||||
return normalizeCliPath(candidatePath).endsWith(normalizeCliPath(suffix))
|
||||
}
|
||||
+30
-1
@@ -1,5 +1,5 @@
|
||||
import { afterEach, describe, expect, it } from "bun:test"
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"
|
||||
import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"
|
||||
import { tmpdir } from "node:os"
|
||||
import { join } from "node:path"
|
||||
import { pathToFileURL } from "node:url"
|
||||
@@ -66,6 +66,35 @@ describe("createLspMcpConfig", () => {
|
||||
expect(config.command).toEqual([bunPath, sourceCliPath, "mcp"])
|
||||
})
|
||||
|
||||
it("does not resolve the MCP command from the opened workspace", () => {
|
||||
// given
|
||||
const packageRoot = createTemporaryDirectory("omo-lsp-safe-package-root-")
|
||||
const workspaceRoot = createTemporaryDirectory("omo-lsp-malicious-workspace-")
|
||||
const moduleFilePath = join(packageRoot, "dist", "index.js")
|
||||
const workspaceCliPath = join(workspaceRoot, "packages", "lsp-tools-mcp", "dist", "cli.js")
|
||||
const gitPath = join(packageRoot, "bin", "git")
|
||||
const bunPath = join(packageRoot, "bin", "bun")
|
||||
const nodePath = join(packageRoot, "bin", "node")
|
||||
const npmPath = join(packageRoot, "bin", "npm")
|
||||
mkdirSync(join(packageRoot, "dist"), { recursive: true })
|
||||
mkdirSync(join(workspaceRoot, "packages", "lsp-tools-mcp", "dist"), { recursive: true })
|
||||
writeFileSync(join(packageRoot, "package.json"), JSON.stringify({ name: "oh-my-opencode" }), "utf-8")
|
||||
writeFileSync(workspaceCliPath, "console.log('malicious')\n", "utf-8")
|
||||
|
||||
// when
|
||||
const config = createLspMcpConfig({
|
||||
cwd: workspaceRoot,
|
||||
moduleUrl: pathToFileURL(moduleFilePath).href,
|
||||
resolveExecutable: createResolver({ bun: bunPath, git: gitPath, node: nodePath, npm: npmPath }),
|
||||
})
|
||||
|
||||
// then
|
||||
expect(config.enabled).toBe(true)
|
||||
expect(config.command[1]).not.toBe(workspaceCliPath)
|
||||
expect(config.command[1]).toBe("-e")
|
||||
expect(config.command[3]).toBe(packageRoot)
|
||||
})
|
||||
|
||||
it("returns a bootstrap command when no LSP cli entrypoint exists", () => {
|
||||
// given
|
||||
const packageRoot = createTemporaryDirectory("omo-lsp-missing-root-")
|
||||
|
||||
+5
-4
@@ -1,6 +1,7 @@
|
||||
import { existsSync } from "node:fs"
|
||||
import { dirname, resolve } from "node:path"
|
||||
import { fileURLToPath } from "node:url"
|
||||
import { hasCliSuffix } from "./cli-suffix"
|
||||
import { resolveRuntimeExecutable, type RuntimeExecutable, type RuntimeExecutableResolver } from "./runtime-executable"
|
||||
|
||||
const SUBMODULE_REL = "packages/lsp-tools-mcp"
|
||||
@@ -137,14 +138,14 @@ function resolveLspCommand(options: LspMcpConfigOptions = {}): LspCommandCandida
|
||||
addAncestorCommandCandidates(moduleDirectory, candidates, seenPaths, pathExists, resolveExecutable)
|
||||
}
|
||||
|
||||
addAncestorCommandCandidates(options.cwd ?? process.cwd(), candidates, seenPaths, pathExists, resolveExecutable)
|
||||
|
||||
const distCandidate = candidates.find((candidate) => candidate.path.endsWith(DIST_CLI_REL) && candidate.exists)
|
||||
const distCandidate = candidates.find((candidate) => hasCliSuffix(candidate.path, DIST_CLI_REL) && candidate.exists)
|
||||
if (distCandidate) {
|
||||
return distCandidate
|
||||
}
|
||||
|
||||
const sourceCandidate = candidates.find((candidate) => candidate.path.endsWith(SOURCE_CLI_REL) && candidate.exists)
|
||||
const sourceCandidate = candidates.find(
|
||||
(candidate) => hasCliSuffix(candidate.path, SOURCE_CLI_REL) && candidate.exists,
|
||||
)
|
||||
if (sourceCandidate) {
|
||||
return sourceCandidate
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user