Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hermes_cli/subcommands/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def build_update_parser(subparsers, *, cmd_update: Callable) -> None:
"-y",
action="store_true",
default=False,
help="Assume yes for interactive prompts (config migration, stash restore). API-key entry is skipped; run 'hermes config migrate' separately for those.",
help="Run without blocking on prompts: accepts the config-migration and stash-restore prompts, skips the fork-upstream prompt without adding a remote. API-key entry is skipped; run 'hermes config migrate' separately for those.",
)
update_parser.add_argument(
"--keep-stash",
Expand Down
89 changes: 71 additions & 18 deletions hermes_cli/update_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -2711,34 +2711,67 @@ def _sync_fork_with_upstream(git_cmd: list[str], cwd: Path) -> bool:
except Exception:
return False

def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None:
def _sync_with_upstream_if_needed(
git_cmd: list[str],
cwd: Path,
*,
assume_yes: bool = False,
input_fn=None,
) -> bool:
"""Check if fork is behind upstream and sync if safe.

This implements the fork upstream sync logic:
- If upstream remote doesn't exist, ask user if they want to add it
- Compare origin/main with upstream/main
- If origin/main is strictly behind upstream/main, pull from upstream
- Try to sync fork back to origin if possible

Returns True when origin/main was actually verified against the official
upstream/main, False when the check never happened (prompt skipped or
declined, remote add failed, fetch or compare failed) so the caller can
avoid reporting the checkout as up to date on the strength of an origin
comparison alone (#97052 review).
"""
has_upstream = _has_upstream_remote(git_cmd, cwd)

if not has_upstream:
# Check if user previously declined
if _should_skip_upstream_prompt():
return
return False

# Ask user if they want to add upstream
print()
print("ℹ Your fork is not tracking the official Hermes repository.")
print(" This means you may miss updates from NousResearch/hermes-agent.")
print()
try:

if assume_yes or (
input_fn is None and not (sys.stdin.isatty() and sys.stdout.isatty())
):
# --yes means "don't block", not "mutate my git remotes". Skip
# without persisting the decline so interactive runs still get asked.
print(" Skipping upstream setup (non-interactive run).")
print(
" Add it later with: git remote add upstream https://github.com/NousResearch/hermes-agent.git"
)
return False

# Ask user if they want to add upstream
if input_fn is not None:
response = (
input("Add official repo as 'upstream' remote? [Y/n]: ").strip().lower()
input_fn("Add official repo as 'upstream' remote? [y/N]", "n")
.strip()
.lower()
)
except (EOFError, KeyboardInterrupt, UnicodeDecodeError):
print()
response = "n"
else:
try:
response = (
input("Add official repo as 'upstream' remote? [Y/n]: ")
.strip()
.lower()
)
except (EOFError, KeyboardInterrupt, UnicodeDecodeError):
print()
response = "n"

if response in {"", "y", "yes"}:
print("→ Adding upstream remote...")
Expand All @@ -2749,13 +2782,13 @@ def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None:
has_upstream = True
else:
print(" ✗ Failed to add upstream remote. Skipping upstream sync.")
return
return False
else:
print(
" Skipped. Run 'git remote add upstream https://github.com/NousResearch/hermes-agent.git' to add later."
)
_mark_skip_upstream_prompt()
return
return False

# Fetch upstream main only. This sync compares upstream/main with
# origin/main, so there's no reason to pull every upstream ref — and a bare
Expand All @@ -2771,7 +2804,7 @@ def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None:
)
except subprocess.CalledProcessError:
print(" ✗ Failed to fetch upstream. Skipping upstream sync.")
return
return False

# Compare origin/main with upstream/main
origin_ahead = _count_commits_between(git_cmd, cwd, "upstream/main", "origin/main")
Expand All @@ -2781,7 +2814,7 @@ def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None:

if origin_ahead < 0 or upstream_ahead < 0:
print(" ✗ Could not compare branches. Skipping upstream sync.")
return
return False

# If origin/main has commits not on upstream, don't trample
if origin_ahead > 0:
Expand All @@ -2790,12 +2823,12 @@ def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None:
print(" Skipping upstream sync to preserve your changes.")
print(" If you want to merge upstream changes, run:")
print(" git pull upstream main")
return
return True

# If upstream is not ahead, fork is up to date
if upstream_ahead == 0:
print(" ✓ Fork is up to date with upstream")
return
return True

# origin/main is strictly behind upstream/main (can fast-forward)
print()
Expand All @@ -2812,7 +2845,7 @@ def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None:
print(
" ✗ Failed to pull from upstream. You may need to resolve conflicts manually."
)
return
return False

print(" ✓ Updated from upstream")

Expand All @@ -2825,6 +2858,7 @@ def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None:
" ℹ Got updates from upstream but couldn't push to fork (no write access?)"
)
print(" Your local repo is updated, but your fork on GitHub may be behind.")
return True

