From 3c2f628f5beddafd07b948ef509cf05dfec3bb5e Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 5 Jul 2026 19:13:33 -0700 Subject: [PATCH] fix(desktop): probe venv python in unwrapWindowsVenvHermesCommand so Repair can escape a broken venv (#59204) A Windows venv broken mid-update (e.g. python-dotenv missing after a partial pip install) still has python.exe + Scripts\hermes.exe on disk. unwrapWindowsVenvHermesCommand() returned that interpreter with no probe -- bypassing even the caller's --version smoke test -- so every recovery action (Retry, Repair install, Use local gateway) re-resolved the same dead backend: ModuleNotFoundError: No module named 'dotenv', same overlay, forever. - unwrapWindowsVenvHermesCommand now runs canImportHermesCli() on the venv python (checkout on PYTHONPATH, mirroring isActiveRuntimeUsable) and returns null on failure so the resolver falls through to the bootstrap installer, which actually repairs the venv. - hermesRuntimeImportProbe() adds 'import dotenv' -- the first third-party import on the CLI boot path (hermes_cli/env_loader.py) -- so a venv missing python-dotenv fails the probe everywhere it's used (isActiveRuntimeUsable, system-python rung, and the new unwrap gate). - Regression tests: probe content + source assertion that the unwrap path probes and falls through. --- apps/desktop/electron/backend-probes.cjs | 2 +- apps/desktop/electron/backend-probes.test.cjs | 4 +++ apps/desktop/electron/main.cjs | 22 ++++++++++++++++ .../windows-hermes-resolution.test.cjs | 25 +++++++++++++++++++ 4 files changed, 52 insertions(+), 1 deletion(-) diff --git a/apps/desktop/electron/backend-probes.cjs b/apps/desktop/electron/backend-probes.cjs index b69bd3f0858..fa0dae88eb7 100644 --- a/apps/desktop/electron/backend-probes.cjs +++ b/apps/desktop/electron/backend-probes.cjs @@ -44,7 +44,7 @@ const PROBE_TIMEOUT_MS = 5000 * @returns {string} */ function hermesRuntimeImportProbe() { - return 'import yaml; import hermes_cli.config' + return 'import yaml; import dotenv; import hermes_cli.config' } /** diff --git a/apps/desktop/electron/backend-probes.test.cjs b/apps/desktop/electron/backend-probes.test.cjs index 93158a32ce8..385e9500cd3 100644 --- a/apps/desktop/electron/backend-probes.test.cjs +++ b/apps/desktop/electron/backend-probes.test.cjs @@ -43,6 +43,10 @@ test('canImportHermesCli returns false when binary does not exist', () => { test('hermes runtime import probe checks config dependencies', () => { const probe = hermesRuntimeImportProbe() assert.match(probe, /\bimport yaml\b/) + // dotenv is the first third-party import on the CLI boot path + // (hermes_cli/env_loader.py); a mid-update venv missing python-dotenv + // passed the old probe and produced an unrecoverable boot loop. + assert.match(probe, /\bimport dotenv\b/) assert.match(probe, /\bimport hermes_cli\.config\b/) }) diff --git a/apps/desktop/electron/main.cjs b/apps/desktop/electron/main.cjs index bd6b867fac0..f1ff1d33b95 100644 --- a/apps/desktop/electron/main.cjs +++ b/apps/desktop/electron/main.cjs @@ -1338,6 +1338,28 @@ function unwrapWindowsVenvHermesCommand(command, backendArgs) { if (!fileExists(python)) return null const root = path.dirname(venvRoot) + + // Smoke-test the venv interpreter before trusting it. A venv whose update + // died mid-`pip install` still has python.exe + hermes.exe on disk, but the + // backend dies on its first import (e.g. ModuleNotFoundError: dotenv) before + // the gateway ever binds. Returning it here also BYPASSED the caller's + // `--version` probe, so Retry/"Repair install" re-resolved the same broken + // venv forever instead of falling through to the bootstrap installer. + // Mirror isActiveRuntimeUsable(): probe with the checkout on PYTHONPATH so a + // healthy source-tree venv passes. + if ( + !canImportHermesCli(python, { + env: { + PYTHONPATH: [...(directoryExists(root) ? [root] : []), process.env.PYTHONPATH].filter(Boolean).join(path.delimiter) + } + }) + ) { + rememberLog( + `Ignoring venv Hermes at ${python}: runtime import probe failed (broken/partial venv); falling through to bootstrap.` + ) + return null + } + return { label: `existing Hermes Python at ${python}`, command: python, diff --git a/apps/desktop/electron/windows-hermes-resolution.test.cjs b/apps/desktop/electron/windows-hermes-resolution.test.cjs index 3e0f9db1d1f..40e2658a122 100644 --- a/apps/desktop/electron/windows-hermes-resolution.test.cjs +++ b/apps/desktop/electron/windows-hermes-resolution.test.cjs @@ -14,6 +14,11 @@ // shim, written at the END of venv setup and absent in interrupted // states), so it escalated to a full venv recreate even on healthy // installs. +// 3. unwrapWindowsVenvHermesCommand() returned the venv python with NO +// runtime probe (bypassing the caller's --version check too), so a venv +// broken mid-update (e.g. missing python-dotenv) was re-selected forever: +// Retry / "Repair install" resolved the same dead interpreter instead of +// falling through to the bootstrap installer. const test = require('node:test') const assert = require('node:assert/strict') @@ -57,3 +62,23 @@ test('Windows bootstrap recovery chooses --update when any real-install signal i 'recovery regressed to gating only on the hermes.exe shim, which forces destructive --repair' ) }) + +test('unwrapWindowsVenvHermesCommand smoke-tests the venv python before trusting it', () => { + const source = readMain() + const fnStart = source.indexOf('function unwrapWindowsVenvHermesCommand(') + assert.notEqual(fnStart, -1, 'unwrapWindowsVenvHermesCommand must exist in main.cjs') + // Slice out just the function body (up to the next top-level function decl) + const fnEnd = source.indexOf('\nfunction ', fnStart + 1) + const body = source.slice(fnStart, fnEnd === -1 ? undefined : fnEnd) + assert.match( + body, + /canImportHermesCli\(python/, + 'unwrap must probe the venv interpreter; returning it unprobed re-selects a broken venv ' + + 'forever (Retry/Repair loop on a mid-update venv missing e.g. python-dotenv)' + ) + assert.match( + body, + /return null\s*\n\s*\}\s*\n\s*return \{/, + 'a failed probe must fall through (return null) so the resolver reaches the bootstrap rung' + ) +})