diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9af52954c..93d70e149 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -19,6 +19,9 @@ ### Fixed - Fixed subagent frontmatter `thinkingLevel` being overridden by `modelRoles.task` model suffixes. ([#3915](https://github.com/can1357/oh-my-pi/issues/3915)) +### Fixed + +- Fixed Ruff LSP auto-detection for Windows Python virtualenvs by checking `.venv/Scripts`, `venv/Scripts`, and `.env/Scripts` before falling back to PATH. ([#3916](https://github.com/can1357/oh-my-pi/issues/3916)) ## [16.2.9] - 2026-06-30 diff --git a/packages/coding-agent/src/lsp/config.ts b/packages/coding-agent/src/lsp/config.ts index 8a0a07a19..553510a60 100644 --- a/packages/coding-agent/src/lsp/config.ts +++ b/packages/coding-agent/src/lsp/config.ts @@ -209,13 +209,27 @@ export function hasRootMarkers(cwd: string, markers: string[]): boolean { * Local bin directories to check before $PATH, ordered by priority. * Each entry maps a root marker to the bin directory to check. */ +const PYTHON_ROOT_MARKERS = [ + "pyproject.toml", + "requirements.txt", + "setup.py", + "setup.cfg", + "Pipfile", + "pyrightconfig.json", + "ruff.toml", + ".ruff.toml", +]; + const LOCAL_BIN_PATHS: Array<{ markers: string[]; binDir: string }> = [ // Node.js - check node_modules/.bin/ { markers: ["package.json", "package-lock.json", "yarn.lock", "pnpm-lock.yaml"], binDir: "node_modules/.bin" }, // Python - check virtual environment bin directories - { markers: ["pyproject.toml", "requirements.txt", "setup.py", "Pipfile"], binDir: ".venv/bin" }, - { markers: ["pyproject.toml", "requirements.txt", "setup.py", "Pipfile"], binDir: "venv/bin" }, - { markers: ["pyproject.toml", "requirements.txt", "setup.py", "Pipfile"], binDir: ".env/bin" }, + { markers: PYTHON_ROOT_MARKERS, binDir: ".venv/bin" }, + { markers: PYTHON_ROOT_MARKERS, binDir: ".venv/Scripts" }, + { markers: PYTHON_ROOT_MARKERS, binDir: "venv/bin" }, + { markers: PYTHON_ROOT_MARKERS, binDir: "venv/Scripts" }, + { markers: PYTHON_ROOT_MARKERS, binDir: ".env/bin" }, + { markers: PYTHON_ROOT_MARKERS, binDir: ".env/Scripts" }, // Ruby - check vendor bundle and binstubs { markers: ["Gemfile", "Gemfile.lock"], binDir: "vendor/bundle/bin" }, { markers: ["Gemfile", "Gemfile.lock"], binDir: "bin" }, diff --git a/packages/coding-agent/test/task/executor-subagent-reminders.test.ts b/packages/coding-agent/test/task/executor-subagent-reminders.test.ts index b475eb4e8..eb0265eff 100644 --- a/packages/coding-agent/test/task/executor-subagent-reminders.test.ts +++ b/packages/coding-agent/test/task/executor-subagent-reminders.test.ts @@ -465,50 +465,6 @@ describe("runSubprocess yield reminders", () => { expect(createAgentSessionSpy).toHaveBeenCalledTimes(1); expect(createAgentSessionSpy.mock.calls[0]?.[0]?.thinkingLevel).toBe(Effort.High); }); - - it("prefers explicit modelOverride thinking suffix over provided thinking level, including off", async () => { - vi.clearAllMocks(); - const modelRegistry = { - refresh: async () => {}, - getAvailable: () => [{ provider: "openai", id: "gpt-4o", name: "GPT-4o" }], - } as unknown as import("@oh-my-pi/pi-coding-agent/config/model-registry").ModelRegistry; - - const cases = [ - { modelOverride: "openai/gpt-4o:low", expectedThinkingLevel: Effort.Low }, - { modelOverride: "openai/gpt-4o:off", expectedThinkingLevel: "off" }, - ] as const; - - const createAgentSessionSpy = vi.spyOn(sdkModule, "createAgentSession"); - - for (const [index, testCase] of cases.entries()) { - const session = createMockSession(({ emit }) => { - emit({ - type: "tool_execution_end", - toolCallId: `tool-thinking-override-${index}`, - toolName: "yield", - result: { - content: [{ type: "text", text: "Result submitted." }], - details: { status: "success", data: { ok: true } }, - }, - isError: false, - }); - }); - - createAgentSessionSpy.mockResolvedValue(createSessionResult(session)); - - await runSubprocess({ - ...baseOptions, - id: `subagent-thinking-override-${index}`, - modelOverride: testCase.modelOverride, - thinkingLevel: Effort.High, - modelRegistry, - }); - } - - expect(createAgentSessionSpy).toHaveBeenCalledTimes(2); - expect(createAgentSessionSpy.mock.calls[0]?.[0]?.thinkingLevel).toBe(cases[0].expectedThinkingLevel); - expect(createAgentSessionSpy.mock.calls[1]?.[0]?.thinkingLevel).toBe(cases[1].expectedThinkingLevel); - }); it("fails after 3 reminders when yield is never called for a structured task", async () => { const prompts: string[] = []; const session = createMockSession(({ text, promptIndex, emit, state }) => { diff --git a/packages/coding-agent/test/tools/lsp-regressions.test.ts b/packages/coding-agent/test/tools/lsp-regressions.test.ts index 284e9e376..ca5851fba 100644 --- a/packages/coding-agent/test/tools/lsp-regressions.test.ts +++ b/packages/coding-agent/test/tools/lsp-regressions.test.ts @@ -1077,6 +1077,91 @@ describe("lsp regressions", () => { } }); + it("detects Ruff in Windows virtualenv Scripts directories", async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, "platform", { value: "win32", configurable: true, writable: true }); + + const tempDir = TempDir.createSync("@omp-lsp-win32-ruff-"); + const whichSpy = vi.spyOn(Bun, "which").mockReturnValue(null); + + try { + await Bun.write(path.join(tempDir.path(), "pyproject.toml"), '[project]\nname = "demo"\n'); + const scriptsDir = path.join(tempDir.path(), ".venv", "Scripts"); + await fs.promises.mkdir(scriptsDir, { recursive: true }); + const localRuff = path.join(scriptsDir, "ruff.exe"); + await Bun.write(localRuff, ""); + + const config = loadConfig(tempDir.path()); + expect(config.servers.ruff?.resolvedCommand).toBe(localRuff); + expect(whichSpy).not.toHaveBeenCalledWith("ruff"); + } finally { + Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true, writable: true }); + vi.restoreAllMocks(); + tempDir.removeSync(); + } + }); + + it("detects Ruff in Windows virtualenv Scripts directories for Ruff-only roots", async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, "platform", { value: "win32", configurable: true, writable: true }); + const whichSpy = vi.spyOn(Bun, "which").mockReturnValue(null); + + try { + for (const marker of ["ruff.toml", ".ruff.toml"] as const) { + const tempDir = TempDir.createSync("@omp-lsp-win32-ruff-marker-"); + try { + await Bun.write(path.join(tempDir.path(), marker), ""); + const scriptsDir = path.join(tempDir.path(), ".venv", "Scripts"); + await fs.promises.mkdir(scriptsDir, { recursive: true }); + const localRuff = path.join(scriptsDir, "ruff.exe"); + await Bun.write(localRuff, ""); + + const config = loadConfig(tempDir.path()); + expect(config.servers.ruff?.resolvedCommand).toBe(localRuff); + } finally { + tempDir.removeSync(); + } + } + expect(whichSpy).not.toHaveBeenCalledWith("ruff"); + } finally { + Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true, writable: true }); + vi.restoreAllMocks(); + } + }); + + it("detects pyright and pylsp in Windows virtualenv Scripts for Python-only roots", async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, "platform", { value: "win32", configurable: true, writable: true }); + const whichSpy = vi.spyOn(Bun, "which").mockReturnValue(null); + + try { + const cases: Array<{ marker: string; server: string; binary: string }> = [ + { marker: "pyrightconfig.json", server: "pyright", binary: "pyright-langserver.exe" }, + { marker: "setup.cfg", server: "pylsp", binary: "pylsp.exe" }, + ]; + for (const { marker, server, binary } of cases) { + const tempDir = TempDir.createSync("@omp-lsp-win32-py-marker-"); + try { + await Bun.write(path.join(tempDir.path(), marker), ""); + const scriptsDir = path.join(tempDir.path(), ".venv", "Scripts"); + await fs.promises.mkdir(scriptsDir, { recursive: true }); + const localBin = path.join(scriptsDir, binary); + await Bun.write(localBin, ""); + + const config = loadConfig(tempDir.path()); + expect(config.servers[server]?.resolvedCommand).toBe(localBin); + } finally { + tempDir.removeSync(); + } + } + expect(whichSpy).not.toHaveBeenCalledWith("pyright-langserver"); + expect(whichSpy).not.toHaveBeenCalledWith("pylsp"); + } finally { + Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true, writable: true }); + vi.restoreAllMocks(); + } + }); + it("detects tlaplus files for LSP startup and language ids", async () => { const tempDir = TempDir.createSync("@omp-lsp-tlaplus-"); const specPath = path.join(tempDir.path(), "Spec.tla");