Skip to content

fix(cli): resolve the desktop-entry Exec independently of argv[0] - #80547

Closed
briandevans wants to merge 3 commits into
NousResearch:mainfrom
briandevans:fix/cli-desktop-entry-stable-exec-80439
Closed

briandevans wants to merge 3 commits into
NousResearch:mainfrom
briandevans:fix/cli-desktop-entry-stable-exec-80439

Conversation

@briandevans

@briandevans briandevans commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

This is a sibling follow-up to #76456 (commit eea6044)

  • What feat(desktop): register a Linux launcher entry for hermes desktop #76456 covered: it introduced hermes_cli/linux_desktop_entry.py, giving hermes desktop a real XDG launcher presence — absolute Exec, absolute Icon, tool-gated menu-cache refresh, and an unchanged-contents guard so a launch does not churn the caches. All of that is right and stays.
  • What feat(desktop): register a Linux launcher entry for hermes desktop #76456 did not touch: which absolute path gets persisted. It reuses relaunch.resolve_hermes_bin(), whose priority order was designed for re-exec, not for a value written to disk.
  • What this adds: a durability filter on the persisted Exec — by location, by shebang, and by keeping the interpreter path lexical — plus an atomic publish of the entry file. Additive only: no restructuring of the module, and all 14 of its original tests still pass (two fixture shebangs were made explicit; no assertion was changed).

What does this PR do?

Fixes both symptoms in #80439. They are one root cause, not two bugs.

resolve_exec_command() delegated to relaunch.resolve_hermes_bin(), which documents its own priority as 1. sys.argv[0] if it resolves to a real executable, 2. shutil.which("hermes"), 3. None. That order is correct for resolve_hermes_bin's actual job — re-exec'ing this process, where argv[0] is runnable by construction — and wrong as the source of a value baked into ~/.local/share/applications/hermes.desktop, which the desktop environment executes later from a process that shares nothing with this one.

Symptom 1 — the launcher goes permanently dead. When the entry itself launches Hermes, argv[0] is the checkout's bare hermes script. That script is:

#!/usr/bin/env python3
if __name__ == "__main__":
    from hermes_cli.main import main

— system python3, no venv shim, hermes_cli imported from the caller's sys.path. A terminal launch supplies that; a cold KDE/GNOME menu launch does not. install_desktop_entry() rewrites Exec to that path, and every subsequent menu launch dies at import. The entry never heals, because the failing launch never reaches the code that would rewrite it.

Symptom 2 — the KDE taskbar pin breaks. This falls out of symptom 1. Exec alternates between the installed wrapper (terminal launch) and the checkout script (menu launch), so the "skip the rewrite when contents are identical" guard never holds across alternating launches. The entry is rewritten and kbuildsycoca6 --noincremental re-run every other launch; Plasma drops the pin's association with the entry and the pinned icon opens a second window instead of focusing the first.

Fixing the Exec derivation makes the rendered contents converge to one stable value, which restores the idempotence guard and therefore fixes the pin. One change, both symptoms.

Symptom 3 — the interpreter fallback lands outside its own venv. Reported against this branch by @magicJie on a uv-created venv, and the same root cause one level down: a per-invocation fact being replaced by a resolved one. The fallback wrote str(Path(sys.executable).resolve()). sys.executable is <venv>/bin/python, a symlink into uv's standalone Python; CPython decides it is inside a venv by looking for pyvenv.cfg beside the path it was invoked through, not beside the symlink target. Dereferencing it produces

/home/jay/.local/share/uv/python/cpython-3.11.15-linux-x86_64-gnu/bin/python3.11 -m hermes_cli.main desktop

— outside the venv, so site-packages is gone and the persisted command fails with ModuleNotFoundError: No module named 'hermes_cli'. Absolute was necessary but not sufficient; the path also has to stay inside the venv. Now os.path.abspath(sys.executable). This module was the only production site in the repo dereferencing the interpreter — relaunch.build_relaunch_argv, uninstall.py's per-profile invocation and kanban_db.py's module-invocation helper all build this exact -m hermes_cli.main argv from a bare sys.executable, so the fix restores the module to the codebase's own convention.

Also durable-but-not-checkout-internal: the env python shebang. _is_inside_checkout rejects a candidate by where it lives. A hand-rolled ~/bin/hermes, or a console script from a non-venv editable install, sits outside the checkout and was kept — yet it still resolves python3 off PATH when the launcher runs it, which is symptom 1 reached by a different route (a cold menu launch supplies the desktop session's PATH, not the shell's). A candidate whose shebang program is env invoking a python* command is now rejected too. The check compares the shebang program's basename against env rather than substring-matching, so a real hardcoded interpreter under a directory named envs is still kept.

