fix(banner): include local HEAD in the update-check cache key

[Kyzcreig]

Problem

check_for_updates() caches the "commits behind" count for 6 hours, keyed on {rev, ver}:

# hermes_cli/banner.py:296-300
now - cached.get("ts", 0) < _UPDATE_CHECK_CACHE_SECONDS
and cached.get("rev") == embedded_rev
and cached.get("ver") == VERSION

Neither component changes when a user updates in place:

• HERMES_REVISION is nix-only, so embedded_rev is None for source installs.
• VERSION only moves on a release bump, not on a git pull.

So an in-place git pull / rebase moves HEAD while the cache key stays constant. The stale count then survives the full TTL, and hermes --version keeps printing e.g. "182 commits behind" immediately after a successful update that brought the checkout up to date. hermes update calls _invalidate_update_cache() explicitly and is unaffected; this is the manual-git path.

Fix

Add the active checkout's HEAD SHA to the cache key, so an in-place update self-invalidates.

Two implementation details that matter:

• Computed after the docker short-circuit. Docker images ship without .git (see .dockerignore) and already return None earlier in the function; placing the read below that guard means containers never shell out to git for this.
• Reuses the existing helpers. _resolve_repo_dir() (banner.py:333) and _git_stdout() (banner.py:157) already exist on main and already handle the not-a-git-install and the Windows-encoding cases, so _local_head_sha() is four lines rather than a second subprocess.run wrapper with its own timeout/encoding policy.

No new config key, no new env var.

Test changes

Two existing tests asserted the old call sequence and had to move with it. Both were tightened rather than loosened:

• test_check_for_updates_uses_cache previously asserted mock_run.assert_not_called() — "no git at all". A git rev-parse HEAD now legitimately runs on the fast path. Rather than relax the assertion to a call count, the test now uses a side_effect that returns the HEAD and raises AssertionError on any other git invocation. That is a strictly stronger claim than the original: it proves no fetch and no rev-list happen, and it names which call is allowed.
• test_check_for_updates_expired_cache counted 4 subprocess calls (origin probe + shallow probe + fetch + rev-list); it is now 5 with the cache-key HEAD read. The comment enumerating them was updated to match.

New: test_check_for_updates_invalidates_when_head_moved pins the actual bug — fresh timestamp, same VERSION, different HEAD must recompute and must rewrite the cache with the new head. It fails on unpatched main.

Verification

# RED — implementation reverted, tests kept
tests/hermes_cli/test_update_check.py → 2 failed, 13 passed
  FAILED test_check_for_updates_invalidates_when_head_moved
  FAILED test_check_for_updates_expired_cache

# GREEN — with the fix
tests/hermes_cli/test_update_check.py → 15 passed

Neighbor suites on this branch:

tests/hermes_cli/test_banner.py
tests/hermes_cli/test_banner_git_state.py
tests/hermes_cli/test_update_check.py
tests/hermes_cli/test_update_stale_dashboard.py
tests/hermes_cli/test_cmd_update_docker.py
→ 68 passed, 0 failed

test_cmd_update_docker.py is included deliberately, since the placement of the HEAD read relative to the docker short-circuit is the one behavioral risk in the diff.

Supersedes

This replaces #40157 (same fix, same premise). That branch is 3,406 commits behind and hard-conflicts on apply. The premise was re-verified on current main before writing this: the cache key at banner.py:298-299 is still {rev, ver} only, and the write at :324 still omits head, so the bug is unfixed upstream.

The re-port is also smaller than the original: #40157 had to introduce its own subprocess.run block because _resolve_repo_dir()/_git_stdout() did not exist yet at the time. They do now, so this version delegates to them.

The review on #40157 noted one gap — no test for the actual HEAD-mismatch transition. test_check_for_updates_invalidates_when_head_moved is that test.

Happy to close #40157 in favor of this once maintainers confirm the approach.

[alt-glitch]

This was generated by AI during triage.

Duplicate of #20653: both make the update-check cache depend on the local Git HEAD so manual source updates invalidate a stale commits-behind banner.

[teknium1]

Thanks for the focused cache-correctness fix. The premise remains valid on current main: hermes_cli/banner.py:311-316 returns a fresh cache before the local checkout is resolved at hermes_cli/banner.py:320-335.

Problems

• The added _local_head_sha() docstring says both VERSION and HERMES_REVISION are None for source installs, but VERSION is defined at hermes_cli/banner.py:68. The code is correct; this is documentation-only. The new regression-test docstring repeats the same statement.

Suggested changes

• Say that VERSION remains unchanged across an in-place pull and that HERMES_REVISION is normally unset for source installs.
• GitHub currently reports this branch as conflicted. Main commit 6b81590c55bcc9c1001a33b51b528784f96c6a07 pruned the surrounding update-cache tests, so salvage should retain the new HEAD-mismatch regression in the current test file rather than restore the removed tests.

Automated hermes-sweeper review.

[teknium1]

VERSION is not None for source installs (hermes_cli/banner.py:68 defines it as a release string); it simply does not change after a manual pull. Please reword this to distinguish unchanged VERSION from normally-unset HERMES_REVISION.

[Kyzcreig]

Superseded by #99294, which carries this change rebased onto current main, unions it with the inconclusive-result guard from #82166 that landed at the same write site, and adds a fresh-cache-with-old-HEAD transition test. The new PR also credits #20653, which diagnosed this bug first. This one has gone stale against main.