Skip to content

fix(banner): invalidate cached update check when local HEAD changes - #22979

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

fix(banner): invalidate cached update check when local HEAD changes#22979
airly7429-dev wants to merge 1 commit into
NousResearch:mainfrom
airly7429-dev:fix/banner-cache-head-invalidation

Conversation

@airly7429-dev

@airly7429-dev airly7429-dev commented May 10, 2026

Copy link
Copy Markdown

Summary

This fixes stale banner update-check results when the local HEAD changes.

Previously, cached update-check state could survive across local branch/commit
changes, which made the banner show outdated information. This change invalidates
the cache when the local HEAD no longer matches the cached revision and adds
test coverage for that behavior.

Changes

  • invalidate cached banner update-check results on local HEAD change
  • keep existing cache behavior when the local revision is unchanged
  • add tests covering cache invalidation and related banner behavior

Testing

  • python -m pytest tests/hermes_cli/test_banner.py -q

Notes

This PR only changes banner/update-check behavior and does not change webhook or
gateway runtime behavior.

What does this PR do?

Related Issue

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform:

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

  • This skill is broadly useful to most users (if bundled) — see Contributing Guide
  • SKILL.md follows the standard format (frontmatter, trigger conditions, steps, pitfalls)
  • No external dependencies that aren't already available (prefer stdlib, curl, existing Hermes tools)
  • I've tested the skill end-to-end: hermes --toolsets skills -q "Use the X skill to do Y"

Screenshots / Logs

@airly7429-dev airly7429-dev changed the title fix(banner): invalidate cached update check on local HEAD change fix(banner): invalidate cached update check when local HEAD changes May 10, 2026
@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 May 10, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #5359, #18236, and #22118 — all address the same stale update-check cache when local HEAD changes. Please consolidate with the earliest open PR (#5359).

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for isolating the stale-HEAD cache case. The underlying issue is still present on current main: hermes_cli/banner.py:340-344 accepts a fresh cache when cached["rev"] == embedded_rev; ordinary git installs have embedded_rev is None and later take the local-git path at hermes_cli/banner.py:351-360.

Problems

  • The branch is conflicting and predates the update-check rewrite in d682f320b. Current check_for_updates() also preserves Docker handling, PyPI fallback, and VERSION cache invalidation (hermes_cli/banner.py:323-365); those must remain intact during salvage.
  • The added fixtures omit ver, but current cache reuse requires cached.get("ver") == VERSION at hermes_cli/banner.py:342. The coverage belongs with the existing update-check tests in tests/hermes_cli/test_update_check.py.

Suggested changes

  • Port the HEAD-derived cache identity into the current local-git path while retaining the current install-method branches and version guard.
  • Update the cache-hit and changed-HEAD regressions in tests/hermes_cli/test_update_check.py to include the current version field.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/install-update Installer, updater, packaging, wheels, doctor labels Jul 13, 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-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