mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-10-30 17:27:09 +00:00
fix(cli): surface a warning when doctor's HYPERFRAMES_PYTHON override is rejected (#3870)
## Summary - `findPython()` correctly resolves `HYPERFRAMES_PYTHON` before falling back to the PATH probe — that part already works, confirmed by direct testing. But when the override is set and fails validation (nonexistent path, non-executable, non-Python-3 output, timeout), it silently falls through to the PATH probe with zero diagnostic. A user whose override had any subtle issue got a plain "Not installed" from `doctor` with no signal the variable was even seen. - This is a corrected, narrower version of a report that originally claimed `doctor` ignores `HYPERFRAMES_PYTHON` entirely — that claim was refuted directly (the override resolution works). The real defect is the silent validation-failure path. - Extracts the override-validation logic into `validatePythonOverride()` and adds `describeRejectedPythonOverride()`, which `doctor`'s TTS (Kokoro) and BGM (MusicGen) checks now call to append the rejection reason to their `detail` when applicable. `findPython()`'s own behavior (including its fallback) is unchanged. PRINFRA-669 ## Test plan - [x] New `packages/cli/src/tts/python.test.ts`: `describeRejectedPythonOverride` returns null when unset / when the override validates; names the override + exception message when the override can't run; names the override + actual output when it isn't Python 3; `findPython` still falls back to the PATH probe when the override is rejected (unchanged behavior) and still uses a valid override directly. - [x] Confirmed RED against the pre-fix source (tagged stash) — all 4 new `describeRejectedPythonOverride` tests failed with "not a function"; GREEN after restoring the fix. - [x] `bunx tsc --noEmit`, `bunx oxlint`, `bunx oxfmt --write` clean on changed files. - [x] `bunx fallow audit --base origin/main --fail-on-issues`: no issues in the 3 changed files. - [x] Full `packages/cli` vitest suite: 3033/3038 passing (2 pre-existing unrelated failures — a PID/socket sandbox quirk and an agent-env-var-pollution test — plus 4 browser-test files failing at collection on a pre-existing `node:` builtin import issue under this sandbox's happy-dom setup; all confirmed identical on pristine `origin/main` and unrelated to this change, consistent with every other fix from this backlog-drain session). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This commit is contained in:
@@ -9,7 +9,7 @@ import type { Example } from "./_examples.js";
|
||||
import { c } from "../ui/colors.js";
|
||||
import { parseToolVersion, runEnvironmentChecks } from "../browser/preflight.js";
|
||||
import { KOKORO_MODULES, KOKORO_PIP, MUSICGEN_MODULES, MUSICGEN_PIP } from "../audio/providers.js";
|
||||
import { hasPythonModules } from "../tts/python.js";
|
||||
import { hasPythonModules, describeRejectedPythonOverride } from "../tts/python.js";
|
||||
import { VERSION } from "../version.js";
|
||||
import { getUpdateMeta, withMeta } from "../utils/updateCheck.js";
|
||||
import {
|
||||
@@ -258,11 +258,16 @@ async function checkWhisper(): Promise<CheckResult> {
|
||||
};
|
||||
}
|
||||
|
||||
function notInstalledDetail(base: string): string {
|
||||
const overrideRejection = describeRejectedPythonOverride();
|
||||
return overrideRejection ? `${base}. ${overrideRejection}` : base;
|
||||
}
|
||||
|
||||
function checkLocalVoice(): CheckResult {
|
||||
if (hasPythonModules(KOKORO_MODULES)) return { ok: true, detail: "Kokoro deps installed" };
|
||||
return {
|
||||
ok: false,
|
||||
detail: "Not installed (optional \u2014 local voice fallback)",
|
||||
detail: notInstalledDetail("Not installed (optional \u2014 local voice fallback)"),
|
||||
hint: KOKORO_PIP,
|
||||
};
|
||||
}
|
||||
@@ -271,7 +276,7 @@ function checkLocalMusic(): CheckResult {
|
||||
if (hasPythonModules(MUSICGEN_MODULES)) return { ok: true, detail: "MusicGen deps installed" };
|
||||
return {
|
||||
ok: false,
|
||||
detail: "Not installed (optional \u2014 local music fallback)",
|
||||
detail: notInstalledDetail("Not installed (optional \u2014 local music fallback)"),
|
||||
hint: MUSICGEN_PIP,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
// Regression coverage for a silent HYPERFRAMES_PYTHON override rejection:
|
||||
// findPython() correctly resolves the env override first, but previously
|
||||
// discarded ANY validation failure (nonexistent path, non-executable,
|
||||
// non-Python-3 output, timeout) via a bare `catch {}` with zero diagnostic,
|
||||
// silently falling back to the PATH probe. A user with a subtly wrong
|
||||
// HYPERFRAMES_PYTHON value had no way to know it was even seen, let alone
|
||||
// why it was rejected.
|
||||
|
||||
const execFileSyncMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("node:child_process", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("node:child_process")>();
|
||||
return { ...actual, execFileSync: execFileSyncMock };
|
||||
});
|
||||
|
||||
const { findPython, describeRejectedPythonOverride } = await import("./python.js");
|
||||
|
||||
describe("findPython / describeRejectedPythonOverride", () => {
|
||||
beforeEach(() => {
|
||||
execFileSyncMock.mockReset();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
delete process.env.HYPERFRAMES_PYTHON;
|
||||
});
|
||||
|
||||
it("describeRejectedPythonOverride returns null when HYPERFRAMES_PYTHON is unset", () => {
|
||||
delete process.env.HYPERFRAMES_PYTHON;
|
||||
expect(describeRejectedPythonOverride()).toBeNull();
|
||||
expect(execFileSyncMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("describeRejectedPythonOverride returns null when the override is a valid Python 3", () => {
|
||||
process.env.HYPERFRAMES_PYTHON = "/opt/venv/bin/python";
|
||||
execFileSyncMock.mockReturnValue("Python 3.11.0");
|
||||
expect(describeRejectedPythonOverride()).toBeNull();
|
||||
});
|
||||
|
||||
it("describeRejectedPythonOverride names the override and the exception when it cannot run", () => {
|
||||
process.env.HYPERFRAMES_PYTHON = "/nonexistent/python";
|
||||
execFileSyncMock.mockImplementation(() => {
|
||||
throw new Error("ENOENT: no such file or directory");
|
||||
});
|
||||
const message = describeRejectedPythonOverride();
|
||||
expect(message).toContain("/nonexistent/python");
|
||||
expect(message).toContain("ENOENT");
|
||||
});
|
||||
|
||||
it("describeRejectedPythonOverride names the override when --version doesn't report Python 3", () => {
|
||||
process.env.HYPERFRAMES_PYTHON = "/opt/venv/bin/python2";
|
||||
execFileSyncMock.mockReturnValue("Python 2.7.18");
|
||||
const message = describeRejectedPythonOverride();
|
||||
expect(message).toContain("/opt/venv/bin/python2");
|
||||
expect(message).toContain("Python 2.7.18");
|
||||
});
|
||||
|
||||
it("findPython still falls back to the PATH probe when the override is rejected", () => {
|
||||
process.env.HYPERFRAMES_PYTHON = "/nonexistent/python";
|
||||
execFileSyncMock.mockImplementation((cmd: string, args: string[]) => {
|
||||
if (args[0] === "--version" && cmd === "/nonexistent/python") {
|
||||
throw new Error("ENOENT: no such file or directory");
|
||||
}
|
||||
if (cmd === "which" || cmd === "where") return "/usr/bin/python3\n";
|
||||
if (args[0] === "--version") return "Python 3.11.0";
|
||||
throw new Error(`unexpected call: ${cmd} ${args.join(" ")}`);
|
||||
});
|
||||
expect(findPython()).toBe("/usr/bin/python3");
|
||||
});
|
||||
|
||||
it("findPython uses the override directly when it validates", () => {
|
||||
process.env.HYPERFRAMES_PYTHON = "/opt/venv/bin/python";
|
||||
execFileSyncMock.mockReturnValue("Python 3.11.0");
|
||||
expect(findPython()).toBe("/opt/venv/bin/python");
|
||||
expect(execFileSyncMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -8,21 +8,30 @@
|
||||
|
||||
import { execFileSync } from "node:child_process";
|
||||
|
||||
type PythonOverrideValidation = { ok: true } | { ok: false; reason: string };
|
||||
|
||||
/** Run `<override> --version` and check it reports Python 3, without throwing. */
|
||||
function validatePythonOverride(override: string): PythonOverrideValidation {
|
||||
try {
|
||||
const version = execFileSync(override, ["--version"], {
|
||||
encoding: "utf-8",
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
timeout: 5000,
|
||||
});
|
||||
if (/Python 3/.test(version)) return { ok: true };
|
||||
return {
|
||||
ok: false,
|
||||
reason: `did not report a Python 3 version (got ${JSON.stringify(version.trim())})`,
|
||||
};
|
||||
} catch (error) {
|
||||
return { ok: false, reason: error instanceof Error ? error.message : String(error) };
|
||||
}
|
||||
}
|
||||
|
||||
/** Locate a Python 3: `HYPERFRAMES_PYTHON` env override first, then PATH. */
|
||||
export function findPython(): string | undefined {
|
||||
const override = process.env.HYPERFRAMES_PYTHON;
|
||||
if (override) {
|
||||
try {
|
||||
const version = execFileSync(override, ["--version"], {
|
||||
encoding: "utf-8",
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
timeout: 5000,
|
||||
});
|
||||
if (/Python 3/.test(version)) return override;
|
||||
} catch {
|
||||
// fall through to the PATH probe
|
||||
}
|
||||
}
|
||||
if (override && validatePythonOverride(override).ok) return override;
|
||||
for (const name of ["python3", "python"]) {
|
||||
try {
|
||||
const cmd = process.platform === "win32" ? "where" : "which";
|
||||
@@ -52,6 +61,22 @@ export function findPython(): string | undefined {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* If `HYPERFRAMES_PYTHON` is set but `findPython()` would reject it, describe
|
||||
* why. `findPython()` itself silently falls back to the PATH probe on any
|
||||
* rejection (nonexistent path, non-executable, non-Python-3 output, timeout)
|
||||
* with no diagnostic — a readiness check like `doctor` calls this to surface
|
||||
* that instead of reporting a plain "not installed" that gives no hint the
|
||||
* override was even seen.
|
||||
*/
|
||||
export function describeRejectedPythonOverride(): string | null {
|
||||
const override = process.env.HYPERFRAMES_PYTHON;
|
||||
if (!override) return null;
|
||||
const validation = validatePythonOverride(override);
|
||||
if (validation.ok) return null;
|
||||
return `HYPERFRAMES_PYTHON="${override}" was rejected: ${validation.reason}`;
|
||||
}
|
||||
|
||||
/** True if `import <pkg>` succeeds — actually executes the module. */
|
||||
export function hasPythonPackage(python: string, pkg: string): boolean {
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user