fix: make Hermes launcher survive home directory changes - #65304
Conversation
Related to closed, unmerged #65292, which carried the same launcher-path change. This is the active submission for the home-directory migration fix. |
|
This is complementary to the recent $HOME-hardening work rather than overlapping with it: #65286 stops runtime |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying a real launcher-migration gap: current main expands the installer-time HERMES_BIN into the shim at scripts/install.sh:1604 and scripts/install.sh:1642.
Problems
scripts/install.sh:1642replaces the resolved executable with$HOME/.hermes/hermes-agent/venv/bin/hermes. That bypasses documented explicit install directories (scripts/install.sh:404-408), root FHS installs at/usr/local/lib/hermes-agent(scripts/install.sh:427-438), and theUSE_VENV=falsePATH branch (scripts/install.sh:1603-1611). Those supported installations would launch the wrong or nonexistent executable.- The changed assertions in
tests/test_install_sh_pythonpath_sanitization.py:30andtests/test_install_sh_symlink_stomp.py:120check a literal generated string rather than executing the launcher after a home change.AGENTS.md:1370-1391prohibits source-shape tests for this reason.
Suggested changes
- Preserve all existing layout-resolution modes while adding runtime resolution for a relocated default installation.
- Add a behavioral launcher test using controlled old/new homes, plus coverage for a non-default install mode.
Automated hermes-sweeper review.
| unset PYTHONPATH | ||
| unset PYTHONHOME | ||
| exec "$HERMES_BIN" "\$@" | ||
| exec "\$HOME/.hermes/hermes-agent/venv/bin/hermes" "\$@" |
There was a problem hiding this comment.
HERMES_BIN is deliberately resolved above for explicit --dir/HERMES_INSTALL_DIR, root-FHS, and --no-venv installs. Replacing it with the default per-home venv path breaks those supported layouts; retain their resolution while making only the default relocated layout dynamic.
setup_path() baked the install-time absolute $HERMES_BIN into the launcher shim, so `hermes` broke outright once the user's home moved — a username change, a profile restored onto a new machine, or any migration left the shim exec'ing a path that no longer existed, before the app ever started. Users had to understand and repair internal launcher paths just to get back to a working install. Emit a $HOME-relative exec target when $HERMES_BIN resolves under the installing user's home, so the path is expanded by the shim at launch time rather than frozen at install time. Installs outside $HOME keep their absolute resolved path, preserving every other supported layout: an explicit --dir / $HERMES_INSTALL_DIR, the root FHS layout under /usr/local/lib/hermes-agent, and USE_VENV=false installs whose $HERMES_BIN came from PATH. Tests execute the generated launcher rather than asserting on install.sh's source text (AGENTS.md: "Never read source code in tests"): one installs under a controlled home, physically relocates it, and re-runs the launcher; one covers a non-default out-of-home install and asserts the absolute path survives a HOME change. Both fail against the respective broken variants. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
e90b65b to
9487c67
Compare
|
Updated — both review points addressed, and the branch is rebased onto current Layout resolution preserved. The shim now rewrites to a Behavioral tests, no source-shape assertions. Both tests were checked against the variants they guard, so neither is vacuous: the migration test fails against |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address the launcher failure caused by embedding an install-time home path: #65292 replaces it with one fixed $HOME layout, while #65304 conditionally makes only in-home installs relocatable and preserves other supported layouts.
Related pull requests
- #65292 [closed]
duplicate— (+3/-3) — superseded by #65304: It targets the reported stale-home launcher path, but hard-codes $HOME/.hermes/hermes-agent/venv/bin/hermes, breaking explicit install directories, root FHS installs, and USE_VENV=false PATH-based installs; although closed unmerged, it remains relevant as the earlier duplicate implementation. - #65304
related— (+240/-9) — preferred fix: The diff converts HERMES_BIN to a runtime $HOME-relative target only when it was installed beneath the original home, retains absolute targets otherwise, and adds behavioral coverage for home relocation, out-of-home layouts, environment sanitization, and argument forwarding. This directly addresses the contributor keep_open review on #65304, including its layout-preservation and behavioral-test concerns.
Duplicates
#65292 and #65304 address the same launcher home-migration defect, but #65304 supersedes #65292 with layout-aware resolution and behavioral regression tests.
Suggested consolidation
Merge #65304 as the complete, review-addressed implementation; keep #65292 closed as a superseded duplicate.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup65292 ["PRs duplicating each other"]
P65292["PR #65292 (closed)"]
P65304["PR #65304 (open)"]
end
class P65292 closed
class P65304 open
class P65304 target
click P65292 "https://github.com/NousResearch/hermes-agent/pull/65292"
click P65304 "https://github.com/NousResearch/hermes-agent/pull/65304"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed or no verify verdict yet (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 14 kB of PR diffs, 2 kB of issue/PR text, 3 kB of discussion (5 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
This fixes Hermes launcher behavior for users who change usernames, restore a profile onto a new machine, or otherwise end up with a different home directory than the one used at install time.
Today the installer-generated launcher can keep pointing at an old absolute path, which breaks
hermesbefore the app even starts. That forces users to understand and repair internal launcher paths just to get back to a working state.This change makes the launcher home-agnostic so it resolves from the current environment instead of assuming one fixed user directory.
Why this matters:
Scope:
scripts/install.shlauncher generationVerification:
uv run --with pytest python -m pytest tests/test_install_sh_pythonpath_sanitization.py tests/test_install_sh_symlink_stomp.py