Inspired by Factory Droid: plugin updates autostash local changes
Factory Droid v0.188.0 (Aug 4, 2026): 'Updating a plugin marketplace now succeeds when its checkout has local changes instead of failing.' Hermes had the same failure: users who tweak an installed plugin in place (config constants, small patches) hit 'Your local changes ... would be overwritten by merge' on every 'hermes plugins update <name>' and the dashboard update path — the plugin becomes permanently un-updatable until they hand-run git. _git_pull_plugin_dir() now autostashes before the pull and re-applies after, reusing the ref-compared stash discipline hermes update already uses for the main checkout (PR #70161): - clean tree → identical single pull, no behavior change - dirty tree → stash push --include-untracked (ref-compared so 'nothing saved' aborts before touching the checkout), pull, stash apply - clean re-apply → drop the stash entry, note in output - conflicted re-apply → reset to the updated revision (plugin stays importable, no conflict markers on disk) and KEEP the stash entry with recovery instructions - failed pull with a stash → restore the user's edits before reporting Covers both callers: cmd_update (CLI) and dashboard_update_user_plugin. Real-git E2E tests for all four paths + sabotage-verified (tests fail on the old single-pull implementation).
This commit is contained in:
Teknium
+106
-14
@@ -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]:
|
||||
|
||||
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user