def _invalidate_update_cache():
"""Delete the update-check cache for ALL profiles so no banner
Expand Down Expand Up @@ -3710,6 +3744,7 @@ def _repair_node_deps_on_current_checkout(
assume_yes: bool = False,
gateway_mode: bool = False,
pre_update_snapshot_id: str | None = None,
completion_message: str = "✓ Already up to date!",
) -> None:
"""Repair Node deps on the ``commit_count == 0`` path (#77211).

Expand Down Expand Up @@ -3740,7 +3775,7 @@ def _repair_node_deps_on_current_checkout(
gateway_mode=gateway_mode,
pre_update_snapshot_id=pre_update_snapshot_id,
)
print_completion("✓ Already up to date!")
print_completion(completion_message)


def _update_node_dependencies() -> list[str]:
Expand Down Expand Up @@ -7833,9 +7868,17 @@ def _cmd_update_impl(args, gateway_mode: bool):
# commit_count == 0 branch, which returns immediately after: an update
# that pulled hundreds of upstream commits printed "Already up to
# date!" and verified nothing).
# Non-fork checkouts have no upstream question: origin IS the official
# repo, so "Already up to date!" is fully verified there.
upstream_checked = True
if commit_count == 0 and is_fork and branch == "main":
pre_sync_sha = _capture_head_sha(git_cmd, _m().PROJECT_ROOT)
_m()._sync_with_upstream_if_needed(git_cmd, _m().PROJECT_ROOT)
upstream_checked = _m()._sync_with_upstream_if_needed(
git_cmd,
_m().PROJECT_ROOT,
assume_yes=assume_yes,
input_fn=gw_input_fn,
)
post_sync_sha = _capture_head_sha(git_cmd, _m().PROJECT_ROOT)
if pre_sync_sha and post_sync_sha and pre_sync_sha != post_sync_sha:
synced_count = _count_commits_between(
Expand Down Expand Up @@ -7994,6 +8037,11 @@ def _cmd_update_impl(args, gateway_mode: bool):
assume_yes=assume_yes,
gateway_mode=gateway_mode,
pre_update_snapshot_id=pre_update_snapshot_id,
completion_message=(
"✓ Already up to date!"
if upstream_checked
else "✓ Up to date with your fork (official repo not checked)."
),
)
if runtime_repaired is not None and not _m()._is_windows():
print()
Expand Down Expand Up @@ -8271,7 +8319,12 @@ def _cmd_update_impl(args, gateway_mode: bool):

# Fork upstream sync logic (only for main branch on forks)
if is_fork and branch == "main":
_m()._sync_with_upstream_if_needed(git_cmd, _m().PROJECT_ROOT)
_m()._sync_with_upstream_if_needed(
git_cmd,
_m().PROJECT_ROOT,
assume_yes=assume_yes,
input_fn=gw_input_fn,
)

# Reinstall Python dependencies. Prefer .[all], but if one optional extra
# breaks on this machine, keep base deps and reinstall the remaining extras
Expand Down
46 changes: 45 additions & 1 deletion tests/hermes_cli/test_cmd_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,54 @@ def test_update_on_fork_checks_upstream_when_origin_up_to_date(
expected_git_cmd = (
["git", "-c", "windows.appendAtomically=false"] if hm._is_windows() else ["git"]
)
sync_mock.assert_called_once_with(expected_git_cmd, PROJECT_ROOT)
sync_mock.assert_called_once_with(
expected_git_cmd,
PROJECT_ROOT,
assume_yes=False,
input_fn=None,
)
captured = capsys.readouterr()
assert "Already up to date!" in captured.out

@patch("shutil.which", return_value=None)
@patch("subprocess.run")
def test_yes_on_fork_without_upstream_does_not_claim_up_to_date(
self, mock_run, _mock_which, capsys
):
"""#97052 review: genuine fork, no upstream remote, HEAD == origin/main,
--yes. The prompt is skipped without mutating remotes, and because the
official repo was never consulted the completion line must not claim
plain "Already up to date!"."""
from hermes_cli import main as hm
from hermes_cli import update_cmd

mock_run.side_effect = _make_run_side_effect(
branch="main", verify_ok=True, commit_count="0"
)

with patch.object(
hm,
"_get_origin_url",
return_value="https://github.com/example/hermes-agent.git",
), patch.object(
update_cmd, "_has_upstream_remote", return_value=False
), patch.object(
update_cmd, "_should_skip_upstream_prompt", return_value=False
), patch.object(
update_cmd, "_add_upstream_remote"
) as add_remote, patch.object(
update_cmd, "_mark_skip_upstream_prompt"
) as mark_skip, patch("builtins.input") as stdin_input:
cmd_update(SimpleNamespace(yes=True))

stdin_input.assert_not_called()
add_remote.assert_not_called()
mark_skip.assert_not_called()
captured = capsys.readouterr()
assert "Skipping upstream setup (non-interactive run)." in captured.out
assert "official repo not checked" in captured.out
assert "Already up to date!" not in captured.out

@patch("shutil.which", return_value=None)
@patch("subprocess.run")
def test_fork_upstream_sync_that_moves_head_runs_post_update_steps(
Expand Down
124 changes: 124 additions & 0 deletions tests/hermes_cli/test_update_upstream_prompt_noninteractive.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
"""The fork-upstream prompt must never block a non-interactive update (#60240).

`_sync_with_upstream_if_needed` asks "Add official repo as 'upstream' remote?"
on fork checkouts with no upstream remote. In unattended contexts (CI, cron,
the desktop updater hand-off) stdin is open but nobody answers, so a bare
``input()`` blocks forever. These tests pin the gate: under ``assume_yes`` or
a non-TTY stdio pair the prompt is skipped as a decline WITHOUT persisting the
skip marker or touching git remotes, and both update call sites forward the
interaction state.
"""

from types import SimpleNamespace
from unittest.mock import patch

import pytest

from hermes_cli import update_cmd


@pytest.fixture
def fork_without_upstream(tmp_path):
with patch.object(
update_cmd, "_has_upstream_remote", return_value=False
), patch.object(
update_cmd, "_should_skip_upstream_prompt", return_value=False
), patch.object(
update_cmd, "_add_upstream_remote", return_value=True
) as add_remote, patch.object(
update_cmd, "_mark_skip_upstream_prompt"
) as mark_skip, patch("builtins.input") as stdin_input:
yield SimpleNamespace(
cwd=tmp_path,
add_remote=add_remote,
mark_skip=mark_skip,
stdin_input=stdin_input,
)


def _tty(stdin: bool, stdout: bool):
return (
patch.object(update_cmd.sys.stdin, "isatty", return_value=stdin),
patch.object(update_cmd.sys.stdout, "isatty", return_value=stdout),
)


class TestUpstreamPromptNonInteractive:
def test_assume_yes_skips_prompt_without_touching_remotes(
self, fork_without_upstream, capsys
):
p_in, p_out = _tty(True, True)
with p_in, p_out:
checked = update_cmd._sync_with_upstream_if_needed(
["git"], fork_without_upstream.cwd, assume_yes=True
)

assert checked is False
fork_without_upstream.stdin_input.assert_not_called()
fork_without_upstream.add_remote.assert_not_called()
fork_without_upstream.mark_skip.assert_not_called()
assert "Skipping upstream setup" in capsys.readouterr().out

@pytest.mark.parametrize(
"stdin_tty,stdout_tty", [(False, False), (False, True), (True, False)]
)
def test_non_tty_skips_prompt_without_persisting_decline(
self, fork_without_upstream, capsys, stdin_tty, stdout_tty
):
p_in, p_out = _tty(stdin_tty, stdout_tty)
with p_in, p_out:
checked = update_cmd._sync_with_upstream_if_needed(
["git"], fork_without_upstream.cwd
)

assert checked is False
fork_without_upstream.stdin_input.assert_not_called()
fork_without_upstream.add_remote.assert_not_called()
fork_without_upstream.mark_skip.assert_not_called()
assert "Skipping upstream setup" in capsys.readouterr().out

def test_gateway_prompt_routes_through_input_fn(self, fork_without_upstream):
prompts = []

def gw_input(prompt, default=""):
prompts.append((prompt, default))
return "n"

p_in, p_out = _tty(False, False)
with p_in, p_out:
update_cmd._sync_with_upstream_if_needed(
["git"], fork_without_upstream.cwd, input_fn=gw_input
)

assert prompts == [("Add official repo as 'upstream' remote? [y/N]", "n")]
fork_without_upstream.stdin_input.assert_not_called()
fork_without_upstream.add_remote.assert_not_called()
fork_without_upstream.mark_skip.assert_called_once_with()

def test_interactive_decline_still_persists_marker(self, fork_without_upstream):
fork_without_upstream.stdin_input.return_value = "n"
p_in, p_out = _tty(True, True)
with p_in, p_out:
update_cmd._sync_with_upstream_if_needed(
["git"], fork_without_upstream.cwd
)

fork_without_upstream.stdin_input.assert_called_once()
fork_without_upstream.add_remote.assert_not_called()
fork_without_upstream.mark_skip.assert_called_once_with()

def test_interactive_accept_adds_upstream(self, fork_without_upstream):
fork_without_upstream.stdin_input.return_value = "y"
with patch.object(
update_cmd, "_count_commits_between", return_value=-1
), patch.object(update_cmd.subprocess, "run"):
p_in, p_out = _tty(True, True)
with p_in, p_out:
update_cmd._sync_with_upstream_if_needed(
["git"], fork_without_upstream.cwd
)

fork_without_upstream.add_remote.assert_called_once_with(
["git"], fork_without_upstream.cwd
)
fork_without_upstream.mark_skip.assert_not_called()
Loading