From ed6e9955dad0b82cecdc111f05124c829eb115d0 Mon Sep 17 00:00:00 2001 From: MoerAI Date: Wed, 20 May 2026 11:43:21 +0900 Subject: [PATCH] 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. --- src/mcp/ast-grep.ts | 8 +++++--- src/mcp/cli-suffix.test.ts | 32 ++++++++++++++++++++++++++++++++ src/mcp/cli-suffix.ts | 7 +++++++ src/mcp/lsp.test.ts | 31 ++++++++++++++++++++++++++++++- src/mcp/lsp.ts | 9 +++++---- 5 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 src/mcp/cli-suffix.test.ts create mode 100644 src/mcp/cli-suffix.ts diff --git a/src/mcp/ast-grep.ts b/src/mcp/ast-grep.ts index 6edd92e07..df53dc72c 100644 --- a/src/mcp/ast-grep.ts +++ b/src/mcp/ast-grep.ts @@ -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 }; } diff --git a/src/mcp/cli-suffix.test.ts b/src/mcp/cli-suffix.test.ts new file mode 100644 index 000000000..f0435d430 --- /dev/null +++ b/src/mcp/cli-suffix.test.ts @@ -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) + }) +}) diff --git a/src/mcp/cli-suffix.ts b/src/mcp/cli-suffix.ts new file mode 100644 index 000000000..a0d55a730 --- /dev/null +++ b/src/mcp/cli-suffix.ts @@ -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)) +} diff --git a/src/mcp/lsp.test.ts b/src/mcp/lsp.test.ts index f4b1c23bb..a04c1df09 100644 --- a/src/mcp/lsp.test.ts +++ b/src/mcp/lsp.test.ts @@ -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-") diff --git a/src/mcp/lsp.ts b/src/mcp/lsp.ts index 712c3cad8..e1b21e2bf 100644 --- a/src/mcp/lsp.ts +++ b/src/mcp/lsp.ts @@ -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 }