Skip to content

fix(update): sync deps into every present venv so the .venv shim uses stays current - #97366

Open
Jackal991 wants to merge 1 commit into
NousResearch:mainfrom
Jackal991:fix/contrib-update-sync-both-venvs-97340
Open

Jackal991 wants to merge 1 commit into
NousResearch:mainfrom
Jackal991:fix/contrib-update-sync-both-venvs-97340

Conversation

@Jackal991

Copy link
Copy Markdown
Contributor

Summary

On an install that carries both venv/ and .venv/ (older git-install flow + uv/bootstrap-installer flow), hermes update only synced Python dependencies into venv/. The ~/.local/bin/hermes launcher shim and Desktop-app-spawned hermes --profile <p> serve --isolated processes run from .venv, so any dependency a release adds is missing there after the update — observed as ModuleNotFoundError: snowballstemmer after v0.20.4 → v0.20.6.

The updater hardcoded PROJECT_ROOT / "venv" at every dependency-sync, import-health-probe, and holder-process-detection site in hermes_cli/update_cmd.py.

Changes

Introduces _project_venv_dirs() which enumerates all present project venvs (venv then .venv; falls back to a single venv target when neither exists, preserving dev-checkout behavior) and routes every previously hardcoded site through it:

  • Dependency sync — the git-install path (_cmd_update_impl), the ZIP fallback path (_update_via_zip), and the repair path all install into every present venv.
  • Health probe (_venv_core_imports_healthy) — reports unhealthy if any present venv is missing a core import, so a stale .venv triggers a repair instead of silently passing.
  • Holder detection (_detect_venv_python_processes, _venv_launcher_ancestors) — Windows lock-holder guards now scan for processes running from any present project venv, not just venv.

The primary (venv) env is kept as the lazy-refresh / tool-dependency / verify target, matching pre-change behavior on both the deps-current and deps-changed paths.

No new env vars, no telemetry, no lockfile churn. Single-venv installs and venv-less dev checkouts behave exactly as before.

Tests

New tests/hermes_cli/test_update_multi_venv_sync.py (4 tests) proves the fix:

  • With both venv/ and .venv/ present, _project_venv_dirs() yields both (venv then .venv).
  • Regression: a stale .venv missing a core import now trips an unhealthy probe (pre-fix code only probed venv/ and returned healthy). Verified: the test suite fails 4/4 against the pre-fix code (AttributeError: no _project_venv_dirs + probe not covering .venv) and passes 4/4 on the fix.
  • Back-compat: single-venv installs still yield a single target; probe runs once.

Existing test_update_stale_virtualenv.py (9) plus test_update_self_lock.py, test_update_concurrent_quarantine.py, test_update_interrupted_recovery.py, test_verify_core_dependencies.py (54) all pass — no regressions.

Closes #97340

… stays current

Fixes the case where an install has both venv/ and .venv/ (git-install +
bootstrap flow): hermes update only synced deps into venv/, leaving the
.venv used by the CLI shim and desktop-served profiles stale
(ModuleNotFoundError: snowballstemmer). Probe, sync, and holder-detect
across all present project venvs.

Closes NousResearch#97340
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/install-update Installer, updater, packaging, wheels, doctor sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 28, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Overall: Ensures ZIP-fallback update syncs deps into every present project venv (both venv and .venv when coexisting) — fixes #97340 where desktop shim (.venv) stayed stale while gateway venv updated.

What it does

  • hermes_cli/update_cmd.py:_update_via_zip now iterates for venv_dir in _project_venv_dirs(): (new helper returning [Path(venv), Path(.venv)] filtered to existing dirs, order venv then .venv), building venv_env with VIRTUAL_ENV=str(venv_dir) per iteration and calling _install_python_dependencies_with_optional_fallback([uv_bin, "pip"], env=venv_env). Keeps primary venv as uv_env["VIRTUAL_ENV"] = str(_project_venv_dirs()[0]) for later tool-dep restore. New helper _project_venv_dirs() -> list[Path] documented: install can carry both venvs (CLI shim vs gateway/webui/dashboard services).
  • tests/hermes_cli/test_update_multi_venv_sync.py new.

Non-blocking notes

  • Per-venv loop allocates dict(uv_env) — isolation correct. If one venv sync fails, exception bubbles and skips remaining venv — consider best-effort continue to update other venv before re-raising.
  • Helper handles case where neither venv exists gracefully (returns empty — loop no-ops, primary index guard needed; verify).

Non-blocking — please use your judgment.

@Jackal991

Copy link
Copy Markdown
Contributor Author

Thanks for the review. The overall read is right — this is the #97340 fix, and the shared _project_venv_dirs() helper is what makes the sync, the health probe, and the holder detection all cover both venv and .venv instead of just venv.

On the two non-blocking notes:

1. Best-effort continue vs. bubbling. I kept the bubble deliberately. If venv syncs but .venv fails, that's a half-updated install — the shim the launcher actually uses is stale while the gateway services are current. Silently continuing and reporting success would leave exactly the #97340 symptom in place, just on the other venv. Failing the update so the user sees the repair path is the honest outcome. The one error we do want to swallow-and-defer is the shim-quarantine case, and that's still caught by the existing except _shim_quarantine_error_type() around the loop. So the current behavior is intentional, not an oversight.

2. "Helper handles case where neither venv exists gracefully (returns empty...)." This one's wrong — the helper never returns empty. It's return present or [_m().PROJECT_ROOT / "venv"]. When neither venv nor .venv exists (a plain dev checkout), it falls back to the single venv target, so _project_venv_dirs()[0] is always defined and the primary-index guard is always safe. The fallback is documented in the docstring precisely so callers keep the historical --python sys.executable pin / healthy-no-venv behavior instead of silently skipping. The test_project_venv_dirs_single_when_only_venv case covers the single-venv path; the no-venv fallback is exercised by the existing dev-checkout behavior. No guard needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

3 participants