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 <BlackishGreen33@users.noreply.github.com> Co-authored-by: salch-cred <salch-cred@users.noreply.github.com> Co-authored-by: jackulau <jackulau@users.noreply.github.com>
2026-08-28 10:33:55 +02:00
parent baa344dee7
commit b33fa127d2
3 changed files with 175 additions and 11 deletions
@@ -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
@@ -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

@@ -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()