fix(banner): bind update-check cache to local HEAD so banner refreshes after hermes update (#20728) - #20792
fix(banner): bind update-check cache to local HEAD so banner refreshes after hermes update (#20728)#20792briandevans wants to merge 1 commit into
Conversation
…s after hermes update (NousResearch#20728) The startup banner kept showing "▲N commits behind" after a successful `hermes update` (and after a plain `git pull`). Two interacting bugs: 1. **In-memory snapshot frozen for long-running processes.** The TUI gateway calls `prefetch_update_check()` once at startup, which sets the module-level `_update_result`. Every later call to `get_update_result()` returns that same value — even when `hermes update` deleted the on-disk cache or a sibling process moved HEAD. `tui_gateway/server.py:1424` reads it on every dashboard request. 2. **Cache validity check was a no-op for git installs.** The `cached.get("rev") == embedded_rev` check only meant something for nix builds (where `HERMES_REVISION` is set). For git installs both sides were `None`, so the 6-hour TTL was the only invalidator. After a manual `git pull` the cached "behind" count survived for hours. Bind the cache to the local HEAD short SHA. `check_for_updates()` writes `local_head` alongside the count and rejects cached entries whose `local_head` doesn't match the current HEAD; legacy caches without the field also fall through to a refresh on first read. `get_update_result()` remembers the HEAD that produced its in-memory snapshot and, when the current HEAD differs, kicks a non-blocking daemon refresh so the next render picks up the fresh value. Caller is never blocked. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
alt-glitch
added
type/bug
|
Duplicate of #20653 (same approach: store repo_head in update-check cache, invalidate when HEAD changes). Also overlaps #18236, #9670, #5359, all addressing the same stale banner cache issue. #20792 additionally fixes the in-memory freeze in long-running TUI processes (Bug 1 from #20728), which is novel. |
|
Thanks @alt-glitch — agreed. Bug 2 (HEAD-binding the on-disk update-check cache) is functionally the same approach as #20653: both add a HEAD field to the cache entry and invalidate when it moves. #20653 was opened first, so deferring to it on that piece makes sense. The novel piece here is Bug 1: the module-level Happy to take either path here:
Whichever the maintainers prefer. |
|
Closing — superseded by @alt-glitch's pointer to #20653, which is the better-positioned fix for the cache-HEAD binding here. |
2 participants
Summary
▲N commits behindbanner afterhermes updatebecause the in-memory_update_resultoutlived both the on-disk cache wipe and a sibling-process pull.cached.get(\"rev\") == embedded_revvalidity check was a no-op for git installs (both sidesNone), so even when read it would return a stale count for up to 6 h aftergit pull.HEADshort SHA at write time and require it to match at read time; kick a non-blocking daemon refresh when HEAD has moved since the in-memory snapshot was taken.The bug
Two interacting issues — both reported with high specificity in #20728:
Bug 1 — in-memory freeze.
tui_gateway/server.py:110callsprefetch_update_check()once at startup. From then on, everyget_update_result()call (including the dashboard info handler attui_gateway/server.py:1424) returns the module-level_update_resultset by the prefetch. There's no IPC, file-watch, or HEAD re-validation. Afterhermes updateruns in a sibling CLI process and invalidates the on-disk cache via_invalidate_update_cache(), the running TUI process never re-reads it.Bug 2 — cache validity is a no-op for git installs.
check_for_updates()cached{ts, behind, rev: None}for git installs. The validity checkcached.get(\"rev\") == embedded_revonly meant something for nix builds (HERMES_REVISIONset). On git installs both sides wereNone, so the 6-hour TTL was the only invalidator — after a manualgit pullthe stalebehindcount survived for hours.The fix
hermes_cli/banner.py:check_for_updates()now writeslocal_head(HEAD short SHA) into the cache JSON for git installs, and rejects cached entries whoselocal_headdoesn't match the current HEAD. Legacy caches without the field also fall through to a refresh so the HEAD-binding contract takes effect on the very next read. The cheapgit rev-parse --short=8 HEADis only invoked when the timestamp + rev pre-checks already pass, so the no-fetch happy path stays no-fetch._current_local_head_short()— single source of truth for the cache-key HEAD lookup. Never does a fetch.prefetch_update_check()records the HEAD it observed alongside_update_result.get_update_result()compares the current HEAD against the snapshot HEAD; on mismatch it kicks a non-blocking daemon thread (guarded by a non-blockingLockso concurrent renders don't pile up refreshes) that recomputes_update_result. The caller still returns immediately with the current snapshot — refresh is asynchronous on purpose so banner renders never block on agit fetch.Test plan
tests/hermes_cli/test_update_check.py:test_check_for_updates_invalidates_when_head_moved— covers Bug 2 (cache rejected when local_head doesn't match)test_check_for_updates_legacy_cache_invalidated— covers the migration of pre-existing caches without the fieldtest_check_for_updates_writes_local_head_to_cache— confirms the new field is persistedtest_get_update_result_refreshes_after_head_moves— covers Bug 1 (long-running process kicks a refresh on HEAD change)test_get_update_result_no_refresh_when_head_unchanged— confirms we don't waste threads on the steady-state pathtest_get_update_result_does_not_block_caller— guards against any future change that would make the refresh synchronoustest_check_for_updates_uses_cacheandtest_check_for_updates_expired_cachefor the new cache schema (addedlocal_headto the cache write/read paths)tests/hermes_cli/test_banner.py,test_banner_git_state.py,test_banner_skills.py— all 15 passcached.get(\"rev\") == embedded_revisNone == Noneand the stale entry is returned indefinitely; the new tests fail onorigin/mainfor the same reason and pass on this branch.Related
hermes update#20728