Skip to content

Commit df500e9

Browse files
committed
fix local Python setup for video gen on macOS Apple Silicon
End-to-end fix for the "configure local Python → install packages → generate a video" flow on Apple Silicon. The previous flow hit five distinct walls in sequence; each one looked like the user's fault but was a real bug: 1. Video Gen's "Local Python not configured" warning only linked to the Settings drawer. Now LocalSetupPanel renders inline below the status pill so detect / install / Create-venv all happen in place; saving a new pythonPath re-polls status so the pill flips green. 2. detectPython() preferred /opt/anaconda3/bin/python3 over /opt/homebrew/bin/python3. On Apple Silicon, anaconda is often x86_64, and `mlx` ships arm64-only wheels, so the install fails with "No matching distribution found for mlx". The detector now probes platform.machine() per candidate on darwin/arm64 and prefers a matching interpreter; /api/image-gen/setup/check returns interpreterArch + suggestedArm64Python so LocalSetupPanel can surface the warning and offer a one-click "Switch to detected arm64 Python" affordance. 3. isExternallyManaged() false-positived on PortOS-owned venvs created from PEP 668 Homebrew Python — sysconfig.get_path("stdlib") inside a venv resolves to the base interpreter's stdlib, so the venv inherited Homebrew's EXTERNALLY-MANAGED marker. Now also checks sys.prefix != sys.base_prefix and short-circuits to false for venvs. 4. /api/video-gen/status returned `connected: !!pythonPath` — the pill went green as soon as any pythonPath was saved, even when none of the required packages were installed. Now probes the imports via checkPackages and returns connected:false + missingPackages list when anything is absent, which keeps the inline panel showing the install button. LocalSetupPanel also notifies the parent on local check transition to "all installed" so the pill flips without waiting on a manual refresh. 5. The PyPI package literally named `mlx_video` is unrelated to the `mlx_video.generate_av` CLI the LTX renderer shells into — that ships in `mlx-video-with-audio`. Both publish an `import mlx_video` namespace, so the missing-check passed against the wrong one and the spawn died with ModuleNotFoundError. pipNameFor('mlx_video') now returns 'mlx-video-with-audio>=0.1.35' on macOS; the import probe checks `import mlx_video.generate_av` to distinguish them; installPackages gained a pre-uninstall step driven by PIP_PRE_UNINSTALL so the conflicting plain `mlx_video` is removed before pip refuses to "downgrade" across the namespace collision. Mirrors what scripts/setup-image-video.sh already did out-of-band. Separately: * The media job worker now re-resolves pythonPath from live settings at dispatch time for every local-Python job, instead of using the snapshot captured at enqueue time. Symptom this fixes: user switches their Python in the UI, clicks Generate, and the worker still shells out to the previous interpreter — because the in-memory queue and data/media-jobs.json carried the old snapshot. Codex image jobs are unaffected (they don't run a local Python). * probePythonHealth() consolidates the three subprocesses /setup/check used to fan out (checkPackages + isExternallyManaged + probePythonArch) into one python invocation that returns everything as JSON. isExternallyManaged becomes a thin wrapper; the standalone export was unused after the consolidation and is removed. * installPackages and installFlux2Venv shared a near-identical spawn-and-stream-stdout helper; extracted to a single streamSpawn(). Tests: 8061 server + 602 client passing (incl. new coverage for mediaJobQueue's live-pythonPath re-resolution and videoGen /status's missing-packages branch). Test count grew by 4 since main.
1 parent 3807aa8 commit df500e9

10 files changed

Lines changed: 351 additions & 87 deletions

File tree

.changelog/NEXT.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,11 @@ TBD
3535
- Series detail page: when the Story Bible drawer is open, the Series Arc + Editorial Roadmap split and the inner text + 260px Themes panel split now respond to the actual content-area width instead of viewport width. Switched to Tailwind v4 container queries — Roadmap drops below Arc when the content area is < 1024px, and Themes stacks below the logline/summary column when the Arc card is < 672px, preventing the text column from being squeezed into an unreadable strip.
3636
- Vite dev server: allow `*.ts.net` hosts so `npm run dev` works when launched via Tailscale MagicDNS (previously rejected with `Blocked request. This host ("…ts.net") is not allowed`).
3737
- Universe Builder base style image lightbox: clicking the rendered probe thumb opened the preview modal with `"Base style"` in the prompt field instead of the actual prompt sent to the renderer. The Universe Builder hydrates each preview item from `galleryByFilename` (built from `listImageGallery()`) so the modal sees the real sidecar prompt/seed/model; that map only refreshes on `runs.length` change or an explicit `bumpGalleryRefresh()` call, and the style-probe path never called the refresh, so its entry fell through to the row label. `StyleProbeImage` now fires an `onRenderComplete` callback after the new filename persists, and `UniverseBuilder` wires it to `bumpGalleryRefresh`.
38+
- Video Gen "Local Python not configured" warning now exposes the Detect / install / Create-venv flow inline on the page instead of linking out to the settings drawer — `LocalSetupPanel` renders directly below the disconnected status pill, wired to the same settings PATCH the drawer uses. Saving a new `pythonPath` re-polls status so the pill flips green without a manual refresh.
39+
- Local video gen on macOS was installing the wrong `mlx_video` PyPI package. The plain `mlx_video` package is unrelated (video classification/captioning) and lacks the `mlx_video.generate_av` CLI that the LTX renderer shells into — both packages publish an `import mlx_video` namespace so the missing-package check passed, but the spawn died with `No module named mlx_video.generate_av`. Fixed: `pipNameFor('mlx_video')` now returns `mlx-video-with-audio>=0.1.35` on macOS; the import probe now checks `import mlx_video.generate_av` instead of `import mlx_video` so the wrong package fails fast and the UI surfaces an install button; `installPackages()` gained a pre-uninstall step driven by a `PIP_PRE_UNINSTALL` map so the conflicting plain `mlx_video` is removed before pip refuses to "downgrade" across the name collision. Mirrors the same conflict-resolution flow `scripts/setup-image-video.sh` already used out-of-band.
40+
- Media job worker now re-resolves `imageGen.local.pythonPath` from live settings at run time for every video job (and every non-codex image job), instead of using the snapshot captured at enqueue time. Symptom this fixes: user switches their Python in the UI, clicks Generate, and the worker still shells out to the previous (broken) interpreter — because both the in-memory queue and the on-disk `media-jobs.json` carried the old path. Now the persisted snapshot is irrelevant; live settings always win. Codex image jobs are unaffected (they don't run a local Python).
41+
- `isExternallyManaged()` no longer false-positives on PortOS-owned venvs created from a PEP 668 base (e.g. Homebrew Python). Inside a venv, `sysconfig.get_path("stdlib")` resolves to the base interpreter's stdlib, so a venv created from Homebrew inherited the `EXTERNALLY-MANAGED` marker even though pip-in-venv ignores PEP 668 entirely. The check now also reads `sys.prefix` and `sys.base_prefix` and short-circuits to `false` when they differ (the canonical "am I in a venv?" test). Symptom this fixes: after switching from Anaconda to Homebrew and clicking "Create PortOS venv", the panel showed the new venv path but still asked to create a venv — because the new venv looked externally-managed too, hiding the regular "Install N missing packages" button.
42+
- Video Gen status pill no longer lies when the saved Python is missing required packages. Previously `/api/video-gen/status` returned `connected: true` whenever any `pythonPath` was stored, so a Python with no `mflux` / `mlx` / `mlx_video` installed showed a green pill until the user clicked Generate and the renderer crashed. `/status` now probes the imports via `checkPackages()` and returns `connected: false` + a `missingPackages` list when anything is absent, which triggers the same inline `LocalSetupPanel`. The panel's header copy adapts: "Set up Local Python" when no path is selected, "Install missing Python packages" (with the count) when the path is valid but packages are missing.
43+
- Local Python auto-detection on Apple Silicon Macs now skips x86_64 candidates: a default Anaconda install (`/opt/anaconda3/bin/python3`) was winning over `/opt/homebrew/bin/python3` and then failing at install time with `No matching distribution found for mlx` because `mlx` ships arm64-only wheels. `detectPython()` now probes `platform.machine()` of each candidate on `darwin/arm64` and prefers a matching interpreter. The `/api/image-gen/setup/check` response also gains `interpreterArch`, `hostArch`, `archMismatch`, and `suggestedArm64Python` fields; `LocalSetupPanel` surfaces a warning and a one-click "Switch to detected arm64 Python" button when the user's saved path is x86_64 on an Apple Silicon host.
3844

3945
## Removed

PLAN.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ _Batch-cleared 2026-05-25: 23 Next Up items shipped together via parallel sub-ag
1212
- [ ] [voice-code-agent-target-managed-app] **Voice code-agent delegation can't target a managed app yet.** The `dispatch_code_agent` voice tool (`server/services/voice/tools.js`) creates a CoS user task with no `app` set, so the agent always runs against the PortOS repo (the CoS default workspace). Add an optional spoken target ("…in BookLoom") → resolve to an app id and pass `app` through to `addTask` (cos.js already persists `metadata.app`, and `agentLifecycle.js#registerAgent` already reads it). Needs a phrase→app resolver (fuzzy match against managed app names) and a guard for "app not found." Deferred from the initial build (2026-05-26) to keep v1 scoped to the self-repo case.
1313
- [ ] [voice-code-agent-status-query] **No mid-task voice status query for dispatched coding agents.** Once a task is dispatched via `dispatch_code_agent`, the user can only learn the outcome from the completion announcement — there's no "how's that coding task going?" tool. Add a `code_agent_status` voice tool that reads `data/cos/state.json` for running agents tagged `metadata.voiceDispatch` and reports phase/elapsed. Deferred from the initial build (2026-05-26); the completion announcement covers the common case.
1414
- [ ] [voice-code-agent-announce-pr-url] **Completion announcement can't speak the PR link.** `agent:completed` (cosAgents.js) fires BEFORE `cleanupAgentWorktree` creates the PR (agentLifecycle.js), so `formatAgentCompletionLine` (proactiveTriggers.js) only speaks success/failure + the task description, not the PR. To include "PR #512 is up," either (a) emit a later `agent:pr-opened` event from the cleanup path carrying `{ taskId, prUrl, voiceDispatch }` and announce on that instead, or (b) have the announcement defer until the agent record gains a `prUrl`. Deferred 2026-05-26 — a spoken GitHub URL is poor UX anyway and the user reviews the PR visually; the "done" announcement is enough for v1.
15+
- [ ] [patch-settings-slice-helper] **Add a `patchSettingsSlice(slicePath, partial)` helper in `client/src/services/`.** The settings PUT shallow-merges top-level keys, so every caller that updates a nested field (`imageGen.local.pythonPath`, `sharing.*`, `backup.*`, etc.) re-implements the same fetch-then-spread pattern. Current sites: `ImageGenTab.handleSave` (`client/src/components/settings/ImageGenTab.jsx:211`), `VideoGen.handleSavePythonPath`, `SharingTab`, `BackupTab`, `MortalLoomTab`, `Sharing.jsx`, `StoryboardPanel`, `NounsStage`, `ComicScriptStage` — 9+ sites. A `patchSettingsSlice('imageGen.local', { pythonPath })` helper would eliminate the slice-clobbering bug class. Surfaced by /simplify on 2026-05-28; deferred because it's a cross-cutting refactor of every settings consumer.
16+
- [ ] [warning-banner-component] **Extract a `<WarningBanner icon title>` component.** The `bg-port-warning/10 border border-port-warning/30 rounded p-2 + AlertTriangle icon` pattern is used 30+ places across `client/src/` (Loras, Security, CreateApp, MemoryTab, ScheduleTab, EditAppModal, BrainGraph, LocalSetupPanel, etc.). No shared component exists — every site re-inlines the same Tailwind classes. Same applies to the matching success/error/info banner variants. Surfaced by /simplify on 2026-05-28.
17+
- [ ] [setup-check-cache] **Server-side cache for `/api/image-gen/setup/check` results, keyed by `(pythonPath, stat(pythonPath).mtimeMs)`.** The `LocalSetupPanel` calls `/setup/check` on debounced (400ms) keystrokes AND on mount AND on refresh-button — each call spawns a python subprocess (~0.5-1s warm). A 30s TTL cache, busted by `/setup/install` completion + settings-PUT-of-pythonPath, would collapse most repeats to memo hits. Less critical now that the 3 subprocesses are consolidated into one, but still hot for typing flows. Surfaced by /simplify on 2026-05-28.
18+
- [ ] [client-use-previous-hook] **Extract `usePrevious(value)` hook in `client/src/hooks/`.** The "compare current to last render via `useRef` + `useEffect`" pattern appears in `LocalSetupPanel.jsx:52-61` (transition-detection for `onPackagesChanged`) and `useMediaJobProgress.js:44` (`prevJobIdRef`). A shared hook + barrel + README row would shrink both to one line each. Surfaced by /simplify on 2026-05-28.
19+
- [ ] [mediajobqueue-resolve-live-params] **Extract `resolveLiveParams(job, safeParams)` in `server/services/mediaJobQueue/index.js#runJob`.** The 8-line block at line 605 that re-resolves `pythonPath` from live settings mixes a settings-read concern into the (already-long) sanitize-uploads section. Pulling it into its own helper makes `runJob` easier to skim and gives the live-settings concern its own seam for future fields (e.g. `model.runtime`-aware overrides). Surfaced by /simplify on 2026-05-28.
20+
- [ ] [pythonsetup-arch-tests] **Test coverage for arch-aware `detectPython()` + `/setup/check` arch fields.** Added 2026-05-28 with the VideoGen inline Local Python setup fix. New helpers (`probePythonArch`, `isArchMismatch`, `detectArm64Python`, `HOST_ARCH`) and the new `/api/image-gen/setup/check` response fields (`interpreterArch`, `hostArch`, `archMismatch`, `suggestedArm64Python`) have no test coverage — the existing `server/routes/imageGen.test.js` skips all `/setup/*` routes entirely. Worth a `pythonSetup.test.js` that mocks `node:os` + `node:child_process` to verify: (1) on `darwin/arm64`, `detectPython` prefers arm64 candidates over x86_64; (2) `/setup/check` includes the new arch fields and only sets `archMismatch: true` when the host is arm64; (3) `suggestedArm64Python` is null when no viable arm64 candidate exists. Deferred from the fix because the `/setup/*` route family has zero existing test harness and adding one is its own piece of work.
1521
- [ ] [mediacard-use-mediaimage-for-syncing-assets] **`MediaCard.jsx` grid thumbnails still use a raw `<img src={previewUrl}>`.** Same peer-sync placeholder/live-swap gap that `[peer-sync-medialightbox-use-mediaimage-for-syncing-assets]` fixed for the lightbox — `client/src/components/media/MediaCard.jsx` (~line 42) doesn't get the "Syncing" placeholder or the `peerSync:asset-arrived` atomic swap. Swap the raw `<img>` for `MediaImage`. Surfaced by that item's cross-check during the batch-clear (2026-05-25); was outside its stated scope.
1622
- [ ] [chrome-canary-followups] **Hardening for the custom-Chrome-binary feature (xhigh code-review 2026-05-25).** _DONE in the v2.10.0 release review: (b) both `spawn()` calls now have `.on('error', …)` listeners; (d) `browser/server.js#loadConfig` now try/catches the `JSON.parse`; (e) `setup-browser.js#loadConfig` now warns + returns `null` and `applyCanaryToConfig` skips the save when the existing config is unreadable; (f) top-level `runCanarySetup()` is now `.catch()`-wrapped. (c) is tracked separately in `[setup-browser-canary-headless]`. Remaining: (a), (g)–(o)._ **High-severity:** (a) On macOS headed mode (the default), `browser/server.js:208` uses `macAppBundle` only and silently ignores `chromePath` — a UI user who fills in `chromePath` for Canary/Chromium/Brave but leaves `macAppBundle` empty gets stock Chrome and the log misreports the binary; couple the two fields in the UI (or auto-derive `macAppBundle` from `chromePath` when the latter is inside a `.app`). (b) Neither `spawn(chromePath, …)` (line 216) nor `spawn('/usr/bin/open', …)` (line 208) has an `.on('error', …)` listener — a typo in `chromePath` emits 'error' with no listener → `uncaughtException` → portos-browser PM2 child crashes and restart-loops. (c) `scripts/setup-browser.js#applyCanaryToConfig` writes `chromePath` + `macAppBundle` but doesn't flip `headless: false`; combined with the seed default of `headless: true` (`data.reference/browser-config.json`), a fresh install accepting Canary runs Canary invisibly. (d) `browser/server.js#loadConfig` does bare `JSON.parse(raw)` with no try/catch — combined with setup-browser's non-atomic `writeFileSync` and the `cachedConfig` race, a partial-file write crashes the supervisor on next start (PM2 restart-loop forever). **Medium-severity:** (e) `setup-browser.js#loadConfig` silently catches all JSON parse errors and returns `{}`, then `applyCanaryToConfig` saves `{chromePath, macAppBundle}` only — wiping every other user-customized key. (f) Top-level `await runCanarySetup()` has no try/catch — any EACCES on data/browser-config.json aborts `npm run setup` / `update.sh` at what was previously a no-op step. (g) Idempotency guard only checks `chromePath`: users who decline get re-prompted on every update, and users who set only `macAppBundle` keep getting re-prompted. (h) `PORTOS_USE_CANARY` only matches literal `'0'`/`'false'` (opt-out) or `'1'`/`'true'` (opt-in) — `'no'`/`'off'`/`'yes'`/`'on'`/`'True'` fall through both branches. (i) `spawnSync(install.cmd, …, { stdio: 'inherit' })` for brew/winget can hang on a sudo password prompt under non-TTY + `PORTOS_USE_CANARY=1` (update.sh stalls). (j) `cachedConfig` in `browserService.js` is stale relative to setup-browser's direct write — GET /api/browser/config returns pre-update values until process restart. (k) `saveConfig` uses bare `writeFileSync` — switch to the canonical `atomicWrite` pattern (`server/lib/fileUtils.js`). **Low-severity:** (l) `spawnSync` failure-status check `result.status !== 0` treats `status: null` (spawn-failure / signal kill) identically to a non-zero exit and never logs `result.error` — masks ENOENT/EPERM/SIGKILL. (m) `optionalPath` Zod schema accepts any string up to 1024 chars; no `.app`/`.exe` sanity check — user pastes the bundle into `chromePath` → spawn() EISDIR. (n) `launchBrowser`'s reuse-existing-Chrome early-return (line 167) fires BEFORE `headlessMode = config.headless === true` (line 170), leaving the module-level default `false` after a PM2 restart that reuses Chrome — /health reports wrong mode (pre-existing, but in a function touched by this change). (o) `detectCanary` on macOS only checks `/Applications/...`; misses per-user `~/Applications/Google Chrome Canary.app` installs (corporate Macs, `HOMEBREW_CASK_OPTS=--appdir=$HOME/Applications`).
1723

client/src/components/settings/LocalSetupPanel.jsx

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { useState, useEffect, useCallback, useRef } from 'react';
2-
import { CheckCircle2, XCircle, Wand2, RefreshCw, Terminal, AlertTriangle, Box } from 'lucide-react';
2+
import { CheckCircle2, XCircle, Wand2, RefreshCw, Terminal, AlertTriangle, Box, Cpu } from 'lucide-react';
33
import toast from '../ui/Toast';
44
import BrailleSpinner from '../BrailleSpinner';
55

6-
export default function LocalSetupPanel({ pythonPath, onPythonPathChange }) {
6+
export default function LocalSetupPanel({ pythonPath, onPythonPathChange, onPackagesChanged }) {
77
const [detecting, setDetecting] = useState(false);
88
const [check, setCheck] = useState(null); // { required, installed, missing, missingPip }
99
const [checking, setChecking] = useState(false);
@@ -46,6 +46,17 @@ export default function LocalSetupPanel({ pythonPath, onPythonPathChange }) {
4646
if (logRef.current) logRef.current.scrollTop = logRef.current.scrollHeight;
4747
}, [installLog]);
4848

49+
// Notify the parent whenever local check transitions from "had missing
50+
// packages" to "all installed" — covers manual refresh, terminal installs,
51+
// and the SSE-complete path. Without this, parent state (e.g. VideoGen's
52+
// status pill) stays stale until the user manually clicks its own refresh.
53+
const prevHadMissingRef = useRef(false);
54+
useEffect(() => {
55+
const allInstalled = !!check && Array.isArray(check.missing) && check.missing.length === 0;
56+
if (allInstalled && prevHadMissingRef.current) onPackagesChanged?.();
57+
prevHadMissingRef.current = !!check && Array.isArray(check.missing) && check.missing.length > 0;
58+
}, [check, onPackagesChanged]);
59+
4960
const handleDetect = async () => {
5061
setDetecting(true);
5162
try {
@@ -167,6 +178,26 @@ export default function LocalSetupPanel({ pythonPath, onPythonPathChange }) {
167178
<p className="text-xs text-gray-500">{checking ? 'Checking…' : 'Set a Python path to check installed packages.'}</p>
168179
) : (
169180
<>
181+
{check.archMismatch && (
182+
<div className="flex items-start gap-2 text-xs text-port-warning bg-port-warning/10 border border-port-warning/30 rounded p-2 mb-3">
183+
<Cpu size={14} className="shrink-0 mt-0.5" />
184+
<div className="flex-1">
185+
<div>
186+
This Python reports <code>{check.interpreterArch}</code> but your Mac is <code>{check.hostArch}</code>.
187+
<code>mlx</code> ships arm64-only wheels — installing it here will fail.
188+
</div>
189+
{check.suggestedArm64Python && (
190+
<button
191+
type="button"
192+
onClick={() => onPythonPathChange(check.suggestedArm64Python)}
193+
className="mt-2 inline-flex items-center gap-1.5 px-2 py-1 text-xs bg-port-accent hover:bg-port-accent/80 text-white rounded"
194+
>
195+
<Wand2 size={12} /> Switch to {check.suggestedArm64Python}
196+
</button>
197+
)}
198+
</div>
199+
</div>
200+
)}
170201
<ul className="grid grid-cols-2 gap-x-4 gap-y-1 text-xs mb-3">
171202
{check.required.map(pkg => {
172203
const ok = check.installed.includes(pkg);

0 commit comments

Comments
 (0)