fix(cli): stop using --depth 1 for update-check fetches on shallow installer checkouts

hermes_cli/banner.py

@@ -225,14 +225,18 @@ def _check_via_local_git(repo_dir: Path) -> Optional[int]:
             return 1
         return checked
 
-    # Installer checkouts are shallow (`git clone --depth 1`). On a shallow
-    # clone the history stops at a single commit, so a plain `git fetch` would
-    # unshallow the repo (dragging in the whole history) and
-    # `rev-list --count HEAD..origin/main` would report a huge bogus "behind"
-    # number (e.g. "12492 commits behind"). Detect shallow up front: fetch with
-    # --depth 1 to preserve the boundary and compare tip SHAs instead of
-    # counting. Full clones (developers, Docker dev images) keep the exact
-    # count path unchanged. Mirrors the desktop fix in apps/desktop/electron/main.cjs.
+    # Installer checkouts are shallow (`git clone --depth 1`). This used to
+    # fetch with `--depth 1` here too, on the theory that a plain fetch
+    # would unshallow the repo and drag in the whole history. Verified
+    # against real git behavior that this isn't the case: a plain,
+    # branch-scoped fetch on an ALREADY-shallow repo extends the existing
+    # shallow boundary forward incrementally -- only the new commits
+    # transfer, and merge-base/rev-list remain exact. --depth 1, in
+    # contrast, unconditionally marks the freshly fetched tip as a NEW
+    # boundary even when its parent is already local, permanently breaking
+    # the count from that fetch onward -- not self-healing, since every
+    # later plain fetch's new tip connects to that now-orphaned boundary
+    # (issue #84591).
     shallow = _git_stdout(["rev-parse", "--is-shallow-repository"], cwd=repo_dir)
     is_shallow = shallow == "true"
 
@@ -246,8 +250,6 @@ def _check_via_local_git(repo_dir: Path) -> Optional[int]:
         # unaffected; the shallow path compares against FETCH_HEAD, which a
         # scoped fetch also updates.
         fetch_args = ["git", "fetch", "origin", "main"]
-        if is_shallow:
-            fetch_args += ["--depth", "1"]
         fetch_args.append("--quiet")
         subprocess.run(
             fetch_args,
@@ -258,17 +260,35 @@ def _check_via_local_git(repo_dir: Path) -> Optional[int]:
         pass  # Offline or timeout — use stale refs, that's fine
 
     if is_shallow:
-        # No history to count across the shallow boundary. `origin/main` may not
-        # be a tracking ref in a `clone --depth 1`, so prefer FETCH_HEAD (just
-        # updated by the fetch above) and fall back to origin/main.
-        head_rev = _git_stdout(["rev-parse", "HEAD"], cwd=repo_dir)
+        # `origin/main` may not be a tracking ref in a `clone --depth 1`, so
+        # prefer FETCH_HEAD (just updated by the fetch above) and fall back
+        # to origin/main.
         target_rev = (
             _git_stdout(["rev-parse", "FETCH_HEAD"], cwd=repo_dir)
             or _git_stdout(["rev-parse", "origin/main"], cwd=repo_dir)
         )
+        head_rev = _git_stdout(["rev-parse", "HEAD"], cwd=repo_dir)
         if not head_rev or not target_rev:
             return None
-        return 0 if head_rev == target_rev else UPDATE_AVAILABLE_NO_COUNT
+        if head_rev == target_rev:
+            return 0
+        # The shallow boundary no longer blocks an exact count in the common
+        # case now that the fetch above isn't --depth-limited. Try rev-list
+        # against the resolved target first, falling back to presence-only
+        # only if it genuinely can't connect (e.g. a rewritten upstream
+        # history orphaning the old boundary).
+        try:
+            result = subprocess.run(
+                ["git", "rev-list", "--count", f"{head_rev}..{target_rev}"],
+                capture_output=True, text=True, encoding="utf-8", errors="replace",
+                timeout=5,
+                cwd=str(repo_dir),
+            )
+            if result.returncode == 0:
+                return int(result.stdout.strip())
+        except Exception:
+            pass
+        return UPDATE_AVAILABLE_NO_COUNT
 
     try:
         result = subprocess.run(

hermes_cli/update_cmd.py

@@ -2290,11 +2290,22 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False):
     # Note: upstream/<branch> may not exist for non-main branches (a fork's
     # bb/gui has no upstream counterpart), so when the caller picks a
     # non-default branch we skip the upstream probe and use origin directly.
-    # Installer checkouts are shallow (`git clone --depth 1`). A plain
-    # `git fetch` would unshallow the repo (dragging in the whole history —
-    # the exact cost the shallow clone avoided) and the rev-list count below
-    # would then report a huge bogus "behind" number. Detect shallow up front:
-    # fetch with --depth 1 to preserve the boundary and report presence-only.
+    #
+    # Installer checkouts are shallow (`git clone --depth 1`). This used to
+    # fetch with `--depth 1` here too, on the theory that a plain fetch would
+    # "unshallow" the repo and drag in the whole history. Verified against
+    # real git behavior that this isn't the case: a plain, branch-scoped
+    # fetch on an ALREADY-shallow repo extends the existing shallow boundary
+    # forward incrementally -- git's fetch negotiation reuses the current
+    # boundary as a "have" reference, so only the new commits transfer, .git
+    # doesn't grow, and merge-base/rev-list remain exact. --depth 1, in
+    # contrast, unconditionally marks the freshly fetched tip as a NEW
+    # boundary even when its parent is already local, permanently breaking
+    # merge-base from that fetch onward -- not self-healing, since every
+    # later plain fetch's new tip connects to that now-orphaned boundary
+    # (issue #84591). Kept `is_shallow` (used below to decide the fallback
+    # path if rev-list genuinely can't connect, e.g. after a history rewrite
+    # upstream) without any depth-limiting on the fetch itself.
     is_shallow = (
         subprocess.run(
             git_cmd + ["rev-parse", "--is-shallow-repository"],
@@ -2304,7 +2315,7 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False):
         ).stdout.strip()
         == "true"
     )
