diff --git a/hermes_cli/banner.py b/hermes_cli/banner.py index a581ba15f..c91b2f728 100644 --- a/hermes_cli/banner.py +++ b/hermes_cli/banner.py @@ -221,7 +221,11 @@ def check_for_updates() -> Optional[int]: cache_file = hermes_home / ".update_check" embedded_rev = os.environ.get("HERMES_REVISION") or None - # Read cache — invalidate if the embedded rev has changed since last check + # Read cache — invalidate if the embedded rev OR installed version has + # changed since the last check. The version guard matters for pip installs: + # `check_via_pypi()` compares against VERSION, so a `pip install --upgrade` + # changes VERSION but leaves rev unchanged (both None), and without this + # the stale "behind" count would survive the upgrade for up to 6h. See #34491. now = time.time() try: if cache_file.exists(): @@ -229,6 +233,7 @@ def check_for_updates() -> Optional[int]: if ( now - cached.get("ts", 0) < _UPDATE_CHECK_CACHE_SECONDS and cached.get("rev") == embedded_rev + and cached.get("ver") == VERSION ): return cached.get("behind") except Exception: @@ -249,7 +254,9 @@ def check_for_updates() -> Optional[int]: behind = _check_via_local_git(repo_dir) try: - cache_file.write_text(json.dumps({"ts": now, "behind": behind, "rev": embedded_rev})) + cache_file.write_text( + json.dumps({"ts": now, "behind": behind, "rev": embedded_rev, "ver": VERSION}) + ) except Exception: pass @@ -691,6 +698,21 @@ def build_welcome_banner(console: Console, model: str, cwd: str, except Exception: pass # Never break the banner over an update check + # Pip-install warning — `pip install hermes-agent` is not the supported + # install path (it exists on PyPI for internal/CI reasons, not end users). + # Such installs miss the git checkout + installer-managed deps, so updates, + # self-update, and issue triage don't behave correctly. Warn, don't block. + try: + from hermes_cli.config import detect_install_method + if detect_install_method() == "pip": + right_lines.append( + "[bold yellow]⚠ pip install not officially supported[/]" + "[dim yellow] — exists for reasons other than user install; " + "expect instability and an inability to support issues[/]" + ) + except Exception: + pass # Never break the banner over the install-method check + right_content = "\n".join(right_lines) layout_table.add_row(left_content, right_content) diff --git a/tests/hermes_cli/test_pip_install_detection.py b/tests/hermes_cli/test_pip_install_detection.py index bfdc8be4f..49df74f62 100644 --- a/tests/hermes_cli/test_pip_install_detection.py +++ b/tests/hermes_cli/test_pip_install_detection.py @@ -59,3 +59,53 @@ def test_docker_detected_via_dockerenv(tmp_path): def test_recommended_update_command_docker(): from hermes_cli.config import recommended_update_command_for_method assert "docker pull" in recommended_update_command_for_method("docker") + + +def test_banner_warns_on_pip_install(tmp_path): + """The welcome banner surfaces a warning when the install method is pip.""" + import io + from rich.console import Console + from hermes_cli import banner + + hh = tmp_path / ".hermes" + hh.mkdir() + (hh / ".install_method").write_text("pip\n") + + with patch("hermes_cli.config.get_hermes_home", return_value=hh), \ + patch("hermes_constants.get_hermes_home", return_value=hh): + buf = io.StringIO() + # Wide console so the warning isn't wrapped across lines in the panel. + console = Console(file=buf, width=400, force_terminal=False, color_system=None) + banner.build_welcome_banner( + console, model="m", cwd="/tmp", + tools=[{"function": {"name": "terminal"}}], + enabled_toolsets=["terminal"], + ) + out = buf.getvalue() + + assert "officially" in out + assert "instability" in out + + +def test_banner_no_pip_warning_on_git_install(tmp_path): + """Git installs must not show the pip-install warning.""" + import io + from rich.console import Console + from hermes_cli import banner + + hh = tmp_path / ".hermes" + hh.mkdir() + (hh / ".install_method").write_text("git\n") + + with patch("hermes_cli.config.get_hermes_home", return_value=hh), \ + patch("hermes_constants.get_hermes_home", return_value=hh): + buf = io.StringIO() + console = Console(file=buf, width=400, force_terminal=False, color_system=None) + banner.build_welcome_banner( + console, model="m", cwd="/tmp", + tools=[{"function": {"name": "terminal"}}], + enabled_toolsets=["terminal"], + ) + out = buf.getvalue() + + assert "officially" not in out diff --git a/tests/hermes_cli/test_update_check.py b/tests/hermes_cli/test_update_check.py index 8a68d6a17..c7cc1c867 100644 --- a/tests/hermes_cli/test_update_check.py +++ b/tests/hermes_cli/test_update_check.py @@ -19,6 +19,7 @@ def test_version_string_no_v_prefix(): def test_check_for_updates_uses_cache(tmp_path, monkeypatch): """When cache is fresh, check_for_updates should return cached value without calling git.""" from hermes_cli.banner import check_for_updates + from hermes_cli import __version__ # Create a fake git repo and fresh cache repo_dir = tmp_path / "hermes-agent" @@ -26,7 +27,7 @@ def test_check_for_updates_uses_cache(tmp_path, monkeypatch): (repo_dir / ".git").mkdir() cache_file = tmp_path / ".update_check" - cache_file.write_text(json.dumps({"ts": time.time(), "behind": 3})) + cache_file.write_text(json.dumps({"ts": time.time(), "behind": 3, "ver": __version__})) monkeypatch.setenv("HERMES_HOME", str(tmp_path)) with patch("hermes_cli.banner.subprocess.run") as mock_run: @@ -36,6 +37,43 @@ def test_check_for_updates_uses_cache(tmp_path, monkeypatch): mock_run.assert_not_called() +def test_check_for_updates_invalidates_on_version_change(tmp_path, monkeypatch): + """A fresh cache from a different installed version must be re-checked, not reused. + + Regression for #34491: after `pip install --upgrade`, VERSION changes but the + cache's 6h TTL hadn't expired and rev was unchanged (both None), so the stale + 'behind' count survived the upgrade. The version guard forces a recheck. + """ + import hermes_cli.banner as banner + + # No local git checkout -> the PyPI path is exercised (pip-install class). + fake_banner = tmp_path / "hermes_cli" / "banner.py" + fake_banner.parent.mkdir(parents=True, exist_ok=True) + fake_banner.touch() + monkeypatch.setattr(banner, "__file__", str(fake_banner)) + + # Fresh (within TTL) cache that says "behind", but stamped with an OLD version. + cache_file = tmp_path / ".update_check" + cache_file.write_text( + json.dumps({"ts": time.time(), "behind": 1, "rev": None, "ver": "0.0.1-old"}) + ) + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.delenv("HERMES_REVISION", raising=False) + with patch("hermes_cli.banner.subprocess.run") as mock_run, \ + patch("hermes_cli.banner.check_via_pypi", return_value=0) as mock_pypi: + result = banner.check_for_updates() + + # Stale-version cache rejected -> fresh check ran -> up-to-date result. + assert result == 0 + mock_pypi.assert_called_once() + mock_run.assert_not_called() + + # Cache rewritten with the current installed version. + written = json.loads(cache_file.read_text()) + assert written["ver"] == banner.VERSION + + def test_check_for_updates_expired_cache(tmp_path, monkeypatch): """When cache is expired, check_for_updates should call git fetch.""" from hermes_cli.banner import check_for_updates