Skip to content

fix(cli): make banner update state branch-aware - #33498

Open
Dusk1e wants to merge 2 commits into
NousResearch:mainfrom
Dusk1e:fix/cli-banner-update-state-branch-aware
Open

fix(cli): make banner update state branch-aware#33498
Dusk1e wants to merge 2 commits into
NousResearch:mainfrom
Dusk1e:fix/cli-banner-update-state-branch-aware

Conversation

@Dusk1e

@Dusk1e Dusk1e commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a state parity gap between branch-aware update operations and localized system surfaces by making the startup banner visualization and the .update_check cache model completely branch-aware.

Why This Is Merge-Worthy

A behavior divergence existed between the branch-aware core update mechanics and adjacent interface surfaces. While a recent update introduced specific branch tracking capabilities to the update engine, the terminal welcome banner and the 6-hour filesystem cache block still evaluated remote drift metrics exclusively against a hard-coded origin/main target. Consequently, users operating on custom engineering or feature branches saw erroneous ahead/behind status logs or inherited invalid configuration keys from stale cache payloads.

Parity Reference Source

This modification directly aligns interface surfaces with the invariant established in:

  • Commit: 9d3e931
  • Message: feat(cli): add --branch flag to hermes update

Scope of Changes

  • hermes_cli/banner.py: Refactored git state resolution paths to evaluate active branch tracking parameters dynamically (origin/<current-branch>) before utilizing traditional origin/main fallbacks.
  • hermes_cli/banner.py: Extended the .update_check local storage configuration schema to scope valid entry arrays explicitly by their resolved compare_ref parameters.
  • tests/hermes_cli/test_banner_git_state.py: Injected target regression checks mapping proper isolated cache storage keys and specific remote target selections.

Verified Test Suites

Targeted git state resolution checks and branch-flag update regressions completed with absolute success:

  • tests/hermes_cli/test_banner_git_state.py
  • tests/hermes_cli/test_cmd_update.py::TestCmdUpdateBranchFlag
  • tests/hermes_cli/test_cmd_update.py::TestCmdUpdateCheckBranchFlag
15 passed, 0 failed

@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 27, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Overlaps with open PRs #18236, #9670, #7303, #4424 which all address branch-aware git state caching for the startup banner/update check. Closed duplicates include #26180, #20792, #5359.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pursuing parity between the update command and startup surfaces.

Problems

  • This branch predates current update-check safeguards. Its _check_via_local_git() at hermes_cli/banner.py:203-228 removes the shallow-clone path now required by hermes_cli/banner.py:204-239; tests/hermes_cli/test_update_check.py:132-170 verifies that shallow clones must not run a history count across the boundary.
  • The PR cache payload at hermes_cli/banner.py:321-326 omits the current ver key used by hermes_cli/banner.py:330-344 to invalidate stale results after a pip upgrade.
  • check_for_updates() also powers the dashboard (hermes_cli/web_server.py:3639-3651), while its changelog remains deliberately pinned to HEAD..origin/main (hermes_cli/web_server.py:3534-3555). Changing only the count reference can make the count and displayed changelog disagree.
  • Current docs define origin/main as the default update target, with non-main selection via --branch (website/docs/getting-started/updating.md:36-43).

Suggested changes

  • Salvage the branch-aware idea onto current banner.py without dropping the current shallow-clone, Docker, SSH-remote, and version-cache safeguards.
  • Decide whether passive checks should remain on the documented main default or make the dashboard/changelog path branch-aware as one coherent behavior.

Automated hermes-sweeper review.

Comment thread hermes_cli/banner.py
try:
result = subprocess.run(
["git", "rev-list", "--count", "HEAD..origin/main"],
["git", "rev-list", "--count", f"HEAD..{compare_ref}"],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unconditional history count regresses the shallow-installer safeguard now on main: shallow clones must fetch with --depth 1 and compare tips rather than run rev-list --count across the boundary. Please preserve that behavior when rebasing this helper.

Comment thread hermes_cli/banner.py
"ts": now,
"behind": behind,
"rev": embedded_rev,
"compare_ref": compare_ref,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please retain the current ver: VERSION cache discriminator here. Main uses it to invalidate a fresh cache after pip install --upgrade; without it, an old behind result survives until the six-hour TTL expires.

@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