Two bugs in ensureLinuxReady(): 1. Branch ordering: "ModuleNotFoundError: No module named 'sounddevice'" contains the word "sounddevice", so the portaudio branch matched first, producing the misleading "install libportaudio2" message even when libportaudio2 was already installed. 2. No venv auto-creation: On PEP 668 systems (Ubuntu 23.10+), system pip is blocked. The code trusted speech-recognizer.py to self-install deps, but its pip install also fails. Now ensureLinuxReady() auto-creates ~/.gsd/voice-venv when the sounddevice module is missing. Fixes: - Extract diagnoseSounddeviceError() with correct branch ordering (check "No module"/"ModuleNotFoundError" BEFORE "sounddevice") - Add ensureVoiceVenv() to auto-create venv with sounddevice+requests - Refactor into linux-ready.ts for testability - Add 20 unit tests covering all error diagnosis paths and venv creation Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
68902466ac
commit
9574c5796d
3 changed files with 222 additions and 21 deletions
|
|
@ -4,9 +4,9 @@ import type { AssistantMessage } from "@gsd/pi-ai";
|
|||
import { isKeyRelease, Key, matchesKey, truncateToWidth, visibleWidth } from "@gsd/pi-tui";
|
||||
import { spawn, execFileSync, type ChildProcess } from "node:child_process";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import * as readline from "node:readline";
|
||||
import { linuxPython, diagnoseSounddeviceError, ensureVoiceVenv, VOICE_VENV_PYTHON } from "./linux-ready.js";
|
||||
|
||||
const __extensionDir = import.meta.dirname!;
|
||||
const SWIFT_SRC = path.join(__extensionDir, "speech-recognizer.swift");
|
||||
|
|
@ -15,19 +15,6 @@ const PYTHON_SCRIPT = path.join(__extensionDir, "speech-recognizer.py");
|
|||
|
||||
const IS_DARWIN = process.platform === "darwin";
|
||||
const IS_LINUX = process.platform === "linux";
|
||||
const VOICE_VENV_PYTHON = path.join(
|
||||
process.env.HOME || process.env.USERPROFILE || os.homedir(),
|
||||
".gsd",
|
||||
"voice-venv",
|
||||
"bin",
|
||||
"python3",
|
||||
);
|
||||
|
||||
/** Return the python3 binary path — prefer venv if it exists, else system. */
|
||||
function linuxPython(): string {
|
||||
if (fs.existsSync(VOICE_VENV_PYTHON)) return VOICE_VENV_PYTHON;
|
||||
return "python3";
|
||||
}
|
||||
|
||||
function ensureBinary(): boolean {
|
||||
if (fs.existsSync(RECOGNIZER_BIN)) return true;
|
||||
|
|
@ -69,17 +56,20 @@ function ensureLinuxReady(ctx: ExtensionContext): boolean {
|
|||
});
|
||||
} catch (err: unknown) {
|
||||
const stderr = (err as { stderr?: Buffer })?.stderr?.toString() ?? "";
|
||||
if (stderr.includes("sounddevice") || stderr.includes("PortAudio") || stderr.includes("portaudio")) {
|
||||
ctx.ui.notify("Voice: install libportaudio2 with: sudo apt install libportaudio2", "error");
|
||||
} else if (stderr.includes("No module") || stderr.includes("ModuleNotFoundError")) {
|
||||
// Deps missing — the Python script handles auto-install on first run,
|
||||
// so we let it through. The script's own ensure_deps() will pip install.
|
||||
ctx.ui.notify("Voice: installing dependencies on first run — this may take a moment", "info");
|
||||
const diagnosis = diagnoseSounddeviceError(stderr);
|
||||
|
||||
if (diagnosis === "missing-module") {
|
||||
// Module not installed — auto-create venv (handles PEP 668 systems
|
||||
// where system pip is blocked). See #2403.
|
||||
if (!ensureVoiceVenv({ notify: (msg, level) => ctx.ui.notify(msg, level) })) {
|
||||
return false;
|
||||
}
|
||||
linuxReady = true;
|
||||
return true;
|
||||
} else if (diagnosis === "missing-portaudio") {
|
||||
ctx.ui.notify("Voice: install libportaudio2 with: sudo apt install libportaudio2", "error");
|
||||
} else {
|
||||
ctx.ui.notify(`Voice: dependency check failed — ${stderr.split("\n")[0] || "unknown error"}`, "error");
|
||||
return false;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
|
|
|||
87
src/resources/extensions/voice/linux-ready.ts
Normal file
87
src/resources/extensions/voice/linux-ready.ts
Normal file
|
|
@ -0,0 +1,87 @@
|
|||
/**
|
||||
* linux-ready.ts — Linux voice readiness logic (extracted for testability).
|
||||
*
|
||||
* Handles:
|
||||
* - Detecting system vs venv python3
|
||||
* - Diagnosing sounddevice import errors (portaudio vs missing module)
|
||||
* - Auto-creating venv on PEP 668 systems
|
||||
*/
|
||||
|
||||
import { execFileSync } from "node:child_process";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
|
||||
export const VOICE_VENV_DIR = path.join(
|
||||
process.env.HOME || process.env.USERPROFILE || os.homedir(),
|
||||
".gsd",
|
||||
"voice-venv",
|
||||
);
|
||||
export const VOICE_VENV_PYTHON = path.join(VOICE_VENV_DIR, "bin", "python3");
|
||||
|
||||
/** Return the python3 binary path — prefer venv if it exists, else system. */
|
||||
export function linuxPython(): string {
|
||||
if (fs.existsSync(VOICE_VENV_PYTHON)) return VOICE_VENV_PYTHON;
|
||||
return "python3";
|
||||
}
|
||||
|
||||
/**
|
||||
* Diagnose a sounddevice import error from its stderr output.
|
||||
*
|
||||
* Returns:
|
||||
* - "missing-module" — sounddevice python package not installed
|
||||
* - "missing-portaudio" — libportaudio2 native library not found
|
||||
* - "unknown" — unrecognized error
|
||||
*
|
||||
* IMPORTANT: Check "No module" / "ModuleNotFoundError" BEFORE checking for the
|
||||
* word "sounddevice", because `ModuleNotFoundError: No module named 'sounddevice'`
|
||||
* contains both strings. The more specific check must come first.
|
||||
*/
|
||||
export function diagnoseSounddeviceError(stderr: string): "missing-module" | "missing-portaudio" | "unknown" {
|
||||
// Check for missing Python module FIRST — the error message
|
||||
// "ModuleNotFoundError: No module named 'sounddevice'" contains the word
|
||||
// "sounddevice", so the old order (checking "sounddevice" first) was wrong.
|
||||
if (stderr.includes("No module") || stderr.includes("ModuleNotFoundError")) {
|
||||
return "missing-module";
|
||||
}
|
||||
// Now check for native portaudio library issues.
|
||||
if (stderr.includes("PortAudio") || stderr.includes("portaudio")) {
|
||||
return "missing-portaudio";
|
||||
}
|
||||
return "unknown";
|
||||
}
|
||||
|
||||
export interface ReadinessCallbacks {
|
||||
notify: (message: string, level: "info" | "error") => void;
|
||||
/** Override for execFileSync — for testing. Uses execFileSync (safe, no shell). */
|
||||
execFile?: typeof execFileSync;
|
||||
/** Override for fs.existsSync — for testing */
|
||||
exists?: typeof fs.existsSync;
|
||||
}
|
||||
|
||||
/**
|
||||
* Auto-create the voice venv if it doesn't exist.
|
||||
* Uses execFileSync internally (no shell, safe from injection).
|
||||
*
|
||||
* Returns true on success, false on failure.
|
||||
*/
|
||||
export function ensureVoiceVenv(cb: ReadinessCallbacks): boolean {
|
||||
const exists = cb.exists ?? fs.existsSync;
|
||||
const execFile = cb.execFile ?? execFileSync;
|
||||
|
||||
if (exists(VOICE_VENV_PYTHON)) return true;
|
||||
|
||||
cb.notify("Voice: setting up Python environment — one-time setup", "info");
|
||||
try {
|
||||
execFile("python3", ["-m", "venv", VOICE_VENV_DIR], { timeout: 30000 });
|
||||
execFile(
|
||||
path.join(VOICE_VENV_DIR, "bin", "pip"),
|
||||
["install", "sounddevice", "requests", "--quiet"],
|
||||
{ timeout: 120000 },
|
||||
);
|
||||
return true;
|
||||
} catch {
|
||||
cb.notify("Voice: failed to create Python venv — run: python3 -m venv ~/.gsd/voice-venv", "error");
|
||||
return false;
|
||||
}
|
||||
}
|
||||
124
src/resources/extensions/voice/tests/linux-ready.test.ts
Normal file
124
src/resources/extensions/voice/tests/linux-ready.test.ts
Normal file
|
|
@ -0,0 +1,124 @@
|
|||
/**
|
||||
* linux-ready.test.ts — Tests for Linux voice readiness logic (#2403).
|
||||
*
|
||||
* Covers:
|
||||
* - diagnoseSounddeviceError branch ordering (ModuleNotFoundError must NOT
|
||||
* match the portaudio branch, even though it contains "sounddevice")
|
||||
* - ensureVoiceVenv auto-creation
|
||||
* - linuxPython venv detection
|
||||
*/
|
||||
|
||||
import { createTestContext } from "../../gsd/tests/test-helpers.ts";
|
||||
import { diagnoseSounddeviceError, ensureVoiceVenv } from "../linux-ready.ts";
|
||||
|
||||
const { assertEq, assertTrue, report } = createTestContext();
|
||||
|
||||
function main(): void {
|
||||
// ── diagnoseSounddeviceError ──────────────────────────────────────────
|
||||
|
||||
// The critical regression: "ModuleNotFoundError: No module named 'sounddevice'"
|
||||
// contains the word "sounddevice", so the old code matched the portaudio branch.
|
||||
console.log("\n=== diagnoseSounddeviceError: ModuleNotFoundError must return missing-module ===");
|
||||
{
|
||||
const stderr = "Traceback (most recent call last):\n File \"<string>\", line 1, in <module>\nModuleNotFoundError: No module named 'sounddevice'";
|
||||
assertEq(diagnoseSounddeviceError(stderr), "missing-module",
|
||||
"ModuleNotFoundError for sounddevice should be 'missing-module', not 'missing-portaudio'");
|
||||
}
|
||||
|
||||
console.log("\n=== diagnoseSounddeviceError: 'No module named sounddevice' variant ===");
|
||||
{
|
||||
const stderr = "ImportError: No module named sounddevice";
|
||||
assertEq(diagnoseSounddeviceError(stderr), "missing-module",
|
||||
"'No module' substring should return missing-module");
|
||||
}
|
||||
|
||||
console.log("\n=== diagnoseSounddeviceError: actual portaudio error ===");
|
||||
{
|
||||
const stderr = "OSError: PortAudio library not found";
|
||||
assertEq(diagnoseSounddeviceError(stderr), "missing-portaudio",
|
||||
"PortAudio library error should return missing-portaudio");
|
||||
}
|
||||
|
||||
console.log("\n=== diagnoseSounddeviceError: lowercase portaudio error ===");
|
||||
{
|
||||
const stderr = "OSError: libportaudio.so.2: cannot open shared object file: No such file or directory";
|
||||
assertEq(diagnoseSounddeviceError(stderr), "missing-portaudio",
|
||||
"lowercase portaudio error should return missing-portaudio");
|
||||
}
|
||||
|
||||
console.log("\n=== diagnoseSounddeviceError: unrelated error ===");
|
||||
{
|
||||
const stderr = "SyntaxError: invalid syntax";
|
||||
assertEq(diagnoseSounddeviceError(stderr), "unknown",
|
||||
"unrelated error should return unknown");
|
||||
}
|
||||
|
||||
console.log("\n=== diagnoseSounddeviceError: empty stderr ===");
|
||||
{
|
||||
assertEq(diagnoseSounddeviceError(""), "unknown",
|
||||
"empty stderr should return unknown");
|
||||
}
|
||||
|
||||
// ── ensureVoiceVenv ──────────────────────────────────────────────────
|
||||
|
||||
console.log("\n=== ensureVoiceVenv: returns true when venv already exists ===");
|
||||
{
|
||||
const notifications: string[] = [];
|
||||
const result = ensureVoiceVenv({
|
||||
notify: (msg) => notifications.push(msg),
|
||||
exists: () => true,
|
||||
execFile: (() => Buffer.from("")) as any,
|
||||
});
|
||||
assertTrue(result, "should return true when venv exists");
|
||||
assertEq(notifications.length, 0, "should not notify when venv exists");
|
||||
}
|
||||
|
||||
console.log("\n=== ensureVoiceVenv: creates venv when missing ===");
|
||||
{
|
||||
const notifications: string[] = [];
|
||||
const commands: string[][] = [];
|
||||
let existsCalled = false;
|
||||
|
||||
const result = ensureVoiceVenv({
|
||||
notify: (msg) => notifications.push(msg),
|
||||
exists: () => { existsCalled = true; return false; },
|
||||
execFile: ((cmd: string, args: string[]) => {
|
||||
commands.push([cmd, ...args]);
|
||||
return Buffer.from("");
|
||||
}) as any,
|
||||
});
|
||||
|
||||
assertTrue(result, "should return true after venv creation");
|
||||
assertTrue(existsCalled, "should check if venv exists");
|
||||
assertEq(commands.length, 2, "should run 2 commands (venv + pip)");
|
||||
assertTrue(commands[0][0] === "python3", "first command is python3");
|
||||
assertTrue(commands[0].includes("-m") && commands[0].includes("venv"),
|
||||
"first command creates venv");
|
||||
assertTrue(commands[1][0].endsWith("bin/pip"), "second command is pip");
|
||||
assertTrue(commands[1].includes("sounddevice"), "pip installs sounddevice");
|
||||
assertTrue(commands[1].includes("requests"), "pip installs requests");
|
||||
assertTrue(notifications[0].includes("one-time setup"),
|
||||
"notifies about one-time setup");
|
||||
}
|
||||
|
||||
console.log("\n=== ensureVoiceVenv: returns false and notifies on failure ===");
|
||||
{
|
||||
const notifications: Array<{ msg: string; level: string }> = [];
|
||||
|
||||
const result = ensureVoiceVenv({
|
||||
notify: (msg, level) => notifications.push({ msg, level }),
|
||||
exists: () => false,
|
||||
execFile: (() => { throw new Error("externally-managed-environment"); }) as any,
|
||||
});
|
||||
|
||||
assertTrue(!result, "should return false on failure");
|
||||
const errorNotif = notifications.find(n => n.level === "error");
|
||||
assertTrue(errorNotif !== undefined, "should emit error notification");
|
||||
assertTrue(errorNotif!.msg.includes("python3 -m venv"),
|
||||
"error message should suggest manual venv creation");
|
||||
}
|
||||
|
||||
report();
|
||||
}
|
||||
|
||||
main();
|
||||
Loading…
Add table
Reference in a new issue