Skip to content

fix(gateway): preserve symlinked venv python in systemd system unit - #8622

Closed
whisky0809 wants to merge 1 commit into
NousResearch:mainfrom
whisky0809:fix/gateway-systemd-venv-symlink
Closed

fix(gateway): preserve symlinked venv python in systemd system unit#8622
whisky0809 wants to merge 1 commit into
NousResearch:mainfrom
whisky0809:fix/gateway-systemd-venv-symlink

Conversation

@whisky0809

Copy link
Copy Markdown
Contributor

What

_remap_path_for_user() in hermes_cli/gateway.py called Path.resolve() on its input, which follows symlinks. In uv-managed venvs, venv/bin/python is a symlink into uv's shared Python store:

venv/bin/python -> ~/.local/share/uv/python/cpython-3.11.x-linux-x86_64-gnu/bin/python3.11

get_python_path() correctly returns .../venv/bin/python, but when generate_systemd_unit(system=True) runs that path through _remap_path_for_user, .resolve() swaps it for the bare uv interpreter. The generated unit's ExecStart= then launches Python outside the venv, so none of the venv's site-packages are importable and the service crash-loops on import:

ModuleNotFoundError: No module named 'yaml'

hermes gateway start --system reports success (systemd accepted the start request) but systemctl status hermes-gateway shows activating (auto-restart) briefly before settling into failed. The CLI and the actual state disagree, which makes it genuinely confusing to diagnose.

Only affects uv-managed venvs combined with --system installs — a traditional python -m venv copies the interpreter, so .resolve() stays inside the venv and the bug is invisible. User-scoped installs skip _remap_path_for_user entirely and are unaffected.

Fix

Prefer the lexical (unresolved) form of the path for the remap, and only fall back to resolving when $HOME itself is a symlink (e.g. /home/alice -> /mnt/users/alice). System paths like /opt/hermes continue to round-trip unchanged. In the unreachable-by-remap fallback branch, return the original path rather than the resolved form — otherwise we'd still leak resolved symlinks for paths outside $HOME.

Test plan

  • pytest tests/hermes_cli/test_gateway_service.py — 58 passed, including new regression test test_preserves_symlink_pointing_outside_home
  • Existing _remap_path_for_user tests (test_remaps_path_under_current_home, test_keeps_system_path_unchanged, test_noop_when_same_user) still pass
  • Reproduced end-to-end on Linux with a uv-managed venv: before the fix, systemctl cat hermes-gateway showed ExecStart=<uv-store-path>/python3.11 ... and the service crash-looped on ModuleNotFoundError: yaml. After the fix, ExecStart=<project>/venv/bin/python ... and the service runs cleanly (Active: active (running)).
  • Not tested on macOS/Windows — change is path-handling only and uses pathlib, so cross-platform impact should be minimal.

Platforms

Tested on Linux (systemd).

_remap_path_for_user() called Path.resolve() on its input, which follows
symlinks. In uv-managed venvs, venv/bin/python is a symlink into uv's
shared Python store (e.g. ~/.local/share/uv/python/cpython-.../bin/python3.11),
so resolving emits the bare interpreter in ExecStart and the service
launches with no venv site-packages — the gateway then crash-loops on
ModuleNotFoundError: yaml (and any other dep) at import time.

Prefer the lexical form for the remap, and only fall back to resolving
when $HOME itself is a symlink. System paths like /opt/hermes continue
to round-trip unchanged.

Adds a regression test covering the uv symlink case.
Copilot AI review requested due to automatic review settings April 12, 2026 22:00

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.

Pull request overview

This PR fixes systemd unit generation for --system installs in uv-managed virtualenvs by preventing _remap_path_for_user() from resolving symlinks (which previously caused ExecStart= to point at uv’s shared interpreter instead of the venv’s bin/python).

Changes:

  • Update _remap_path_for_user() to prefer lexical (unresolved) paths and only use resolved comparisons when needed.
  • Adjust fallback behavior to return the original path (not the resolved path) when remapping doesn’t apply.
  • Add a regression test ensuring a symlinked venv/bin/python that points outside $HOME is preserved.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
hermes_cli/gateway.py Avoids Path.resolve()-driven symlink following during home-dir remapping so systemd ExecStart= stays within the venv.
tests/hermes_cli/test_gateway_service.py Adds regression coverage for uv-style venv interpreter symlink behavior during remapping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +856 to +864
# Simulate a uv-style venv: venv/bin/python -> external interpreter
venv_bin = root_home / "src" / "hermes-agent" / "venv" / "bin"
venv_bin.mkdir(parents=True)
external_python = tmp_path / "uv-store" / "python3.11"
external_python.parent.mkdir(parents=True)
external_python.write_text("")
venv_python = venv_bin / "python"
venv_python.symlink_to(external_python)

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

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

This regression test creates a real filesystem symlink via symlink_to(), which can raise OSError/NotImplementedError on platforms or CI environments where symlinks aren’t supported (notably Windows without dev-mode/admin). Other tests in the repo guard symlink creation and pytest.skip when unavailable; doing the same here will prevent platform-specific failures while still exercising the behavior when symlinks work.

Copilot uses AI. Check for mistakes.
@teknium1

Copy link
Copy Markdown
Contributor

Already fixed on main via PR #8861 (salvage of #7735 by @akhater). The current code drops .resolve() entirely and uses .expanduser() for lexical-only expansion, preserving venv symlinks. Thanks for the contribution!

@teknium1 teknium1 closed this Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants