Skip to content

fix(desktop): fail the hermes launcher with an actionable error when deps are missing - #93170

Open
Finn763 wants to merge 1 commit into
NousResearch:mainfrom
Finn763:fix/issue-92882-desktop-exec-venv-message
Open

Finn763 wants to merge 1 commit into
NousResearch:mainfrom
Finn763:fix/issue-92882-desktop-exec-venv-message

Conversation

@Finn763

@Finn763 Finn763 commented Aug 23, 2026 •

Copy link
Copy Markdown

Problem

hermes desktop — and the generated ~/.local/share/applications/hermes.desktop — die at import time with a bare ModuleNotFoundError: No module named 'yaml' (or 'dotenv') whenever the interpreter that ends up running the hermes entry script is not the install venv: a uv-managed CPython invoked directly (the issue's step 2), or an Exec= whose interpreter escaped the venv.

Scope (rebased after #99492 landed)

The Exec= half of this PR — writing sys.executable verbatim in resolve_exec_command() instead of Path(sys.executable).resolve() — is on main now: #99492 / 6fe933e7099 rewrote hermes_cli/linux_desktop_entry.py to resolve the interpreter lexically (os.path.abspath(sys.executable)), which is the same fix by a better route. That half has been dropped here and the branch rebased onto current main, so the PR is no longer conflicting.

What remains is the half no sibling PR touches: the repo's hermes entry script. It does a bare from hermes_cli.main import main, so any invocation under a dep-less interpreter surfaces as an opaque import traceback with no hint about which interpreter ran or where the deps live.

Fix

The hermes launcher catches an import-time ModuleNotFoundError and exits 1 with an actionable diagnostic: the running interpreter (sys.executable), the missing module, and — when discoverable next to the checkout (venv/.venv + pyvenv.cfg) — the expected venv interpreter and the exact command to re-run.

A ModuleNotFoundError for a hermes_cli.* module raised from a file inside the package is classified as an internal error (reported with the real traceback, never as a missing dependency), so the launcher guard cannot mask an import bug in hermes_cli itself.

Re-review fixes (Enough1122 nits)

  1. Shell-quoted suggested fix command — _report_launcher_failure renders the re-run command with shlex.join(sys.argv[1:]) instead of " ".join(...), so hermes gateway run --model "a b" comes out as ... --model 'a b' and copy-pastes verbatim instead of flattening into two arguments.
  2. raise ... from err attribution — _classify_import_failure classifies by the innermost frame of the deepest traceback in the __cause__ chain, so an import failure re-raised via raise ... from err is attributed to its original import site rather than to whatever re-raised it (which could sit outside the package and demote an internal bug to a "missing dependency"). The contract is pinned in the docstring.
  3. Machine-greppable missing-dependency field — when exc.name is empty, the whole No module named 'x' sentence used to land in the missing dependency: field; it is now normalized to the bare module token x.

Evidence / regression tests (RED pre-fix, GREEN with this patch)

  • test_launcher_fails_actionably_when_deps_are_missing — runs ./hermes under python -S (site-packages stripped). Pre-fix stderr is the issue's exact Traceback ... No module named 'yaml' (same main.py:723); post-fix it names the running interpreter + missing module + fix, with no Traceback and no ModuleNotFoundError.
  • test_classify_flags_hermes_cli_internal_import_as_bug / test_classify_reports_missing_third_party_dep / test_classify_hermes_cli_name_from_outside_is_not_internal — the import-failure classifier: a hermes_cli.* module missing from inside the package is an internal bug; a missing external module (yaml, dotenv, …) is a dependency problem even when the failing import sits inside hermes_cli; the same hermes_cli.* failure raised from outside the package is not internal.
  • test_classify_attributes_re_raised_import_to_cause_chain — a raise ... from err-wrapped import failure whose re-raise site is outside hermes_cli is still classified as internal via the __cause__ chain.
  • test_classify_normalizes_unnamed_module_error_to_module_token — No module named 'x' with empty exc.name normalizes to x.
  • test_report_launcher_failure_missing_dependency_field_is_bare_token — the report field carries only the module token.
  • test_report_launcher_failure_fix_command_is_shell_quoted — argv entries with spaces must be shell-quoted in the suggested fix and round-trip through a POSIX shell.
  • test_report_launcher_failure_distinguishes_internal_bug — the internal-bug report says "internal error", never "missing dependency".
  • test_launcher_flags_internal_import_bug_not_missing_deps — end-to-end: a checkout whose hermes_cli imports its own missing submodule exits 1 with the internal-error diagnostic + real traceback, and is never misreported as a missing dependency.
  • test_launcher_still_runs_cli_when_deps_are_present — ./hermes --help still exits 0 with deps installed; the guard does not change the healthy path.

Local runs on this rebased head: pytest tests/hermes_cli/test_launcher.py → 12 passed; the same suite against main's hermes script → 10 failed, 2 passed (RED before / GREEN after); ruff check + ruff format --check clean.

Fixes #92882.

@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 comp/desktop Electron desktop app (apps/desktop/*) area/install-update Installer, updater, packaging, wheels, doctor sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 23, 2026
@Finn763

Finn763 commented Aug 23, 2026

Copy link
Copy Markdown
Author

Thanks for the review. All three findings addressed (pushed in de0f8ff):

  1. Overlap with fix(desktop-entry): keep the venv interpreter in Exec= instead of its symlink target #92090 — agreed, and stated plainly in the PR body now: the Exec= half (verbatim venv interpreter in the generated .desktop entry) is synonymous with open fix(desktop-entry): keep the venv interpreter in Exec= instead of its symlink target #92090 and overlaps fix(desktop): resolve a Hermes-capable interpreter for the .desktop Exec #92122 / fix(cli): keep venv symlink in desktop entry Exec, don't dereference #92516 / fix(desktop): don't follow venv python symlink in Linux .desktop Exec #92278 / fix(cli): Linux hermes.desktop Exec escapes the venv via symlink resolution #92790 / fix(desktop): preserve venv symlink in .desktop Exec line #92285. It is kept because the issue's Exec= traceback goes through exactly this path, but I'm happy to rebase onto whichever sibling PR lands first and drop the overlapping half — or maintainers can merge per-half. The launcher diagnostic half is unique to this PR (none of the siblings touch the hermes entry script).

  2. Bare except ModuleNotFoundError masking internal bugs — the launcher now classifies the failure: a ModuleNotFoundError for a hermes_cli.* module raised from a file inside the package is reported as an internal error with the real traceback, never as a missing dependency. The missing-yaml/dotenv install case keeps the actionable venv fix unchanged.

  3. Tests added: classifier unit tests (internal bug vs third-party dep vs outside-package frame), the internal-bug report wording, and an end-to-end launcher test with a checkout whose hermes_cli imports its own missing submodule. RED-before/GREEN-after verified; pytest tests/hermes_cli/test_launcher.py → 8 passed, ruff check/ruff format --check clean.

@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference, author can ignore or act on any point.

Both halves are solid now. The Exec= verbatim-interpreter change is the correct root-cause fix (on POSIX a venv's bin/python is a symlink; resolve() walks Exec= out of the venv where nothing is installed), the docstrings explain the CPython pyvenv.cfg-next-to-symlink mechanics accurately, and the new symlink tests assert the base interpreter never leaks into the line. On the launcher half, the internal-bug vs missing-dependency classification is exactly the right response to the masking concern — a hermes_cli.* failure raised from inside the package reports the real traceback instead of gaslighting users into reinstalling. Tests are genuinely end-to-end (-S subprocess for the dep-less path, a synthetic broken checkout for the internal path). Remaining nits:

  1. _report_launcher_failure builds the suggested fix command by joining sys.argv[1:] with spaces (hermes:99): fix: /venv/bin/python ./hermes gateway run --model "a b" renders unquoted and copy-pastes wrong. One shlex.join(sys.argv[1:]) fixes it.

  2. _classify_import_failure inspects only exc.__traceback__; an import failure re-raised via raise ... from err keeps its cause in __cause__, whose frames won't be consulted — the innermost frame would then be whatever re-raised it, potentially misclassifying an internal bug as external. Rare, but a short walk of the __cause__ chain (or a comment saying single-traceback-only) would pin the contract.

  3. Cosmetic: when exc.name is empty, missing = str(exc) puts the whole "No module named 'x'" sentence into the missing dependency: field of the report. Normalizing that to just the module token would keep the output machine-greppable.

On the disclosed overlap with #92090/#92122/#92516: keeping the Exec= half until siblings land while owning the unique launcher-diagnostics contribution is a reasonable sequencing offer for maintainers to arbitrate.

@Finn763

Finn763 commented Aug 24, 2026

Copy link
Copy Markdown
Author

Thanks for the re-review. All three nits are fixed and pushed (b15fa34):

  1. shlex.join for the suggested fix command — _report_launcher_failure now renders the re-run command with shlex.join(sys.argv[1:]) instead of " ".join(...). hermes gateway run --model "a b" now prints fix: /venv/bin/python ./hermes gateway run --model 'a b' and copy-pastes verbatim instead of flattening into two arguments. I verified both ways: shlex-splitting the rendered argv part round-trips to the original argument list, and executing the rendered command through a real shell with an echo probe delivers every argument — including a b and two words — intact. Covered by the new test_report_launcher_failure_fix_command_is_shell_quoted (RED pre-fix: the command flattened to gateway run --model a b; GREEN after).

  2. __cause__ chain walk in _classify_import_failure — classification now uses the innermost frame of the deepest traceback in the __cause__ chain rather than only exc.__traceback__, so an import failure re-raised via raise ... from err is attributed to its original import site, not to whatever re-raised it. The contract is pinned in the docstring ("the innermost frame of the deepest traceback in the __cause__ chain"). Covered by the new test_classify_attributes_re_raised_import_to_cause_chain, which builds exactly your scenario: the original import site is inside hermes_cli, the re-raise site is outside the package. Pre-fix the classifier returned internal=False (misclassifying the internal bug as a missing dependency); post-fix it returns internal=True.

  3. Bare module token in the missing dependency: field — when exc.name is empty, str(exc)'s whole No module named 'x' sentence is normalized to just the module token. Before: missing dependency: No module named 'definitely_missing_dependency'; after: missing dependency: definitely_missing_dependency. Covered by the new test_classify_normalizes_unnamed_module_error_to_module_token and test_report_launcher_failure_missing_dependency_field_is_bare_token.

Test results: pytest tests/hermes_cli/test_launcher.py → 12 passed (8 baseline + 4 new). RED-before/GREEN-after re-verified: with the launcher fixes reverted to the previous head, exactly the 4 new tests fail; with the fixes restored, all 12 pass. ruff check and ruff format --check are clean on both touched files (hermes, tests/hermes_cli/test_launcher.py).

…deps are missing

The launcher (the top-level `hermes` entry script) now reports which
interpreter it actually ran under, which dependency is missing, and the
venv interpreter to use instead of dying with a bare ModuleNotFoundError
(NousResearch#92882, NousResearch#90292, NousResearch#91504, NousResearch#80439). A ModuleNotFoundError for a
`hermes_cli.*` module raised from inside the package is classified as an
internal error with its real traceback, so a genuine bug in the checkout
is never misreported as a missing dependency.

Scope narrowed: the `Exec=` half (keeping the venv interpreter verbatim
in the generated .desktop entry) landed on main via NousResearch#99492 / 6fe933e,
which resolves the interpreter lexically, so this PR now carries only the
launcher diagnostic half - no sibling PR touches the entry script.
@Finn763
Finn763 force-pushed the fix/issue-92882-desktop-exec-venv-message branch from b15fa34 to 81f7468 Compare September 24, 2026 19:53
@Finn763 Finn763 changed the title fix(desktop): keep the venv interpreter in Exec= and fail the hermes launcher with an actionable error when deps are missing fix(desktop): fail the hermes launcher with an actionable error when deps are missing Sep 24, 2026

This branch has not been deployed

No deployments
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 comp/desktop Electron desktop app (apps/desktop/*) 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

Development

Successfully merging this pull request may close these issues.

[Bug]: hermes desktop fails with ModuleNotFoundError (yaml/dotenv) when launched with a uv-managed CPython outside the install venv

3 participants