From b33fa127d2c4cc7b71fa0aea312979d7b7209c49 Mon Sep 17 00:00:00 2001 From: yoniebans Date: Fri, 28 Aug 2026 10:33:55 +0200 Subject: [PATCH] fix(update): gate the fork-upstream prompt for --yes and non-tty runs _sync_with_upstream_if_needed called bare input() with no assume_yes parameter and no tty check, so a fork checkout without an upstream remote wedged hermes update forever in any non-interactive context (CI, cron, the desktop updater hand-off): stdin stays open, EOFError never fires. Thread assume_yes and the gateway input_fn into the helper and skip the prompt as a decline under assume_yes or a non-tty stdio pair, without writing the decline marker or touching git remotes, so interactive runs still get asked later. Both call sites forward the interaction state; the config-migration and stash-restore prompts already carry this gate. Closes #60240 (prompt half). Supersedes #78678, #92448, #92410. Co-authored-by: BlackishGreen33 Co-authored-by: salch-cred Co-authored-by: jackulau --- hermes_cli/update_cmd.py | 57 ++++++-- tests/hermes_cli/test_cmd_update.py | 7 +- ...t_update_upstream_prompt_noninteractive.py | 122 ++++++++++++++++++ 3 files changed, 175 insertions(+), 11 deletions(-) create mode 100644 tests/hermes_cli/test_update_upstream_prompt_noninteractive.py diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 46d4ea449a..75b923a6b1 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -2711,7 +2711,13 @@ 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, +) -> None: """Check if fork is behind upstream and sync if safe. This implements the fork upstream sync logic: @@ -2727,18 +2733,39 @@ def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None: if _should_skip_upstream_prompt(): return - # 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: - response = ( - input("Add official repo as 'upstream' remote? [Y/n]: ").strip().lower() + + 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" ) - except (EOFError, KeyboardInterrupt, UnicodeDecodeError): - print() - response = "n" + return + + # Ask user if they want to add upstream + if input_fn is not None: + response = ( + input_fn("Add official repo as 'upstream' remote? [y/N]", "n") + .strip() + .lower() + ) + 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...") @@ -7835,7 +7862,12 @@ def _cmd_update_impl(args, gateway_mode: bool): # date!" and verified nothing). 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) + _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( @@ -8271,7 +8303,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 diff --git a/tests/hermes_cli/test_cmd_update.py b/tests/hermes_cli/test_cmd_update.py index 5dac178dfd..a34dfd3ea9 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -301,7 +301,12 @@ class TestCmdUpdateBranchFallback: 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 diff --git a/tests/hermes_cli/test_update_upstream_prompt_noninteractive.py b/tests/hermes_cli/test_update_upstream_prompt_noninteractive.py new file mode 100644 index 0000000000..c97257d916 --- /dev/null +++ b/tests/hermes_cli/test_update_upstream_prompt_noninteractive.py @@ -0,0 +1,122 @@ +"""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: + update_cmd._sync_with_upstream_if_needed( + ["git"], fork_without_upstream.cwd, assume_yes=True + ) + + 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: + update_cmd._sync_with_upstream_if_needed( + ["git"], fork_without_upstream.cwd + ) + + 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()