-    depth_args = ["--depth", "1"] if is_shallow else []
+    depth_args: list[str] = []
 
     if branch == "main":
         # Probe locally (~6 ms) whether an 'upstream' remote exists at all
@@ -2382,8 +2393,32 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False):
         sys.exit(1)
 
     if is_shallow:
-        # No history to count across the shallow boundary. Compare tip SHAs and
-        # report presence-only (mirrors the banner's _check_via_local_git).
+        # The shallow boundary no longer blocks an exact count in the
+        # common case (see the fetch comment above) -- try rev-list first
+        # and only fall back to presence-only if it genuinely can't
+        # connect HEAD to compare_branch (e.g. upstream history was
+        # rewritten and the old boundary is no longer an ancestor).
+        rev_probe = subprocess.run(
+            git_cmd + ["rev-list", f"HEAD..{compare_branch}", "--count"],
+            cwd=_m().PROJECT_ROOT,
+            capture_output=True,
+            text=True, encoding="utf-8", errors="replace",
+        )
+        if rev_probe.returncode == 0:
+            behind = int(rev_probe.stdout.strip())
+            if behind == 0:
+                print("✓ Already up to date.")
+            else:
+                commits_word = "commit" if behind == 1 else "commits"
+                print(f"⚕ Update available: {behind} {commits_word} behind {compare_branch}.")
+                from hermes_cli.config import recommended_update_command
+
+                print(f"  Run '{recommended_update_command()}' to install.")
+            return
+
+        # rev-list couldn't connect the history -- fall back to
+        # presence-only. Compare tip SHAs and report accordingly (mirrors
+        # the banner's _check_via_local_git).
         head_sha = subprocess.run(
             git_cmd + ["rev-parse", "HEAD"],
             cwd=_m().PROJECT_ROOT, capture_output=True, text=True, encoding="utf-8", errors="replace",

tests/hermes_cli/test_shallow_checkout_update_count_84591.py

@@ -0,0 +1,164 @@
+"""Regression for #84591 — a --depth 1 check-fetch permanently poisons
+merge-base on shallow installer checkouts, stalling the update indicator
+at a placeholder "1 commit behind" forever.
+
+Uses REAL git repositories (not mocked subprocess calls): the bug is in
+git's own shallow-fetch semantics (whether --depth 1 marks an unconditional
+new shallow boundary vs. a plain fetch correctly extending an existing one
+incrementally), which a mock cannot meaningfully exercise.
+"""
+
+from __future__ import annotations
+
+import subprocess
+from pathlib import Path
+
+import pytest
+
+
+def _run(args: list[str], cwd: Path) -> subprocess.CompletedProcess:
+    return subprocess.run(
+        args, cwd=str(cwd), capture_output=True, text=True, check=True,
+        env={"GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@t.com",
+             "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@t.com",
+             "PATH": "/usr/bin:/bin"},
+    )
+
+
+def _commit(repo: Path, message: str) -> None:
+    (repo / "file.txt").write_text(message)
+    _run(["git", "add", "file.txt"], cwd=repo)
+    _run(["git", "commit", "-q", "-m", message], cwd=repo)
+
+
+@pytest.fixture
+def shallow_checkout(tmp_path):
+    """A real shallow clone of a real 'remote' repo, matching the
+    installer's `git clone --depth 1`. Returns (clone_dir, remote_dir)."""
+    remote = tmp_path / "remote.git"
+    remote.mkdir()
+    _run(["git", "init", "-q", "-b", "main"], cwd=remote)
+    for i in range(1, 21):
+        _commit(remote, f"commit {i}")
+
+    clone = tmp_path / "shallow_clone"
+    _run(
+        ["git", "clone", "--depth", "1", f"file://{remote}", str(clone)],
+        cwd=tmp_path,
+    )
+    return clone, remote
+
+
+def test_shallow_clone_fixture_is_actually_shallow(shallow_checkout):
+    """Sanity check on the test fixture itself."""
+    clone, _ = shallow_checkout
+    result = subprocess.run(
+        ["git", "rev-parse", "--is-shallow-repository"],
+        cwd=str(clone), capture_output=True, text=True,
+    )
+    assert result.stdout.strip() == "true"
+
+
+def test_plain_fetch_after_shallow_clone_preserves_exact_count(shallow_checkout):
+    """The core empirical claim behind this fix: a PLAIN (no --depth)
+    branch-scoped fetch on an already-shallow clone correctly extends the
+    shallow boundary forward, without dragging in full history, and
+    merge-base/rev-list stay exact. This is what banner.py/update_cmd.py's
+    fetch now does instead of --depth 1."""
+    clone, remote = shallow_checkout
+    for i in range(21, 24):
+        _commit(remote, f"commit {i}")
+
+    branch_result = subprocess.run(
+        ["git", "symbolic-ref", "--short", "HEAD"],
+        cwd=str(clone), capture_output=True, text=True,
+    )
+    branch = branch_result.stdout.strip()
+
+    fetch = subprocess.run(
+        ["git", "fetch", "origin", branch],
+        cwd=str(clone), capture_output=True, text=True,
+    )
+    assert fetch.returncode == 0
+
+    merge_base = subprocess.run(
+        ["git", "merge-base", "HEAD", f"origin/{branch}"],
+        cwd=str(clone), capture_output=True, text=True,
+    )
+    assert merge_base.returncode == 0, (
+        "merge-base must succeed after a plain fetch on a shallow clone"
+    )
+
+    count = subprocess.run(
+        ["git", "rev-list", "--count", f"HEAD..origin/{branch}"],
+        cwd=str(clone), capture_output=True, text=True,
+    )
+    assert count.returncode == 0
+    assert count.stdout.strip() == "3"
+
+
+def test_depth_1_fetch_poisons_merge_base_permanently(shallow_checkout):
+    """Confirms the reported bug's exact mechanism, as a negative control:
+    a --depth 1 fetch on an already-shallow clone marks a NEW shallow
+    boundary and breaks merge-base -- and does not self-heal on a later
+    plain fetch, since the next tip connects to the now-orphaned boundary.
+    This is the behavior the fix removes from the check paths."""
+    clone, remote = shallow_checkout
+    for i in range(21, 23):
+        _commit(remote, f"commit {i}")
+
+    branch_result = subprocess.run(
+        ["git", "symbolic-ref", "--short", "HEAD"],
+        cwd=str(clone), capture_output=True, text=True,
+    )
+    branch = branch_result.stdout.strip()
+
+    subprocess.run(
+        ["git", "fetch", "--depth", "1", "origin", branch],
+        cwd=str(clone), capture_output=True, text=True,
+    )
+    merge_base = subprocess.run(
+        ["git", "merge-base", "HEAD", f"origin/{branch}"],
+        cwd=str(clone), capture_output=True, text=True,
+    )
+    assert merge_base.returncode != 0, (
+        "sanity: --depth 1 must reproduce the merge-base failure "
+        "this fix works around"
+    )
+
+    # And it does NOT self-heal on a later plain fetch, even with more
+    # commits landing -- matching the reporter's "stuck forever" claim.
+    _commit(remote, "commit 23")
+    subprocess.run(
+        ["git", "fetch", "origin", branch],
+        cwd=str(clone), capture_output=True, text=True,
+    )
+    merge_base_2 = subprocess.run(
+        ["git", "merge-base", "HEAD", f"origin/{branch}"],
+        cwd=str(clone), capture_output=True, text=True,
+    )
+    assert merge_base_2.returncode != 0, (
+        "the poisoned boundary must not self-heal from a plain fetch alone"
+    )
+
+
+def test_check_via_local_git_reports_exact_count_on_shallow_checkout(
+    shallow_checkout, monkeypatch
+):
+    """End-to-end: banner.py's _check_via_local_git() against a real
+    shallow checkout must report the EXACT commit count, not the
+    presence-only placeholder, once its fetch no longer uses --depth 1."""
+    clone, remote = shallow_checkout
+    for i in range(21, 25):
+        _commit(remote, f"commit {i}")
+
+    import hermes_cli.banner as banner_mod
+
+    monkeypatch.setattr(banner_mod, "_is_official_ssh_remote", lambda url: False)
+
+    result = banner_mod._check_via_local_git(clone)
+    assert result == 4, (
+        f"expected the exact behind-count (4), got {result!r} -- if this "
+        f"is banner_mod.UPDATE_AVAILABLE_NO_COUNT (-1), the placeholder "
+        f"path fired instead of the exact count"
+    )