Second, unrelated-to-argv0 but same durable artifact: the entry was published with a bare entry_path.write_text(...), a truncate-then-write. An interruption between truncate and write leaves a zero-length hermes.desktop — Hermes disappears from the menu and the pin dies for good. utils.atomic_write_text's own docstring states it exists so "every destructive file rewrite in the codebase shares one implementation", so this call site was a deviation. It is now routed through the shared helper (same precedent as #79137 and #79746). preserve_mode=True keeps an already-0o755 entry from transiting mkstemp's 0o600, and preserves the owner when a privileged process rewrites a user-owned file; the unconditional chmod(0o755) afterwards is retained because the module requires the entry to be executable regardless of the mode it previously had.

Resolution precedence now persisted

  1. resolve_hermes_bin() — unchanged, still argv[0] then PATH.
  2. If that candidate is not durable — it lives inside project_root, or its shebang is env dispatching a python* — discard it and take shutil.which("hermes") instead; if that is also not durable, discard that too. (resolve_hermes_bin() already consults PATH last, so the retry only fires when the candidate came from argv[0].)
  3. os.path.abspath(sys.executable) -m hermes_cli.main desktop — absolute, but not symlink-resolved, so a venv interpreter stays in its venv.

An installed wrapper reached through argv[0] (~/.local/bin/hermes, a venv shim, a nix store path) is still preferred — only the candidates that cannot start Hermes from a cold session are rejected. tests/hermes_cli/test_relaunch.py and resolve_hermes_bin itself are deliberately untouched: their argv[0]-first order is correct for re-exec and is pinned by 12 tests. The defect is persistence, not the helper.

Related Issue

Fixes #80439

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

  • New private _is_inside_checkout(path, project_root) — resolves both sides and reports whether an entry point lives inside the source tree. Returns False for None/empty and swallows only ValueError/OSError from resolve()/relative_to().
  • New private _is_env_python_wrapper(path) — reads at most 256 bytes, requires a #!, requires the program's basename to be exactly env, tolerates env -S, and requires the command it runs to start with python. An unreadable candidate returns False: absence of evidence is not evidence of a PATH dependency, and this runs on the launch path, so it must not raise.
  • New private _is_durable_entry_point(path, project_root) — the two rejection clauses behind one name, so resolve_exec_command reads as one question asked twice.
  • resolve_exec_command() takes an optional project_root and discards a non-durable candidate, falling through to PATH and then to the interpreter. Default stays None, so calling it with no argument preserves the old behaviour exactly.
  • The interpreter fallback is os.path.abspath(sys.executable) rather than str(Path(sys.executable).resolve()). os is already imported at module scope.
  • install_desktop_entry() passes its project_root through, and publishes via utils.atomic_write_text(..., preserve_mode=True, create_mode=0o755). The utils import is function-local, matching the module's existing lazy-import idiom (from hermes_cli.relaunch import ...) so the module stays import-light for the uninstaller and the Electron main process, as its docstring promises.

