From 24cbc9391368f14142a977ab73252cbb95cb07c3 Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 10 Jun 2026 08:04:47 +0200 Subject: [PATCH] fix(eval): thread python.interpreter from session settings and expand ~ Resolve the explicit interpreter from the session's Settings instance (ToolSession.settings / AgentSession.settings) instead of re-reading the process-global Settings.init() singleton, so project-scoped and cloned session settings take effect. The availability cache is now keyed by cwd + interpreter, and PythonKernel.start/executePython accept the resolved interpreter as an option. Also expand home-relative paths (~/...) before resolving against cwd, and document the contract of resolveExplicitPythonRuntime. Addresses review feedback on #2204. --- packages/coding-agent/src/eval/py/executor.ts | 11 +++++- packages/coding-agent/src/eval/py/index.ts | 7 +++- packages/coding-agent/src/eval/py/kernel.ts | 39 +++++++++++-------- packages/coding-agent/src/eval/py/runtime.ts | 18 ++++++++- .../coding-agent/src/session/agent-session.ts | 1 + packages/coding-agent/src/tools/index.ts | 7 +++- .../test/core/python-kernel-env.test.ts | 24 ++++++++++++ 7 files changed, 87 insertions(+), 20 deletions(-) diff --git a/packages/coding-agent/src/eval/py/executor.ts b/packages/coding-agent/src/eval/py/executor.ts index c33a0b25c..266be1ecc 100644 --- a/packages/coding-agent/src/eval/py/executor.ts +++ b/packages/coding-agent/src/eval/py/executor.ts @@ -42,6 +42,11 @@ export interface PythonExecutorOptions { kernelOwnerId?: string; /** Kernel mode (session reuse vs per-call) */ kernelMode?: PythonKernelMode; + /** + * Explicit interpreter path (`python.interpreter` resolved from the + * session's settings). Skips automatic runtime discovery when set. + */ + interpreter?: string; /** Restart the kernel before executing */ reset?: boolean; /** Session file path for accessing task outputs */ @@ -326,6 +331,7 @@ async function startKernel(cwd: string, options: PythonExecutorOptions): Promise env: buildKernelEnv(options), signal: options.signal, deadlineMs: options.deadlineMs, + interpreter: options.interpreter, }); } @@ -587,7 +593,10 @@ async function executeWithKernel( } async function ensureKernelAvailable(cwd: string, options: PythonExecutorOptions): Promise { - const availability = await waitForPromiseWithCancellation(checkPythonKernelAvailability(cwd), options); + const availability = await waitForPromiseWithCancellation( + checkPythonKernelAvailability(cwd, options.interpreter), + options, + ); if (!availability.ok) { throw new Error(availability.reason ?? "Python kernel unavailable"); } diff --git a/packages/coding-agent/src/eval/py/index.ts b/packages/coding-agent/src/eval/py/index.ts index c470b97ed..1aa8f6173 100644 --- a/packages/coding-agent/src/eval/py/index.ts +++ b/packages/coding-agent/src/eval/py/index.ts @@ -19,13 +19,17 @@ function readSetting(session: ToolSession, key: string): T | undefined { return settings?.get?.(key); } +function readInterpreterSetting(session: ToolSession): string | undefined { + return readSetting(session, "python.interpreter")?.trim() || undefined; +} + export default { id: "python", label: "Python", highlightLang: "python", async isAvailable(session: ToolSession): Promise { - const availability = await checkPythonKernelAvailability(session.cwd); + const availability = await checkPythonKernelAvailability(session.cwd, readInterpreterSetting(session)); return availability.ok; }, @@ -37,6 +41,7 @@ export default { signal: opts.signal, sessionId: namespaceSessionId(opts.sessionId), kernelMode, + interpreter: readInterpreterSetting(opts.session), sessionFile: opts.sessionFile, artifactsDir: opts.session.getArtifactsDir?.() ?? undefined, localRoots: resolveEvalUrlRoots(opts.session), diff --git a/packages/coding-agent/src/eval/py/kernel.ts b/packages/coding-agent/src/eval/py/kernel.ts index 6e7fbfc80..46b21c9dd 100644 --- a/packages/coding-agent/src/eval/py/kernel.ts +++ b/packages/coding-agent/src/eval/py/kernel.ts @@ -102,6 +102,11 @@ interface KernelLifecycleOptions { interface KernelStartOptions extends KernelLifecycleOptions { cwd: string; env?: Record; + /** + * Explicit interpreter path (`python.interpreter` from the session's + * settings). When set, runtime discovery is skipped entirely. + */ + interpreter?: string; } interface KernelShutdownOptions { @@ -135,20 +140,24 @@ function throwIfAborted(signal: AbortSignal | undefined, fallbackReason: string) throw createAbortError("AbortError", typeof reason === "string" ? reason : fallbackReason); } -// Cache successful probes per resolved cwd: every cell otherwise pays one (or -// two — backend.isAvailable + ensureKernelAvailable) interpreter spawns even -// when the kernel is already hot. Failures are not cached so installing a -// Python mid-session is picked up on the next attempt. +// Cache successful probes per resolved cwd + explicit interpreter: every cell +// otherwise pays one (or two — backend.isAvailable + ensureKernelAvailable) +// interpreter spawns even when the kernel is already hot. Failures are not +// cached so installing a Python mid-session is picked up on the next attempt. const availabilityCache = new Map>(); -export async function checkPythonKernelAvailability(cwd: string): Promise { +export async function checkPythonKernelAvailability( + cwd: string, + interpreter?: string, +): Promise { if (isBunTestRuntime() || $flag("PI_PYTHON_SKIP_CHECK")) { return { ok: true }; } - const key = path.resolve(cwd); + const resolvedCwd = path.resolve(cwd); + const key = `${resolvedCwd}\0${interpreter ?? ""}`; const cached = availabilityCache.get(key); if (cached) return await cached; - const probe = probePythonKernelAvailability(key); + const probe = probePythonKernelAvailability(resolvedCwd, interpreter); availabilityCache.set(key, probe); const result = await probe; if (!result.ok && availabilityCache.get(key) === probe) { @@ -157,14 +166,13 @@ export async function checkPythonKernelAvailability(cwd: string): Promise { +async function probePythonKernelAvailability(cwd: string, interpreter?: string): Promise { try { const settings = await Settings.init(); const { env } = settings.getShellConfig(); const baseEnv = filterEnv(env); - const explicitInterpreter = settings.get("python.interpreter")?.trim(); - const runtimes = explicitInterpreter - ? [resolveExplicitPythonRuntime(explicitInterpreter, cwd, baseEnv)] + const runtimes = interpreter + ? [resolveExplicitPythonRuntime(interpreter, cwd, baseEnv)] : enumeratePythonRuntimes(cwd, baseEnv); if (runtimes.length === 0) { return { ok: false, reason: "Python executable not found on PATH" }; @@ -248,6 +256,7 @@ export class PythonKernel { "PythonKernel.start:availabilityCheck", checkPythonKernelAvailability, options.cwd, + options.interpreter, ); if (!availability.ok) { throw new Error(availability.reason ?? "Python kernel unavailable"); @@ -259,11 +268,9 @@ export class PythonKernel { // PI_PYTHON_SKIP_CHECK), where no candidate was probed. let runtime = availability.runtime; if (!runtime) { - const settings = await Settings.init(); - const { env: shellEnv } = settings.getShellConfig(); - const explicitInterpreter = settings.get("python.interpreter")?.trim(); - runtime = explicitInterpreter - ? resolveExplicitPythonRuntime(explicitInterpreter, options.cwd, filterEnv(shellEnv)) + const { env: shellEnv } = (await Settings.init()).getShellConfig(); + runtime = options.interpreter + ? resolveExplicitPythonRuntime(options.interpreter, options.cwd, filterEnv(shellEnv)) : resolvePythonRuntime(options.cwd, filterEnv(shellEnv)); } const spawnEnv: Record = {}; diff --git a/packages/coding-agent/src/eval/py/runtime.ts b/packages/coding-agent/src/eval/py/runtime.ts index e5e89d0e0..367c9444a 100644 --- a/packages/coding-agent/src/eval/py/runtime.ts +++ b/packages/coding-agent/src/eval/py/runtime.ts @@ -5,6 +5,7 @@ * for both the shared gateway and local kernel spawning. */ import * as fs from "node:fs"; +import * as os from "node:os"; import * as path from "node:path"; import { $env, $which, getPythonEnvDir } from "@oh-my-pi/pi-utils"; @@ -191,18 +192,33 @@ function detectExplicitVenv(pythonPath: string): { venvPath: string; binDir: str return undefined; } +/** + * Resolve an explicitly configured interpreter (`python.interpreter`) into a + * runtime, bypassing discovery. Does not probe or validate the executable — + * callers must check it actually runs. `~` expands to the home directory and + * relative paths resolve against `cwd`. When the interpreter sits inside a + * virtualenv (a `pyvenv.cfg` above its bin dir), the venv activation env is + * applied so subprocesses and `pip` resolve consistently. + */ export function resolveExplicitPythonRuntime( interpreter: string, cwd: string, baseEnv: Record, ): PythonRuntime { - const pythonPath = path.isAbsolute(interpreter) ? interpreter : path.resolve(cwd, interpreter); + const expanded = + interpreter === "~" + ? os.homedir() + : interpreter.startsWith("~/") + ? path.join(os.homedir(), interpreter.slice(2)) + : interpreter; + const pythonPath = path.isAbsolute(expanded) ? expanded : path.resolve(cwd, expanded); const venv = detectExplicitVenv(pythonPath); if (venv) { return { pythonPath, env: applyVenvEnv(baseEnv, venv.venvPath, venv.binDir), venvPath: venv.venvPath }; } return { pythonPath, env: { ...baseEnv } }; } + /** * Enumerate candidate Python runtimes in priority order: an active/project venv, * the managed `~/.omp/python-env`, then the system interpreter on PATH. Every diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index a86fce63f..0c9e8f717 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -8690,6 +8690,7 @@ export class AgentSession { sessionId: namespacePythonSessionId(sessionId), kernelOwnerId: this.#evalKernelOwnerId, kernelMode: this.settings.get("python.kernelMode"), + interpreter: this.settings.get("python.interpreter")?.trim() || undefined, onChunk, signal: abortController.signal, }); diff --git a/packages/coding-agent/src/tools/index.ts b/packages/coding-agent/src/tools/index.ts index 1c6209e1a..349acc361 100644 --- a/packages/coding-agent/src/tools/index.ts +++ b/packages/coding-agent/src/tools/index.ts @@ -463,7 +463,12 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P !allowJs && (requestedTools === undefined || requestedTools.includes("eval")) ) { - const availability = await logger.time("createTools:pythonCheck", checkPythonKernelAvailability, session.cwd); + const availability = await logger.time( + "createTools:pythonCheck", + checkPythonKernelAvailability, + session.cwd, + session.settings.get("python.interpreter")?.trim() || undefined, + ); pythonAvailable = availability.ok; if (!availability.ok) { logger.warn("Python kernel unavailable and JS backend disabled; eval will be unavailable", { diff --git a/packages/coding-agent/test/core/python-kernel-env.test.ts b/packages/coding-agent/test/core/python-kernel-env.test.ts index cc257bb4f..b1dd5504f 100644 --- a/packages/coding-agent/test/core/python-kernel-env.test.ts +++ b/packages/coding-agent/test/core/python-kernel-env.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, vi } from "bun:test"; import * as fs from "node:fs"; +import * as os from "node:os"; import * as path from "node:path"; import { enumeratePythonRuntimes, @@ -126,6 +127,29 @@ describe("enumeratePythonRuntimes", () => { expect(runtime.env.VIRTUAL_ENV).toBe(venvDir); expect(runtime.env.PATH).toBe(`${binDir}${path.delimiter}${path.join(path.sep, "usr", "bin")}`); }); + + it("expands a home-relative explicit interpreter instead of resolving against cwd", () => { + const home = path.join(path.sep, "home", "tester"); + vi.spyOn(os, "homedir").mockReturnValue(home); + vi.spyOn(fs, "existsSync").mockReturnValue(false); + + const runtime = resolveExplicitPythonRuntime("~/venvs/py/bin/python", path.join(path.sep, "work"), {}); + + expect(runtime.pythonPath).toBe(path.join(home, "venvs", "py", "bin", "python")); + }); + + it("resolves a relative explicit interpreter against cwd", () => { + vi.spyOn(fs, "existsSync").mockReturnValue(false); + + const runtime = resolveExplicitPythonRuntime( + path.join(".venv", "bin", "python"), + path.join(path.sep, "work"), + {}, + ); + + expect(runtime.pythonPath).toBe(path.join(path.sep, "work", ".venv", "bin", "python")); + }); + it("throws from resolvePythonRuntime when no interpreter can be found", () => { vi.spyOn(piUtils, "getPythonEnvDir").mockReturnValue(managedDir); vi.spyOn(piUtils, "$which").mockReturnValue(null);