fix(coding-agent): probed all Python runtimes to bypass broken managed env
- Replaced single-candidate resolution with `enumeratePythonRuntimes`, returning venv, managed env, and system interpreter in priority order. - Availability check now probes each candidate and falls through to the first that executes, so a stale `uv`-managed Python no longer fails the whole session. - Kernel spawn reuses the probed runtime from the availability result instead of re-resolving independently. - Expanded test coverage for enumeration, fallback, and env isolation between candidates.
This commit is contained in:
@@ -10,6 +10,7 @@
|
||||
- Added a `/switch` slash command that opens the temporary model selector for the current session, mirroring the `alt+p` keybinding.
|
||||
- Added `replace block N:` and `delete block N` operators to the `edit` tool: they resolve the syntactic block beginning on line N via tree-sitter (native `blockRangeAt`) and replace or delete its full line span, so a construct can be rewritten or removed without counting its closing line. Unresolvable blocks (unsupported language, blank/closing-delimiter line, or a parse error) are rejected with guidance to use an explicit `replace N..M:` / `delete N..M` range.
|
||||
- Added an animated pending border for `bash` and `eval` execution blocks: while a command/cell is running, a single dark segment glides clockwise around the block's outer edge (top → right → bottom → left), replacing the previous static accent border. Motion is eased per edge (decelerating into each corner) and timed against a fixed lap duration mapped onto the live perimeter, so streaming a new output line or resizing the terminal nudges the segment proportionally instead of resetting its position. Driven by the existing spinner cadence and gated on the `display.shimmer` setting (no motion when `disabled`).
|
||||
- Added a `PI_TINY_DTYPE` environment variable that overrides the ONNX quantization/precision used for local tiny models (session titles and Mnemosyne memory tasks), mirroring `PI_TINY_DEVICE`. Unset keeps each model's shipped dtype (currently `q4`); `PI_TINY_DTYPE=fp16` trades speed for fidelity and `PI_TINY_DTYPE=q8` is also accepted, alongside `auto`, `fp32`, `int8`, `uint8`, `bnb4`, `q4f16`, `q2`, `q2f16`, `q1`, and `q1f16`. An unrecognized value fails loudly at worker startup instead of silently loading a different precision.
|
||||
|
||||
### Changed
|
||||
|
||||
@@ -27,6 +28,7 @@
|
||||
- Fixed the `read` tool description advertising `inspect_image` ("for visual analysis, call `inspect_image`") even when the `inspect_image` tool was disabled, which left the model hunting for a tool absent from its function list. The image section is now gated on `inspect_image.enabled`: when disabled it instead states that reading an image path returns the decoded image inline.
|
||||
- Fixed session-title generation latching onto literal text inside fenced code blocks — a pasted UI mockup containing "Welcome to Claude Code v2.1.158" titled the session "Setup Screen for Claude Code v2.1.158" instead of capturing the actual request. The first user message now has fenced code blocks stripped before titling (both the online `pi/smol` and local on-device model paths share the same preprocessing), with a fallback to the original message when stripping would leave too little to title from (e.g. a message that is essentially just a code block).
|
||||
- Fixed slash-command autocomplete repaint requests so Windows Terminal sessions with unknown native viewport state keep updating the input box and candidate list. ([#1550](https://github.com/can1357/oh-my-pi/issues/1550))
|
||||
- Fixed Python `eval` failing the whole session when the managed `~/.omp/python-env` interpreter exists on disk but no longer runs (e.g. a stale `uv`-managed Python that was removed or upgraded). Availability resolution now enumerates every candidate — active/project venv, the managed env, then the system interpreter — and probes each in priority order, falling through to the first that actually executes instead of failing fast on the first resolved path. The kernel spawns whichever interpreter the probe selected, so a working system Python takes over transparently.
|
||||
|
||||
### Removed
|
||||
|
||||
|
||||
@@ -17,7 +17,7 @@ import { Settings } from "../../config/settings";
|
||||
import { type KernelDisplayOutput, renderKernelDisplay } from "./display";
|
||||
import { PYTHON_PRELUDE } from "./prelude";
|
||||
import RUNNER_SCRIPT from "./runner.py" with { type: "text" };
|
||||
import { filterEnv, resolvePythonRuntime } from "./runtime";
|
||||
import { enumeratePythonRuntimes, filterEnv, type PythonRuntime, resolvePythonRuntime } from "./runtime";
|
||||
|
||||
export type { KernelDisplayOutput, PythonStatusEvent } from "./display";
|
||||
export { renderKernelDisplay } from "./display";
|
||||
@@ -106,6 +106,8 @@ export interface PythonKernelAvailability {
|
||||
ok: boolean;
|
||||
pythonPath?: string;
|
||||
reason?: string;
|
||||
/** The probed-working runtime, when one was found. */
|
||||
runtime?: PythonRuntime;
|
||||
}
|
||||
|
||||
function getRemainingTimeMs(deadlineMs?: number): number | undefined {
|
||||
@@ -134,19 +136,34 @@ export async function checkPythonKernelAvailability(cwd: string): Promise<Python
|
||||
const settings = await Settings.init();
|
||||
const { env } = settings.getShellConfig();
|
||||
const baseEnv = filterEnv(env);
|
||||
const runtime = resolvePythonRuntime(cwd, baseEnv);
|
||||
const probe = await $`${runtime.pythonPath} -c "import sys;sys.exit(0)"`
|
||||
.quiet()
|
||||
.nothrow()
|
||||
.cwd(cwd)
|
||||
.env(runtime.env);
|
||||
if (probe.exitCode === 0) {
|
||||
return { ok: true, pythonPath: runtime.pythonPath };
|
||||
const runtimes = enumeratePythonRuntimes(cwd, baseEnv);
|
||||
if (runtimes.length === 0) {
|
||||
return { ok: false, reason: "Python executable not found on PATH" };
|
||||
}
|
||||
// Probe each candidate in priority order and use the first that actually
|
||||
// runs. A managed env left behind by a removed `uv` install can exist on
|
||||
// disk yet fail to execute; falling through to the next candidate lets a
|
||||
// working system Python take over instead of failing the whole session.
|
||||
const failures: string[] = [];
|
||||
for (const runtime of runtimes) {
|
||||
try {
|
||||
const probe = await $`${runtime.pythonPath} -c "import sys;sys.exit(0)"`
|
||||
.quiet()
|
||||
.nothrow()
|
||||
.cwd(cwd)
|
||||
.env(runtime.env);
|
||||
if (probe.exitCode === 0) {
|
||||
return { ok: true, pythonPath: runtime.pythonPath, runtime };
|
||||
}
|
||||
failures.push(`${runtime.pythonPath} (exit code ${probe.exitCode})`);
|
||||
} catch (err) {
|
||||
failures.push(`${runtime.pythonPath} (${err instanceof Error ? err.message : String(err)})`);
|
||||
}
|
||||
}
|
||||
return {
|
||||
ok: false,
|
||||
pythonPath: runtime.pythonPath,
|
||||
reason: `Python interpreter at ${runtime.pythonPath} returned exit code ${probe.exitCode}`,
|
||||
pythonPath: runtimes[0].pythonPath,
|
||||
reason: `No working Python interpreter found. Tried: ${failures.join("; ")}`,
|
||||
};
|
||||
} catch (err) {
|
||||
return { ok: false, reason: err instanceof Error ? err.message : String(err) };
|
||||
@@ -207,10 +224,15 @@ export class PythonKernel {
|
||||
throw new Error(availability.reason ?? "Python kernel unavailable");
|
||||
}
|
||||
|
||||
const settings = await Settings.init();
|
||||
const { env: shellEnv } = settings.getShellConfig();
|
||||
const baseEnv = filterEnv(shellEnv);
|
||||
const runtime = resolvePythonRuntime(options.cwd, baseEnv);
|
||||
// Reuse the interpreter the availability probe selected so the spawned
|
||||
// kernel matches what we verified actually runs. The fallback computes a
|
||||
// runtime only for the skip-check fast path (test runtime /
|
||||
// PI_PYTHON_SKIP_CHECK), where no candidate was probed.
|
||||
let runtime = availability.runtime;
|
||||
if (!runtime) {
|
||||
const { env: shellEnv } = (await Settings.init()).getShellConfig();
|
||||
runtime = resolvePythonRuntime(options.cwd, filterEnv(shellEnv));
|
||||
}
|
||||
const spawnEnv: Record<string, string> = {};
|
||||
for (const [key, value] of Object.entries(runtime.env)) {
|
||||
if (typeof value === "string") spawnEnv[key] = value;
|
||||
|
||||
@@ -162,49 +162,78 @@ export function resolveVenvPath(cwd: string): string | undefined {
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve Python runtime including executable path, environment, and venv detection.
|
||||
* Apply a venv-style PATH/VIRTUAL_ENV layout onto a fresh copy of `baseEnv` for
|
||||
* the interpreter living in `binDir`.
|
||||
*/
|
||||
export function resolvePythonRuntime(cwd: string, baseEnv: Record<string, string | undefined>): PythonRuntime {
|
||||
function applyVenvEnv(
|
||||
baseEnv: Record<string, string | undefined>,
|
||||
venvPath: string,
|
||||
binDir: string,
|
||||
): Record<string, string | undefined> {
|
||||
const env = { ...baseEnv };
|
||||
const venvPath = env.VIRTUAL_ENV ?? resolveVenvPath(cwd);
|
||||
env.VIRTUAL_ENV = venvPath;
|
||||
const pathKey = resolvePathKey(env);
|
||||
const currentPath = env[pathKey];
|
||||
env[pathKey] = currentPath ? `${binDir}${path.delimiter}${currentPath}` : binDir;
|
||||
return env;
|
||||
}
|
||||
|
||||
function venvBinDir(venvPath: string): string {
|
||||
return process.platform === "win32" ? path.join(venvPath, "Scripts") : path.join(venvPath, "bin");
|
||||
}
|
||||
|
||||
/**
|
||||
* Enumerate candidate Python runtimes in priority order: an active/project venv,
|
||||
* the managed `~/.omp/python-env`, then the system interpreter on PATH. Every
|
||||
* candidate that physically exists is returned so callers can probe each in turn
|
||||
* rather than committing to the first — a managed env left behind by a removed
|
||||
* `uv` install no longer shadows a working system Python.
|
||||
*/
|
||||
export function enumeratePythonRuntimes(cwd: string, baseEnv: Record<string, string | undefined>): PythonRuntime[] {
|
||||
const runtimes: PythonRuntime[] = [];
|
||||
const seen = new Set<string>();
|
||||
const push = (runtime: PythonRuntime): void => {
|
||||
if (seen.has(runtime.pythonPath)) return;
|
||||
seen.add(runtime.pythonPath);
|
||||
runtimes.push(runtime);
|
||||
};
|
||||
|
||||
const venvPath = baseEnv.VIRTUAL_ENV ?? resolveVenvPath(cwd);
|
||||
if (venvPath) {
|
||||
env.VIRTUAL_ENV = venvPath;
|
||||
const binDir = process.platform === "win32" ? path.join(venvPath, "Scripts") : path.join(venvPath, "bin");
|
||||
const binDir = venvBinDir(venvPath);
|
||||
const pythonCandidate = path.join(binDir, process.platform === "win32" ? "python.exe" : "python");
|
||||
if (fs.existsSync(pythonCandidate)) {
|
||||
const pathKey = resolvePathKey(env);
|
||||
const currentPath = env[pathKey];
|
||||
env[pathKey] = currentPath ? `${binDir}${path.delimiter}${currentPath}` : binDir;
|
||||
return {
|
||||
pythonPath: pythonCandidate,
|
||||
env,
|
||||
venvPath,
|
||||
};
|
||||
push({ pythonPath: pythonCandidate, env: applyVenvEnv(baseEnv, venvPath, binDir), venvPath });
|
||||
}
|
||||
}
|
||||
|
||||
const managed = resolveManagedPythonCandidate();
|
||||
if (fs.existsSync(managed.pythonPath)) {
|
||||
env.VIRTUAL_ENV = managed.venvPath;
|
||||
const pathKey = resolvePathKey(env);
|
||||
const currentPath = env[pathKey];
|
||||
const managedBin =
|
||||
process.platform === "win32" ? path.join(managed.venvPath, "Scripts") : path.join(managed.venvPath, "bin");
|
||||
env[pathKey] = currentPath ? `${managedBin}${path.delimiter}${currentPath}` : managedBin;
|
||||
return {
|
||||
const managedBin = path.dirname(managed.pythonPath);
|
||||
push({
|
||||
pythonPath: managed.pythonPath,
|
||||
env,
|
||||
env: applyVenvEnv(baseEnv, managed.venvPath, managedBin),
|
||||
venvPath: managed.venvPath,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
const pythonPath = $which("python") ?? $which("python3");
|
||||
if (!pythonPath) {
|
||||
const systemPath = $which("python") ?? $which("python3");
|
||||
if (systemPath) {
|
||||
push({ pythonPath: systemPath, env: { ...baseEnv } });
|
||||
}
|
||||
|
||||
return runtimes;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the highest-priority Python runtime. Prefer {@link enumeratePythonRuntimes}
|
||||
* when you can probe candidates; this returns only the first one and throws when
|
||||
* no interpreter exists.
|
||||
*/
|
||||
export function resolvePythonRuntime(cwd: string, baseEnv: Record<string, string | undefined>): PythonRuntime {
|
||||
const [runtime] = enumeratePythonRuntimes(cwd, baseEnv);
|
||||
if (!runtime) {
|
||||
throw new Error("Python executable not found on PATH");
|
||||
}
|
||||
return {
|
||||
pythonPath,
|
||||
env,
|
||||
};
|
||||
return runtime;
|
||||
}
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { filterEnv } from "@oh-my-pi/pi-coding-agent/eval/py/runtime";
|
||||
import { afterEach, describe, expect, it, vi } from "bun:test";
|
||||
import * as fs from "node:fs";
|
||||
import * as path from "node:path";
|
||||
import { enumeratePythonRuntimes, filterEnv, resolvePythonRuntime } from "@oh-my-pi/pi-coding-agent/eval/py/runtime";
|
||||
import * as piUtils from "@oh-my-pi/pi-utils";
|
||||
|
||||
describe("Python gateway environment filtering", () => {
|
||||
it("filters sensitive and unknown variables from shell env", () => {
|
||||
@@ -42,3 +45,59 @@ describe("Python gateway environment filtering", () => {
|
||||
expect(filtered.LC_MESSAGES).toBe("en_US.UTF-8");
|
||||
});
|
||||
});
|
||||
|
||||
describe("enumeratePythonRuntimes", () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
const managedDir = path.join(path.sep, "fake", ".omp", "python-env");
|
||||
const managedBin = path.join(managedDir, process.platform === "win32" ? "Scripts" : "bin");
|
||||
const managedPy = path.join(managedBin, process.platform === "win32" ? "python.exe" : "python");
|
||||
const systemPy = path.join(path.sep, "usr", "bin", "python3");
|
||||
|
||||
it("enumerates the managed env AND the system interpreter so a broken managed env can fall through", () => {
|
||||
vi.spyOn(piUtils, "getPythonEnvDir").mockReturnValue(managedDir);
|
||||
vi.spyOn(piUtils, "$which").mockImplementation(bin => (bin === "python" ? systemPy : null));
|
||||
// Only the managed interpreter physically exists; no project/active venv.
|
||||
vi.spyOn(fs, "existsSync").mockImplementation(candidate => candidate === managedPy);
|
||||
|
||||
const runtimes = enumeratePythonRuntimes(path.join(path.sep, "work"), {
|
||||
PATH: path.join(path.sep, "usr", "bin"),
|
||||
});
|
||||
|
||||
expect(runtimes.map(r => r.pythonPath)).toEqual([managedPy, systemPy]);
|
||||
|
||||
const [managed, system] = runtimes;
|
||||
expect(managed.venvPath).toBe(managedDir);
|
||||
expect(managed.env.VIRTUAL_ENV).toBe(managedDir);
|
||||
expect(managed.env.PATH).toBe(`${managedBin}${path.delimiter}${path.join(path.sep, "usr", "bin")}`);
|
||||
|
||||
// The system candidate must not inherit the managed env's VIRTUAL_ENV/PATH mutation.
|
||||
expect(system.venvPath).toBeUndefined();
|
||||
expect(system.env.VIRTUAL_ENV).toBeUndefined();
|
||||
expect(system.env.PATH).toBe(path.join(path.sep, "usr", "bin"));
|
||||
});
|
||||
|
||||
it("falls back to the system interpreter when no managed env or venv exists", () => {
|
||||
vi.spyOn(piUtils, "getPythonEnvDir").mockReturnValue(managedDir);
|
||||
vi.spyOn(piUtils, "$which").mockImplementation(bin => (bin === "python" ? systemPy : null));
|
||||
vi.spyOn(fs, "existsSync").mockReturnValue(false);
|
||||
|
||||
const runtimes = enumeratePythonRuntimes(path.join(path.sep, "work"), {
|
||||
PATH: path.join(path.sep, "usr", "bin"),
|
||||
});
|
||||
|
||||
expect(runtimes.map(r => r.pythonPath)).toEqual([systemPy]);
|
||||
expect(resolvePythonRuntime(path.join(path.sep, "work"), {}).pythonPath).toBe(systemPy);
|
||||
});
|
||||
|
||||
it("throws from resolvePythonRuntime when no interpreter can be found", () => {
|
||||
vi.spyOn(piUtils, "getPythonEnvDir").mockReturnValue(managedDir);
|
||||
vi.spyOn(piUtils, "$which").mockReturnValue(null);
|
||||
vi.spyOn(fs, "existsSync").mockReturnValue(false);
|
||||
|
||||
expect(enumeratePythonRuntimes(path.join(path.sep, "work"), {})).toEqual([]);
|
||||
expect(() => resolvePythonRuntime(path.join(path.sep, "work"), {})).toThrow("Python executable not found");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -20,8 +20,8 @@ import {
|
||||
type BenchmarkConfig,
|
||||
type BenchmarkResult,
|
||||
buildBenchmarkResult,
|
||||
percentile,
|
||||
type ProgressEvent,
|
||||
percentile,
|
||||
runBenchmark,
|
||||
} from "./runner";
|
||||
import { type EditTask, loadTasksFromDir, validateFixturesFromDir } from "./tasks";
|
||||
|
||||
Reference in New Issue
Block a user