Skip to content

fix(banner): stop fabricating "1 commit behind" on SSH-official remotes - #61050

Closed
gkd2323c wants to merge 1 commit into
NousResearch:mainfrom
gkd2323c:fix/banner-ssh-behind-count-20260709
Closed

fix(banner): stop fabricating "1 commit behind" on SSH-official remotes#61050
gkd2323c wants to merge 1 commit into
NousResearch:mainfrom
gkd2323c:fix/banner-ssh-behind-count-20260709

Conversation

@gkd2323c

@gkd2323c gkd2323c commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Problem

The SSH-official-remote branch in banner._check_via_local_git was hard-coded to return 1 whenever _check_via_rev reported UPDATE_AVAILABLE_NO_COUNT, so hermes --version and the CLI banner surfaced a stable but false Update available: 1 commit behind — run hermes update message.

The count never grew: whether upstream was 1 commit or 100 commits ahead, the banner always said "1 commit behind".

Root cause

Introduced in #52828 to "align with the desktop client checker". The intent was to give the desktop UI something to show, but the change picked the wrong lever: an ls-remote probe against the upstream URL can only tell us tip SHAs, not a real commit count. Fabricating 1 in the source path breaks every downstream consumer that renders a count.

The UPDATE_AVAILABLE_NO_COUNT sentinel (-1) already means "update exists, count unknown" — that's the exact shape this path should return. It's what check_via_nix_revision and (via #50784) check_via_pypi already do.

Why no UI consumer needs the fake 1

  • REST /api/hermes/update/check (hermes_cli/web_server.py::check_hermes_update) treats any nonzero behind as update_available=true, and the docstring explicitly documents -1 as a legitimate value ("-1 if behind by an unknown count (nix/pypi)").
  • Desktop store (apps/desktop/src/store/updates.ts::mapBackendCheck) already clamps behind <= 0 to 0 and reads updateAvailable as a separate boolean field — so a -1 payload from the backend renders as updateAvailable=true, behind=0, exactly the "yes update, no number" UX the original PR was trying to achieve.

So restoring the sentinel is a strict improvement: the CLI banner and hermes --version say "Update available" honestly instead of inventing a count, and every REST/desktop consumer keeps working.

Changes

  • hermes_cli/banner.py::_check_via_local_git — drop the return 1 override in the SSH branch; propagate the sentinel unchanged. Comment explains why every UI consumer already handles -1 correctly.
  • hermes_cli/main.py::_print_version_info — render the sentinel as Update available — run <cmd> (no count).
  • tests/hermes_cli/test_update_check.py
    • Update test_check_for_updates_official_ssh_origin_uses_https_probe to assert on the sentinel (was asserting on the fabricated 1).
    • Add three new tests locking in the CLI renderer's three branches: UPDATE_AVAILABLE_NO_COUNT → "Update available" without a count; > 0 → exact-count rendering; == 0 → "Up to date".

Test plan

$ ./venv/bin/python -m pytest tests/hermes_cli/test_update_check.py \
    tests/gateway/test_version_command.py tests/hermes_cli/test_banner_git_state.py \
    tests/hermes_cli/test_banner_pip_update.py tests/hermes_cli/test_banner_skills.py \
    tests/hermes_cli/test_pip_install_detection.py \
    -o 'addopts=' -q -n 0 --tb=short
51 passed in 8.30s

Repro (before this PR)

Clone git@github.com:NousResearch/hermes-agent.git (SSH remote) via the installer (--depth 1). Fully update to the latest tag. Run hermes --version:

Update available: 1 commit behind — run 'hermes update'

Now run hermes update. It reports Already up to date. hermes --version still says 1 commit behind. Rinse and repeat — the message is permanent because the SSH branch never actually counted anything.

After this PR the same environment reports Up to date (when caught up) or Update available — run 'hermes update' (when upstream is genuinely ahead).

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels Jul 8, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. Current main still turns the unknown-count sentinel into a literal 1 in hermes_cli/banner.py:199-202, despite _check_via_rev() returning UPDATE_AVAILABLE_NO_COUNT for differing tip SHAs at hermes_cli/banner.py:191. The PR restores that sentinel and adds the corresponding no-count branch for hermes --version.

Existing consumers are compatible: hermes_cli/web_server.py:3543-3549 treats any nonzero value as an available update, and apps/desktop/src/store/updates.ts:286-294 preserves update_available while clamping a negative count. The added regression coverage complements the existing SSH probe test in tests/hermes_cli/test_update_check.py:100-129.

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 labels Jul 10, 2026
@gkd2323c
gkd2323c force-pushed the fix/banner-ssh-behind-count-20260709 branch from 46e7120 to 6a80a21 Compare July 13, 2026 15:38
The SSH-official-remote path in _check_via_local_git was hard-coded to
return 1 when _check_via_rev reported UPDATE_AVAILABLE_NO_COUNT, so
'hermes --version' and the CLI banner surfaced a stable but false
'Update available: 1 commit behind — run hermes update' message. The
count never grew: whether upstream was 1 commit or 100 commits ahead,
the banner always said '1 commit behind'.

Root cause: an ls-remote probe against the upstream URL can only tell
us tip SHAs, not a real commit count. Returning the sentinel
UPDATE_AVAILABLE_NO_COUNT (-1) already means 'update exists, count
unknown' — the exact right shape for this path.

The dashboard/desktop UI does not depend on the fabricated 1:

- The REST /api/hermes/update/check endpoint
  (hermes_cli/web_server.py::check_hermes_update) treats any nonzero
  behind as update_available=true, and its docstring explicitly
  documents -1 as a legitimate value.
- The desktop store (apps/desktop/src/store/updates.ts::mapBackendCheck)
  clamps behind<=0 to 0 and reads updateAvailable as a separate boolean
  field.

So restoring the sentinel is a strict improvement: CLI banner and
hermes --version now say 'Update available' honestly instead of
inventing a count, and every REST/desktop consumer keeps working.

Changes:
- hermes_cli/banner.py: drop the 'return 1' override in the SSH branch;
  propagate the sentinel unchanged.
- hermes_cli/main.py::_print_version_info: render the sentinel as
  'Update available — run <cmd>' (without a count).
- tests/hermes_cli/test_update_check.py: update the SSH-official test
  to assert on the sentinel; add 3 new tests covering the CLI
  renderer's -1 / >0 / 0 branches.
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #86257 — your commit was cherry-picked onto current main with your authorship preserved in git log (bf10349). Thanks! Your fix was the earliest in this cluster and is credited as the base of the salvage: the SSH-official path now passes the UPDATE_AVAILABLE_NO_COUNT sentinel through untouched and hermes version renders it without a fabricated count. On top of it we added exact-count recovery via the GitHub compare API, so most installs now show the real number instead of the generic message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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