fix(cli): banner/TUI/dashboard update checks go through the GitHub API, cached 24h

The Python passive check (`banner.check_for_updates`, used by the CLI
banner, `hermes --tui`, every `tui_gateway` spawn and the dashboard's
/api/hermes/update/check) also ran `git fetch origin main` on every cache
miss, and never cached an inconclusive result so a flaky line retried on
every start. Same GitHub complaint, same fix:

- remote tip via GET /repos/{slug}/commits/main (vnd.github.sha), local tip
  via rev-parse, exact count + changelog via the compare API when they
  differ. HTTPS `ls-remote` remains only as the fallback when the API is
  unreachable or the origin isn't on GitHub.
- cache TTL 6h -> 24h, failures cached 1h; the cache is keyed on HEAD so
  `hermes update` invalidates it immediately.
- the dashboard's "what's changed" list comes from the memoized compare
  payload (`upstream_commits_behind`) instead of `git log HEAD..origin/main`,
  which was stale without a fetch.

Tests rewritten to the new contract: passive checks must not run
`git fetch`/`ls-remote` for a GitHub origin; the daily cache invalidates
when HEAD moves and re-asks after the failure window.
This commit is contained in:
Teknium
2026-09-10 12:11:26 -07:00
parent 6dfcf15685
commit 338bf9ea9a
8 changed files with 267 additions and 402 deletions

View File

