feat(hermes): declarative gateway deps via uv dependency group - #1660
Conversation
Add hermes dependency group to pyproject.toml for Renovate tracking. Install python-telegram-bot and croniter into hermes venv at activation.
|
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR extends the Hermes service infrastructure by adding a dashboard service, updating token resolution with environment variable fallback priority, defining service dependencies, and installing runtime packages during activation. ChangesHermes Service Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Mesa DescriptionTL;DRMake Hermes gateway runtime deps declarative with a new What changed?
Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces automated installation of Hermes gateway runtime dependencies via the activate.sh script and defines these dependencies in pyproject.toml. Feedback suggests replacing hardcoded paths with configurable environment variables and removing stderr redirection to improve debuggability during dependency installation.
| chmod 700 "$HOME_DIR/.hermes" | ||
|
|
||
| # Install hermes gateway runtime deps from pyproject.toml dependency group | ||
| HERMES_VENV="$HOME_DIR/ghq/github.com/NousResearch/hermes-agent/.venv" |
There was a problem hiding this comment.
The path to the virtual environment is hardcoded to a specific ghq directory structure. This makes the script brittle if the repository is cloned elsewhere. Consider allowing this path to be overridden via an environment variable to improve portability.
| HERMES_VENV="$HOME_DIR/ghq/github.com/NousResearch/hermes-agent/.venv" | |
| HERMES_VENV="${HERMES_VENV_PATH:-$HOME_DIR/ghq/github.com/NousResearch/hermes-agent/.venv}" |
| uv pip install --python "$HERMES_VENV/bin/python" \ | ||
| --group hermes \ | ||
| --project "$HOME_DIR/dotfiles" 2>/dev/null || true |
There was a problem hiding this comment.
Silencing stderr with 2>/dev/null makes it difficult to diagnose failures during the dependency installation (e.g., if uv is missing, the network is down, or there are version conflicts). It is better to allow the error output to be visible, especially since the exit code is already safely ignored with || true. Additionally, the --project path is hardcoded to ~/dotfiles; using an environment variable would make this more flexible.
| uv pip install --python "$HERMES_VENV/bin/python" \ | |
| --group hermes \ | |
| --project "$HOME_DIR/dotfiles" 2>/dev/null || true | |
| uv pip install --python "$HERMES_VENV/bin/python" \ | |
| --group hermes \ | |
| --project "${DOTFILES_DIR:-$HOME_DIR/dotfiles}" || true |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="home-manager/services/hermes/activate.sh">
<violation number="1" location="home-manager/services/hermes/activate.sh:20">
P1: Do not silence and ignore `uv pip install` failures; this can leave Hermes missing runtime dependencies while activation appears successful.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if [ -d "$HERMES_VENV" ]; then | ||
| uv pip install --python "$HERMES_VENV/bin/python" \ | ||
| --group hermes \ | ||
| --project "$HOME_DIR/dotfiles" 2>/dev/null || true |
There was a problem hiding this comment.
P1: Do not silence and ignore uv pip install failures; this can leave Hermes missing runtime dependencies while activation appears successful.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/hermes/activate.sh, line 20:
<comment>Do not silence and ignore `uv pip install` failures; this can leave Hermes missing runtime dependencies while activation appears successful.</comment>
<file context>
@@ -11,3 +11,11 @@ mkdir -p "$HOME_DIR/.hermes/memories"
+if [ -d "$HERMES_VENV" ]; then
+ uv pip install --python "$HERMES_VENV/bin/python" \
+ --group hermes \
+ --project "$HOME_DIR/dotfiles" 2>/dev/null || true
+fi
</file context>
| --project "$HOME_DIR/dotfiles" 2>/dev/null || true | |
| --project "$HOME_DIR/dotfiles" |
![indent-zero[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
| if [ -d "$HERMES_VENV" ]; then | ||
| uv pip install --python "$HERMES_VENV/bin/python" \ | ||
| --group hermes \ | ||
| --project "$HOME_DIR/dotfiles" 2>/dev/null || true |
There was a problem hiding this comment.
Next uv sync on hermes-agent will undo this install.
Upstream NousResearch/hermes-agent's pyproject.toml only declares python-telegram-bot[webhooks]>=22.6,<23 under the optional [messaging] (and [termux]) extras — croniter is already a core dep. Meanwhile scripts/update-local-binaries.sh:223 runs a bare uv sync against the hermes-agent repo, and uv sync removes packages that aren't in the project's pyproject by default (verified locally: a uv sync after uv pip install python-telegram-bot removed it: - python-telegram-bot==22.7, - httpx, - httpcore, ...).
So the runtime sequence becomes:
ulbbuilds the venv → nopython-telegram-bot(themessagingextra wasn't requested).home-manager switch→ this script reinstallspython-telegram-bot.- User runs
ulbagain for any reason →uv syncstripspython-telegram-bot. - systemd starts
hermes-gateway→ import fails,Restart=always+StartLimitBurst=10kicks in until it stops.
Until the user remembers to re-run home-manager switch, the gateway is dead. Cleaner long-term fix: pass --extra messaging to the uv sync call inside scripts/update-local-binaries.sh for the hermes-agent build (and drop this whole install block + the hermes group from pyproject.toml). Alternative: add an ExecStartPre to home-manager/services/hermes/default.nix that re-runs this install before each gateway start.
| if [ -d "$HERMES_VENV" ]; then | ||
| uv pip install --python "$HERMES_VENV/bin/python" \ | ||
| --group hermes \ | ||
| --project "$HOME_DIR/dotfiles" 2>/dev/null || true |
There was a problem hiding this comment.
Silent failure hides every install error.
With 2>/dev/null || true, all of the following exit 0 with zero output:
uvnot on PATH (verified:PATH=/usr/bin:/bin bash -c 'uv pip install ... 2>/dev/null || true'→ exit 0, nothing printed even thoughuv: command not foundfired on stderr).- Network unreachable.
- Resolver/version conflicts (e.g. if hermes-agent later pins an incompatible version).
- The existing
exclude-newer = "1 week"parse warning thatuv 0.8.xemits on this repo'spyproject.toml(visible in dry-run).
When hermes-gateway later crashes due to missing deps, there will be nothing in the home-manager activation log to point at the cause.
Compare to the codebase's own pattern in home-manager/modules/uv-globals/install-uv-globals.sh:24-27, 70:
if ! command -v uv &>/dev/null; then
echo "uv not found, skipping uv globals install"
exit 0
fi
...
uv tool install "$pkg" ... 2>/dev/null || echo "Failed to install $pkg, skipping..."Suggested change here: drop the 2>/dev/null and replace || true with || echo "[hermes] failed to install runtime deps" >&2, plus an explicit command -v uv guard. That keeps the script non-fatal but lets the user see why nothing was installed.
| # Hermes gateway runtime deps - installed into hermes venv via uv pip install | ||
| hermes = [ | ||
| "python-telegram-bot[webhooks]>=22.6,<23", | ||
| "croniter>=6.0.0,<7", |
There was a problem hiding this comment.
croniter is already a core dep of hermes-agent.
Upstream NousResearch/hermes-agent's pyproject.toml lists croniter>=6.0.0,<7 as a core dependency (the project explicitly notes "croniter is now a core dependency; this extra kept for back-compat"). So uv sync against the hermes-agent repo will always install it — the line here just creates a second version pin that has to be kept in sync with upstream.
The only dep that genuinely benefits from this declarative install path is python-telegram-bot[webhooks], since it lives in upstream's optional [messaging] (and [termux]) extras and is therefore not installed by a bare uv sync.
Safe to drop just this line. Even better: switch the build to uv sync --extra messaging in scripts/update-local-binaries.sh and remove the whole hermes group + the install block in activate.sh (see the other comment).
| "croniter>=6.0.0,<7", | |
| hermes = [ | |
| "python-telegram-bot[webhooks]>=22.6,<23", | |
| ] |
- Add hermes-dashboard systemd service (port 9119, --insecure for k3s) - Use HERMES_TELEGRAM_TOKEN env var for dedicated bot - Add fastapi/uvicorn to hermes dependency group
1 participant
Summary
hermesdependency group topyproject.tomlwithpython-telegram-bot[webhooks]andcroniterhome-manager/services/hermes/activate.shto install deps into hermes venv at activation time viauv pip install --group hermesTest plan
uv pip install --group hermesinstalls correctly into hermes venvmake shell-testpasses (1397 examples, 0 failures)Summary by cubic
Make Hermes gateway runtime deps declarative via a
hermesuv group with auto-install on activation, and add ahermes-dashboardservice. Prefer a dedicatedHERMES_TELEGRAM_TOKEN; Renovate tracks versions to avoid missing-deps warnings.New Features
hermes-dashboardsystemd user service (port 9119,--insecurefor k3s).HERMES_TELEGRAM_TOKENis now used for the Telegram bot, with fallback toTELEGRAM_TOKEN/secrets.Dependencies
hermesgroup inpyproject.toml:python-telegram-bot[webhooks]>=22.6,<23,croniter>=6.0.0,<7,fastapi[standard]>=0.115.0,uvicorn>=0.30.0.uv pipwith the venv’s Python.Written for commit 7a9f188. Summary will update on new commits.