fix(banner): count "commits behind" against myfork/main on fork-tracking checkouts

[sahilm-ai]

Problem

On an inverted-topology checkout (this box: origin = NousResearch upstream, myfork = the fork the machine tracks and deploys from), the startup banner perpetually nags:

⚠ 5 commits behind — run <upstream update command> to update

even on a freshly fork-synced tree. Two defects:

1. Wrong baseline. _check_via_local_git counted HEAD..origin/main (upstream). The checkout is 0 behind its own fork but N behind upstream, so the nag never clears.
2. Wrong remedy. The upstream auto-pull path resets to upstream and discards fork-only commits — exactly the wrong action on a fork-tracking checkout (upstream is integrated via the fork-sync workflow, never reset).

banner.py predated the fork-tracking work (#47) and never consumed hermes_cli.fork_tracking.

Fix

Wire _check_via_local_git to fork_tracking.detect_fork_tracking (read-only consumer):

• Fork-tracking checkout (resolver returns a config): count HEAD..{cfg.fork_ref} (myfork/main), fetching the fork remote — never origin. Banner reads "N behind your fork" — 0 on a synced checkout, non-zero only on genuine local drift behind the fork. Remedy copy points at the fork-sync workflow, not the upstream update command.
• Else (canonical upstream-tracking install): byte-identical to before — count behind origin/main, suggest the upstream update command. PyPI / container / official-remote installs unchanged.

_check_via_local_git now returns (behind, baseline). check_for_updates threads the baseline into the 6h .update_check cache (new baseline key) plus a module-level getter, so a fork cache is never read as an upstream cache (or vice-versa) and the render picks the correct remedy.

Verification

On the live inverted checkout (origin = NousResearch, myfork = sahilm-ti):

behind: 0 | baseline: fork          # new _check_via_local_git
HEAD..origin/main (upstream): 5     # old, wrong baseline -> phantom nag
HEAD..myfork/main  (fork):     0    # new, correct -> no nag

Tests (tests/hermes_cli/test_update_check.py, 17 pass; tests/test_fork_tracking.py, 16 pass; tests/gateway/test_update_command.py, 35 pass):

• test_check_via_local_git_fork_tracking_counts_against_fork — counts HEAD..myfork/main, fetches myfork not origin.
• test_check_via_local_git_fork_tracking_reports_drift — non-zero fork-relative count on drift.
• test_check_via_local_git_non_fork_unchanged — upstream path invariant (origin/main, fetch origin, baseline=upstream).
• test_fork_cache_carries_baseline_back / test_check_for_updates_persists_fork_baseline — baseline persisted to and read back from cache.
• Existing test_check_for_updates_expired_cache updated to assert the upstream-path git calls are present rather than an exact subprocess call count (fork-detection adds one leading git remote get-url origin probe).

ruff check clean; ty introduces zero new diagnostics (4 pre-existing build_welcome_banner signature warnings, unrelated, present on base).

Scope / non-goals

• Read-only consumer of fork_tracking.py — no change to its merge logic or the cron fork-sync workflow.
• Non-fork installs stay byte-identical.

🤖 Authored by sahilm-ai (AI worker) for kanban task t_6c2d0631.

Summary by CodeRabbit

Release Notes

• New Features

  • Added fork-tracking support for update checks, providing fork-specific sync guidance instead of generic upstream recommendations
  • Baseline values now cached for 6 hours to optimize performance

• Bug Fixes

  • Improved cache invalidation to prevent stale update counts from being reused across different baseline configurations

[coderabbitai]

📝 Walkthrough

Walkthrough

The banner update-check logic in hermes_cli/banner.py is extended with a fork-tracking baseline mode. _check_via_local_git() now returns a (behind, baseline) tuple, fetching from the fork remote and counting HEAD..myfork/main when a fork-tracking checkout is detected. The baseline is persisted in the .update_check cache, exposed via get_update_baseline(), and used to select fork-specific vs upstream remedy text in the banner. Tests are updated and expanded to match.

Changes

Fork-tracking update check baseline

Layer / File(s)	Summary
Baseline constants, module state, and accessor hermes_cli/banner.py	Defines BASELINE_UPSTREAM and BASELINE_FORK constants, a module-level _update_baseline variable, _set_update_baseline() helper, and the public get_update_baseline() accessor consumed by banner messaging.
Fork-tracking detection and _check_via_local_git() tuple return hermes_cli/banner.py	Adds _resolve_fork_tracking() to detect fork-tracking setups via hermes_cli.fork_tracking.detect_fork_tracking, rewrites _check_via_local_git() to fetch the fork remote and count HEAD..myfork/main when active, and returns (behind, baseline) tuples for both branches including on failure.
Cache read/write and check_for_updates() baseline wiring hermes_cli/banner.py	Cache-read guard now calls _set_update_baseline() from the cached baseline value before returning; adds BASELINE_UPSTREAM defaulting on cache miss; check_for_updates() unpacks (behind, baseline), writes baseline into the .update_check JSON payload, and calls _set_update_baseline().
Banner fork-specific remedy text hermes_cli/banner.py	Banner update messaging branches on get_update_baseline(): upstream installs render the existing recommended-update-command text; fork-tracking installs render a sync-to-myfork/main remedy instead.
Test updates and new fork/cache test cases tests/hermes_cli/test_update_check.py	Updates expired-cache and local-git tests to assert specific git commands and unpack (behind, baseline) tuples; adds fork-tracking scaffolding, fork-vs-upstream git ref counting tests, and cache-read/cache-write tests for baseline persistence via get_update_baseline().

Sequence Diagram(s)

sequenceDiagram
    participant Banner
    participant check_for_updates
    participant _check_via_local_git
    participant fork_tracking
    participant git

    Banner->>check_for_updates: check_for_updates()
    check_for_updates->>check_for_updates: read .update_check cache
    alt cache hit (fresh)
        check_for_updates->>check_for_updates: _set_update_baseline(cached_baseline)
        check_for_updates-->>Banner: cached behind count
    else cache miss / expired
        check_for_updates->>_check_via_local_git: _check_via_local_git(repo_dir)
        _check_via_local_git->>fork_tracking: detect_fork_tracking()
        alt fork tracking active
            _check_via_local_git->>git: git fetch myfork --quiet
            _check_via_local_git->>git: git rev-list --count HEAD..myfork/main
            _check_via_local_git-->>check_for_updates: (behind, BASELINE_FORK)
        else upstream
            _check_via_local_git->>git: git fetch origin --quiet
            _check_via_local_git->>git: git rev-list --count HEAD..origin/main
            _check_via_local_git-->>check_for_updates: (behind, BASELINE_UPSTREAM)
        end
        check_for_updates->>check_for_updates: write behind + baseline to .update_check
        check_for_updates->>check_for_updates: _set_update_baseline(baseline)
        check_for_updates-->>Banner: behind count
    end
    Banner->>check_for_updates: get_update_baseline()
    check_for_updates-->>Banner: baseline string
    alt baseline == BASELINE_FORK
        Banner->>Banner: render "sync to myfork/main" remedy
    else baseline == BASELINE_UPSTREAM
        Banner->>Banner: render "run recommended update command" remedy
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• sahilm-ti/hermes-agent#47: Introduces hermes_cli.fork_tracking.detect_fork_tracking, the exact function this PR calls in _resolve_fork_tracking() to determine fork-tracking checkout status and resolve the fork remote ref.

Poem

🐇 A fork in the path, a remote to track,
I count the commits behind and come back.
The baseline is cached, the banner is bright,
"Sync to myfork/main" — that's the right byte!
Upstream or fork, I know which you are,
Hopping through git logs, near or far. 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 76.19% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing commit-behind counting on fork-tracking checkouts by counting against myfork/main instead of origin/main.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch kanban/t_6c2d0631

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

🔎 Lint report: kanban/t_6c2d0631 vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 10957 on HEAD, 10955 on base (🆕 +2)

🆕 New issues (2):

Rule	Count
unresolved-attribute	2

First entries

run_agent.py:2920: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`

✅ Fixed issues (1):

Rule	Count
invalid-assignment	1

First entries

tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`

Unchanged: 5756 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

hermes_cli/banner.py (1)

357-377: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Cache does not invalidate when baseline mode changes.

The comment on lines 362-364 states the baseline guard "prevents a fork-tracking checkout from reading an upstream-baseline cache (or vice versa)", but the cache validity check on lines 369-373 does not compare the cached baseline against the current detection result.

If a checkout transitions between fork-tracking and upstream-tracking (e.g., user adds/removes the myfork remote), the stale behind count from the old baseline will be returned for up to 6 hours. The remedy text will be correct (line 374 sets the baseline from cache), but the commit count will be measured against the wrong ref.

Consider adding a baseline check to the cache guard, or note in the comment that this edge case is intentionally tolerated.

Proposed fix to add baseline validation

+    # Pre-compute current baseline to validate cache
+    repo_dir = Path(__file__).parent.parent.resolve()
+    if not (repo_dir / ".git").exists():
+        repo_dir = get_hermes_home() / "hermes-agent"
+    current_baseline = BASELINE_FORK if (repo_dir / ".git").exists() and _resolve_fork_tracking(repo_dir) else BASELINE_UPSTREAM
+
     now = time.time()
     try:
         if cache_file.exists():
             cached = json.loads(cache_file.read_text())
             if (
                 now - cached.get("ts", 0) < _UPDATE_CHECK_CACHE_SECONDS
                 and cached.get("rev") == embedded_rev
                 and cached.get("ver") == VERSION
+                and cached.get("baseline", BASELINE_UPSTREAM) == current_baseline
             ):
                 _set_update_baseline(cached.get("baseline", BASELINE_UPSTREAM))
                 return cached.get("behind")

Alternatively, if this edge case (switching baseline mode mid-cache-lifetime) is rare enough to tolerate, update the comment to reflect the actual behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hermes_cli/banner.py` around lines 357 - 377, The cache validity check in the
section starting at line 369 validates the timestamp, rev, and version but does
not compare the cached baseline against the current baseline, even though the
comment states the baseline guard prevents cross-pollution between fork-tracking
and upstream-tracking modes. Add a baseline check to the cache validity
condition to ensure that if the baseline mode has changed (e.g., user
adds/removes the myfork remote), the stale cached behind count is invalidated.
Determine the current baseline before the cache check and add a comparison of
cached.get("baseline") against the detected current baseline to the conditional
on lines 370-373.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@hermes_cli/banner.py`:
- Around line 357-377: The cache validity check in the section starting at line
369 validates the timestamp, rev, and version but does not compare the cached
baseline against the current baseline, even though the comment states the
baseline guard prevents cross-pollution between fork-tracking and
upstream-tracking modes. Add a baseline check to the cache validity condition to
ensure that if the baseline mode has changed (e.g., user adds/removes the myfork
remote), the stale cached behind count is invalidated. Determine the current
baseline before the cache check and add a comparison of cached.get("baseline")
against the detected current baseline to the conditional on lines 370-373.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9b23edb0-7784-457d-a3a2-4daef1452dae

📥 Commits

Reviewing files that changed from the base of the PR and between 40305a4 and 282c72d.

📒 Files selected for processing (2)

• hermes_cli/banner.py
• tests/hermes_cli/test_update_check.py

[sahilm-ti]

auto-review: approved, awaiting human merge + kanban_approve.

Matrix checks (U1–U6, C1–C6)

• U1 in-scope ✓ — only hermes_cli/banner.py (+110/-16) and tests/hermes_cli/test_update_check.py (+171/-2); both in AC scope.
• U2 deletions ✓ — none.
• U3 secrets ✓ — clean.
• U4 AC coverage ✓ — fork baseline (HEAD..myfork/main, fetches the fork not origin), fork remedy copy ("N behind your fork — sync the live checkout to myfork/main"), baseline threaded into the 6h .update_check cache key, non-fork path byte-identical, + 5 new regression tests. All 4 "Verify before review" items covered.
• U5 mergeable — MERGEABLE. See CI caveat below.
• U6/C6 UI — N/A (terminal banner; no GChat/web/UI-glob files).
• C1 CI — PR's own gates green: ruff + ty diff, ruff enforcement, check-attribution, e2e, all typechecks, test_banner.py (8✓), test_fork_tracking.py (16✓), test_update_check.py (17✓). Caveat (CI red, NOT this PR): the test (1) shard fails on 6 tests in files this PR never touched — tests/gateway/test_kanban_notifier.py (5× ImportError: cannot import name 'NOTIFY_BLOCKED_REASON_MAX'/'NOTIFY_DONE_SUMMARY_MAX'/'NOTIFY_GAVE_UP_ERROR_MAX' from 'gateway.run') and tests/hermes_cli/test_kanban_db.py::test_cleanup_workspace_swept_after_last_child_completes. Verified these reproduce identically against the PR head tree and the constants are absent from gateway/run.py on base — a pre-existing base-branch breakage, orthogonal to the banner change. Not grounds to bounce this PR.
• C2 type-discipline ✓ — no new type: ignore/cast/Any (ty-diff CI green). The 4 ty diagnostics on banner.py:636/638 are pre-existing build_welcome_banner signature warnings, present on base.
• C3 lint ✓ — ruff check clean.
• C4 tests touched ✓ — 5 new fork-baseline regression tests + 1 updated expired-cache test.
• C5 worker identity ✓ — commit 282c72d6 authored + committed by Sahil (AI) <266772320+sahilm-ai@…>.

Code-quality judgment (role-reviewer)

Verdict: APPROVED (no Blocker/Major).

API consumption matches fork_tracking.py exactly (detect_fork_tracking → cfg.fork_remote/cfg.fork_ref). The try/except: pass paths follow the file's established "can't compute → render nothing (None)" pattern, not a new silent fallback on a required value. Threaded _update_baseline global mirrors the existing _update_result ordering. Tests are black-box at the subprocess + injection boundary. All _check_via_local_git callers updated to the new (behind, baseline) tuple (web_server.py reference is a comment, not a call).

• [Minor / follow-up] sibling changelog ref hermes_cli/web_server.py:2193 _recent_upstream_commits
  • Evidence: hardcodes HEAD..origin/main for the dashboard "what's changed" list.
  • Why: out of this card's AC (it computes a changelog, not the behind-count). Self-limiting — gated on behind != 0, and on a synced fork checkout check_for_updates() (now fork-aware) returns 0, so this branch never fires for the reported symptom. Only on genuine fork-drift (behind>0) would the dashboard list upstream commits against the wrong ref.
  • Fix: follow-up card to teach _recent_upstream_commits the fork baseline (count/changelog against myfork/main). Not blocking this merge.