diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 77cffed56a6..7dc7ae50425 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -345,6 +345,58 @@ def recommended_update_command() -> str: return recommended_update_command_for_method(method) +# Long-form text for ``hermes update`` / ``--check`` when running inside the +# Docker image. Surfaced by ``cmd_update`` and ``_cmd_update_check`` in +# hermes_cli/main.py; lives here so the wording stays consistent and we +# don't grow two slightly-different copies. +# +# Why this matters: +# - The published image excludes ``.git`` (see .dockerignore), so the +# git-based update path can never succeed inside the container. +# - The pre-existing fallback message ("✗ Not a git repository. Please +# reinstall: curl ... install.sh") is actively misleading inside Docker +# — that script installs a *new* host-side Hermes, it doesn't update +# the running container. +# - The right action is ``docker pull`` + restart the container; this +# helper spells that out, with notes on tag pinning and config +# persistence so users don't get blindsided. +_DOCKER_UPDATE_MESSAGE = """\ +✗ ``hermes update`` doesn't apply inside the Docker container. + +Hermes Agent runs as a published image (nousresearch/hermes-agent), not a +git checkout — the container has no working tree to pull into. Update by +pulling a fresh image and restarting your container instead: + + docker pull nousresearch/hermes-agent:latest + # then restart whatever started the container, e.g.: + docker compose up -d --force-recreate hermes-agent + # or, for ad-hoc runs, exit the current container and `docker run` again + +Verify the new version after restart: + docker run --rm nousresearch/hermes-agent:latest --version + +Notes: + • If you pinned a specific tag (e.g. ``:v0.14.0``) the ``:latest`` tag + won't move your container — pull the newer tag you actually want, or + switch to ``:latest`` / ``:main`` for rolling updates. See available + tags at https://hub.docker.com/r/nousresearch/hermes-agent/tags + • Your config and session history live under ``$HERMES_HOME`` (``/opt/data`` + in the container, typically bind-mounted from the host) and persist + across image upgrades — re-pulling doesn't lose any state. + • Running a fork? Build your own image with this repo's ``Dockerfile`` + and replace the ``docker pull`` step with your build/push pipeline.""" + + +def format_docker_update_message() -> str: + """Return the user-facing message for ``hermes update`` inside Docker. + + Centralised so ``cmd_update`` (the apply path) and ``_cmd_update_check`` + (the dry-run path) share the same wording. See ``_DOCKER_UPDATE_MESSAGE`` + above for the full rationale. + """ + return _DOCKER_UPDATE_MESSAGE + + def format_managed_message(action: str = "modify this Hermes installation") -> str: """Build a user-facing error for managed installs.""" managed_system = get_managed_system() or "a package manager" diff --git a/hermes_cli/main.py b/hermes_cli/main.py index f1f83333124..adbea3682c9 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -8416,6 +8416,14 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): """ from hermes_cli.config import detect_install_method method = detect_install_method(PROJECT_ROOT) + if method == "docker": + # Docker can't ``git fetch`` from within the container. Surface the + # same long-form ``docker pull`` guidance ``hermes update`` (apply + # path) uses — telling the user to "reinstall via curl" or that + # ".git is missing" would point them at the wrong remediation. + from hermes_cli.config import format_docker_update_message + print(format_docker_update_message()) + sys.exit(1) if method == "pip": from hermes_cli.config import recommended_update_command from hermes_cli.banner import check_via_pypi @@ -8716,12 +8724,27 @@ def cmd_update(args): runs the update, then restores stdio on the way out (even on ``sys.exit`` or unhandled exceptions). """ - from hermes_cli.config import is_managed, managed_error + from hermes_cli.config import ( + detect_install_method, + format_docker_update_message, + is_managed, + managed_error, + ) if is_managed(): managed_error("update Hermes Agent") return + # Docker users can't ``git pull`` — the image excludes ``.git`` from + # the build context. Bail with a friendly explanation pointing at + # ``docker pull`` BEFORE any of the apply-path / check-path branches + # below get a chance to error out with misleading "Not a git + # repository" text. See format_docker_update_message() for the full + # rationale and tag-pinning / config-persistence notes. + if detect_install_method(PROJECT_ROOT) == "docker": + print(format_docker_update_message()) + sys.exit(1) + if getattr(args, "check", False): # --check honors --branch so the "any new commits?" answer matches # what a subsequent `hermes update --branch=` would actually pull. diff --git a/tests/hermes_cli/test_cmd_update_docker.py b/tests/hermes_cli/test_cmd_update_docker.py new file mode 100644 index 00000000000..7bccfe89782 --- /dev/null +++ b/tests/hermes_cli/test_cmd_update_docker.py @@ -0,0 +1,173 @@ +"""Tests for ``hermes update`` / ``--check`` inside the Docker container. + +Background: ``.dockerignore`` excludes ``.git``, so the existing git-pull +update path can never succeed inside the published image. Before this +fix, ``hermes update`` would fall through to ``"✗ Not a git repository. +Please reinstall: curl ... install.sh"`` — that script installs a *new* +host-side Hermes, not an update to the running container, so the message +was actively misleading. + +These tests pin the new behaviour: when ``detect_install_method`` reports +``"docker"`` (stamped by ``docker/stage2-hook.sh``), both the apply path +(``cmd_update``) and the check path (``_cmd_update_check``) print the +``docker pull`` guidance from ``format_docker_update_message`` and exit +with status 1, without running ``git fetch`` / ``subprocess.run``. +""" + +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + +from hermes_cli.main import _cmd_update_check, cmd_update + + +# ---------- cmd_update (apply path) ---------- + + +@patch("hermes_cli.config.is_managed", return_value=False) +@patch("hermes_cli.config.detect_install_method", return_value="docker") +@patch("subprocess.run") +def test_cmd_update_in_docker_prints_guidance_and_exits( + mock_run, _mock_method, _mock_managed, capsys +): + """``hermes update`` inside Docker → friendly message + exit 1, no git calls.""" + with pytest.raises(SystemExit) as excinfo: + cmd_update(SimpleNamespace(check=False)) + + assert excinfo.value.code == 1 + out = capsys.readouterr().out + # Spot-check the key guidance — exhaustive wording is locked in by the + # config-module test below to keep these CLI tests resilient to copy edits. + assert "doesn't apply inside the Docker container" in out + assert "docker pull nousresearch/hermes-agent:latest" in out + + # No git invocations — the early-return must beat every git command. + git_calls = [c for c in mock_run.call_args_list if c.args and c.args[0] and "git" in str(c.args[0][0])] + assert git_calls == [], f"expected no git calls, got: {git_calls}" + + +@patch("hermes_cli.config.is_managed", return_value=False) +@patch("hermes_cli.config.detect_install_method", return_value="docker") +@patch("subprocess.run") +def test_cmd_update_check_in_docker_prints_guidance_and_exits( + mock_run, _mock_method, _mock_managed, capsys +): + """``hermes update --check`` inside Docker → same message + exit 1, no fetch.""" + with pytest.raises(SystemExit) as excinfo: + cmd_update(SimpleNamespace(check=True, branch=None)) + + assert excinfo.value.code == 1 + out = capsys.readouterr().out + assert "doesn't apply inside the Docker container" in out + assert "docker pull nousresearch/hermes-agent:latest" in out + + git_calls = [c for c in mock_run.call_args_list if c.args and c.args[0] and "git" in str(c.args[0][0])] + assert git_calls == [], f"expected no git calls, got: {git_calls}" + + +@patch("hermes_cli.config.is_managed", return_value=False) +@patch("hermes_cli.config.detect_install_method", return_value="docker") +@patch("subprocess.run") +def test_cmd_update_in_docker_ignores_yes_and_force( + mock_run, _mock_method, _mock_managed, capsys +): + """``--yes`` / ``--force`` don't bypass the Docker bail-out. + + The point of the bail-out is "git pull will never work here", so even + a user trying to barge through with ``--yes --force`` should see the + docker-pull guidance. + """ + with pytest.raises(SystemExit): + cmd_update(SimpleNamespace(check=False, yes=True, force=True)) + + assert "docker pull" in capsys.readouterr().out + git_calls = [c for c in mock_run.call_args_list if c.args and c.args[0] and "git" in str(c.args[0][0])] + assert git_calls == [] + + +# ---------- _cmd_update_check (check path, direct entry) ---------- + + +@patch("hermes_cli.config.detect_install_method", return_value="docker") +@patch("subprocess.run") +def test_cmd_update_check_direct_in_docker(mock_run, _mock_method, capsys): + """Calling ``_cmd_update_check`` directly (no apply path) also bails.""" + with pytest.raises(SystemExit) as excinfo: + _cmd_update_check() + + assert excinfo.value.code == 1 + assert "docker pull" in capsys.readouterr().out + git_calls = [c for c in mock_run.call_args_list if c.args and c.args[0] and "git" in str(c.args[0][0])] + assert git_calls == [] + + +# ---------- Non-Docker installs unaffected ---------- + + +@patch("hermes_cli.config.is_managed", return_value=False) +@patch("hermes_cli.config.detect_install_method", return_value="git") +def test_cmd_update_on_git_install_does_not_print_docker_message( + _mock_method, _mock_managed, capsys +): + """Source/git installs MUST NOT hit the Docker branch. + + Regression guard: an over-eager detection refactor could accidentally + route git users through the docker-pull message. We swallow the + SystemExit / errors from the rest of the update flow — those don't + matter for this assertion; what matters is that the docker text is + absent. + """ + try: + cmd_update(SimpleNamespace(check=True, branch=None)) + except SystemExit: + # Update flow may exit for unrelated reasons in a test env (no + # network, no real .git) — that's fine; we only care about the + # banner not appearing. + pass + + assert "doesn't apply inside the Docker container" not in capsys.readouterr().out + + +@patch("hermes_cli.config.detect_install_method", return_value="pip") +@patch("hermes_cli.banner.check_via_pypi", return_value=0) +def test_cmd_update_check_on_pip_install_still_uses_pypi( + _mock_pypi, _mock_method, capsys +): + """PyPI installs route to PyPI check, not the Docker bail-out.""" + _cmd_update_check() + + out = capsys.readouterr().out + assert "Already up to date" in out + assert "doesn't apply inside the Docker container" not in out + + +# ---------- format_docker_update_message — content lock ---------- + + +def test_format_docker_update_message_contents(): + """Lock in the high-value content of the Docker update message. + + These are the bits a user actually needs to act on; if any of them + disappear in a copy edit, the message has lost its value. Specific + wording around them is free to evolve (we don't assert full text). + """ + from hermes_cli.config import format_docker_update_message + + msg = format_docker_update_message() + + # Primary command — the entire reason this message exists. + assert "docker pull nousresearch/hermes-agent:latest" in msg + + # The four key concepts the message must cover: + assert "restart" in msg.lower(), "must explain that a restart is required" + assert "--version" in msg, "must show how to verify the new version" + assert ":latest" in msg, "must mention tag pinning caveat" + assert "HERMES_HOME" in msg or "/opt/data" in msg, ( + "must address config persistence across upgrades" + ) + + # Acknowledges that forks exist (build-your-own-image escape hatch). + assert "fork" in msg.lower() or "Dockerfile" in msg