Skip to content

fix(desktop): derive the backend venv root from the chosen interpreter - #288

Merged
OmarB97 merged 1 commit into
mainfrom
fix/desktop-venv-interpreter-mismatch
Aug 2, 2026
Merged

fix(desktop): derive the backend venv root from the chosen interpreter#288
OmarB97 merged 1 commit into
mainfrom
fix/desktop-venv-interpreter-mismatch

Conversation

@OmarB97

@OmarB97 OmarB97 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

The desktop app could launch the Python backend with one environment's interpreter and another environment's site-packages, which made the backend unstartable.

findPythonForRoot() prefers .venv over venv (matching the convention in AGENTS.md), but createPythonBackend() hardcoded path.join(root, 'venv') when it built the child environment's PYTHONPATH and PATH. A checkout carrying both environments therefore ran the .venv interpreter against venv's site-packages.

The two trees routinely hold different Python versions — 3.13 and 3.11 on the machine where this was found. Pure-Python packages still import, so the failure surfaces late and points at the wrong thing: the first compiled extension resolves to a wheel built for the wrong ABI, and the backend exits(1) at import with

Import error: No module named 'pydantic_core._pydantic_core'

which the app reports as Web UI dependencies not installed (need fastapi + uvicorn). Electron then respawns the backend in a loop. Neither environment is broken — each works perfectly on its own — so the usual debugging instinct (reinstall or delete a venv) chases a ghost.

The fix follows the interpreter that was actually selected instead of guessing. Windows was never affected, because its branch already forced the matching venv interpreter; that behavior is unchanged.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • apps/desktop/electron/backend-env.ts — new exported venvRootForInterpreter(). Maps an interpreter path back to its environment root (<root>/bin/python on POSIX, <root>\Scripts\python.exe on Windows) and returns null for anything not laid out like a venv, so the caller keeps control of its own fallback. Pure path logic, with platform/pathModule injectable to match the rest of the module.
  • apps/desktop/electron/main.tscreatePythonBackend() derives the venv root from the chosen interpreter, using it when it holds a pyvenv.cfg and falling back to the previous <root>/venv otherwise (system interpreters, first-run bootstrap).
  • apps/desktop/electron/backend-env.test.ts — two regression tests: the helper resolves .venv and venv interpreters to their own roots on POSIX and Windows, and declines interpreters that are not venv-shaped.

createActiveBackend() was audited and left alone: it already takes both the interpreter and the site-packages from VENV_ROOT, so it cannot mismatch. Those are the only two call sites of getVenvSitePackagesEntries().

How to Test

Reproducing needs a checkout with two environments of different Python versions. With a 3.13 .venv and a 3.11 venv in the same root, this is what the pre-fix code assembled:

env PYTHONPATH="$PWD:$PWD/venv/lib/python3.11/site-packages" ./.venv/bin/python -m hermes_cli.main serve --host 127.0.0.1 --port 0
  1. Run the command above — it exits 1 with No module named 'pydantic_core._pydantic_core'.
  2. Swap the PYTHONPATH entry for .venv/lib/python3.13/site-packages (what this PR now builds) — the backend reaches HERMES_BACKEND_READY port=<n>.
  3. Note that ./.venv/bin/python -c "import pydantic_core" passes on its own in both cases. That is the trap: the environment is only broken in combination.

Screenshots / Logs

Both runs against the same interpreter, differing only in the PYTHONPATH the desktop app builds:

=== PRE-FIX  (hardcoded 'venv'): EXITED rc=1
    Import error: No module named 'pydantic_core._pydantic_core'
=== POST-FIX (follows interpreter): STARTED OK (still running, killed)
    HERMES_BACKEND_READY port=51241

Checklist

Code

  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix (no unrelated commits)
  • I've added tests for my changes
  • I've tested on my platform: macOS 15 (Darwin 25.6.0), Apple Silicon
  • pytest tests/ -q — N/A, this is an Electron main-process change with no Python surface. The equivalent gates were run instead, re-run on the rebased branch (base 7b09c55): vitest run --project electron (49 files, 490 passed, 1 skipped), tsc -p tsconfig.electron.json --noEmit clean, and eslint electron/ at its unchanged 24-warning baseline with none introduced.

