Skip to content

fix: keep venv interpreter path in desktop entry Exec - #94051

Open
GitTradWang wants to merge 2 commits into
NousResearch:mainfrom
GitTradWang:fix/desktop-entry-venv-symlink
Open

GitTradWang wants to merge 2 commits into
NousResearch:mainfrom
GitTradWang:fix/desktop-entry-venv-symlink

Conversation

@GitTradWang

@GitTradWang GitTradWang commented Aug 24, 2026 •

Copy link
Copy Markdown

What does this PR do?

Fixes the Linux desktop-entry Exec line being rewritten with a bare (non-venv) Python interpreter, which made launching Hermes Desktop from the launcher icon crash silently after every upgrade on uv-created venvs.

Root cause: resolve_exec_command() in hermes_cli/linux_desktop_entry.py called Path(sys.executable).resolve(). uv venv (and pip) make bin/python a symlink into a shared base-interpreter tree, and .resolve() follows that link — so the generated Exec prefixed the bare base interpreter (e.g. ~/.local/share/uv/python/.../bin/python3.11). That interpreter never activates the venv (CPython discovers pyvenv.cfg from the unresolved argv[0]), so it crashes on the first third-party import (ModuleNotFoundError: yaml), with nothing on screen because Terminal=false.

Every hermes desktop run re-installs the desktop entry (main.py → _register_linux_desktop_entry()), and hermes update rebuilds it through python -m hermes_cli.main desktop --build-only. So after each upgrade the icon entry was broken again, while hermes desktop from a shell kept working — confusing: "works in terminal, flashes and dies from the icon".

Fix:

  • Add _resolve_exec_path(): when the executable sits inside a venv (a pyvenv.cfg exists above it), keep the unresolved venv path; resolve only otherwise. Use it for both sys.executable and the resolved hermes binary, so the Exec prefix stays on the venv interpreter that actually loads site-packages.
  • Add _is_python_interpreter(): when the resolved bin is the interpreter itself (the updater drives python -m hermes_cli.main desktop --build-only, so argv[0] is the interpreter), emit <venv-python> -m hermes_cli.main desktop instead of <venv-python> desktop (which would treat desktop as a script path).

Related Issue

Fixes #94058

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/linux_desktop_entry.py:
    • add _resolve_exec_path() — venv-symlink-preserving path resolution
    • add _is_python_interpreter() — detect interpreter-drive form
    • use both in resolve_exec_command() and _needs_interpreter()
  • tests/hermes_cli/test_linux_desktop_entry.py:
    • update two assertions to use the venv-preserving executable
    • add test_exec_keeps_venv_symlink_in_interpreter_prefix (uv-style symlink regression)
    • add test_exec_uses_module_form_when_bin_is_python_interpreter (updater drive form)
    • follow-up: tighten _is_python_interpreter() to real interpreter names; tests assert concrete venv paths instead of mirroring the implementation

How to Test

  1. Reproduce on any uv-created venv (uv venv): run hermes desktop once, then inspect ~/.local/share/applications/hermes.desktop — before the fix, Exec starts with the resolved base interpreter (~/.local/share/uv/python/.../bin/python3.11); after, it starts with the venv python (.../venv/bin/python).
  2. Launch from the launcher icon → app starts (before: instant silent crash).
  3. hermes update → entry survives with the venv path intact.
  4. pytest tests/hermes_cli/test_linux_desktop_entry.py -q → 17 passed, 2 skipped.

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 pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Fedora Linux x86_64, uv venv (python 3.11)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

resolve_exec_command() used Path.resolve() on sys.executable, which
follows the venv's bin/python symlink (uv and pip both symlink into a
shared base-interpreter tree). The generated .desktop Exec then prefixed
the bare base interpreter, which never activates the venv (pyvenv.cfg is
discovered from the unresolved argv[0]) and crashes on the first
third-party import — ModuleNotFoundError: yaml, silent since
Terminal=false. Every 'hermes desktop' or 'hermes update' rewrote the
entry, so launching from the launcher icon broke after every upgrade.

- add _resolve_exec_path(): keep the venv path when the executable sits
  inside a venv (pyvenv.cfg found above), resolve otherwise
- use it for both sys.executable and the resolved hermes bin
- updater drive form (argv[0] == the interpreter itself, e.g.
  'python -m hermes_cli.main desktop --build-only') now emits
  '<venv-python> -m hermes_cli.main desktop' instead of treating
  'desktop' as a script path
