diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 82df98f395d..68255b12696 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -6988,7 +6988,25 @@ def _update_via_zip(args): import zipfile from urllib.request import urlretrieve - branch = "main" + # The ZIP fallback exists for Windows git-file-I/O breakage. It pulls a + # static archive from GitHub, which is fine for the default "main" + # channel but would silently ignore --branch and update from main even + # if the user asked for something else — exactly the silent-divergence + # bug --branch was added to prevent. Refuse to proceed in that case + # rather than lie. + branch = _resolve_update_branch(args) + if branch != "main": + print( + f"✗ --branch={branch} is not supported on the Windows ZIP-fallback " + "update path." + ) + print( + " This path runs when git file I/O is broken on the system. " + "Either resolve the git-side breakage (typically an antivirus " + "or NTFS filter holding files open) and rerun `hermes update " + f"--branch {branch}`, or update against main with `hermes update`." + ) + sys.exit(1) zip_url = ( f"https://github.com/NousResearch/hermes-agent/archive/refs/heads/{branch}.zip" ) @@ -8395,13 +8413,36 @@ def _finalize_update_output(state): pass -def _cmd_update_check(): - """Implement ``hermes update --check``: fetch and report without installing.""" +def _resolve_update_branch(args) -> str: + """Normalize ``args.branch`` into a non-empty branch name. + + Centralizes the "default to main, accept --branch override, treat empty + or whitespace-only values as the default" parsing so every consumer of + ``--branch`` (check path, git-update path, ZIP-fallback path) agrees on + the same answer. + """ + return (getattr(args, "branch", None) or "main").strip() or "main" + + +def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): + """Implement ``hermes update --check``: fetch and report without installing. + + ``branch`` selects which branch the check compares against. Default is + "main"; callers can pass another branch to ask "are there new commits + on origin/?" without performing the update. + + ``branch_explicit`` is True iff the caller passed --branch on the CLI. + PyPI installs can't honor non-default branches, so when this is True + on a PyPI install we surface a one-line notice instead of silently + dropping the flag. + """ from hermes_cli.config import detect_install_method method = detect_install_method(PROJECT_ROOT) if method == "pip": from hermes_cli.config import recommended_update_command from hermes_cli.banner import check_via_pypi + if branch_explicit and branch != "main": + print(f"⚠ --branch is ignored for PyPI installs (would have checked '{branch}').") result = check_via_pypi() if result is None: print("✗ Could not reach PyPI to check for updates.") @@ -8422,16 +8463,34 @@ def _cmd_update_check(): if sys.platform == "win32": git_cmd = ["git", "-c", "windows.appendAtomically=false"] - # Fetch both origin and upstream; prefer upstream as the canonical reference - print("→ Fetching from upstream...") - fetch_result = subprocess.run( - git_cmd + ["fetch", "upstream"], - cwd=PROJECT_ROOT, - capture_output=True, - text=True, - ) - if fetch_result.returncode != 0: - # Fallback to origin if upstream doesn't exist + # Fetch both origin and upstream; prefer upstream as the canonical reference. + # Note: upstream/ 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. + if branch == "main": + print("→ Fetching from upstream...") + fetch_result = subprocess.run( + git_cmd + ["fetch", "upstream"], + cwd=PROJECT_ROOT, + capture_output=True, + text=True, + ) + if fetch_result.returncode != 0: + # Fallback to origin if upstream doesn't exist + print("→ Fetching from origin...") + fetch_result = subprocess.run( + git_cmd + ["fetch", "origin"], + cwd=PROJECT_ROOT, + capture_output=True, + text=True, + ) + upstream_exists = False + compare_branch = f"origin/{branch}" + else: + upstream_exists = True + compare_branch = f"upstream/{branch}" + else: + # Non-default branch: compare against origin/ directly. print("→ Fetching from origin...") fetch_result = subprocess.run( git_cmd + ["fetch", "origin"], @@ -8440,10 +8499,7 @@ def _cmd_update_check(): text=True, ) upstream_exists = False - compare_branch = "origin/main" - else: - upstream_exists = True - compare_branch = "upstream/main" + compare_branch = f"origin/{branch}" if fetch_result.returncode != 0: stderr = fetch_result.stderr.strip() @@ -8457,6 +8513,20 @@ def _cmd_update_check(): print(f" {stderr.splitlines()[0]}") sys.exit(1) + # Verify the compare ref actually exists before asking rev-list about it. + # Without this, `git rev-list HEAD..origin/ --count` exits 128 and + # (with check=True) raises CalledProcessError, surfacing a Python + # traceback. Friendlier to detect-and-report. + verify_result = subprocess.run( + git_cmd + ["rev-parse", "--verify", "--quiet", compare_branch], + cwd=PROJECT_ROOT, + capture_output=True, + text=True, + ) + if verify_result.returncode != 0: + print(f"✗ Branch '{branch}' not found on {compare_branch.split('/', 1)[0]}.") + sys.exit(1) + rev_result = subprocess.run( git_cmd + ["rev-list", f"HEAD..{compare_branch}", "--count"], cwd=PROJECT_ROOT, @@ -8675,7 +8745,13 @@ def cmd_update(args): return if getattr(args, "check", False): - _cmd_update_check() + # --check honors --branch so the "any new commits?" answer matches + # what a subsequent `hermes update --branch=` would actually pull. + branch = _resolve_update_branch(args) + _cmd_update_check( + branch=branch, + branch_explicit=bool(getattr(args, "branch", None)), + ) return gateway_mode = getattr(args, "gateway", False) @@ -8835,26 +8911,57 @@ def _cmd_update_impl(args, gateway_mode: bool): ) current_branch = result.stdout.strip() - # Always update against main - branch = "main" + # Determine the target branch. Default is "main" (the long-standing + # CLI behavior); --branch overrides for callers that want to update + # against a non-default channel. + branch = _resolve_update_branch(args) - # If user is on a non-main branch or detached HEAD, switch to main - if current_branch != "main": + # If user is on a different branch than the update target, switch + # to the target. When the target is "main" this is the historical + # "always update against main" behavior; for any other target it's + # the same thing — get HEAD onto the requested branch first, then + # fast-forward. + if current_branch != branch: label = ( "detached HEAD" if current_branch == "HEAD" else f"branch '{current_branch}'" ) - print(f" ⚠ Currently on {label} — switching to main for update...") + 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) - subprocess.run( - git_cmd + ["checkout", "main"], + checkout_result = subprocess.run( + git_cmd + ["checkout", branch], cwd=PROJECT_ROOT, capture_output=True, text=True, - check=True, ) + if checkout_result.returncode != 0: + # Local checkout doesn't have this branch yet. Try to set + # it up as a tracking branch of origin/. This is + # the common case when the requested branch exists upstream + # but was never checked out locally. + track_result = subprocess.run( + git_cmd + ["checkout", "-B", branch, f"origin/{branch}"], + cwd=PROJECT_ROOT, + capture_output=True, + 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) @@ -8885,7 +8992,7 @@ def _cmd_update_impl(args, gateway_mode: bool): prompt_user=prompt_for_restore, input_fn=gw_input_fn, ) - if current_branch not in {"main", "HEAD"}: + if current_branch not in {branch, "HEAD"}: subprocess.run( git_cmd + ["checkout", current_branch], cwd=PROJECT_ROOT, @@ -8947,7 +9054,7 @@ def _cmd_update_impl(args, gateway_mode: bool): if reset_result.stderr.strip(): print(f" {reset_result.stderr.strip()}") print( - " Try manually: git fetch origin && git reset --hard origin/main" + f" Try manually: git fetch origin && git reset --hard origin/{branch}" ) sys.exit(1) @@ -13449,6 +13556,17 @@ Examples: default=False, help="Assume yes for interactive prompts (config migration, stash restore). API-key entry is skipped; run 'hermes config migrate' separately for those.", ) + update_parser.add_argument( + "--branch", + default=None, + metavar="NAME", + help=( + "Update against this branch instead of the default (main). " + "If the local checkout is on a different branch, hermes will " + "switch to the requested branch first (auto-stashing any " + "uncommitted changes)." + ), + ) update_parser.add_argument( "--force", action="store_true", diff --git a/tests/hermes_cli/test_cmd_update.py b/tests/hermes_cli/test_cmd_update.py index b9087c06663..e4cf849f261 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -276,6 +276,315 @@ class TestCmdUpdateProfileSkillSync: assert default_p.path in synced_paths +class TestCmdUpdateBranchFlag: + """``hermes update --branch `` targets the requested branch. + + The CLI default stays 'main'; --branch lets callers pick a different + target without monkey-patching the implementation. + """ + + def _branch_side_effect(self, current_branch, target_branch, *, checkout_fails=False, track_fails=False, commit_count="0"): + """Mock side-effect that knows about checkout/track behavior. + + - ``current_branch`` what ``git rev-parse --abbrev-ref HEAD`` returns + - ``target_branch`` passed via --branch; what we expect the code to switch to + - ``checkout_fails`` if True, ``git checkout `` returns non-zero + (simulates branch absent locally; code should retry with -B) + - ``track_fails`` if True, ``git checkout -B origin/`` ALSO fails + (simulates branch absent on origin too) + - ``commit_count`` rev-list count returned (0 = up-to-date, >0 = behind) + """ + + def side_effect(cmd, **kwargs): + joined = " ".join(str(c) for c in cmd) + + if "rev-parse" in joined and "--abbrev-ref" in joined: + return subprocess.CompletedProcess(cmd, 0, stdout=f"{current_branch}\n", stderr="") + + if "checkout" in joined and "-B" in joined: + rc = 128 if track_fails else 0 + err = f"fatal: '{target_branch}' did not match any file(s) known to git\n" if track_fails else "" + return subprocess.CompletedProcess(cmd, rc, stdout="", stderr=err) + + if "checkout" in joined and "-B" not in joined and "rev-parse" not in joined: + rc = 128 if checkout_fails else 0 + err = f"error: pathspec '{target_branch}' did not match\n" if checkout_fails else "" + return subprocess.CompletedProcess(cmd, rc, stdout="", stderr=err) + + if "rev-list" in joined: + return subprocess.CompletedProcess(cmd, 0, stdout=f"{commit_count}\n", stderr="") + + return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") + + return side_effect + + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_branch_flag_pulls_against_named_branch(self, mock_run, _mock_which, capsys): + """--branch bb/gui makes rev-list and pull target origin/bb/gui.""" + mock_run.side_effect = self._branch_side_effect( + current_branch="bb/gui", target_branch="bb/gui", commit_count="3" + ) + args = SimpleNamespace(branch="bb/gui") + + cmd_update(args) + + commands = [" ".join(str(a) for a in c.args[0]) for c in mock_run.call_args_list] + + # rev-list must compare against origin/bb/gui, not origin/main + rev_list_cmds = [c for c in commands if "rev-list" in c] + assert any("origin/bb/gui" in c for c in rev_list_cmds), rev_list_cmds + assert not any("origin/main" in c for c in rev_list_cmds), rev_list_cmds + + # pull must target bb/gui + pull_cmds = [c for c in commands if "pull" in c and "ff-only" in c] + assert any("bb/gui" in c and "main" not in c.split() for c in pull_cmds), pull_cmds + + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_branch_flag_defaults_to_main_when_none(self, mock_run, _mock_which, capsys): + """No --branch (or --branch=None) preserves the historical 'main' default.""" + mock_run.side_effect = self._branch_side_effect( + current_branch="main", target_branch="main", commit_count="0" + ) + args = SimpleNamespace(branch=None) + + cmd_update(args) + + commands = [" ".join(str(a) for a in c.args[0]) for c in mock_run.call_args_list] + rev_list_cmds = [c for c in commands if "rev-list" in c] + assert all("origin/main" in c for c in rev_list_cmds), rev_list_cmds + + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_branch_flag_switches_from_different_branch(self, mock_run, _mock_which, capsys): + """When HEAD is on main and --branch=bb/gui, switch to bb/gui first.""" + mock_run.side_effect = self._branch_side_effect( + current_branch="main", target_branch="bb/gui", commit_count="2" + ) + args = SimpleNamespace(branch="bb/gui") + + cmd_update(args) + + commands = [" ".join(str(a) for a in c.args[0]) for c in mock_run.call_args_list] + # First checkout call should switch us to bb/gui (not -B; happy-path branch exists locally) + checkout_cmds = [c for c in commands if "checkout" in c and "rev-parse" not in c] + assert len(checkout_cmds) >= 1 + assert "bb/gui" in checkout_cmds[0] + + out = capsys.readouterr().out + assert "switching to bb/gui" in out + + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_branch_flag_tracks_remote_when_branch_absent_locally(self, mock_run, _mock_which, capsys): + """If local lacks the branch but origin has it, fall back to ``checkout -B``.""" + mock_run.side_effect = self._branch_side_effect( + current_branch="main", + target_branch="bb/gui", + checkout_fails=True, # plain checkout fails + track_fails=False, # -B from origin/bb/gui succeeds + commit_count="2", + ) + args = SimpleNamespace(branch="bb/gui") + + cmd_update(args) + + commands = [" ".join(str(a) for a in c.args[0]) for c in mock_run.call_args_list] + # Should have BOTH a failed `checkout bb/gui` AND a successful `checkout -B bb/gui origin/bb/gui` + track_cmds = [c for c in commands if "checkout" in c and "-B" in c] + assert len(track_cmds) == 1 + assert "bb/gui" in track_cmds[0] + assert "origin/bb/gui" in track_cmds[0] + + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_branch_flag_fails_when_branch_missing_everywhere(self, mock_run, _mock_which, capsys): + """If branch doesn't exist locally OR on origin, exit non-zero with clear error.""" + mock_run.side_effect = self._branch_side_effect( + current_branch="main", + target_branch="nonexistent", + checkout_fails=True, + track_fails=True, + commit_count="0", + ) + args = SimpleNamespace(branch="nonexistent") + + with pytest.raises(SystemExit) as exc_info: + cmd_update(args) + assert exc_info.value.code == 1 + + out = capsys.readouterr().out + assert "does not exist locally or on origin" in out + assert "nonexistent" in out + + +class TestCmdUpdateCheckBranchFlag: + """``hermes update --check --branch `` honors the branch override. + + The check path used to call ``git rev-list HEAD..origin/ --count`` + with ``check=True``. When the branch didn't exist on origin, the fetch + silently succeeded (no refspec) but rev-list exited 128 and a raw + ``CalledProcessError`` propagated to the user. These tests pin the + friendlier behavior: detect-the-missing-ref before rev-list, exit 1 + with a clear message. + """ + + def _check_side_effect( + self, + target_branch: str, + *, + verify_ok: bool = True, + commit_count: str = "0", + upstream_fetch_ok: bool = True, + ): + """Mock side-effect for the _cmd_update_check git pipeline. + + - ``target_branch`` what we expect compare ref to point at + - ``verify_ok`` if False, ``git rev-parse --verify --quiet + origin/`` fails (branch missing + on origin) + - ``commit_count`` rev-list count (0 = up-to-date) + - ``upstream_fetch_ok`` if False, ``git fetch upstream`` fails + (forces fallback to origin on branch==main) + """ + + def side_effect(cmd, **kwargs): + joined = " ".join(str(c) for c in cmd) + + if "fetch" in joined and "upstream" in joined: + rc = 0 if upstream_fetch_ok else 128 + err = "" if upstream_fetch_ok else "fatal: 'upstream' does not appear to be a git repository\n" + return subprocess.CompletedProcess(cmd, rc, stdout="", stderr=err) + + if "fetch" in joined and "origin" in joined: + return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") + + if "rev-parse" in joined and "--verify" in joined: + rc = 0 if verify_ok else 1 + return subprocess.CompletedProcess(cmd, rc, stdout="", stderr="") + + if "rev-list" in joined: + return subprocess.CompletedProcess(cmd, 0, stdout=f"{commit_count}\n", stderr="") + + return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") + + return side_effect + + @patch("hermes_cli.config.detect_install_method", return_value="git") + @patch("subprocess.run") + def test_check_branch_compares_against_named_origin_branch( + self, mock_run, _mock_method, capsys + ): + """--check --branch bb/gui compares against origin/bb/gui, never origin/main.""" + mock_run.side_effect = self._check_side_effect( + target_branch="bb/gui", verify_ok=True, commit_count="2" + ) + args = SimpleNamespace(check=True, branch="bb/gui") + + cmd_update(args) + + commands = [" ".join(str(a) for a in c.args[0]) for c in mock_run.call_args_list] + # Non-main branch skips upstream probe entirely. + assert not any("fetch" in c and "upstream" in c for c in commands), commands + # Verify and rev-list both target origin/bb/gui. + verify_cmds = [c for c in commands if "rev-parse" in c and "--verify" in c] + assert any("origin/bb/gui" in c for c in verify_cmds), verify_cmds + rev_list_cmds = [c for c in commands if "rev-list" in c] + assert any("origin/bb/gui" in c for c in rev_list_cmds), rev_list_cmds + assert not any("origin/main" in c for c in rev_list_cmds), rev_list_cmds + + @patch("hermes_cli.config.detect_install_method", return_value="git") + @patch("subprocess.run") + def test_check_branch_missing_on_origin_exits_cleanly( + self, mock_run, _mock_method, capsys + ): + """If origin/ doesn't exist, surface a friendly error and exit 1. + + Pre-fix this case raised CalledProcessError from rev-list's check=True + and dumped a Python traceback to stdout. + """ + mock_run.side_effect = self._check_side_effect( + target_branch="ghost", verify_ok=False + ) + args = SimpleNamespace(check=True, branch="ghost") + + with pytest.raises(SystemExit) as exc_info: + cmd_update(args) + assert exc_info.value.code == 1 + + out = capsys.readouterr().out + # No raw Python traceback. + assert "Traceback" not in out + assert "CalledProcessError" not in out + # Friendly message naming the branch. + assert "ghost" in out + assert "not found" in out + + # rev-list must never have been called once verify failed. + commands = [" ".join(str(a) for a in c.args[0]) for c in mock_run.call_args_list] + assert not any("rev-list" in c for c in commands), commands + + @patch("hermes_cli.config.detect_install_method", return_value="git") + @patch("subprocess.run") + def test_check_default_main_still_prefers_upstream( + self, mock_run, _mock_method, capsys + ): + """No --branch (or --branch=None) preserves the upstream-then-origin probe.""" + mock_run.side_effect = self._check_side_effect( + target_branch="main", verify_ok=True, commit_count="0" + ) + args = SimpleNamespace(check=True, branch=None) + + cmd_update(args) + + commands = [" ".join(str(a) for a in c.args[0]) for c in mock_run.call_args_list] + # Should have tried upstream first. + assert any("fetch" in c and "upstream" in c for c in commands), commands + # Compare ref is upstream/main (upstream fetch succeeded). + rev_list_cmds = [c for c in commands if "rev-list" in c] + assert any("upstream/main" in c for c in rev_list_cmds), rev_list_cmds + + @patch("hermes_cli.config.detect_install_method", return_value="pip") + @patch("hermes_cli.banner.check_via_pypi", return_value=0) + @patch("subprocess.run") + def test_check_branch_warns_on_pypi_install( + self, mock_run, _mock_pypi, _mock_method, capsys + ): + """PyPI install + --branch= surfaces a warning instead of silent drop.""" + args = SimpleNamespace(check=True, branch="bb/gui") + + cmd_update(args) + + out = capsys.readouterr().out + assert "--branch is ignored for PyPI installs" in out + assert "bb/gui" in out + + +class TestCmdUpdateZipBranchRefusal: + """``hermes update --branch=`` must refuse on the ZIP fallback path. + + The ZIP fallback hard-codes a GitHub archive URL for main.zip; honoring + --branch arbitrarily would require remote-branch existence checks the + fallback can't easily do. Refusing is the right move — silently lying + about which branch got installed is the bug --branch was meant to prevent. + """ + + def test_zip_fallback_refuses_non_main_branch(self, capsys): + from hermes_cli.main import _update_via_zip + + args = SimpleNamespace(branch="bb/gui") + with pytest.raises(SystemExit) as exc_info: + _update_via_zip(args) + assert exc_info.value.code == 1 + + out = capsys.readouterr().out + assert "bb/gui" in out + assert "not supported" in out + # No actual download attempted. + assert "Downloading latest version" not in out + + def test_is_termux_env_true_for_termux_prefix(): from hermes_cli import main as hm