Skip to content

fix(banner): invalidate update-check cache when local HEAD moves - #38337

Open
ssiweifnag wants to merge 1 commit into
NousResearch:mainfrom
ssiweifnag:fix/banner-cache-invalidation
Open

fix(banner): invalidate update-check cache when local HEAD moves#38337
ssiweifnag wants to merge 1 commit into
NousResearch:mainfrom
ssiweifnag:fix/banner-cache-invalidation

Conversation

@ssiweifnag

Copy link
Copy Markdown
Contributor

Summary

hermes_cli/banner.py:check_for_updates() writes a 6h cache file at ~/.hermes/.update_check and keys it on (HERMES_REVISION, VERSION, ts). None of those change after a successful git pull/rebase, so the cache survives for up to 6 hours and hermes --version keeps reporting the pre-pull "Update available: N commits behind" even when the local checkout is now caught up.

Fix

Add the local short HEAD hash to both the cache write and the read-invalidation condition. For pip installs (no local HEAD) the value is None and the cache key collapses back to the original (rev, ver) pair, preserving the #34491 fix.

While moving the HEAD resolution out of the cache block, also flattened the embedded-rev / git / pypi dispatch into a single if/elif/else so repo_dir is resolved once at the top.

Repro

# 1. fall behind upstream by 38 commits
git checkout ecac659d7    # 38 commits behind origin/main
./hermes --version        # writes cache: behind=38, head=ecac659d7

# 2. catch up
git pull --rebase origin main   # HEAD now efccb7001
./hermes --version        # BEFORE fix: still says "38 commits behind"
                          # AFTER  fix: writes cache: behind=0, head=efccb7001

The repro's "Update available: 38 commits behind" survives up to 6h today; with this patch it disappears the moment HEAD moves.

Test plan

  • Existing tests/hermes_cli/test_cmd_update.py (25 cases) still passes — verified locally.
  • Manual: with cache primed at behind=N, head=H1, run a no-op commit, run hermes --version, confirm cache file is rewritten (ts advanced, head updated to H2).
  • Manual: with no local git checkout (pip install), confirm head is null in the cache and invalidation still works via rev/ver changes only.

The 6h update-check cache was keyed on (HERMES_REVISION, VERSION, ts).
After a successful 'git pull' or rebase, neither of those changes but
HEAD has moved, so the cache would keep returning the pre-pull 'behind'
count for up to 6 hours — printing 'Update available: N commits behind'
even when the local checkout is now caught up.

Add the local short HEAD hash to both the cache write and the read
invalidation condition so any local-history movement refreshes the
count. For pip installs (no local HEAD) the value is None and behaves
identically to before. Repo_dir is now resolved once at the top of the
function instead of inside the if/else branch, which also flattens the
embedded_rev / git / pypi dispatch into a single if/elif/else.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels Jun 3, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This is a duplicate of the long-running update-check cache invalidation chain — same root cause (cache keyed on (rev, ver, ts) not invalidated when local HEAD moves). See #9670, #18236, #21447, #26006 which all add the HEAD hash to .update_check. Consolidating on one of those would avoid yet another competing PR for the same cosmetic banner fix.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for targeting a real cache-invalidation gap. Current main still returns a fresh .update_check entry when only rev and ver match (hermes_cli/banner.py:330-344), before it resolves the local checkout (hermes_cli/banner.py:348-360), so a pull/rebase can retain a stale behind count.

Problems

  • The PR contains no regression test for a changed local HEAD. Existing cache-hit coverage at tests/hermes_cli/test_update_check.py:19-37 does not exercise that invalidation path.
  • The branch is now conflict-bearing (mergeStateStatus: DIRTY). Current main added the Docker early return at hermes_cli/banner.py:323-328; salvage must retain it.

Suggested changes

  • Preserve current main's Docker guard and source-location-first repo selection, then add the HEAD key before cache acceptance.
  • Add a test that primes H1, resolves H2 within the TTL, and verifies a recheck plus cache rewrite.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/install-update Installer, updater, packaging, wheels, doctor labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants