diff --git a/hermes_cli/plugins_cmd.py b/hermes_cli/plugins_cmd.py index aeed0d8d14..2f321779cb 100644 --- a/hermes_cli/plugins_cmd.py +++ b/hermes_cli/plugins_cmd.py @@ -2724,29 +2724,121 @@ def _clear_plugin_bytecode(target: Path) -> int: return removed +def _run_plugin_git( + git_exe: str, target: Path, *args: str, timeout: int = 60 +) -> subprocess.CompletedProcess: + """Run one git command inside a plugin checkout (non-interactive).""" + return subprocess.run( + [git_exe, *args], + capture_output=True, + text=True, encoding='utf-8', errors='replace', + timeout=timeout, + cwd=str(target), + stdin=subprocess.DEVNULL, + env=noninteractive_git_env(), + ) + + +def _stash_ref(git_exe: str, target: Path) -> str: + """Current ``refs/stash`` commit, or empty string when no stash exists.""" + probe = _run_plugin_git(git_exe, target, "rev-parse", "--verify", "refs/stash") + return probe.stdout.strip() if probe.returncode == 0 else "" + + def _git_pull_plugin_dir(target: Path) -> tuple[bool, str]: + """``git pull --ff-only`` a plugin checkout, autostashing local edits. + + Users tweak installed plugins in place (config constants, small patches), + and a plain ``pull --ff-only`` then aborts with "Your local changes ... + would be overwritten by merge" — making the plugin permanently + un-updatable until they hand-run git. Same UX class Factory Droid fixed + in v0.188 ("Updating a plugin marketplace now succeeds when its checkout + has local changes"), and the same autostash approach ``hermes update`` + already uses for the main checkout (PR #70161). + + Flow: clean tree → plain pull (unchanged). Dirty tree → stash push + (ref-compared, so "nothing saved" is distinguished from "saved but exit + 1"), pull, stash apply. A clean re-apply drops the entry; a conflicted + re-apply resets the tree to the updated revision and KEEPS the stash so + the plugin still imports and no local work is lost. + """ git_exe = _resolve_git_executable() if not git_exe: return False, "git is not installed or not in PATH." try: - result = subprocess.run( - [git_exe, "pull", "--ff-only"], - capture_output=True, - text=True, encoding='utf-8', errors='replace', - timeout=60, - cwd=str(target), - stdin=subprocess.DEVNULL, - env=noninteractive_git_env(), + status = _run_plugin_git(git_exe, target, "status", "--porcelain") + dirty = status.returncode == 0 and bool(status.stdout.strip()) + + stash_created = False + pre_stash = "" + if dirty: + pre_stash = _stash_ref(git_exe, target) + push = _run_plugin_git( + git_exe, target, + "stash", "push", "--include-untracked", + "-m", "hermes-plugin-update-autostash", + ) + post_stash = _stash_ref(git_exe, target) + stash_created = bool(post_stash) and post_stash != pre_stash + if not stash_created: + # Nothing was saved — do not risk the pull clobbering edits. + err = _safe_git_error(push) + return False, ( + "Local changes in the plugin checkout could not be " + "stashed; update aborted before touching the checkout." + + (f"\n{err}" if err else "") + ) + if push.returncode != 0: + # Saved-but-couldn't-clean (undeletable untracked files): + # the stash entry is complete; reset tracked mods so the + # pull isn't blocked by a still-dirty tree. + _run_plugin_git(git_exe, target, "reset", "--hard", "HEAD") + + result = _run_plugin_git(git_exe, target, "pull", "--ff-only") + + if result.returncode != 0: + err = _safe_git_error(result) + if stash_created: + # Put the user's edits back before reporting the failure. + restore = _run_plugin_git(git_exe, target, "stash", "apply", "stash@{0}") + if restore.returncode == 0: + _run_plugin_git(git_exe, target, "stash", "drop", "stash@{0}") + note = "Local changes were restored." + else: + note = ( + "Local changes are preserved in git stash " + "(restore with: git stash pop)." + ) + return False, (err or "git pull failed.") + f"\n{note}" + return False, err or "git pull failed." + + pulled = result.stdout.strip() + if not stash_created: + return True, pulled + + restore = _run_plugin_git(git_exe, target, "stash", "apply", "stash@{0}") + unmerged = _run_plugin_git( + git_exe, target, "diff", "--name-only", "--diff-filter=U" + ) + has_conflicts = bool(unmerged.stdout.strip()) + + if restore.returncode == 0 and not has_conflicts: + _run_plugin_git(git_exe, target, "stash", "drop", "stash@{0}") + return True, pulled + "\nLocal changes were re-applied on top of the update." + + # Conflicted re-apply: leave the plugin importable on the updated + # revision; the user's edits stay safe in the stash entry. + _run_plugin_git(git_exe, target, "reset", "--hard", "HEAD") + return True, pulled + ( + "\n⚠ Local changes in this plugin conflicted with the update and " + "were NOT re-applied. They are preserved in git stash — inspect " + "with `git stash show -p stash@{0}` and re-apply with " + f"`git stash pop` inside {target}." ) except FileNotFoundError: return False, "git is not installed or not in PATH." except subprocess.TimeoutExpired: - return False, "Git pull timed out after 60 seconds." - - if result.returncode != 0: - err = _safe_git_error(result) - return False, err or "git pull failed." - return True, result.stdout.strip() + return False, "Git operation timed out after 60 seconds." def dashboard_remove_user_plugin(name: str) -> dict[str, Any]: diff --git a/tests/hermes_cli/test_plugins_cmd.py b/tests/hermes_cli/test_plugins_cmd.py index 2c49b12441..4002698b0e 100644 --- a/tests/hermes_cli/test_plugins_cmd.py +++ b/tests/hermes_cli/test_plugins_cmd.py @@ -153,11 +153,146 @@ class TestResolveGitExecutable: return_value="/resolved/git", ): with patch.object(pc.subprocess, "run") as run: - run.return_value = MagicMock(returncode=0, stdout="Already up to date\n", stderr="") + # First call is `git status --porcelain` (clean tree), + # second is the pull itself. + run.side_effect = [ + MagicMock(returncode=0, stdout="", stderr=""), + MagicMock(returncode=0, stdout="Already up to date\n", stderr=""), + ] ok, msg = pc._git_pull_plugin_dir(tmp_path) assert ok is True - run.assert_called_once() - assert run.call_args[0][0][0] == "/resolved/git" + assert run.call_count == 2 + for call in run.call_args_list: + assert call.args[0][0] == "/resolved/git" + assert run.call_args_list[1].args[0][1:] == ["pull", "--ff-only"] + + def test_git_pull_clean_tree_never_stashes(self, tmp_path): + import hermes_cli.plugins_cmd as pc + + _resolve_git_executable.cache_clear() + with patch.object(pc, "_resolve_git_executable", return_value="/g"): + with patch.object(pc.subprocess, "run") as run: + run.side_effect = [ + MagicMock(returncode=0, stdout="", stderr=""), # status + MagicMock(returncode=0, stdout="Updated\n", stderr=""), # pull + ] + ok, msg = pc._git_pull_plugin_dir(tmp_path) + assert ok is True + assert msg == "Updated" + commands = [c.args[0][1] for c in run.call_args_list] + assert "stash" not in commands + + +class TestGitPullPluginDirAutostash: + """Real-git E2E: local edits in a plugin checkout must not block updates.""" + + @staticmethod + def _make_repos(tmp_path): + import subprocess as sp + + def git(cwd, *args): + r = sp.run(["git", *args], cwd=str(cwd), capture_output=True, text=True) + assert r.returncode == 0, r.stderr + return r.stdout + + origin = tmp_path / "origin" + origin.mkdir() + git(origin, "init", "-q", "-b", "main") + git(origin, "config", "user.email", "t@t") + git(origin, "config", "user.name", "t") + pad = "\n".join(f"# pad {i}" for i in range(12)) + (origin / "plugin.py").write_text( + f"VALUE = 1\n{pad}\nOTHER = 'a'\n", encoding="utf-8" + ) + git(origin, "add", ".") + git(origin, "commit", "-qm", "init") + + checkout = tmp_path / "checkout" + git(tmp_path, "clone", "-q", str(origin), str(checkout)) + git(checkout, "config", "user.email", "t@t") + git(checkout, "config", "user.name", "t") + return origin, checkout, git + + @staticmethod + def _set_line(repo, prefix, new_line): + """Replace the line starting with ``prefix`` in plugin.py, keep the rest.""" + f = repo / "plugin.py" + lines = f.read_text(encoding="utf-8").splitlines() + lines = [new_line if ln.startswith(prefix) else ln for ln in lines] + f.write_text("\n".join(lines) + "\n", encoding="utf-8") + + def test_dirty_checkout_pulls_and_reapplies_local_edit(self, tmp_path): + import hermes_cli.plugins_cmd as pc + + if not pc._resolve_git_executable(): + pytest.skip("git not available") + origin, checkout, git = self._make_repos(tmp_path) + + # Upstream changes one line; local edit touches a DIFFERENT line. + self._set_line(origin, "VALUE", "VALUE = 2") + git(origin, "commit", "-qam", "bump value") + self._set_line(checkout, "OTHER", "OTHER = 'local'") + + ok, msg = pc._git_pull_plugin_dir(checkout) + assert ok is True + content = (checkout / "plugin.py").read_text(encoding="utf-8") + assert "VALUE = 2" in content # update landed + assert "OTHER = 'local'" in content # local edit survived + assert "re-applied" in msg + # Clean re-apply drops the autostash entry. + assert git(checkout, "stash", "list").strip() == "" + + def test_conflicting_local_edit_is_preserved_in_stash(self, tmp_path): + import hermes_cli.plugins_cmd as pc + + if not pc._resolve_git_executable(): + pytest.skip("git not available") + origin, checkout, git = self._make_repos(tmp_path) + + # Upstream and local both change the SAME line → re-apply conflicts. + self._set_line(origin, "VALUE", "VALUE = 2") + git(origin, "commit", "-qam", "bump value") + self._set_line(checkout, "VALUE", "VALUE = 99") + + ok, msg = pc._git_pull_plugin_dir(checkout) + assert ok is True + content = (checkout / "plugin.py").read_text(encoding="utf-8") + # Checkout is importable on the updated revision — no conflict markers. + assert "<<<<<<<" not in content + assert "VALUE = 2" in content + assert "preserved in git stash" in msg + # The local edit is recoverable from the kept stash entry. + stash_list = git(checkout, "stash", "list") + assert "hermes-plugin-update-autostash" in stash_list + stash_diff = git(checkout, "stash", "show", "-p", "stash@{0}") + assert "VALUE = 99" in stash_diff + + def test_untracked_local_file_survives_update(self, tmp_path): + import hermes_cli.plugins_cmd as pc + + if not pc._resolve_git_executable(): + pytest.skip("git not available") + origin, checkout, git = self._make_repos(tmp_path) + + self._set_line(origin, "VALUE", "VALUE = 2") + git(origin, "commit", "-qam", "bump value") + (checkout / "local_notes.txt").write_text("keep me\n", encoding="utf-8") + + ok, msg = pc._git_pull_plugin_dir(checkout) + assert ok is True + assert (checkout / "local_notes.txt").read_text(encoding="utf-8") == "keep me\n" + assert "VALUE = 2" in (checkout / "plugin.py").read_text(encoding="utf-8") + + def test_clean_checkout_unchanged_behavior(self, tmp_path): + import hermes_cli.plugins_cmd as pc + + if not pc._resolve_git_executable(): + pytest.skip("git not available") + origin, checkout, git = self._make_repos(tmp_path) + + ok, msg = pc._git_pull_plugin_dir(checkout) + assert ok is True + assert "Already up to date" in msg # ── _repo_name_from_url ────────────────────────────────────────────────── @@ -277,11 +412,14 @@ class TestCmdUpdate: ) mock_sanitize.return_value = mock_target - mock_run.return_value = MagicMock(returncode=0, stdout="Updated", stderr="") + mock_run.side_effect = [ + MagicMock(returncode=0, stdout="", stderr=""), # status: clean + MagicMock(returncode=0, stdout="Updated", stderr=""), # pull + ] cmd_update("test-plugin") - mock_run.assert_called_once() + assert mock_run.call_count == 2 @patch("hermes_cli.plugins_cmd._sanitize_plugin_name") @patch("hermes_cli.plugins_cmd._plugins_dir")