Skip to content

fix(desktop): keep venv interpreter unresolved in .desktop Exec - #95190

Open
thehamsti wants to merge 1 commit into
NousResearch:mainfrom
thehamsti:fix/desktop-entry-venv-interpreter
Open

thehamsti wants to merge 1 commit into
NousResearch:mainfrom
thehamsti:fix/desktop-entry-venv-interpreter

Conversation

@thehamsti

Copy link
Copy Markdown

What does this PR do?

Fixes the Linux launcher icon silently failing after every update on uv-based installs (the standard installer layout).

Root cause: resolve_exec_command() prefixes the resolved hermes script with str(Path(sys.executable).resolve()). uv-created venvs make venv/bin/python a symlink into the shared base-interpreter tree (~/.local/share/uv/python/cpython-3.11.16-.../bin/python3.11), and .resolve() follows that symlink. CPython discovers pyvenv.cfg from the executable path as invoked, so the generated Exec= line ran the bare base interpreter with no venv site-packages and died instantly on the first third-party import:

ModuleNotFoundError: No module named 'yaml'

— silently, because Terminal=false. hermes desktop from a terminal worked (the bash wrapper execs the venv python), and each launch/rewrite of the entry re-broke it, which is why it recurred after every update.

Fix: use sys.executable verbatim (it is already absolute). The running process's imports came through exactly that path, so it is always at least as correct as the resolved path — resolving can only lose venv context, never gain anything.

The existing code comment already states the intent ("sys.executable is the interpreter actually running Hermes (the venv one)"); .resolve() defeated it.

Related Issue

Fixes #94058

(Also matches the symptoms in #92882, #92095, #92086, #91504, and duplicates #94564 / #94110. Related prior PR #94593 was closed by its author; this takes the simpler root-cause approach instead of the sibling-venv heuristic, and also fixes the -m hermes_cli.main fallback branch which had the same .resolve().)

Type of Change

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

Changes Made

  • hermes_cli/linux_desktop_entry.py — resolve_exec_command(): use sys.executable without .resolve() in both the script-prefix branch and the -m hermes_cli.main fallback branch; comment explains why resolving is wrong.
  • tests/hermes_cli/test_linux_desktop_entry.py — updated test_exec_prefixes_interpreter_for_env_shebang_python_script to expect the unresolved interpreter; added test_exec_keeps_venv_symlink_interpreter_unresolved, a regression test that builds a uv-style layout (venv bin/python → base-store symlink) and asserts Exec= uses the unresolved venv path and never mentions the base interpreter.

How to Test

  1. On a standard uv-based install, check out this branch and run the entry generator as the launcher would (venv python, argv[0] = repo script):
    venv/bin/python -c "
    import sys; sys.argv = ['<repo>/hermes', 'desktop']
    from pathlib import Path
    from hermes_cli.linux_desktop_entry import install_desktop_entry
    print(install_desktop_entry(Path('<repo>')))"
  2. Inspect ~/.local/share/applications/hermes.desktop — Exec= must start with <repo>/venv/bin/python, not ~/.local/share/uv/python/....
  3. Run the Exec line's interpreter against a third-party import (<repo>/venv/bin/python -c "import yaml") — succeeds; the previously generated base interpreter fails.
  4. Launch Hermes from the desktop launcher icon — the app starts.

Verified on a live Omarchy/Arch install: before the fix the generated Exec= failed with ModuleNotFoundError: No module named 'yaml'; after the fix the regenerated entry imports cleanly and desktop-file-validate passes.

Checklist

Code

  • I've read the Contributing Guide
  • 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/feature (no unrelated commits)
  • I've run scripts/run_tests.sh — tests/hermes_cli/test_linux_desktop_entry.py: 16/16 pass. Full suite: 37,409 passed, 202 failed; all failures reproduce identically on unmodified main in this environment (15 in update-flow tests verified by re-running those files on main; the rest are ImportErrors in gateway/messaging tests because the throwaway test venv has only .[dev], not .[all,dev], extras). Zero failures overlap with this change.
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Arch Linux (Omarchy/Hyprland), Python 3.11.16, uv-managed venv, Hermes v0.20.5 (f751a8c)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A (behavior-only fix; updated the code comment that documented the old approach)
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) — N/A: module is already gated to Linux/BSD via is_supported(); sys.executable is absolute on all platforms
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

Generated entry before (broken):

Exec=/home/hamsti/.local/share/uv/python/cpython-3.11.16-linux-x86_64-gnu/bin/python3.11 /home/hamsti/.hermes/hermes-agent/hermes desktop
$ <that Exec line>
ModuleNotFoundError: No module named 'yaml'

Generated entry after (works):

Exec=/home/hamsti/.hermes/hermes-agent/venv/bin/python /home/hamsti/.hermes/hermes-agent/hermes desktop
$ /home/hamsti/.hermes/hermes-agent/venv/bin/python -c "import yaml; print(yaml.__version__)"
6.0.3

resolve_exec_command() prefixed sys.executable after Path.resolve(), but
uv-created venvs make bin/python a symlink into a shared base-interpreter
tree. Resolving escapes the venv: CPython only discovers pyvenv.cfg from
the executable path as invoked, so the generated hermes.desktop Exec ran
the bare base interpreter and died silently (Terminal=false) on the first
third-party import (ModuleNotFoundError: yaml) when launched from a
launcher icon. Use sys.executable verbatim in both branches.

Fixes NousResearch#94058
@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 26, 2026
@Enough1122

Copy link
Copy Markdown

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

This is a precise fix for a real bug: Path(sys.executable).resolve() follows the venv's bin/python symlink to the base interpreter, which then can't find venv-installed packages because CPython discovers pyvenv.cfg from the invoked path, not the resolved one. Using sys.executable verbatim preserves the venv context. The fix is minimal and correct — two .resolve() removals, one in each branch.

The symlink test is well-constructed: it creates a real symlink chain, verifies that .resolve() would indeed escape the venv, and asserts both that the Exec line uses the unresolved path and that the base interpreter path does not appear. This pins the exact regression.

One minor observation: the test creates base_python as an empty file rather than a real executable. This is sufficient for testing the path logic (the test doesn't execute the Python), but a comment noting that the file is a placeholder (not a functional interpreter) would prevent confusion. The test's assertion venv_python.resolve() == base_python confirms the symlink resolves as expected before the Exec-line assertion, which is good defensive testing.

No concerns about the implementation — the .desktop Exec format doesn't require resolved paths, and _quote_exec_arg handles any spaces or special characters in the path.

@kvnloo

kvnloo commented Sep 15, 2026

Copy link
Copy Markdown

exact-head f61255b — void if moved

CHECK / pick-one: keep venv interpreter unresolved in .desktop Exec; CONFLICTING. Broader open fix is #92122 (Hermes-capable resolve + refreshed tests on same files).

Recommend: close as superseded by #92122, or rebase any unique bit after #92122 lands.

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.

Linux desktop entry Exec resolves venv symlink to bare interpreter, breaking launcher launch after upgrade

4 participants