fix(cli): restore visible git fetch progress and defer autostash until update confirmed (#3523)

hermes_cli/main.py

@@ -9571,14 +9571,26 @@ def _cmd_update_impl(args, gateway_mode: bool):
         branch = _resolve_update_branch(args)
 
         print("→ Fetching updates...")
+        # Don't capture stdout/stderr — surface git's fetch progress to the
+        # user (object enumeration, remote branch listing). We still get the
+        # returncode to detect failure; only on failure do we re-run with
+        # capture_output so we can grep stderr for network/auth errors.
+        # Regression introduced by #3492.
         fetch_result = subprocess.run(
             git_cmd + ["fetch", "origin", branch],
             cwd=PROJECT_ROOT,
-            capture_output=True,
+            capture_output=False,
             text=True,
         )
         if fetch_result.returncode != 0:
-            stderr = fetch_result.stderr.strip()
+            # Re-fetch with captured stderr so we can pattern-match the failure.
+            _diag = subprocess.run(
+                git_cmd + ["fetch", "origin", branch],
+                cwd=PROJECT_ROOT,
+                capture_output=True,
+                text=True,
+            )
+            stderr = _diag.stderr.strip()
             if "Could not resolve host" in stderr or "unable to access" in stderr:
                 print("✗ Network error — cannot reach the remote repository.")
                 print(f"  {stderr.splitlines()[0]}" if stderr else "")
@@ -9609,15 +9621,17 @@ def _cmd_update_impl(args, gateway_mode: bool):
         # "always update against main" behavior; for any other target it's
         # the same thing — get HEAD onto the requested branch first, then
         # fast-forward.
+        # NB: We do NOT stash here yet. We defer the stash until after we've
+        # confirmed there's actually an update to apply — otherwise every
+        # `hermes update` on an up-to-date tree with local changes would
+        # autostash and pop for nothing. Regression introduced by #3492.
         if current_branch != branch:
             label = (
                 "detached HEAD"
                 if current_branch == "HEAD"
                 else f"branch '{current_branch}'"
             )
             print(f"  ⚠ Currently on {label} — switching to {branch} for update...")
-            # Stash before checkout so uncommitted work isn't lost
-            auto_stash_ref = _stash_local_changes_if_needed(git_cmd, PROJECT_ROOT)
             checkout_result = subprocess.run(
                 git_cmd + ["checkout", branch],
                 cwd=PROJECT_ROOT,
@@ -9636,30 +9650,13 @@ def _cmd_update_impl(args, gateway_mode: bool):
                     text=True,
                 )
                 if track_result.returncode != 0:
-                    # Restore the user's prior branch + stash before bailing
-                    # so we don't leave them stranded in a weird state.
-                    if auto_stash_ref is not None:
-                        _restore_stashed_changes(
-                            git_cmd,
-                            PROJECT_ROOT,
-                            auto_stash_ref,
-                            prompt_user=False,
-                            input_fn=gw_input_fn,
-                        )
                     print(f"✗ Branch '{branch}' does not exist locally or on origin.")
                     if track_result.stderr.strip():
                         print(f"  {track_result.stderr.strip().splitlines()[0]}")
                     sys.exit(1)
-        else:
-            auto_stash_ref = _stash_local_changes_if_needed(git_cmd, PROJECT_ROOT)
 
-        prompt_for_restore = (
-            auto_stash_ref is not None
-            and not assume_yes
-            and (gateway_mode or (sys.stdin.isatty() and sys.stdout.isatty()))
-        )
-
-        # Check if there are updates
+        # Check if there are updates BEFORE stashing — there's no point
+        # stashing local changes if origin has nothing new to apply.
         result = subprocess.run(
             git_cmd + ["rev-list", f"HEAD..origin/{branch}", "--count"],
             cwd=PROJECT_ROOT,
@@ -9669,14 +9666,51 @@ def _cmd_update_impl(args, gateway_mode: bool):
         )
         commit_count = int(result.stdout.strip())
 
+        # Default: nothing stashed yet. Set by the deferred-stash below.
+        auto_stash_ref: Optional[str] = None
+
         if commit_count == 0:
             _invalidate_update_cache()
 
             # Even if origin is up to date, the fork may be behind upstream
             if is_fork and branch == "main":
                 _sync_with_upstream_if_needed(git_cmd, PROJECT_ROOT)
 
-            # Restore stash and switch back to original branch if we moved
+            # No update to apply, and we never stashed — nothing to restore,
+            # nothing to switch back from (we did the branch switch above
+            # only when it was needed, but here `current_branch == branch`
+            # for the common case).
+
+            # If we DID switch branches earlier (different current_branch),
+            # switch back to the user's original branch.
+            if current_branch not in {branch, "HEAD"}:
+                subprocess.run(
+                    git_cmd + ["checkout", current_branch],
+                    cwd=PROJECT_ROOT,
+                    capture_output=True,
+                    text=True,
+                    check=False,
+                )
+        else:
+            # We have an update — now (and only now) is the right moment to
+            # stash any local changes that would conflict with the fast-forward.
+            auto_stash_ref = _stash_local_changes_if_needed(git_cmd, PROJECT_ROOT)
+
+        prompt_for_restore = (
+            auto_stash_ref is not None
+            and not assume_yes
+            and (gateway_mode or (sys.stdin.isatty() and sys.stdout.isatty()))
+        )
+
+        # commit_count is from the pre-stash check above. Stashing doesn't
+        # move HEAD, so it stays valid. The original code re-ran rev-list
+        # here for safety, but with the deferred-stash fix the pre-check
+        # is sufficient — we know whether there's work to do before any
+        # stash happened.
+
+        if commit_count == 0:
+            # Race: someone else pulled between our check and our stash.
+            # Restore the stash and bail.
             if auto_stash_ref is not None:
                 _restore_stashed_changes(
                     git_cmd,

tests/hermes_cli/test_cmd_update.py

@@ -872,3 +872,147 @@ def test_load_installable_optional_extras_supports_termux_group(tmp_path, monkey
 
     assert hm._load_installable_optional_extras(group="all") == ["mcp"]
     assert hm._load_installable_optional_extras(group="termux-all") == ["termux", "mcp"]
+
+
+# ---------------------------------------------------------------------------
+# Regression tests for issue #3523:
+# `hermes update` regressed to silent git fetch output and autostash on
+# every run, even when already up to date.
+# ---------------------------------------------------------------------------
+
+
+def _make_3523_side_effect(branch="main", commit_count="0", git_status_dirty=False):
+    """Side effect: simulate `git fetch` silent, `rev-list` returns commit_count,
+    `git status --porcelain` returns dirty/clean, `git stash` records calls."""
+    def side_effect(cmd, **kwargs):
+        joined = " ".join(str(c) for c in cmd)
+        # git rev-parse --abbrev-ref HEAD
+        if "rev-parse" in joined and "--abbrev-ref" in joined:
+            return subprocess.CompletedProcess(cmd, 0, stdout=f"{branch}\n", stderr="")
+        # git rev-parse --verify origin/{branch}
+        if "rev-parse" in joined and "--verify" in joined:
+            return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+        # git rev-list HEAD..origin/{branch} --count
+        if "rev-list" in joined:
+            return subprocess.CompletedProcess(cmd, 0, stdout=f"{commit_count}\n", stderr="")
+        # git status --porcelain — used by _stash_local_changes_if_needed
+        if "status" in joined and "--porcelain" in joined:
+            dirty = "M hermes_cli/main.py\n" if git_status_dirty else ""
+            return subprocess.CompletedProcess(cmd, 0, stdout=dirty, stderr="")
+        # git ls-files --unmerged — used by _stash_local_changes_if_needed
+        if "ls-files" in joined:
+            return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+        # git stash push / drop — track if called
+        if "stash" in joined and "push" in joined:
+            return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+        # git stash apply / drop
+        if "stash" in joined:
+            return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+        # git checkout
+        if "checkout" in joined:
+            return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+        # git fetch — track whether stderr was captured
+        if "fetch" in joined:
+            return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+        # Fallback
+        return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+    return side_effect
+
+
+@patch("shutil.which", return_value=None)
+@patch("subprocess.run")
+def test_3523_no_stash_when_up_to_date_with_local_changes(
+    mock_run, _mock_which, mock_args, capsys, tmp_path, monkeypatch
+):
+    """Regression #3523: `hermes update` on an up-to-date tree with local
+    changes must NOT autostash-and-pop. The deferred-stash logic should
+    skip the stash entirely because commit_count==0."""
+    from hermes_cli import main as hm
+
+    monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
+    # Make the .git directory "exist" so we don't bail out as Docker/pip
+    (tmp_path / ".git").mkdir()
+
+    mock_run.side_effect = _make_3523_side_effect(
+        branch="main", commit_count="0", git_status_dirty=True
+    )
+
+    cmd_update(mock_args)
+
+    commands = [c.args[0] for c in mock_run.call_args_list]
+    stash_pushes = [c for c in commands if "stash" in c and "push" in " ".join(str(x) for x in c)]
+    assert stash_pushes == [], (
+        f"Expected no `git stash push` when up to date, but got: {stash_pushes}"
+    )
+
+    captured = capsys.readouterr()
+    assert "Already up to date" in captured.out
+
+
+@patch("shutil.which", return_value=None)
+@patch("subprocess.run")
+def test_3523_fetch_stderr_visible_on_success(
+    mock_run, _mock_which, mock_args, tmp_path, monkeypatch
+):
+    """Regression #3523: `git fetch` stdout/stderr should reach the terminal
+    so users see object enumeration. We verify the fetch subprocess is run
+    with capture_output=False (vs the regression's capture_output=True)."""
+    from hermes_cli import main as hm
+
+    monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
+    (tmp_path / ".git").mkdir()
+
+    mock_run.side_effect = _make_3523_side_effect(branch="main", commit_count="0")
+
+    cmd_update(mock_args)
+
+    commands = list(mock_run.call_args_list)
+    # Find the fetch call. With the regression fix, fetch runs first (capture_output=False).
+    # The diagnostic re-fetch (capture_output=True) only fires on failure.
+    fetch_calls = [c for c in commands if "fetch" in " ".join(str(x) for x in c.args[0])]
+    assert len(fetch_calls) >= 1, "Expected at least one fetch call"
+
+    # The first fetch (the user-facing one) should NOT capture output.
+    first_fetch_kwargs = fetch_calls[0].kwargs
+    assert first_fetch_kwargs.get("capture_output", None) is False or first_fetch_kwargs.get("capture_output") is False, (
+        f"Expected first fetch to have capture_output=False, got: {first_fetch_kwargs}"
+    )
+
+
+@patch("shutil.which", return_value=None)
+@patch("subprocess.run")
+def test_3523_stash_still_happens_when_update_available(
+    mock_run, _mock_which, mock_args, tmp_path, monkeypatch, capsys
+):
+    """Regression #3523: ensure the deferred-stash fix didn't break the
+    normal update-with-local-changes path — we MUST still stash."""
+    from hermes_cli import main as hm
+
+    monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
+    (tmp_path / ".git").mkdir()
+
+    base_side_effect = _make_3523_side_effect(
+        branch="main", commit_count="3", git_status_dirty=True
+    )
+
+    def _side_effect_with_pull_noop(*args, **kwargs):
+        joined = " ".join(str(c) for c in args[0])
+        if "pull" in joined:
+            return subprocess.CompletedProcess(
+                args[0], 0, stdout="Already up to date.", stderr=""
+            )
+        return base_side_effect(*args, **kwargs)
+
+    mock_run.side_effect = _side_effect_with_pull_noop
+
+    cmd_update(mock_args)
+
+    commands = [c.args[0] for c in mock_run.call_args_list]
+    stash_pushes = [
+        c for c in commands
+        if "stash" in c and any("push" in str(x) for x in c)
+    ]
+    assert len(stash_pushes) >= 1, (
+        f"Expected at least one `git stash push` when update is available, got: {commands}"
+    )