From a0bf660db68e96087198c47787a21bd267fd99a5 Mon Sep 17 00:00:00 2001 From: kshitijk4poor Date: Tue, 14 Apr 2026 18:41:16 +0530 Subject: [PATCH] fix: invalidate update check cache when HEAD moves (e.g. git pull) The update check cached the 'commits behind' count with a 6-hour TTL based purely on timestamp. When a user ran git pull manually, HEAD moved forward but the banner still showed the stale count until the cache expired. hermes update worked because it explicitly deleted the cache file. Fix: store the local HEAD hash in the .update_check cache. On each check, compare current HEAD against the cached value. If they differ (git pull, checkout, rebase, etc.), treat the cache as stale and re-fetch. The rev-parse call is ~5ms so the fast path (cache hit) remains fast. Backward compat: old cache files without the head key get a one-time invalidation and rewrite with the HEAD hash. --- hermes_cli/banner.py | 24 ++++++++++--- tests/hermes_cli/test_update_check.py | 50 +++++++++++++++++++++++---- 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/hermes_cli/banner.py b/hermes_cli/banner.py index fb6068a81b39..2401539ce49b 100644 --- a/hermes_cli/banner.py +++ b/hermes_cli/banner.py @@ -140,12 +140,25 @@ def check_for_updates() -> Optional[int]: if not (repo_dir / ".git").exists(): return None - # Read cache + # Read cache — invalidate if HEAD has moved (e.g. manual git pull) now = time.time() + current_head = None + try: + head_result = subprocess.run( + ["git", "rev-parse", "HEAD"], + capture_output=True, text=True, timeout=5, + cwd=str(repo_dir), + ) + if head_result.returncode == 0: + current_head = head_result.stdout.strip() + except Exception: + pass try: if cache_file.exists(): cached = json.loads(cache_file.read_text()) - if now - cached.get("ts", 0) < _UPDATE_CHECK_CACHE_SECONDS: + cache_fresh = now - cached.get("ts", 0) < _UPDATE_CHECK_CACHE_SECONDS + head_unchanged = current_head is None or cached.get("head") == current_head + if cache_fresh and head_unchanged: return cached.get("behind") except Exception: pass @@ -174,9 +187,12 @@ def check_for_updates() -> Optional[int]: except Exception: behind = None - # Write cache + # Write cache (include HEAD hash so manual git pull invalidates it) try: - cache_file.write_text(json.dumps({"ts": now, "behind": behind})) + cache_data = {"ts": now, "behind": behind} + if current_head: + cache_data["head"] = current_head + cache_file.write_text(json.dumps(cache_data)) except Exception: pass diff --git a/tests/hermes_cli/test_update_check.py b/tests/hermes_cli/test_update_check.py index 84d5475228ba..3b9afa6a941f 100644 --- a/tests/hermes_cli/test_update_check.py +++ b/tests/hermes_cli/test_update_check.py @@ -17,23 +17,27 @@ 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.""" + """When cache is fresh and HEAD hasn't moved, check_for_updates should return cached value.""" from hermes_cli.banner import check_for_updates - # Create a fake git repo and fresh cache + # Create a fake git repo and fresh cache with matching HEAD repo_dir = tmp_path / "hermes-agent" repo_dir.mkdir() (repo_dir / ".git").mkdir() + fake_head = "abc123def456" 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, "head": fake_head})) + + head_result = MagicMock(returncode=0, stdout=fake_head + "\n") monkeypatch.setenv("HERMES_HOME", str(tmp_path)) - with patch("hermes_cli.banner.subprocess.run") as mock_run: + with patch("hermes_cli.banner.subprocess.run", return_value=head_result) as mock_run: result = check_for_updates() assert result == 3 - mock_run.assert_not_called() + # Only git rev-parse HEAD should be called (to check if HEAD moved), no fetch/rev-list + assert mock_run.call_count == 1 def test_check_for_updates_expired_cache(tmp_path, monkeypatch): @@ -55,7 +59,41 @@ def test_check_for_updates_expired_cache(tmp_path, monkeypatch): result = check_for_updates() assert result == 5 - assert mock_run.call_count == 2 # git fetch + git rev-list + assert mock_run.call_count == 3 # git rev-parse HEAD + git fetch + git rev-list + + +def test_check_for_updates_head_changed_invalidates_cache(tmp_path, monkeypatch): + """When HEAD changes (e.g. manual git pull), fresh cache should be invalidated.""" + from hermes_cli.banner import check_for_updates + + repo_dir = tmp_path / "hermes-agent" + repo_dir.mkdir() + (repo_dir / ".git").mkdir() + + # Fresh cache says 5 behind, recorded at old HEAD + cache_file = tmp_path / ".update_check" + cache_file.write_text(json.dumps({ + "ts": time.time(), "behind": 5, "head": "old_head_abc123" + })) + + def mock_subprocess_run(cmd, **kwargs): + if "rev-parse" in cmd: + return MagicMock(returncode=0, stdout="new_head_def456\n") + # git fetch + git rev-list both succeed, now 0 behind + return MagicMock(returncode=0, stdout="0\n") + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + with patch("hermes_cli.banner.subprocess.run", side_effect=mock_subprocess_run) as mock_run: + result = check_for_updates() + + # Should have re-fetched and recounted because HEAD moved + assert result == 0 + assert mock_run.call_count == 3 # rev-parse + fetch + rev-list + + # Cache should now contain the new HEAD + cached = json.loads(cache_file.read_text()) + assert cached["head"] == "new_head_def456" + assert cached["behind"] == 0 def test_check_for_updates_no_git_dir(tmp_path, monkeypatch):