- update affected tests and add regressions for the uv-style symlink and
  the interpreter-drive form
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard area/install-update Installer, updater, packaging, wheels, doctor P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 24, 2026
- _is_python_interpreter() now matches only real interpreter names
  (python, python3, python3.11, python2.7) and rejects lookalikes such
  as python3-config
- the env-shebang and venv-shebang tests build a simulated uv-style
  venv and assert concrete paths instead of mirroring the
  implementation via _resolve_exec_path()
- add a direct unit test for _is_python_interpreter()
@Enough1122

Copy link
Copy Markdown

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

Overall: correct and well-explained fix — venv detection really does key off unresolved argv[0], so preserving the bin/python symlink (with the pyvenv.cfg walk-up check) instead of blind .resolve() addresses the actual flash-crash mechanism. The new -m hermes_cli.main branch for the updater's drive form is right, ordering after _needs_interpreter() avoids regressing console-scripts, and the tests simulate a concrete uv layout with negative assertions rather than mirroring the implementation.

Suggestions:

  1. CI portability of the new tests — test_exec_prefixes_interpreter_for_env_shebang_python_script, test_exec_keeps_venv_symlink_in_interpreter_prefix, and test_exec_uses_module_form_when_bin_is_python_interpreter all call Path.symlink_to(). On the repo's native Windows lane, creating symlinks raises OSError without developer-mode/privilege, so these will error rather than skip. Per house convention (@pytest.mark.posix_only-style markers; "use the marker, never a bare skipif"), either gate them or build the fake interpreter as a copy instead of a link.

  2. _is_python_interpreter regex misses ABI-tagged names like python3.11m (present in some BSD/libexec layouts); such an interpreter hitting the drive-form branch would get <python> desktop instead of the -m form. Trivial widening ((\.\d+)?[a-z]*) closes it.

  3. _resolve_exec_path walks every ancestor for pyvenv.cfg; a stray pyvenv.cfg above the install (e.g. in $HOME) would freeze an unrelated path unresolved. Cheap tightening: stop at the first ancestor that isn't named bin (or limit depth to 2).

Minor: consider asserting in test_is_python_interpreter_distinguishing_binary_from_launcher that python3.12 style names pass too, pinning the two-digit-minor case.

gokhanyildirimlar added a commit to gokhanyildirimlar/hermes-agent that referenced this pull request Aug 25, 2026
…preter match

Three hardening pieces that no other open PR in this space carries
together, consolidating the good ideas from the sibling PRs with credit:

- _running_interpreter(): keep sys.executable LEXICAL only when it is
  venv-semantic (pyvenv.cfg at or above it in the tree); otherwise
  resolve(). Blanket abspath (this PR's previous form, NousResearch#92516/NousResearch#94115/
  NousResearch#94544) preserves venv semantics but loses durability when the
  executable is a re-pointable symlink OUTSIDE any venv; blanket
  resolve() (NousResearch#90492) loses venv semantics. Detection picks the right
  one per path. Idea lineage credited in the docstring.

- Atomic entry write: install_desktop_entry now goes through
  utils.atomic_write_text (temp+fsync+rename) instead of a plain
  write_text. An interrupted plain write leaves a zero-byte entry that
  permanently breaks the taskbar pin. This piece was in NousResearch#80547, which
  closed unmerged with it unlanded - ported here.

- _is_interpreter(): strict regex basename match (python[23]?(\d+)?(\.\d+)?)
  with the bin/Scripts parent guard - rejects python3-config, pythonw
  and other lookalikes the startswith() form accepted (regex approach
  independently proposed in NousResearch#94051).

Verified live: venv context (pyvenv.cfg present) keeps the lexical path;
non-venv context resolves; three-context convergence intact (A==B,
C falls back to runnable -m under real wrapper-absence); atomic write
produces non-empty entries with 0755 on create; suites 35 passed/6
skipped; ruff clean.
gokhanyildirimlar added a commit to gokhanyildirimlar/hermes-agent that referenced this pull request Aug 25, 2026
…preter match

Three hardening pieces that no other open PR in this space carries
together, consolidating the good ideas from the sibling PRs with credit:

- _running_interpreter(): keep sys.executable LEXICAL only when it is
  venv-semantic (pyvenv.cfg at or above it in the tree); otherwise
  resolve(). Blanket abspath (this PR's previous form, NousResearch#92516/NousResearch#94115/
  NousResearch#94544) preserves venv semantics but loses durability when the
  executable is a re-pointable symlink OUTSIDE any venv; blanket
  resolve() (NousResearch#90492) loses venv semantics. Detection picks the right
  one per path. Idea lineage credited in the docstring.

- Atomic entry write: install_desktop_entry now goes through
  utils.atomic_write_text (temp+fsync+rename) instead of a plain
  write_text. An interrupted plain write leaves a zero-byte entry that
  permanently breaks the taskbar pin. This piece was in NousResearch#80547, which
  closed unmerged with it unlanded - ported here.

- _is_interpreter(): strict regex basename match (python[23]?(\d+)?(\.\d+)?)
  with the bin/Scripts parent guard - rejects python3-config, pythonw
  and other lookalikes the startswith() form accepted (regex approach
  independently proposed in NousResearch#94051).

Verified live: venv context (pyvenv.cfg present) keeps the lexical path;
non-venv context resolves; three-context convergence intact (A==B,
C falls back to runnable -m under real wrapper-absence); atomic write
produces non-empty entries with 0755 on create; suites 35 passed/6
skipped; ruff clean.

@actavisbulgaria-ai actavisbulgaria-ai left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified end-to-end on a real uv-managed install (Fedora 44 / KDE Plasma, Wayland)

Ran this PR's code on the exact setup the bug targets — ~/.hermes/hermes-agent/venv/bin/python is a symlink into ~/.local/share/uv/python/cpython-3.11.15-linux-x86_64-gnu/ (the real uv venv condition):

1. Unit tests — pytest tests/hermes_cli/test_linux_desktop_entry.py in a fresh env (pytest + PyYAML only): 18 passed, 2 skipped.

2. Generated Exec= on this machine via resolve_exec_command() with sys.argv[0] = the venv launcher:

Exec=/home/kiril/.hermes/hermes-agent/venv/bin/hermes desktop

Clean form, no interpreter prefix — _needs_interpreter() correctly returns False because _resolve_exec_path keeps the venv shim, so exe_dir matches the #!/…/venv/bin/python3 shebang. The pre-fix code on this same machine generated the broken:

Exec=…/.local/share/uv/python/cpython-3.11.15-linux-x86_64-gnu/bin/python3.11 …/hermes desktop

→ ModuleNotFoundError: No module named 'hermes_cli' on launch (invisible, Terminal=false).

_resolve_exec_path walking all parents for pyvenv.cfg (rather than just parent.parent) is the right call — it survives deeper shim layouts. This covers the whole bug class: both argv sites and the _needs_interpreter sibling path that earlier PRs missed.

For maintainers consolidating the pile: #92122 and #92278 also touch all three sites and generate the same correct Exec= on this machine — but this PR additionally carries passing tests and the most robust pyvenv.cfg walk.

@YannZhou

Copy link
Copy Markdown

Confirmed on a second environment (Ubuntu 26.04.1 LTS, KDE Plasma 6 / X11)

Exact same repro as the issue — and the fix direction is verified end-to-end here.

Environment

  • Ubuntu 26.04.1 LTS, KDE Plasma 6 (X11)
  • Hermes Agent v0.20.6, uv-managed venv at ~/.hermes/hermes-agent/venv
  • venv/bin/python is a symlink → ~/.local/share/uv/python/cpython-3.11.15-linux-x86_64-gnu/bin/python3.11 (pyvenv.cfg home points at the same uv tree)

Observed

  • After hermes desktop runs, ~/.local/share/applications/hermes.desktop Exec= was rewritten to prefix the resolved bare interpreter: .../uv/python/cpython-3.11.15-.../bin/python3.11 .../hermes-agent/hermes desktop
  • Clicking the launcher icon failed silently (Terminal=false) with ModuleNotFoundError: No module named hermes_cli
  • Every launch re-broke the entry, because hermes desktop re-installs the desktop entry on each run — so any manual edit of the .desktop file was overwritten right back to the broken Exec=.

Verification of the fix
Applied the same one-line change (drop .resolve() on sys.executable in resolve_exec_command()) locally:

  • Generated Exec= now stays on the venv interpreter: .../venv/bin/python .../hermes-agent/hermes desktop
  • hermes desktop launches normally (Electron GUI comes up, kwallet6 backend OK)
  • Re-running hermes desktop no longer rewrites the entry to the broken form (idempotent)
  • desktop-file-validate passes

This PR looks correct and covers the updater path too (python -m hermes_cli.main desktop --build-only), which our minimal local fix did not. Thanks for the thorough fix — would love to see it merged.

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 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

6 participants