Skip to content

fix(desktop): resolve a Hermes-capable interpreter for the .desktop Exec - #92088

Closed
autumn8-builds wants to merge 1 commit into
NousResearch:mainfrom
autumn8-builds:fix/desktop-entry-venv-wrapper
Closed

autumn8-builds wants to merge 1 commit into
NousResearch:mainfrom
autumn8-builds:fix/desktop-entry-venv-wrapper

Conversation

@autumn8-builds

Copy link
Copy Markdown

Summary

Fixes the Linux .desktop launcher failing silently when Hermes is launched via an interpreter that cannot import hermes_cli (e.g. a uv-managed or system python shim). Closes #92086.

resolve_exec_command() previously prefixed the resolved launcher with sys.executable whenever the script's shebang pointed outside the running interpreter's directory. When sys.executable itself lacks hermes_cli, the generated entry dies on import hermes_cli — invisibly, because Terminal=false. Clicking the pinned launcher does nothing.

Fix

The prefix interpreter is now chosen by capability, not blindly from sys.executable:

  1. sys.executable, only if it can import hermes_cli
  2. the installed venv-backed wrapper on PATH (e.g. ~/.local/bin/hermes)
  3. sys.executable -m hermes_cli.main

Also sets StartupNotify=false: the Electron startup token does not propagate through the wrapper, so true leaves a pinned launcher spinning forever.

Test plan

  • Added regression tests for the "sys.executable lacks hermes_cli" path (falls to wrapper) and the "no wrapper available" path (falls to -m).
  • Full tests/hermes_cli/test_linux_desktop_entry.py passes (18 passed, 2 platform-skipped) on a venv install where sys.executable is a uv shim.

resolve_exec_command() previously prefixed the resolved launcher with
sys.executable whenever the script's shebang pointed outside the running
interpreter's directory. When Hermes is launched via an interpreter that
cannot import hermes_cli (e.g. a uv-managed or system python shim), the
generated .desktop fails silently under Terminal=false: clicking the
pinned launcher does nothing.

Now the prefix interpreter is chosen by capability, not blindly from
sys.executable:
  1. sys.executable, only if it can import hermes_cli
  2. the installed venv-backed wrapper on PATH (e.g. ~/.local/bin/hermes)
  3. sys.executable -m hermes_cli.main

Also set StartupNotify=false: the Electron startup token does not
propagate through the wrapper, so a true value leaves a pinned launcher
spinning forever.

Adds regression tests covering the "sys.executable lacks hermes_cli" and
"no wrapper available" paths.

Refs NousResearch#92086
@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 needs-decision Awaiting maintainer decision before any implementation labels Aug 22, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Related: #92086 reports this interpreter-capability failure. This is a distinct competing repair in the active Linux desktop-entry launcher cluster with #91179, #87269, and #83358; maintainer selection is needed.

@Enough1122

Copy link
Copy Markdown

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

Capability-based selection is the right instinct: probing whether sys.executable can actually import hermes_cli before trusting it as the Exec prefix converts a silent-failure assumption into a checked one, and the wrapper → -m hermes_cli.main ladder gives two real escapes. The StartupNotify=false change is plausibly correct (the Electron startup token indeed doesn't survive a wrapper), though it's a separate behavior change bundled with the fix — fine if you observed the spinning cursor, worth a sentence saying so if you did.

Substantive points:

  1. Keeps .resolve() — which may reintroduce the same bug class on standard installs. _interpreter_for probes and prefixes Path(sys.executable).resolve(). On a uv venv/python -m venv POSIX install, sys.executable is a symlink to the base interpreter, so the probe tests the base interpreter, fails, and — if no PATH wrapper exists — falls to -m hermes_cli.main under the base interpreter, which cannot import hermes_cli either. See fix(desktop-entry): keep the venv interpreter in Exec= instead of its symlink target #92090, which fixes the sibling defect by using abspath(sys.executable) (absolute without leaving the venv) and argues from the same reporter setup that the "uv-managed shim" is the reporter's own symlinked venv python. Suggest probing/preferring the un-resolved sys.executable first, then the resolved form — that covers both topologies.
  2. Probe latency and timeout: _can_import_hermes_cli spawns a subprocess with timeout=30 and runs during desktop-entry generation, which happens on launches. 30s is very generous for an import probe; if the interpreter hangs (slow network home, broken install), entry generation stalls visibly. A 5s ceiling with fallthrough to the next candidate would bound the worst case.
  3. Coordinate with fix(desktop-entry): keep the venv interpreter in Exec= instead of its symlink target #92090 — same issue, same file, competing root-cause theories; the maintainers will want a combined resolution rather than a race.
  4. Test nit: the selective subprocess stub keyed on "import hermes_cli" in str(a) is brittle to how args are passed (list vs str); matching on the command list contents more structurally would survive refactors of the call site.

@jackulau

Copy link
Copy Markdown

I have #92090 open against the same issue with a different theory, and I would rather put the overlap in front of you than let two PRs race. Short version: I think your capability gate is correct and covers a case mine does not, and I think one expression inside it is load-bearing in a way that currently breaks it on the default POSIX layout.

Your theory is real and I do not handle it. If sys.executable is a genuinely foreign interpreter (a uv or system shim outside any venv), my PR keeps it and prefixes it verbatim, which reproduces the silent failure. A capability check is the right answer to that and I do not have one.

The expression. Every branch of _interpreter_for names Path(sys.executable).resolve():

if _can_import_hermes_cli(Path(sys.executable).resolve()):
    return [str(Path(sys.executable).resolve()), str(script), "desktop"]
wrapper = shutil.which("hermes")
if wrapper:
    return [str(Path(wrapper).resolve()), "desktop"]
return [str(Path(sys.executable).resolve()), "-m", "hermes_cli.main", "desktop"]

On POSIX, python -m venv and uv venv both create venv/bin/python as a symlink to the base interpreter, and only the venv path has the site-packages that make import hermes_cli work. .resolve() follows that symlink, so on such a host the three branches read:

  1. The probe runs against the base interpreter, correctly answers "cannot import hermes_cli", and in answering it discards sys.executable unresolved, which is the one interpreter on the box that would have worked.
  2. Falls to shutil.which("hermes"). Fine where ~/.local/bin/hermes exists, and I suspect that is why this did not show up in your testing.
  3. With no wrapper on PATH, the final branch runs <base interpreter> -m hermes_cli.main — the same interpreter branch 1 just proved cannot import hermes_cli. That branch cannot succeed on a symlinked venv. It is unreachable-by-correctness, not merely unlikely.

This is measured, not theoretical. @uncrayon ran an affected host on #92090 (Ubuntu 26.04, git/uv install, symlinked venv/bin/python):

sys.executable = <install>/venv/bin/python
resolved       = <uv-cache>/cpython-3.11.16-linux-x86_64-gnu/bin/python3.11

The resolved one raises ModuleNotFoundError: No module named 'dotenv'; the unresolved one imports dotenv, openai and yaml. That is your probe's "no" and the reason branch 3 is dead.

The fix is a substitution, not a redesign. Probe and emit os.path.abspath(sys.executable) rather than Path(sys.executable).resolve(). abspath normalises without following symlinks, so on a symlinked venv branch 1 fires and emits <install>/venv/bin/python; branches 2 and 3 are then reached only in the genuinely-foreign case this PR is actually about. Your ladder keeps its shape and starts being right on the common layout.

Worth knowing this cannot reproduce on Windows: Windows venvs copy the interpreter into Scripts\, so abspath and resolve are the same string and the bug is invisible there. If you develop on Windows, as I do, no amount of local testing surfaces it.

There is a second half in #92090 you may want regardless: _needs_interpreter compares a script's shebang against only the resolved interpreter directory, which misclassifies a correct venv console script as foreign (a console script installed into a venv carries the venv's own bin/python in its shebang), and the comparison is not case-folded. So the entry gets an interpreter prefix it does not need, naming the interpreter that cannot import Hermes.

I have no preference about which PR carries the combined fix and I am not asking you to close this one. If maintainers prefer yours as the base, I will port _running_interpreter() and the _needs_interpreter fix onto it myself and close mine. What I would flag is that either landing alone reintroduces the other's bug: yours routes around the venv on the common layout, mine prefixes a foreign shim verbatim in yours. Same note is on #92090 so both threads have it.

One unrelated observation while I was reading: StartupNotify=false is a separate behaviour change from the interpreter fix, and the reasoning for it (the Electron startup token not propagating through the wrapper) is worth its own line in the PR body — a reviewer scanning for scope will otherwise read it as drive-by.

@autumn8-builds

Copy link
Copy Markdown
Author

Closing in favor of #92122 (same fix, cleaner history + the abspath hardening from the review). Thanks!

@autumn8-builds
autumn8-builds deleted the fix/desktop-entry-venv-wrapper branch August 23, 2026 00:45
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/*) needs-decision Awaiting maintainer decision before any implementation 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 launcher fails silently when sys.executable lacks hermes_cli (e.g. uv-managed install)

4 participants