@@ -126,7 +126,12 @@ def get_available_skills() -> Dict[str, List[str]]:
# === Update check ===
_UPDATE_CHECK_CACHE_SECONDS = 6 * 3600 # avoid repeated git fetches
# Passive checks hit GitHub at most once a day per install; a failed check retries after an hour
# so a flaky line can't turn every startup into a request (nor stay wrong for a day).
_UPDATE_CHECK_CACHE_SECONDS = 24 * 3600
_UPDATE_CHECK_FAILURE_CACHE_SECONDS = 3600
# Upstream tip seen by the most recent check; recorded in the cache file for the changelog.
_last_target_rev: Optional[str] = None
# Returned when an update is known to exist but commits can't be counted (e.g. nix builds).
UPDATE_AVAILABLE_NO_COUNT = -1
@@ -197,10 +202,7 @@ def _git_ok(args: list[str], **kw) -> bool:
def _git_count(args: list[str], *, cwd: Path) -> Optional[int]:
"""``int`` of a successful ``git rev-list --count``-style command, else None.
Deliberately bypasses ``_git_stdout`` so tests can stub the two layers independently.
"""
"""``int`` of a successful ``git rev-list --count``-style command, else None."""
result = _git_run(args, cwd=cwd)
if result is not None and result.returncode == 0:
return _quiet(lambda: int(result.stdout.strip()))
@@ -211,14 +213,21 @@ def _is_full_sha(value: Optional[str]) -> bool:
return isinstance(value, str) and len(value) == 40 and all(c in "0123456789abcdefABCDEF" for c in value)
def _github_compare_behind(current_rev: str, target_rev: str) -> Optional[int]:
"""Exact behind-count via the GitHub compare API for uncountable graphs.
_compare_payload_cache: Dict[tuple, dict] = {}
Shallow installer clones and ls-remote-only probes know the two tip SHAs but have no local
history to run ``rev-list --count`` across.
def _github_compare(current_rev: str, target_rev: str) -> Optional[dict]:
"""Compare payload for ``current...target`` from the GitHub API; memoized per process.
Shallow installer clones and API-only probes know the two tip SHAs but have no local history
to run ``rev-list --count`` or ``git log`` across; the payload carries both the count
(``ahead_by``) and the commit list the dashboard/desktop render as "what's changed".
"""
if not (_is_full_sha(current_rev) and _is_full_sha(target_rev)):
return None
key = (current_rev, target_rev)
if key in _compare_payload_cache:
return _compare_payload_cache[key]
url = f"https://api.github.com/repos/nousresearch/hermes-agent/compare/{current_rev}...{target_rev}"
def _fetch():
@@ -229,10 +238,49 @@ def _github_compare_behind(current_rev: str, target_rev: str) -> Optional[int]:
with urllib.request.urlopen(req, timeout=10) as resp:
return json.loads(resp.read().decode("utf-8"))
payload = _quiet(_fetch)
ahead = payload.get("ahead_by") if isinstance(payload, dict) else None
if not isinstance(payload, dict):
return None
_compare_payload_cache[key] = payload
return payload
def _github_compare_behind(current_rev: str, target_rev: str) -> Optional[int]:
"""Exact behind-count via the GitHub compare API for uncountable graphs."""
payload = _github_compare(current_rev, target_rev)
ahead = payload.get("ahead_by") if payload else None
return ahead if isinstance(ahead, int) and not isinstance(ahead, bool) and ahead >= 0 else None
def upstream_commits_behind(n: int = 20) -> List[Dict[str, Any]]:
"""Commits between the last checked HEAD and upstream tip, newest first; [] when unknown.
Reads the tips recorded by ``check_for_updates`` so it costs no extra request when the
compare payload is already memoized for this process.
"""
cached = _read_json(get_hermes_home() / ".update_check") or {}
head_rev, target_rev = cached.get("head"), cached.get("target")
if not head_rev or not target_rev or head_rev == target_rev:
return []
payload = _github_compare(head_rev, target_rev)
rows: List[Dict[str, Any]] = []
for entry in (payload or {}).get("commits", []) if isinstance(payload, dict) else []:
commit = entry.get("commit") or {}
when = ((commit.get("committer") or {}).get("date") or "")
try:
from datetime import datetime
at = int(datetime.fromisoformat(when.replace("Z", "+00:00")).timestamp()) if when else 0
except ValueError:
at = 0
rows.append({
"sha": str(entry.get("sha", ""))[:7],
"summary": str(commit.get("message", "")).split("\n", 1)[0],
"author": str((commit.get("author") or {}).get("name", "")),
"at": at,
})
rows.reverse() # compare returns oldest first
return rows[:n]
def _tips_behind(head_rev: Optional[str], target_rev: Optional[str], repo_dir: Optional[Path] = None) -> Optional[int]:
"""Behind-count from two tip SHAs: None if either is unknown, 0 when equal, else count/sentinel.
@@ -250,8 +298,27 @@ def _tips_behind(head_rev: Optional[str], target_rev: Optional[str], repo_dir: O
return counted if counted is not None else UPDATE_AVAILABLE_NO_COUNT
def _github_branch_tip(repo_slug: str, branch: str) -> Optional[str]:
"""Tip SHA of ``branch`` on GitHub via the REST API (40-byte body, no git, no auth)."""
from urllib.parse import quote
url = f"https://api.github.com/repos/{repo_slug}/commits/{quote(branch, safe='')}"
def _fetch():
import urllib.request
req = urllib.request.Request(
url, headers={"Accept": "application/vnd.github.sha", "User-Agent": "hermes-cli-update-check"})
with urllib.request.urlopen(req, timeout=10) as resp:
return resp.read().decode("utf-8").strip()
sha = _quiet(_fetch)
return sha if _is_full_sha(sha) else None
def _upstream_main_sha() -> Optional[str]:
"""Tip SHA of upstream main via HTTPS ls-remote (no auth, no prompts)."""
"""Tip SHA of upstream main; API first, HTTPS ``ls-remote`` (no auth, no prompts) as fallback."""
sha = _github_branch_tip(_OFFICIAL_REPO_CANONICAL.removeprefix("github.com/"), "main")
if sha:
return sha
result = _git_run(["ls-remote", _UPSTREAM_REPO_URL, "refs/heads/main"], timeout=10, network=True)
if result is None or result.returncode != 0 or not result.stdout:
return None
@@ -259,68 +326,40 @@ def _upstream_main_sha() -> Optional[str]:
def _check_via_rev(local_rev: str) -> Optional[int]:
"""Compare an embedded git revision to upstream main via ls-remote (see ``_tips_behind``)."""
return _tips_behind(local_rev, _upstream_main_sha())
"""Compare an embedded git revision to upstream main via the API (see ``_tips_behind``)."""
global _last_target_rev
_last_target_rev = _upstream_main_sha()
return _tips_behind(local_rev, _last_target_rev)
def _check_via_local_git(repo_dir: Path) -> Optional[int]:
"""Count commits behind origin/main in a local checkout."""
# Probe the origin URL under the same config-isolated env as the fetch below. A plain
# get-url applies a global url.<https>.insteadOf rewrite, so an SSH origin masquerades as
# HTTPS, the SSH-avoiding fast path is skipped — and the fetch, whose env drops global
# config (GIT_CONFIG_GLOBAL=/dev/null), dials the raw SSH origin; its host-key prompt opens
# /dev/tty directly and steals the CLI's keystrokes (#104591).
"""Count commits behind origin/main in a local checkout.
Passive checks never run ``git fetch``: every CLI/TUI/gateway start used to negotiate a pack
with GitHub, and across the install base that was tens of millions of fetch requests a day
(GitHub asked us to poll the API instead). Two tip SHAs are enough — the remote one from the
API, the local one from ``rev-parse`` — and ``_tips_behind`` recovers the exact count through
the compare API when they differ. ``git fetch`` happens only inside ``hermes update``.
"""
# Probe the origin URL under the config-isolated env: a global url.<https>.insteadOf rewrite
# otherwise makes an SSH origin masquerade as HTTPS (#104591).
origin_url = _git_stdout(["remote", "get-url", "origin"], cwd=repo_dir, network=True)
if _is_official_ssh_remote(origin_url):
head_rev = _git_stdout(["rev-parse", "HEAD"], cwd=repo_dir)
if not head_rev:
return None
# Passive probe via HTTPS ls-remote (never SSH — no hardware-key prompts). Tip SHAs alone
# can't distinguish "behind" from a local commit AHEAD of origin/main, and misreporting an
# ahead checkout nudges the user into `hermes update`, which can wipe carried work — hence
# the ancestor check, against the FRESH upstream SHA (a stale tracking ref can't fake an
# up-to-date report).
return _tips_behind(head_rev, _upstream_main_sha(), repo_dir)
# Installer checkouts are shallow (`git clone --depth 1`): a plain `git fetch` would unshallow
# the repo and `rev-list --count HEAD..origin/main` would report a bogus "12492 commits
# behind". Fetch with --depth 1 to preserve the boundary and compare tip SHAs instead. Full
# clones keep the exact count path. Mirrors apps/desktop/electron/main.cjs.
is_shallow = _git_stdout(["rev-parse", "--is-shallow-repository"], cwd=repo_dir) == "true"
def _fetch() -> bool:
# Self-heal abandoned git lock files first. A stale .git/shallow.lock from a crashed fetch
# makes every fetch fail silently and stale refs get compared against HEAD until a human
# removes the lock. This passive check is also the main tmp_pack GENERATOR on flaky lines,
# so it must be the janitor too (#93732).
from hermes_cli.gitlock import clear_stale_git_locks, clear_stale_tmp_packs
clear_stale_git_locks(repo_dir)
clear_stale_tmp_packs(repo_dir)
# Scope the fetch to the one branch compared against: an unscoped ``git fetch origin``
# transfers ~1,400 remote heads (3.0 s vs 0.55 s measured) and can burn the full timeout.
# A scoped fetch still updates ``origin/main`` and FETCH_HEAD; ``--depth 1`` preserves
# the shallow boundary.
fetch_args = ["fetch", "origin", "main", *(["--depth", "1"] if is_shallow else []), "--quiet"]
return _git_ok(fetch_args, cwd=repo_dir, timeout=10, network=True)
fetch_ok = _quiet(_fetch, False) # Offline or timeout — don't use stale refs
# When the fetch fails the local origin/main ref is stale: it cannot prove *currentness*, but
# if it already shows HEAD behind, that is sound evidence an update exists. Return the positive
# stale count; None (inconclusive) otherwise so the caller doesn't cache a false "up to date".
if is_shallow:
# (#82166, review #92578)
if not fetch_ok:
return None
# No history across the shallow boundary. `origin/main` may not be a tracking ref in a
# `clone --depth 1`, so prefer FETCH_HEAD (just updated) and fall back to origin/main.
head_rev = _git_stdout(["rev-parse", "HEAD"], cwd=repo_dir)
target_rev = (
_git_stdout(["rev-parse", "FETCH_HEAD"], cwd=repo_dir)
or _git_stdout(["rev-parse", "origin/main"], cwd=repo_dir))
return _tips_behind(head_rev, target_rev)
behind = _git_count(["rev-list", "--count", "HEAD..origin/main"], cwd=repo_dir)
return behind if fetch_ok or (behind is not None and behind > 0) else None
head_rev = _git_stdout(["rev-parse", "HEAD"], cwd=repo_dir)
if not head_rev:
return None
canonical = _canonical_github_remote(origin_url)
if canonical.startswith("github.com/"):
target_rev = _github_branch_tip(canonical.removeprefix("github.com/"), "main")
else:
# Non-GitHub origin: one ls-remote for the tip (ref advertisement only, no pack transfer).
result = _git_run(["ls-remote", "origin", "refs/heads/main"], cwd=repo_dir, timeout=10, network=True)
target_rev = result.stdout.split()[0] if result is not None and result.returncode == 0 and result.stdout else None
global _last_target_rev
_last_target_rev = target_rev
# Tip SHAs alone can't distinguish "behind" from a local commit AHEAD of origin/main, and
# misreporting an ahead checkout nudges the user into `hermes update`, which can wipe carried
# work — hence the ancestor check inside _tips_behind, against the FRESH upstream SHA.
return _tips_behind(head_rev, target_rev, repo_dir)
def _read_json(path: Path) -> Optional[dict]:
@@ -332,8 +371,8 @@ def _read_json(path: Path) -> Optional[dict]:
def check_for_updates(*, passive: bool = False) -> Optional[int]:
"""Check whether a Hermes update is available.
If ``HERMES_REVISION`` is set (nix builds embed it), compare it to upstream main via
``git ls-remote``; otherwise count commits behind ``origin/main`` in the local checkout.
If ``HERMES_REVISION`` is set (nix builds embed it), compare it to upstream main; otherwise
compare the local checkout's HEAD. Both go through the GitHub API, never ``git fetch``.
"""
def _read_config_opt_out():
from hermes_cli.config import load_config
@@ -354,22 +393,26 @@ def check_for_updates(*, passive: bool = False) -> Optional[int]:
if _quiet(_install_method) in {"docker", "apt"}:
return None
# Cache is invalidated when the embedded rev OR installed version changed since the last check.
# For a git checkout the local HEAD is part of the key too: `hermes update` moves HEAD, and a
# stale "3 behind" must not survive the update it just prompted.
now = time.time()
repo_dir = None if embedded_rev else _resolve_repo_dir()
head_rev = _git_stdout(["rev-parse", "HEAD"], cwd=repo_dir) if repo_dir is not None else None
cached = _read_json(cache_file)
if (cached is not None and now - cached.get("ts", 0) < _UPDATE_CHECK_CACHE_SECONDS
and cached.get("rev") == embedded_rev and cached.get("ver") == VERSION):
return cached.get("behind")
if cached is not None and cached.get("rev") == embedded_rev and cached.get("ver") == VERSION \
and cached.get("head") == head_rev:
ttl = _UPDATE_CHECK_CACHE_SECONDS if cached.get("behind") is not None else _UPDATE_CHECK_FAILURE_CACHE_SECONDS
if now - cached.get("ts", 0) < ttl:
return cached.get("behind")
if embedded_rev:
behind = _check_via_rev(embedded_rev)
else:
# No checkout and no embedded revision — status can't be determined.
repo_dir = _resolve_repo_dir()
behind = _check_via_local_git(repo_dir) if repo_dir is not None else None
# Don't cache inconclusive results: None means the check could not run (typically a failed
# fetch), and caching it would suppress retries for the full 6-hour window (#82166).
if behind is not None:
_quiet(lambda: cache_file.write_text(
json.dumps({"ts": now, "behind": behind, "rev": embedded_rev, "ver": VERSION}), encoding="utf-8"))
_quiet(lambda: cache_file.write_text(
json.dumps({"ts": now, "behind": behind, "rev": embedded_rev, "ver": VERSION,
"head": head_rev or embedded_rev, "target": _last_target_rev}),
encoding="utf-8"))
return behind

View File

@@ -230,36 +230,6 @@ async def update_hermes():
return {"ok": True, "pid": proc.pid, "name": "hermes-update", "action_id": action_id}
def _recent_upstream_commits(n: int = 20) -> List[Dict[str, Any]]:
"""Commits the local checkout is behind ``origin/main`` by, newest first; [] on any failure.
Logs the SAME range the behind-count uses (``HEAD..origin/main``, see
``banner._check_via_local_git``), NOT ``@{upstream}``: on a feature branch that is
the branch's own tip (zero commits), leaving the changelog empty while the count is non-zero.
"""
try:
# git log emits UTF-8 (emoji/CJK subjects). On Windows text=True defaults to
# the ANSI code page; an undefined cp1252 byte crashed the stdlib
# _readerthread and killed the desktop backend — hence encoding="utf-8".
out = subprocess.run(
[
"git", "-C", str(_server_path("PROJECT_ROOT")), "log", "--format=%H%x1f%s%x1f%an%x1f%ct",
"HEAD..origin/main", f"-n{int(n)}",
],
capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=5,
)
if out.returncode != 0:
return []
rows: List[Dict[str, Any]] = []
for line in out.stdout.splitlines():
if line.strip():
sha, summary, author, at = (line.split("\x1f") + ["", "", "", "0"])[:4]
rows.append({"sha": sha[:7], "summary": summary, "author": author, "at": int(at or 0)})
return rows
except Exception:
return []
_NON_APPLYABLE_MESSAGES = {
"docker": format_docker_update_message,
"apt": lambda: "Hermes is managed by Termux APT; run `pkg upgrade hermes-agent`.",
@@ -295,10 +265,10 @@ async def check_hermes_update(force: bool = False):
payload["message"] = non_applyable()
return payload
# banner.check_for_updates() handles git / nix-revision paths and caches
# the result for 6h. ``force`` busts the cache so "Check now" reflects reality.
# banner.check_for_updates() handles git / nix-revision paths through the GitHub API and
# caches the result for 24h. ``force`` busts the cache so "Check now" reflects reality.
try:
from hermes_cli.banner import check_for_updates
from hermes_cli.banner import check_for_updates, upstream_commits_behind
if force:
with contextlib.suppress(OSError):
@@ -315,10 +285,9 @@ async def check_hermes_update(force: bool = False):
payload["message"] = "You're on the latest version."
else:
payload["update_available"] = True
# "What's changed" for the desktop's remote update overlay; git only,
# best-effort (empty list on any failure).
if install_method == "git":
payload["commits"] = await asyncio.to_thread(_recent_upstream_commits)
# "What's changed" for the desktop's remote update overlay; best-effort
# (empty list on any failure).
payload["commits"] = await asyncio.to_thread(upstream_commits_behind)
return payload

View File

@@ -177,13 +177,12 @@ def test_check_via_local_git_insteadof_rewrite_routes_to_ssh_fastpath(tmp_path,
def spy_run(args, **kwargs):
calls.append((list(args), kwargs))
if args[1] == "ls-remote":
return MagicMock(returncode=0, stdout=f"{head_sha}\trefs/heads/main\n")
if args[1] == "fetch":
return MagicMock(returncode=1, stdout="")
if args[1] in {"ls-remote", "fetch"}:
raise AssertionError(f"a GitHub origin must be probed via the API, not git {args[1]}")
return real_run(args, **kwargs)
monkeypatch.setattr(banner.subprocess, "run", spy_run)
monkeypatch.setattr(banner, "_github_branch_tip", lambda slug, branch: head_sha)
behind = banner._check_via_local_git(repo_dir)

View File

@@ -10,8 +10,11 @@ def test_passive_check_obeys_config_before_using_cached_notice(monkeypatch):
from hermes_cli import banner
home = get_hermes_home()
# The cache is keyed on the checkout's HEAD (an update moving HEAD invalidates it).
repo_dir = banner._resolve_repo_dir()
head = banner._git_stdout(["rev-parse", "HEAD"], cwd=repo_dir) if repo_dir else None
(home / ".update_check").write_text(json.dumps({
"ts": time.time(), "behind": 17, "rev": None, "ver": banner.VERSION,
"ts": time.time(), "behind": 17, "rev": None, "ver": banner.VERSION, "head": head,
}), encoding="utf-8")
monkeypatch.delenv("HERMES_REVISION", raising=False)
config = home / "config.yaml"

View File

@@ -44,12 +44,20 @@ class _FakeResponse:
def _patch_urlopen(payload):
banner._compare_payload_cache.clear()
return patch(
"urllib.request.urlopen",
return_value=_FakeResponse(json.dumps(payload).encode()),
)
@pytest.fixture(autouse=True)
def _fresh_compare_cache():
banner._compare_payload_cache.clear()
yield
banner._compare_payload_cache.clear()
# ---------------------------------------------------------------------------
# _github_compare_behind
# ---------------------------------------------------------------------------
@@ -98,69 +106,54 @@ def test_compare_behind_rejects_malformed_payloads(payload):
# ---------------------------------------------------------------------------
def _ls_remote_result(sha):
return MagicMock(returncode=0, stdout=f"{sha}\trefs/heads/main\n")
def _upstream_tip(sha):
return patch.object(banner, "_github_branch_tip", return_value=sha)
def test_check_via_rev_recovers_exact_count():
with patch(
"hermes_cli.banner.subprocess.run", return_value=_ls_remote_result(SHA_B)
), patch.object(banner, "_github_compare_behind", return_value=61) as compare:
with _upstream_tip(SHA_B), patch.object(banner, "_github_compare_behind", return_value=61) as compare:
assert banner._check_via_rev(SHA_A) == 61
compare.assert_called_once_with(SHA_A, SHA_B)
def test_check_via_rev_falls_back_to_sentinel_offline():
"""FAIL-BEFORE (class): this path returned a fabricated 1 via callers."""
with patch(
"hermes_cli.banner.subprocess.run", return_value=_ls_remote_result(SHA_B)
), patch.object(banner, "_github_compare_behind", return_value=None):
with _upstream_tip(SHA_B), patch.object(banner, "_github_compare_behind", return_value=None):
assert banner._check_via_rev(SHA_A) == banner.UPDATE_AVAILABLE_NO_COUNT
def test_check_via_rev_up_to_date_short_circuits_compare():
with patch(
"hermes_cli.banner.subprocess.run", return_value=_ls_remote_result(SHA_A)
), patch.object(banner, "_github_compare_behind") as compare:
with _upstream_tip(SHA_A), patch.object(banner, "_github_compare_behind") as compare:
assert banner._check_via_rev(SHA_A) == 0
compare.assert_not_called()
def test_check_via_rev_local_ahead_reports_up_to_date():
"""ahead_by == 0 with differing tips = local commits on top, not behind."""
with patch(
"hermes_cli.banner.subprocess.run", return_value=_ls_remote_result(SHA_B)
), patch.object(banner, "_github_compare_behind", return_value=0):
with _upstream_tip(SHA_B), patch.object(banner, "_github_compare_behind", return_value=0):
assert banner._check_via_rev(SHA_A) == 0
# ---------------------------------------------------------------------------
# _check_via_local_git: shallow path recovers the exact count
# _check_via_local_git: tips from the API, exact count via compare, no fetch
# ---------------------------------------------------------------------------
def _shallow_git(head_sha, fetch_head_sha):
def _local_git(head_sha):
def fake_run(cmd, **kwargs):
if cmd[:4] == ["git", "remote", "get-url", "origin"]:
return MagicMock(
returncode=0,
stdout="https://github.com/NousResearch/hermes-agent.git\n",
)
if cmd[:3] == ["git", "rev-parse", "--is-shallow-repository"]:
return MagicMock(returncode=0, stdout="true\n")
if cmd[:2] == ["git", "fetch"]:
return MagicMock(returncode=0, stdout="")
return MagicMock(returncode=0, stdout="https://github.com/NousResearch/hermes-agent.git\n")
if cmd[:3] == ["git", "rev-parse", "HEAD"]:
return MagicMock(returncode=0, stdout=f"{head_sha}\n")
if cmd[:3] == ["git", "rev-parse", "FETCH_HEAD"]:
return MagicMock(returncode=0, stdout=f"{fetch_head_sha}\n")
if cmd[:3] == ["git", "merge-base", "--is-ancestor"]:
return MagicMock(returncode=1, stdout="")
raise AssertionError(f"unexpected git command: {cmd!r}")
return fake_run
def test_shallow_checkout_recovers_exact_count(tmp_path):
"""The #84591 shape: shallow boundary kills merge-base, tips differ.
def test_local_checkout_recovers_exact_count(tmp_path):
"""The #84591 shape: no local history across the tips (shallow clone), tips differ.
FAIL-BEFORE (class): reported UPDATE_AVAILABLE_NO_COUNT (or, further back,
a fabricated 1) even though the compare API could count exactly.
@@ -168,31 +161,28 @@ def test_shallow_checkout_recovers_exact_count(tmp_path):
repo_dir = tmp_path / "hermes-agent"
repo_dir.mkdir()
with patch(
"hermes_cli.banner.subprocess.run", side_effect=_shallow_git(SHA_A, SHA_B)
), patch.object(banner, "_github_compare_behind", return_value=61):
with patch("hermes_cli.banner.subprocess.run", side_effect=_local_git(SHA_A)), \
patch.object(banner, "_github_branch_tip", return_value=SHA_B), \
patch.object(banner, "_github_compare_behind", return_value=61):
assert banner._check_via_local_git(repo_dir) == 61
def test_shallow_checkout_offline_keeps_honest_sentinel(tmp_path):
def test_local_checkout_offline_compare_keeps_honest_sentinel(tmp_path):
repo_dir = tmp_path / "hermes-agent"
repo_dir.mkdir()
with patch(
"hermes_cli.banner.subprocess.run", side_effect=_shallow_git(SHA_A, SHA_B)
), patch.object(banner, "_github_compare_behind", return_value=None):
assert (
banner._check_via_local_git(repo_dir)
== banner.UPDATE_AVAILABLE_NO_COUNT
)
with patch("hermes_cli.banner.subprocess.run", side_effect=_local_git(SHA_A)), \
patch.object(banner, "_github_branch_tip", return_value=SHA_B), \
patch.object(banner, "_github_compare_behind", return_value=None):
assert banner._check_via_local_git(repo_dir) == banner.UPDATE_AVAILABLE_NO_COUNT
def test_shallow_checkout_equal_tips_up_to_date_without_compare(tmp_path):
def test_local_checkout_equal_tips_up_to_date_without_compare(tmp_path):
repo_dir = tmp_path / "hermes-agent"
repo_dir.mkdir()
with patch(
"hermes_cli.banner.subprocess.run", side_effect=_shallow_git(SHA_A, SHA_A)
), patch.object(banner, "_github_compare_behind") as compare:
with patch("hermes_cli.banner.subprocess.run", side_effect=_local_git(SHA_A)), \
patch.object(banner, "_github_branch_tip", return_value=SHA_A), \
patch.object(banner, "_github_compare_behind") as compare:
assert banner._check_via_local_git(repo_dir) == 0
compare.assert_not_called()

View File

@@ -1,70 +1,119 @@
"""Tests for the update check mechanism in hermes_cli.banner."""
"""Tests for the update check mechanism in hermes_cli.banner.
Passive checks go through the GitHub REST API — never ``git fetch``. Every CLI, TUI and desktop
start used to fetch; across the install base that was tens of millions of fetch requests a day
and GitHub asked us to poll the API instead. These tests pin that contract plus the cache
policy that keeps the API traffic to one request a day per install.
"""
import json
import os
import threading
import time
from pathlib import Path
from unittest.mock import MagicMock, patch
import pytest
import hermes_cli.banner as banner
SHA_A = "a" * 40
SHA_B = "b" * 40
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
@pytest.fixture
def git_repo(tmp_path, monkeypatch):
"""A fake checkout the update check resolves to, with git calls stubbed out."""
repo_dir = tmp_path / "hermes-agent"
repo_dir.mkdir()
(repo_dir / ".git").mkdir()
cache_file = tmp_path / ".update_check"
cache_file.write_text(
json.dumps({"ts": time.time(), "behind": 3, "ver": __version__}),
encoding="utf-8",
)
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
with patch("hermes_cli.banner.subprocess.run") as mock_run:
result = check_for_updates()
assert result == 3
mock_run.assert_not_called()
monkeypatch.delenv("HERMES_REVISION", raising=False)
monkeypatch.setattr(banner, "_resolve_repo_dir", lambda: repo_dir)
monkeypatch.setattr("hermes_cli.config.detect_install_method", lambda root: "git")
monkeypatch.setattr("hermes_cli.config.get_project_root", lambda: repo_dir)
return repo_dir
def _stub_git(monkeypatch, *, head=SHA_A, origin="https://github.com/NousResearch/hermes-agent.git"):
calls = []
def fake_run(args, **kwargs):
calls.append(list(args))
sub = args[1]
if sub == "rev-parse":
return MagicMock(returncode=0, stdout=f"{head}\n")
if sub == "remote":
return MagicMock(returncode=0, stdout=f"{origin}\n")
if sub == "merge-base":
return MagicMock(returncode=1, stdout="")
raise AssertionError(f"passive check must not run git {sub}: {args}")
monkeypatch.setattr(banner.subprocess, "run", fake_run)
return calls
def test_passive_check_uses_the_api_and_never_fetches(git_repo, monkeypatch):
"""The whole point: no ``git fetch`` / ``ls-remote`` for a GitHub origin, exact count via compare."""
calls = _stub_git(monkeypatch, head=SHA_A)
tip = MagicMock(return_value=SHA_B)
monkeypatch.setattr(banner, "_github_branch_tip", tip)
monkeypatch.setattr(banner, "_github_compare_behind", lambda cur, tgt: 61)
assert banner.check_for_updates() == 61
tip.assert_called_once_with("nousresearch/hermes-agent", "main")
assert not any(c[1] in {"fetch", "ls-remote"} for c in calls)
cached = json.loads((git_repo.parent / ".update_check").read_text())
assert (cached["head"], cached["target"], cached["behind"]) == (SHA_A, SHA_B, 61)
def test_cache_is_daily_but_invalidated_when_head_moves(git_repo, monkeypatch):
"""A fresh cache answers without any network; ``hermes update`` moving HEAD busts it at once;
an inconclusive (None) result is retried after the shorter failure window, not never."""
from hermes_cli import __version__
cache_file = git_repo.parent / ".update_check"
_stub_git(monkeypatch, head=SHA_A)
tip = MagicMock(return_value=None)
monkeypatch.setattr(banner, "_github_branch_tip", tip)
def write_cache(*, ts, head, behind):
cache_file.write_text(json.dumps(
{"ts": ts, "behind": behind, "rev": None, "ver": __version__, "head": head}))
write_cache(ts=time.time() - banner._UPDATE_CHECK_CACHE_SECONDS + 60, head=SHA_A, behind=3)
assert banner.check_for_updates() == 3
tip.assert_not_called()
write_cache(ts=time.time(), head=SHA_B, behind=3) # cached for a different HEAD
assert banner.check_for_updates() is None # API unreachable → inconclusive, re-asked
tip.assert_called_once()
tip.reset_mock()
write_cache(ts=time.time() - banner._UPDATE_CHECK_FAILURE_CACHE_SECONDS + 60, head=SHA_A, behind=None)
assert banner.check_for_updates() is None
tip.assert_not_called()
write_cache(ts=time.time() - banner._UPDATE_CHECK_FAILURE_CACHE_SECONDS - 1, head=SHA_A, behind=None)
banner.check_for_updates()
tip.assert_called_once()
def test_prefetch_non_blocking():
"""prefetch_update_check() should return immediately without blocking."""
import hermes_cli.banner as banner
# Reset module state
banner._update_result = None
banner._update_check_done = threading.Event()
with patch.object(banner, "check_for_updates", return_value=5):
start = time.monotonic()
banner.prefetch_update_check()
elapsed = time.monotonic() - start
# Should return almost immediately (well under 1 second)
assert elapsed < 1.0
# Wait for the background thread to finish
assert time.monotonic() - start < 1.0
banner._update_check_done.wait(timeout=5)
assert banner._update_result == 5
def test_upstream_main_sha_disables_git_prompts(monkeypatch):
"""The passive HTTPS probe must never inherit the interactive terminal."""
from hermes_cli import banner
def test_upstream_main_sha_ls_remote_fallback_disables_git_prompts(monkeypatch):
"""When the API is unreachable the HTTPS ls-remote fallback must never inherit the terminal."""
monkeypatch.setattr(banner, "_github_branch_tip", lambda slug, branch: None)
completed = MagicMock(returncode=1, stdout="", stderr="auth required")
run = MagicMock(return_value=completed)
monkeypatch.setattr(banner.subprocess, "run", run)
@@ -74,202 +123,3 @@ def test_upstream_main_sha_disables_git_prompts(monkeypatch):
assert kwargs["stdin"] is banner.subprocess.DEVNULL
assert kwargs["env"]["GIT_TERMINAL_PROMPT"] == "0"
assert kwargs["env"]["GCM_INTERACTIVE"] == "Never"
def test_check_via_local_git_fetch_failure_returns_none(tmp_path, monkeypatch):
"""When git fetch fails and the stale origin/main ref is not ahead,
_check_via_local_git must return None (#82166).
A stale tracking ref cannot prove *currentness* (rev-list 0 just means
the ref hasn't caught up), so returning None is the honest inconclusive
result — and the caller must not cache it as "up to date".
"""
from hermes_cli import banner
repo_dir = tmp_path / "hermes-agent"
repo_dir.mkdir()
(repo_dir / ".git").mkdir()
# Simulate a non-shallow, non-SSH-remote checkout
def mock_git_stdout(args, *, cwd, timeout=5, network=False):
if args[:2] == ["remote", "get-url"]:
return "https://github.com/NousResearch/hermes-agent.git"
if args[:2] == ["rev-parse", "--is-shallow-repository"]:
return "false"
return None
# Fetch fails (returncode != 0); stale rev-list reports 0 behind
failed_proc = MagicMock()
failed_proc.returncode = 1
failed_proc.stdout = ""
failed_proc.stderr = "fatal: could not reach remote"
stale_zero_proc = MagicMock()
stale_zero_proc.returncode = 0
stale_zero_proc.stdout = "0"
fetch_kwargs = None
def mock_run(args, **kwargs):
nonlocal fetch_kwargs
if args[:2] == ["git", "fetch"]:
fetch_kwargs = kwargs
return failed_proc
if args[:2] == ["git", "rev-list"]:
return stale_zero_proc
raise AssertionError(f"unexpected subprocess.run: {args}")
monkeypatch.setattr(banner, "_git_stdout", mock_git_stdout)
monkeypatch.setattr(banner.subprocess, "run", mock_run)
result = banner._check_via_local_git(repo_dir)
assert result is None, (
"Fetch failure with stale 0-behind must return None, not 'up to date'"
)
assert fetch_kwargs is not None
assert fetch_kwargs["stdin"] is banner.subprocess.DEVNULL
assert fetch_kwargs["env"]["GIT_TERMINAL_PROMPT"] == "0"
assert fetch_kwargs["env"]["GCM_INTERACTIVE"] == "Never"
def test_check_via_local_git_fetch_failure_keeps_positive_stale_count(tmp_path, monkeypatch):
"""A failed fetch must preserve sound evidence: if the stale origin/main
ref already shows HEAD behind, that positive count is still an update
signal and must be returned (review #92578)."""
from hermes_cli import banner
repo_dir = tmp_path / "hermes-agent"
repo_dir.mkdir()
(repo_dir / ".git").mkdir()
def mock_git_stdout(args, *, cwd, timeout=5, network=False):
if args[:2] == ["remote", "get-url"]:
return "https://github.com/NousResearch/hermes-agent.git"
if args[:2] == ["rev-parse", "--is-shallow-repository"]:
return "false"
return None
failed_proc = MagicMock()
failed_proc.returncode = 1
failed_proc.stdout = ""
failed_proc.stderr = "fatal: could not reach remote"
stale_behind_proc = MagicMock()
stale_behind_proc.returncode = 0
stale_behind_proc.stdout = "5"
def mock_run(args, **kwargs):
if args[:2] == ["git", "fetch"]:
return failed_proc
if args[:2] == ["git", "rev-list"]:
return stale_behind_proc
raise AssertionError(f"unexpected subprocess.run: {args}")
monkeypatch.setattr(banner, "_git_stdout", mock_git_stdout)
monkeypatch.setattr(banner.subprocess, "run", mock_run)
result = banner._check_via_local_git(repo_dir)
assert result == 5, "Stale positive behind-count must be preserved on fetch failure"
def test_check_via_local_git_fetch_failure_rev_list_error_returns_none(tmp_path, monkeypatch):
"""If the stale rev-list itself fails, the check stays inconclusive (None)."""
from hermes_cli import banner
repo_dir = tmp_path / "hermes-agent"
repo_dir.mkdir()
(repo_dir / ".git").mkdir()
def mock_git_stdout(args, *, cwd, timeout=5, network=False):
if args[:2] == ["remote", "get-url"]:
return "https://github.com/NousResearch/hermes-agent.git"
if args[:2] == ["rev-parse", "--is-shallow-repository"]:
return "false"
return None
failed_proc = MagicMock()
failed_proc.returncode = 1
failed_proc.stdout = ""
failed_proc.stderr = "fatal: could not reach remote"
bad_rev_list = MagicMock()
bad_rev_list.returncode = 128
bad_rev_list.stdout = ""
bad_rev_list.stderr = "fatal: ambiguous argument 'HEAD..origin/main'"
def mock_run(args, **kwargs):
if args[:2] == ["git", "fetch"]:
return failed_proc
if args[:2] == ["git", "rev-list"]:
return bad_rev_list
raise AssertionError(f"unexpected subprocess.run: {args}")
monkeypatch.setattr(banner, "_git_stdout", mock_git_stdout)
monkeypatch.setattr(banner.subprocess, "run", mock_run)
result = banner._check_via_local_git(repo_dir)
assert result is None
def test_check_for_updates_does_not_cache_none(tmp_path, monkeypatch):
"""check_for_updates must not cache None results so a transient fetch
failure doesn't suppress retries for the full 6-hour cache window (#82166).
Instead of mocking the full Path resolution chain, we verify the cache-write
guard directly: call check_for_updates with a mocked _check_via_local_git
that returns None, and confirm no cache file is created.
"""
import hermes_cli.banner as banner
cache_file = tmp_path / ".update_check"
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
monkeypatch.delenv("HERMES_REVISION", raising=False)
# Create a fake repo dir so the .git check passes
repo_dir = tmp_path / "hermes-agent"
repo_dir.mkdir()
(repo_dir / ".git").mkdir()
# Mock the internal functions to force the local-git path returning None
monkeypatch.setattr(banner, "_check_via_local_git", lambda rd: None)
monkeypatch.setattr(
"hermes_cli.config.detect_install_method", lambda root: "git"
)
monkeypatch.setattr(
"hermes_cli.config.get_project_root", lambda: repo_dir
)
# Patch __file__ resolution by monkeypatching the module's Path calls.
# check_for_updates does: Path(__file__).parent.parent.resolve()
# We intercept by making the resolve() return our fake repo_dir.
original_init = Path.__init__
def patched_path_init(self, *args, **kwargs):
original_init(self, *args, **kwargs)
# Simpler: just patch the get_hermes_home and the repo_dir resolution
# by making check_for_updates find our fake repo via hermes_home fallback.
# The code checks Path(__file__).parent.parent/.git first, then falls
# back to hermes_home / "hermes-agent". We ensure the fallback hits.
# To do this, we make Path(__file__).parent.parent.resolve() return
# a path without .git, so it falls through to hermes_home / "hermes-agent".
real_resolve = Path.resolve
def fake_resolve(self, *args, **kwargs):
s = str(self)
if "banner.py" in s or s.endswith("hermes_cli"):
# Return a path that has no .git, forcing the fallback
return tmp_path / "no-git-here"
return real_resolve(self, *args, **kwargs)
monkeypatch.setattr(Path, "resolve", fake_resolve)
result = banner.check_for_updates()
assert result is None
# The cache file must NOT have been written with a None result
assert not cache_file.exists(), "None result must not be cached"

View File

@@ -152,11 +152,22 @@ Leaving these unset keeps the legacy defaults (`HERMES_API_TIMEOUT=1800`s, `HERM
## Update Behavior
### Background checks and SSH authentication
### Background checks
Passive update checks (CLI banner, TUI badge, dashboard, desktop app) ask the
GitHub REST API for the tip of `main` and, when it differs from your checkout,
the compare endpoint for the exact count and changelog. They never run
`git fetch`, and every install asks at most **once per 24 hours** (a failed check
retries after an hour). Applying an update (`hermes update`, or the desktop's
Update button) always fetches fresh and invalidates the cached answer. Explicit
checks — `hermes update --check`, the desktop's "Check for Updates…" menu item,
Settings → About → "Check now" — bypass the cache.
### SSH authentication
The startup update check reads the origin URL with the same isolated Git
configuration used for its network calls. Global `url.*.insteadOf` rewrites
therefore cannot hide an official SSH remote from the public HTTPS check.
therefore cannot hide an official SSH remote from the public HTTPS path.
Hermes's isolated internal Git commands default to `ssh -o BatchMode=yes`:
unknown host keys, passwords, and encrypted keys needing a passphrase fail

View File

@@ -586,7 +586,7 @@ same auth gate as the rest of `/api/`.
| `GET /api/ops/checkpoints` · `POST .../prune` | Inspect / prune the `/rollback` store |
| `POST /api/ops/hooks` · `DELETE /api/ops/hooks` | Create / remove a shell hook (consent-gated) |
| `GET /api/system/stats` | Host stats — OS, CPU, memory, disk, uptime |
| `GET /api/hermes/update/check` | Report update availability (commits behind, install method) without applying. For git installs that are behind, also returns a `commits` list (`sha`, `summary`, `author`, `at`) of what's changed. `?force=1` busts the 6h cache |
| `GET /api/hermes/update/check` | Report update availability (commits behind, install method) without applying. For git installs that are behind, also returns a `commits` list (`sha`, `summary`, `author`, `at`) of what's changed. `?force=1` busts the 24h cache (the check goes through the GitHub API, never `git fetch`) |
| `GET /api/curator` · `PUT .../paused` · `POST .../run` | Skill-curator status + pause/resume + run |
| `GET /api/portal` | Nous Portal auth + Tool Gateway routing (read-only) |
| `POST /api/ops/prompt-size` · `/dump` · `/config-migrate` | Diagnostics (backgrounded) |