Documentation & Housekeeping

  • I've considered cross-platform impact — Windows path layout is covered by a test and its resolution behavior is unchanged; the bug was POSIX-only
  • I've updated relevant documentation — N/A, no user-facing surface or config key changes
  • I've updated cli-config.yaml.example — N/A
  • I've updated CONTRIBUTING.md / AGENTS.md — N/A, this makes the code match the .venv-preferred convention AGENTS.md already documents
  • I've updated tool descriptions/schemas — N/A

Risks / gaps

  • Behavior for single-environment checkouts is unchanged: the derived root equals the old hardcoded one, so the common case is a no-op. Low risk.
  • A system interpreter (no pyvenv.cfg) still falls back to <root>/venv, exactly as before. When that venv is absent, getVenvSitePackagesEntries() already returns nothing, so no foreign site-packages can be injected. Accepted tradeoff — preserves the bootstrap path.
  • The .venv on the affected machine is stale in a separate way (editable install pinned to an older version). Out of scope for this PR; a uv sync is the operator-side cleanup.

@OmarB97

OmarB97 commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

CI: the two red checks are inherited from main, not from this diff

Check uv.lock and Desktop E2E / Playwright E2E (Linux) both fail here, and both come down to a single cause that this branch cannot influence:

error: The lockfile at `uv.lock` needs to be updated, but `--check` was provided.

Desktop E2E never got as far as Playwright. It failed at step 8, Install Python dependencies, running uv sync --locked --python 3.11 --extra all --extra dev; steps 9–12 (build, restore baselines, run E2E, save baselines) were all skipped. So this run carries no E2E signal in either direction.

Evidence that it predates this branch

main is already red for the same reason, and much more broadly. Run 30733377837 at commit 7b09c55 (the PR #284 merge, ~10 minutes before this PR opened) fails: all eight Python tests / Run tests slice N/8, Python tests / e2e, Python lints / ruff enforcement (blocking), Desktop E2E, Check uv.lock, and the aggregate gate. Those jobs all install dependencies through uv sync --locked, which refuses a stale lockfile.

This PR shows only two of them because the path filter correctly skips the Python jobs for a TypeScript-only change.

Locally, against a clean checkout of the same base:

$ git diff --name-only origin/main...HEAD
apps/desktop/electron/backend-env.test.ts
apps/desktop/electron/backend-env.ts
apps/desktop/electron/main.ts

$ uv lock --check
Resolved 234 packages in 30ms
error: The lockfile at `uv.lock` needs to be updated, but `--check` was provided.

Three .ts files, and the lockfile check still fails on that base. A Python lockfile cannot drift from an Electron edit.

What is green

Everything that actually exercises this change: all eight JS & TS checks / Typecheck & Test jobs including apps/desktop, plus Detect affected areas, Deny unrelated histories, List npm workspaces, and both OSV scan jobs — 13 passing, 0 failing among the checks relevant to this diff.

Deliberately not fixed here

Refreshing uv.lock in this PR would break the "only changes related to this fix" guarantee above, and uv lock re-resolves against PyPI — pyproject.toml carries intentional pins and upper bounds (requires-python <3.14, the pinned web extras) whose diff deserves its own review. That work is tracked separately; once it lands, a rebase here should clear all three red checks.

findPythonForRoot() prefers `.venv` over `venv`, but createPythonBackend()
hardcoded `<root>/venv` when it built the backend environment. A checkout
carrying both environments therefore launched the `.venv` interpreter with
`venv`'s site-packages on PYTHONPATH. The two trees routinely hold different
Python versions (3.13 vs 3.11 here), so every compiled extension resolved to a
wheel built for the wrong ABI: the backend exited(1) at import with
"No module named 'pydantic_core._pydantic_core'", the desktop app surfaced it as
"Web UI dependencies not installed (need fastapi + uvicorn)", and Electron
respawned the backend in a loop.

backend-env.ts gains venvRootForInterpreter(), which maps an interpreter path
back to its environment root (`<root>/bin/python`, `<root>\Scripts\python.exe`)
and returns null for anything not laid out like a venv, so the caller keeps its
own fallback. createPythonBackend() uses that root when it holds a pyvenv.cfg
and falls back to `<root>/venv` otherwise.

Windows was never affected — its branch already forced the matching `venv`
python — and its behavior is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@OmarB97
OmarB97 force-pushed the fix/desktop-venv-interpreter-mismatch branch from e6eb3eb to d057f2a Compare August 2, 2026 05:24
@OmarB97
OmarB97 merged commit 94c2ed4 into main Aug 2, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant