feat: support package-managed installs (Snap/Homebrew) - #37710
Open
beriberikix wants to merge 4 commits into
Open
feat: support package-managed installs (Snap/Homebrew)#37710beriberikix wants to merge 4 commits into
beriberikix wants to merge 4 commits into
Conversation
… installs Hermes previously had a single `is_managed()` gate that blocked ALL interactive config/credential writes for every package-manager-managed install. That is correct for NixOS, which generates the config declaratively and owns a read-only result, but wrong for package managers like Homebrew and Snap: those own only the install tree, while the user still configures Hermes in a writable HERMES_HOME. This adds `is_config_managed()` (true only for NixOS) and recognizes Snap as a managed system: - `_MANAGED_SYSTEM_NAMES` maps `snap`/`snapcraft` -> "Snap" so the HERMES_MANAGED env var set by the snap launcher is understood. - `is_config_managed()` is used by the file-permission and config-write paths in this module instead of `is_managed()`, so Snap/Homebrew users can save settings and credentials to their writable HERMES_HOME while NixOS still refuses mutations. - Update-command and managed-message helpers learn the Snap variants (`snap refresh hermes-agent`). No behavior change for NixOS or unmanaged installs; this only unblocks the package-managed-but-config-writable case that Snap needs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stalls Builds on the config-managed vs package-managed split. Applies it across the user-facing commands and routes lifecycle actions to the snapd-native equivalents so a confined Snap behaves sensibly: - gateway.py: `gateway setup` now gates on `is_config_managed()` (so Snap users can configure the gateway), and `gateway install/uninstall/ start/stop/restart/status` print snapd guidance (`snap start hermes-agent.gateway`, `snap services …`, etc.) instead of trying to manage a systemd unit that the snap already declares as a daemon. - main.py: `postinstall` is a no-op with guidance under Snap (deps are baked into the snap, not pip-installed), and `uninstall` points at `snap remove hermes-agent` (`--purge` to drop user data). - setup.py and tui_gateway/server.py: swap `is_managed()` -> `is_config_managed()` so the setup wizard and the dashboard's credential endpoint allow writes to the writable HERMES_HOME on Snap/Homebrew. Tests cover get_managed_system()/is_config_managed(), the update/ postinstall/uninstall rewrites, and the gateway service guidance for Snap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "Command Installation" section assumes a pip/editable install: it looks for a venv entry point and a ~/.local/bin/hermes symlink, and on a miss it advises `pip install -e '.[all]'`. On a package-managed install (Snap/Homebrew/NixOS) none of that applies — the package manager provides the command, the tree is read-only, and the pip advice is impossible to follow. On Snap this surfaced as two spurious warnings plus a bogus remediation line in the summary. When `get_managed_system()` is set, doctor now reports "Managed by <system>" and skips the venv/symlink checks entirely. Unmanaged installs are unaffected. Also improves Homebrew/NixOS, which had the same false warning. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "no user allowlists configured" warning hardcoded `~/.hermes/.env`, which is misleading whenever HERMES_HOME is relocated — most visibly under Snap, where the env file lives at `$SNAP_USER_COMMON/hermes/.env`. Use `get_env_path()` (the canonical helper, = `get_hermes_home()/.env`) so the message always points at the file the user should actually edit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
13 tasks
1 task
teknium1
reviewed
Jul 13, 2026
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for identifying the package-managed versus declarative-config distinction. The premise still holds on current main: packaging/homebrew/hermes-agent.rb:32-37 exports HERMES_MANAGED=homebrew, while hermes_cli/config.py:7140-7142, hermes_cli/setup.py:2709-2712, and tui_gateway/server.py:12932-12941 still treat every managed install as configuration-read-only.
Problems
- The added classification tests in
tests/hermes_cli/test_managed_installs.py:14-34do not prove the central behavioral claim: thatconfig.yamland.envwrites succeed for Homebrew/Snap while NixOS remains blocked. - Current main has a direct TUI config writer at
tui_gateway/server.py:1951-1957that bypassessave_config; salvage should preserve the NixOS config-managed boundary there too.
Suggested changes
- Add temp-
HERMES_HOMEpersistence tests forsave_configandsave_env_valueunder Homebrew/Snap, plus a NixOS rejection regression. - Reapply the policy split against current writer paths rather than only the PR's June snapshot. GitHub currently reports this branch as conflicting.
This is an automated hermes-sweeper review.
| monkeypatch.setenv("HERMES_MANAGED", "homebrew") | ||
|
|
||
| assert get_managed_system() == "Homebrew" | ||
| assert is_config_managed() is False |
Contributor
There was a problem hiding this comment.
This checks only the new predicate. Please add a temp-HERMES_HOME behavioral test that exercises save_config and save_env_value: Homebrew/Snap should persist, while NixOS must still reject the writes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Distinguishes config-managed installs (NixOS — declarative, read-only config) from package-managed installs (Snap/Homebrew — the package owns the install tree, but the user still configures Hermes in a writable
HERMES_HOME).Previously a single
is_managed()gate blocked all interactive config/credential writes for any managed install. That is correct for NixOS, but wrong for Snap/Homebrew, where the user must still be able to save settings. This is the prerequisite for snap packaging (#37709), and it independently fixes spurious managed-install guidance on Homebrew/NixOS.Related Issue
Part of #37709
Type of Change
(Also folds in two small, same-theme fixes: a
doctorfalse warning and a misleading gateway log path under relocatedHERMES_HOME.)Changes Made
hermes_cli/config.py: addis_config_managed()(true only for NixOS); recognizesnap/snapcraft→ "Snap"; Snap update/message helpers; swapis_managed()→is_config_managed()in the file-permission and config-write paths so package-managed installs can write toHERMES_HOME.hermes_cli/gateway.py,hermes_cli/setup.py,tui_gateway/server.py: allow config writes on package-managed installs, and routegateway install/uninstall/start/stop/statusto snapd-native commands.hermes_cli/main.py:postinstallis a no-op with guidance under Snap;uninstallpoints atsnap remove hermes-agent.hermes_cli/doctor.py: skip the pip/venv "Command Installation" checks on managed installs (thepip install -eadvice can't apply to a read-only package).gateway/run.py: the allowlist warning now references the resolvedHERMES_HOME/.envpath viaget_env_path()instead of a hardcoded~/.hermes/.env.tests/hermes_cli/test_managed_installs.py,test_gateway_service.py,test_doctor_command_install.py.How to Test
pytest tests/hermes_cli/test_managed_installs.py \ tests/hermes_cli/test_gateway_service.py \ tests/hermes_cli/test_doctor_command_install.py -qManual:
Checklist
Code
feat(config):,fix(doctor):, …)scripts/run_tests.shhas unrelated pre-existing env failures on my machine — optional backends needing API keys/network: web/voice/browser/image/wecom/acp — none in files this PR touches.)Documentation & Housekeeping
cli-config.yaml.example— N/A (no config keys added/changed)CONTRIBUTING.md/AGENTS.md— N/Adoctorchange sits inside the existingsys.platform != "win32"guard