tests/hermes_cli/test_linux_desktop_entry.py — 11 new tests (20 cases with parametrisation), placed beside the existing test_exec_* and install clusters. Helpers _make_executable and _fake_which added next to the existing _make_project/_stub_tools; _make_executable now takes the shebang so a test can say which kind of entry point it is standing up (INSTALLED_SHEBANG for a console script, ENV_PYTHON_SHEBANG for the checkout's launcher). No pre-existing assertion was changed.

How to Test

uv run --with pytest --with pytest-asyncio python3 -m pytest tests/hermes_cli/test_linux_desktop_entry.py -v

Before → after, verified in both directions. Applying the new test file to clean main (prod file untouched, run in a detached worktree) fails exactly the four regression tests, and passes on this branch:

Test On clean main On this branch Pins
test_exec_prefers_path_wrapper_over_checkout_argv0 ❌ Exec = checkout script ✅ Exec = PATH wrapper symptom 1
test_exec_rejects_checkout_argv0_when_no_wrapper_on_path ❌ Exec = checkout script ✅ Exec = <python> -m hermes_cli.main desktop symptom 1, the reporter's case
test_entry_is_stable_across_a_relaunch_through_itself ❌ contents differ, cache refresh runs twice ✅ byte-identical, refresh runs once symptom 2 (the pin)
test_failed_publish_leaves_the_existing_entry_intact ❌ returns the path; a real failure would truncate ✅ returns None, prior entry byte-identical, no temp residue atomic publish
test_exec_accepts_argv0_outside_the_checkout ✅ ✅ guard: argv[0] was narrowed, not deleted
test_install_publishes_atomically_and_leaves_no_temp ✅ ✅ guard: no .tmp residue, mode 0o755

The last two pass on both sides by design — they are guards against over-correcting, not regression proofs.

Second round — symptom 3 and the shebang clause, verified against the previous head of this branch (38e94f0) rather than clean main, since they regress code this PR itself introduced:

Test On 38e94f0 On this branch Pins
test_exec_keeps_the_lexical_venv_interpreter_path ❌ Exec = the base interpreter behind the venv symlink ✅ Exec = the venv's own bin/python, pyvenv.cfg still beside it symptom 3
test_exec_rejects_env_python_wrapper_outside_the_checkout ❌ Exec = the env python wrapper ✅ Exec = <python> -m hermes_cli.main desktop symptom 1, reached by shebang
test_exec_keeps_an_interpreter_under_a_directory_named_envs ✅ ✅ guard: env is matched as a basename, not a substring
test_env_python_wrapper_detection (10 cases) n/a — new predicate ✅ env -S python3 -u, env node, #!/bin/sh, bare env, a non-shebang ELF header, an empty file

Each clause was reverted in isolation to confirm it is the one doing the work: with os.path.abspath reverted, test_exec_keeps_the_lexical_venv_interpreter_path fails; with the shebang clause reverted and abspath kept, only test_exec_rejects_env_python_wrapper_outside_the_checkout fails.

Full run on this branch: 34 passed in test_linux_desktop_entry.py (14 pre-existing + 20 new cases; no pre-existing test was modified). Adjacent suites also green: tests/hermes_cli/test_gui_command.py, tests/hermes_cli/test_gui_uninstall.py, tests/hermes_cli/test_relaunch.py — 34 passed, 68 combined.

Manual, on the reporter's setup (Linux):

  1. hermes desktop from a terminal via ~/.local/bin/hermes → grep Exec ~/.local/share/applications/hermes.desktop shows the wrapper.
  2. Launch Hermes from the KDE application menu.
  3. grep Exec again → unchanged (before this PR it flipped to .../hermes-agent/hermes desktop).
  4. stat -c %Y ~/.local/share/applications/hermes.desktop before and after step 2 → unchanged, so no kbuildsycoca6 run and the taskbar pin keeps focusing the existing window.

Root-cause coverage (sibling-site sweep)

The concern is a per-invocation path baked into a durable, later-executed artifact. grep -rn --include='*.py' 'resolve_hermes_bin' plus a sweep for Exec= / .desktop writers enumerates every site:

Site Disposition
linux_desktop_entry.resolve_exec_command Covered — the only place an argv[0]-derived path is persisted, and (per git grep -n 'sys\.executable' -- '*.py' | grep -E 'resolve\(\)|realpath') the only production site in the repo that dereferenced the interpreter.
linux_desktop_entry.render_desktop_entry (Exec=) Covered via its input.
linux_desktop_entry.install_desktop_entry (write_text) Covered — atomic publish.
relaunch.resolve_hermes_bin / build_relaunch_argv Excluded — transient re-exec of the current process. argv[0]-first is deliberate there and pinned by its own tests. Nothing is persisted. Already passes a bare sys.executable.
linux_desktop_entry.resolve_exec_command, binary branch (str(Path(bin_path).resolve())) Deliberately unchanged — resolving is load-bearing for _is_inside_checkout, and for a wrapper the two choices trade off in opposite directions (lexical survives the target being replaced by an upgrade; resolved survives the symlink being removed). Neither can produce an import failure, so it reads as a maintainer's call rather than a bug.
uninstall.py per-profile invocation, kanban_db.py module-invocation helper Excluded — already bare sys.executable; they are the convention symptom 3 restores.
gateway/run.py::_resolve_hermes_bin Excluded — shutil.which first, and the result feeds a Popen within the same process lifetime, never a file.
hermes_cli/kanban_db.py Excluded — documented mirror of gateway.run._resolve_hermes_bin; same transient use.
tools/environments/local.py::_resolve_hermes_bin_dir Excluded — builds a PATH prefix for a child environment; not argv[0]-derived, not persisted.
systemd unit / launchd plist ExecStart (hermes_cli/gateway.py) Excluded — generated from the venv interpreter path; grep resolve_hermes_bin hermes_cli/gateway.py → 0 hits.
hermes_cli/gui_uninstall.py Excluded — reads desktop_entry_path() for removal only; no Exec derivation, no write.

resolve_exec_command has exactly one call site, so the signature change is fully covered.

Deliberately not included

The issue also raises four items that are maintainer calls rather than bug fixes, so they are left out to keep this reviewable as one concern. Happy to follow up on any of them:

  • "Don't rewrite if a user-customized version exists." This needs a policy decision about how to distinguish a user edit from a stale entry (marker comment? checksum sidecar?). With Exec stabilised, the entry stops churning on its own, which is the part that actually broke the pin.
  • --skip-build in the generated Exec. Changes what a menu launch does, not whether it works.
  • NVIDIA + Wayland ELECTRON_OZONE_PLATFORM_HINT=x11 autodetection. New runtime detection with its own compatibility surface.
  • A Path= key pointing at the checkout. Would let the sys.executable -m hermes_cli.main fallback work from a bare, uninstalled checkout, but it is a different concern (working directory, not path durability) and a bad Path= makes some launchers refuse the entry outright.

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 — ran the focused suite plus the adjacent CLI suites listed above, not the full tree
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15 (the suite is platform-gated via monkeypatch on sys.platform, exactly as the existing tests in this file are, so it runs headlessly off-Linux). The manual KDE/Plasma steps above are not something I can verify first-hand.

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — docstrings on resolve_exec_command and the new helper explain the durability requirement and why resolve_hermes_bin's order is right for its own caller
  • 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) per the compatibility guide — install_desktop_entry is already a no-op off Linux/BSD via is_supported(); _is_inside_checkout is pure pathlib, _is_env_python_wrapper reads bytes and never executes the candidate (a Windows .exe has no #! and is simply kept), the one symlink-creating test is skipif os.name == "nt", and the atomic writer is the one the rest of the codebase already uses on every platform
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Related / Positioning

Three open PRs touch resolve_exec_command(). To be explicit about the overlap:

Happy to defer on any overlapping hunk, or to rebase onto whichever of these lands first.

Screenshots / Logs

Focused suite on this branch:

tests/hermes_cli/test_linux_desktop_entry.py::test_exec_prefers_path_wrapper_over_checkout_argv0 PASSED
tests/hermes_cli/test_linux_desktop_entry.py::test_exec_rejects_checkout_argv0_when_no_wrapper_on_path PASSED
tests/hermes_cli/test_linux_desktop_entry.py::test_exec_keeps_the_lexical_venv_interpreter_path PASSED
tests/hermes_cli/test_linux_desktop_entry.py::test_exec_rejects_env_python_wrapper_outside_the_checkout PASSED
tests/hermes_cli/test_linux_desktop_entry.py::test_exec_keeps_an_interpreter_under_a_directory_named_envs PASSED
tests/hermes_cli/test_linux_desktop_entry.py::test_exec_accepts_argv0_outside_the_checkout PASSED
tests/hermes_cli/test_linux_desktop_entry.py::test_entry_is_stable_across_a_relaunch_through_itself PASSED
tests/hermes_cli/test_linux_desktop_entry.py::test_install_publishes_atomically_and_leaves_no_temp PASSED
tests/hermes_cli/test_linux_desktop_entry.py::test_failed_publish_leaves_the_existing_entry_intact PASSED
============================== 34 passed in 0.31s ==============================

First-round tests against clean main (production file unmodified):

FAILED tests/hermes_cli/test_linux_desktop_entry.py::test_exec_prefers_path_wrapper_over_checkout_argv0
FAILED tests/hermes_cli/test_linux_desktop_entry.py::test_exec_rejects_checkout_argv0_when_no_wrapper_on_path
FAILED tests/hermes_cli/test_linux_desktop_entry.py::test_entry_is_stable_across_a_relaunch_through_itself
FAILED tests/hermes_cli/test_linux_desktop_entry.py::test_failed_publish_leaves_the_existing_entry_intact
========================= 4 failed, 16 passed in 0.94s =========================

Second-round tests against the previous head of this branch (38e94f0), each clause reverted on its own:

# os.path.abspath reverted to Path(...).resolve()
FAILED tests/hermes_cli/test_linux_desktop_entry.py::test_exec_keeps_the_lexical_venv_interpreter_path
E   - <venv>/bin/python3 -m hermes_cli.main desktop
E   + <uv standalone build>/cpython-3.11.15-.../bin/python3.11 -m hermes_cli.main desktop

# shebang clause reverted, abspath kept
FAILED tests/hermes_cli/test_linux_desktop_entry.py::test_exec_rejects_env_python_wrapper_outside_the_checkout
E   assert '<tmp>/home/bin/hermes' not in '<tmp>/home/bin/hermes desktop'
========================= 1 failed, 3 passed in 0.16s ==========================

`resolve_exec_command()` delegated to `relaunch.resolve_hermes_bin()`,
which ranks `sys.argv[0]` first. That order is correct for its own job --
re-exec'ing *this* process, where argv[0] is runnable by construction --
and wrong as the source of a value baked into `~/.local/share/
applications/hermes.desktop`, which the desktop environment runs later
from a process that shares nothing with this one.

When the entry itself launches Hermes, argv[0] is the checkout's bare
`hermes` script. That script runs under `/usr/bin/env python3` with no
venv shim and imports `hermes_cli` from the caller's `sys.path`, which a
cold menu launch does not provide. Persisting it strands the launcher,
and the entry never heals because the broken launch dies at import
before it can rewrite anything.

The same root cause produces the second reported symptom. Because `Exec`
alternates between the installed wrapper and the checkout script, the
unchanged-contents guard in `install_desktop_entry()` never holds across
alternating launches, so the entry is rewritten and `kbuildsycoca6`
re-run on every other launch. Plasma drops the taskbar pin association
and the pinned icon opens a second window instead of focusing the first.

Discard a checkout-internal entry point and fall through to PATH, then
to the interpreter. An installed wrapper reached through argv[0] is
still preferred -- only the non-self-sufficient case is rejected.

Also publish the entry through `utils.atomic_write_text` instead of
`Path.write_text`. The entry is a durable user-facing artifact, and a
truncate-then-write leaves a zero-length file if the write is
interrupted, removing Hermes from the menu and killing the pin for good.
`preserve_mode` keeps an existing 0o755 entry from transiting mkstemp's
0o600, and preserves the owner when a privileged process rewrites a
user-owned file.

Fixes NousResearch#80439
Copilot AI lite review requested due to automatic review settings August 6, 2026 19:21
@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 P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@magicJie

magicJie commented Aug 7, 2026

Copy link
Copy Markdown

Additional real-world confirmation for the git/venv install path:

We hit this exact cold-launch crash on Deepin 25 (XDG launcher click → ModuleNotFoundError: No module named 'yaml') with a git checkout install where the only CLI entry is a bash wrapper at ~/.local/bin/hermes that execs an absolute venv python + the checkout script (unset PYTHONPATH/PYTHONHOME; exec "<venv>/bin/python" "<checkout>/hermes" "$@").

For that layout your fall-through (drop checkout-internal entry → shutil.which("hermes") → interpreter) resolves correctly: the wrapper is outside the checkout, is a bash script (no env python PATH dependency), and the absolute venv path inside it survives a minimal desktop PATH. We verified the wrapper works standalone in a stripped PATH=/usr/bin:/bin environment.

One note: we also append --skip-build and ELECTRON_DISABLE_SANDBOX=1 locally (slow-mirror / sandbox-helper environments) — those stay as user-level concerns, but the Exec resolution fix itself is what unblocks the default installs. Thanks for covering the durability filter + atomic publish too.

@briandevans

Copy link
Copy Markdown
Contributor Author

Thanks for the Deepin 25 repro — that's a useful independent confirmation, and the layout you describe is exactly the shape the fall-through targets.

Your wrapper is the case that motivated the durability filter rather than a plain "prefer PATH" rule. On a cold menu launch argv[0] is the checkout's bare hermes script, _is_inside_checkout discards it, and shutil.which("hermes") then resolves ~/.local/bin/hermes, which is kept because it is outside the checkout. That candidate is self-sufficient for the reason you found: it is a bash wrapper (so no #!/usr/bin/env python3 PATH dependency at exec time) and it execs an absolute venv interpreter after clearing PYTHONPATH/PYTHONHOME, so the venv's site-packages is on sys.path no matter how stripped the desktop environment's PATH is. That is precisely the ModuleNotFoundError: No module named 'yaml' failure mode — the checkout script hands the desktop session a system python3 with no venv on sys.path. Verifying it standalone under PATH=/usr/bin:/bin is the right test; that is close to what a cold XDG launch actually supplies.

Agreed on --skip-build and ELECTRON_DISABLE_SANDBOX=1 — those are environment-local knobs and out of scope here. This PR only changes which absolute path gets persisted into Exec and makes the entry publish atomically, so it should compose cleanly with whatever you append locally.

@magicJie

magicJie commented Aug 9, 2026

Copy link
Copy Markdown

Follow-up to my earlier Deepin 25 confirmation: the installed-wrapper path is good, but I found a real cold-launch failure in the no-wrapper fallback on a uv-created venv.

resolve_exec_command() currently uses Path(sys.executable).resolve() for the fallback. Here sys.executable is /home/jay/.hermes/hermes-agent/venv/bin/python, a symlink into uv's standalone Python. Resolving it strips the venv identity and produces this persisted Exec:

/home/jay/.local/share/uv/python/cpython-3.11.15-linux-x86_64-gnu/bin/python3.11 -m hermes_cli.main desktop

I reproduced it from this PR's head by forcing the checkout-script argv0 and removing the wrapper from PATH. Executing the generated interpreter from a cold cwd then fails:

$ cd /tmp
$ env -i HOME=/home/jay PATH=/usr/bin:/bin \
  /home/jay/.local/share/uv/python/cpython-3.11.15-linux-x86_64-gnu/bin/python3.11 \
  -c 'import hermes_cli'
ModuleNotFoundError: No module named 'hermes_cli'

Preserving sys.executable without resolving the symlink works under the same cold environment:

$ cd /tmp
$ env -i HOME=/home/jay PATH=/usr/bin:/bin \
  /home/jay/.hermes/hermes-agent/venv/bin/python \
  -c 'import hermes_cli; print(hermes_cli.__file__)'
/home/jay/.hermes/hermes-agent/hermes_cli/__init__.py

So the wrapper-present path still fixes the reported Deepin case, but test_exec_rejects_checkout_argv0_when_no_wrapper_on_path currently only checks that the fallback path is absolute; it does not execute the generated command. I suggest preserving the lexical venv interpreter path in the fallback and adding a regression test with a venv-python symlink whose resolved target cannot import hermes_cli.

Local verification on PR head: 38 passed across test_linux_desktop_entry.py, test_gui_command.py, and test_gui_uninstall.py; this is an additional runtime-path gap rather than an existing test failure.

The interpreter fallback wrote `Path(sys.executable).resolve()`. That
dereferences a venv's `bin/python` symlink, and CPython decides it is
inside a venv by looking for `pyvenv.cfg` beside the path it was
*invoked* through, not beside the symlink target. A uv-created venv
points that symlink at `~/.local/share/uv/python/cpython-*/bin/python3.11`,
outside the venv, so the persisted `Exec=` starts an interpreter with the
venv's site-packages missing and cannot `import hermes_cli` at all —
`ModuleNotFoundError` on every cold menu launch, and it never heals.

Write `os.path.abspath(sys.executable)` instead: absolute, but lexical.
This is also what every other production site that spawns this same
module already does (`relaunch.py`, `uninstall.py`, `kanban_db.py` all
pass a bare `sys.executable`); this module was the only one dereferencing
it.

Widen the durability filter while here. `_is_inside_checkout` rejects a
non-durable entry point by location only, so it misses a hand-rolled
`~/bin/hermes` or a console script from a non-venv editable install: both
sit outside the checkout and both still resolve `python3` off `PATH` at
exec time. A cold menu launch supplies the desktop session's `PATH`, not
the shell's, so the interpreter they land on need not have Hermes on
`sys.path`. Reject a candidate whose shebang program is `env` running a
`python*` command, and fall through to the interpreter. The check matches
the shebang program's basename exactly, so a hardcoded interpreter under
a directory named `envs` is still kept.
@briandevans

Copy link
Copy Markdown
Contributor Author

You are right on both counts, and thank you for chasing the no-wrapper branch — that is the half of the fall-through your first repro never reached, so it went untested by construction.

Fixed on the branch. resolve_exec_command()'s interpreter fallback now writes os.path.abspath(sys.executable) instead of str(Path(sys.executable).resolve()) — absolute, but lexical. CPython decides it is inside a venv by looking for pyvenv.cfg beside the path it was invoked through, so dereferencing venv/bin/python to uv's standalone build under ~/.local/share/uv/python/ moves the interpreter out of the venv and drops site-packages with it. That is exactly your ModuleNotFoundError: No module named 'hermes_cli'.

Worth noting this module was the only production site in the repo dereferencing the interpreter. relaunch.py's build_relaunch_argv(), uninstall.py's per-profile invocation and kanban_db.py's module-invocation helper all build the same -m hermes_cli.main shape from a bare sys.executable, so this was a local deviation rather than a policy, and the fix restores the module to the convention.

Your point about the test is also fair: test_exec_rejects_checkout_argv0_when_no_wrapper_on_path only asserted is_absolute(), which the resolved uv path satisfies. The new regression test is test_exec_keeps_the_lexical_venv_interpreter_path in tests/hermes_cli/test_linux_desktop_entry.py — it builds a venv whose bin/python symlinks to a base interpreter outside the tree, pins sys.executable to it, and asserts the emitted Exec= is the lexical venv path and that pyvenv.cfg is still beside what got persisted. It fails on the previous code with precisely your symptom; on my own macOS checkout the old code rendered .hermes-runtime/python/generation-*/cpython-3.11.15-macos-aarch64-none/bin/python3.11, so the uv layout reproduces off Linux too.

I should be upfront that @darzi-admin's #82040 already carries the same os.path.abspath(sys.executable) change with a pyvenv.cfg rationale, so I am not claiming to have found this independently — your comment is what prompted me to look, and their PR landed the same conclusion a few hours earlier. If the maintainers prefer to take that one for the interpreter line, no objection from me.

While in there I widened the durability filter, because your original Deepin report exposes a gap the location check cannot cover. _is_inside_checkout rejects a candidate by where it lives; a hand-rolled ~/bin/hermes, or a console script from a non-venv editable install, sits outside the checkout and is kept — yet it still resolves python3 off PATH when the launcher runs it, which is the same stranded entry reached by a different route. _is_durable_entry_point() now also rejects a candidate whose shebang program is env invoking a python* command (test_exec_rejects_env_python_wrapper_outside_the_checkout). Your Deepin wrapper is unaffected: the check only rejects an env-dispatched python*, so a bash wrapper is kept whichever shebang form it uses, and it execs an absolute venv interpreter after clearing PYTHONPATH/PYTHONHOME anyway. It stays the preferred candidate and that path behaves exactly as before. The check compares the shebang program's basename against env rather than substring-matching, so a real interpreter under a directory named envs is still kept; test_exec_keeps_an_interpreter_under_a_directory_named_envs pins that, and test_env_python_wrapper_detection covers env -S python3 -u, #!/bin/sh, env node and a non-shebang binary.

One site I deliberately left alone: the binary branch still writes str(Path(bin_path).resolve()). Resolving there is load-bearing for _is_inside_checkout, and for a wrapper the two choices trade off in opposite directions — the lexical path survives the target being replaced by an upgrade, the resolved path survives the symlink being removed. There is no import failure either way, so that reads as a maintainer's call rather than a bug, and I would rather not fold a judgement call into a crash fix.

On your exact three files (test_linux_desktop_entry.py, test_gui_command.py, test_gui_uninstall.py) that is 38 passed → 52 passed, and 68 passed with test_relaunch.py added. Head is now 7468eb7b516070189ea20b502b51d64b81956102.

@monerostar monerostar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ubuntu 26.04 on linux-5800x (kernel 7.0.0-28-generic). Compared main 3f832978d vs this tip 7468eb7b5 and sibling #80563.

Live matrix (mocked resolve_hermes_bin / sys.executable):

  • host launcher ~/.local/bin/hermes is #!/usr/bin/env bash that execs the venv python with absolute paths. main and this PR keep it as Exec. #80563 rejects any #!/usr/bin/env and falls back to -m, so it drops a working bash wrapper.
  • bare #!/usr/bin/env python3 script: main writes it into Exec (broken under a stripped desktop env). this PR discards it and lands on PATH hermes or -m.
  • venv bin/python symlink to a base interpreter outside the venv (same shape as this box: venv/bin/python -> .hermes-runtime/.../python3.11): main and #80563 use Path.resolve() and persist the base path. this PR keeps the lexical venv path via os.path.abspath, so pyvenv.cfg still applies.

Also on this install, Path(venv/bin/python).resolve() != os.path.abspath(...) (runtime generation dir). that is exactly the durability hole.

pytest: tests/hermes_cli/test_linux_desktop_entry.py -> 34 passed.

Looks good. Prefer this over #80563 for the env-python-only shebang check, PATH retry, lexical interpreter, and atomic publish.

@briandevans

Copy link
Copy Markdown
Contributor Author

@monerostar Thanks for running the three-way matrix on Ubuntu 26.04. The venv-symlink case is awkward to reproduce deliberately, so a live install where venv/bin/python points at a runtime generation dir outside the venv is genuinely useful.

Your Path(...).resolve() vs os.path.abspath(...) reading matches what is on head: resolve_exec_command()'s interpreter fallback in hermes_cli/linux_desktop_entry.py writes os.path.abspath(sys.executable), so the lexical venv path is what gets persisted and pyvenv.cfg still applies beside it. The regression test is test_exec_keeps_the_lexical_venv_interpreter_path in tests/hermes_cli/test_linux_desktop_entry.py. Head is 7468eb7b516070189ea20b502b51d64b81956102. The same divergence shows up off Linux — on a macOS checkout the old code rendered a .hermes-runtime/python/generation-*/.../bin/python3.11 path.

Restating what I noted earlier today so it stays visible next to your comparison: @darzi-admin's #82040 carries the same os.path.abspath(sys.executable) line on the same production file, independently and a few hours ahead of mine. Both are open, so the maintainers have a clean comparison to make rather than a priority claim from me.

@gokhanyildirimlar

Copy link
Copy Markdown

Just hit this bug in the wild and independently applied a similar fix before finding this PR. Tested locally - 34/34 tests pass, .desktop now stably resolves to ~/.local/bin/hermes across relaunches (the self-reinforcing overwrite cycle is gone).

Two things in this PR that made the difference over a simpler PATH-first approach (#80563 and my own first attempt):

  1. The os.path.abspath vs .resolve() distinction - on uv-managed venvs where bin/python symlinks outside the venv tree, .resolve() follows the symlink and CPython no longer finds pyvenv.cfg. The entry silently fails with ModuleNotFoundError even though the path looks correct. The only one of the three approaches that catches this.
  2. Atomic write - a zero-byte entry from an interrupted write permanently breaks the taskbar pin. Small edge case but a permanent failure mode worth preventing.

The shebang parsing edge cases (-S flag, envs/ directory false-positive, ELF binaries) are thorough. 👍

@alt-glitch alt-glitch added P2 Medium — degraded but workaround exists and removed P3 Low — cosmetic, nice to have labels Aug 9, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related to #80563: both repair #80439's generated desktop-entry Exec failure through different durable-resolution approaches.

@briandevans

Copy link
Copy Markdown
Contributor Author

@gokhanyildirimlar Thanks for the independent repro and the 34/34 run — an unprompted second confirmation from someone who hit this in the wild is worth more than another round of my own testing, especially on the two points you singled out.

Both of those are pinned by named tests on head 7468eb7b516070189ea20b502b51d64b81956102, so neither can silently regress:

1. os.path.abspath vs .resolve(). The interpreter fallback is hermes_cli/linux_desktop_entry.py:170 — argv = [os.path.abspath(sys.executable), "-m", "hermes_cli.main", "desktop"]. The regression test is test_exec_keeps_the_lexical_venv_interpreter_path in tests/hermes_cli/test_linux_desktop_entry.py. It builds a venv whose bin/python symlinks to a base interpreter outside the venv tree, guards the fixture first (venv_python.is_symlink() and os.path.realpath(...) != str(...), so it cannot pass vacuously if the symlink ever stops being a symlink), then asserts the persisted Exec is the lexical path and that pyvenv.cfg is beside it. That last assertion is the mechanism you described: absolute is not the property that matters, pyvenv.cfg sitting adjacent to the invocation path is, and that is what CPython keys off to decide it is in a venv.

2. Atomic write. Publishing goes through the shared utils.atomic_write_text at linux_desktop_entry.py:270. test_install_publishes_atomically_and_leaves_no_temp covers the no-stray-temp half on a genuine overwrite (it changes the rendered contents so it is not the unchanged-contents fast path). test_failed_publish_leaves_the_existing_entry_intact injects an OSError at the replace step and asserts the previously published entry is byte-identical afterwards — that is exactly the zero-byte-entry-breaks-the-pin failure you called out, and it is permanent because nothing later rewrites a file that already exists at the right path.

The shebang cases are test_exec_rejects_env_python_wrapper_outside_the_checkout (an #!/usr/bin/env python3 wrapper outside the checkout is still rejected — being outside the checkout is not the same as being durable under a cold desktop PATH) and test_exec_keeps_an_interpreter_under_a_directory_named_envs (the envs/ false positive; the check keys off the program's basename, not a substring).

One thing worth restating next to your comparison, since you are weighing approaches: @darzi-admin's #82040 carries the same os.path.abspath(sys.executable) line on the same production file, independently and a few hours ahead of mine. That PR and #80563 are both still open, so this is a comparison for the maintainers to make on the merits rather than a priority claim from me.

@briandevans

Copy link
Copy Markdown
Contributor Author

Closing — superseded by @teknium1's #90492, which is the better fix here. It handles Python-script launchers that would escape the active virtual environment while preserving native binaries, shell wrappers, and venv-shebang scripts.

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

Review case 3 (this PR's own bug): known-location probing ran before
the primary was proven non-durable, so a valid external launcher as
argv[0] (e.g. /opt/.../bin/hermes) plus a PATH miss plus a different
~/.local/bin/hermes silently switched installations. Reorder: an
external primary is now returned immediately; probing runs only after
the primary is proven checkout-internal AND PATH missed.

Review case 1 (inherited from NousResearch#90492): _needs_interpreter() compared
the shebang against Path(sys.executable).resolve().parent - on uv venvs
the resolved parent is the base interpreter's dir, so a valid
.venv/bin/python shebang was classified foreign. Compare the LEXICAL
interpreter directory (abspath) instead.

Review case 2 (inherited from NousResearch#90492): the no-wrapper fallback emitted
the RESOLVED interpreter, which on uv venvs is the base python outside
the venv tree - a dead entry (pyvenv.cfg no longer adjacent). Emit the
lexical sys.executable (os.path.abspath), preserving venv semantics -
the exact durability rule NousResearch#80547's review established.

Tests updated: interpreter expectations now assert the lexical path;
the venv-shebang fixture writes a realistic console-script shebang
(lexical, as pip writes them) rather than a resolved one.
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
…bang

Review case 3 (this PR's own bug): known-location probing ran before
the primary was proven non-durable, so a valid external launcher as
argv[0] (e.g. /opt/.../bin/hermes) plus a PATH miss plus a different
~/.local/bin/hermes silently switched installations. Reorder: an
external primary is now returned immediately; probing runs only after
the primary is proven checkout-internal AND PATH missed.

Review case 1 (inherited from NousResearch#90492): _needs_interpreter() compared
the shebang against Path(sys.executable).resolve().parent - on uv venvs
the resolved parent is the base interpreter's dir, so a valid
.venv/bin/python shebang was classified foreign. Compare the LEXICAL
interpreter directory (abspath) instead.

Review case 2 (inherited from NousResearch#90492): the no-wrapper fallback emitted
the RESOLVED interpreter, which on uv venvs is the base python outside
the venv tree - a dead entry (pyvenv.cfg no longer adjacent). Emit the
lexical sys.executable (os.path.abspath), preserving venv semantics -
the exact durability rule NousResearch#80547's review established.

Tests updated: interpreter expectations now assert the lexical path;
the venv-shebang fixture writes a realistic console-script shebang
(lexical, as pip writes them) rather than a resolved one.
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.
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.

bug(desktop): auto-generated hermes.desktop uses wrong Exec path, breaking KDE taskbar pinning

6 participants