From 3e03da41e1ec9855bac523a1651414971c026fa4 Mon Sep 17 00:00:00 2001 From: JoaoMarcos44 Date: Fri, 25 Sep 2026 02:37:08 -0300 Subject: [PATCH] fix(plugins): carry user files across staged updates (cherry picked from commit 8b17cdef5a1cb45a5f9f79ec5590376147ffe239) --- hermes_cli/plugins_cmd_catalog.py | 109 +++++++++++++++++++++--------- hermes_cli/plugins_cmd_update.py | 21 ++++-- hermes_cli/plugins_transaction.py | 11 +-- 3 files changed, 99 insertions(+), 42 deletions(-) diff --git a/hermes_cli/plugins_cmd_catalog.py b/hermes_cli/plugins_cmd_catalog.py index 8066588e87..2692728b8f 100644 --- a/hermes_cli/plugins_cmd_catalog.py +++ b/hermes_cli/plugins_cmd_catalog.py @@ -10,6 +10,7 @@ from __future__ import annotations import datetime import json import logging +import os import shutil import sys import tempfile @@ -284,17 +285,17 @@ def refuse_if_installed_removed(name: str, plugin_dir) -> None: _PRESERVE_SKIP = ("__pycache__", CATALOG_SIDECAR) -def _local_changes(target: Path) -> tuple[list[str], list[str]]: - """``(untracked_or_ignored, modified_tracked)`` relative paths in a git checkout; empty for a - non-git tree (subdir installs carry no ``.git``, so nothing can be told apart from the clone).""" +def _local_changes(target: Path) -> Optional[tuple[list[str], list[str]]]: + """``(untracked_or_ignored, modified_tracked)`` in a git checkout, or ``None`` when git + cannot classify the installed tree (notably subdirectory installs, which carry no ``.git``).""" from hermes_cli.plugins_cmd import _resolve_git_executable, _run_plugin_git git_exe = _resolve_git_executable() if not git_exe or not (target / ".git").exists(): - return [], [] + return None status = _run_plugin_git(git_exe, target, "status", "--porcelain", "--ignored", "-z", "--untracked-files=all", "--ignored=matching", timeout=30) if status.returncode != 0: - return [], [] + return None local, modified = [], [] for item in status.stdout.split("\0"): if len(item) < 4: @@ -315,6 +316,49 @@ def _stash_local_files(target: Path, rels: list[str], stash: Path) -> None: shutil.copy2(src, dst) +def _carry_user_files(old: Path, new: Path, local: Optional[list[str]]) -> None: + """Carry user-owned files into a staged replacement without reviving old plugin code. + + For a git checkout, *local* is the ``??``/``!!`` set and may contain a directory entry + such as ``data/``; descendants of those entries are copied and win over same-path files in + the new tree. ``None`` means git cannot classify the tree, so only non-Python files that + the new tree does not already ship are carried. Path-type conflicts are owned by the new tree. + """ + keep = {Path(rel) for rel in local or ()} + for dirpath, dirnames, filenames in os.walk(old): + here = Path(dirpath) + links = [name for name in dirnames if (here / name).is_symlink()] + dirnames[:] = [ + name for name in dirnames + if name not in links and name != ".git" and name not in _PRESERVE_SKIP + ] + for name in (*filenames, *links): + src = here / name + rel = src.relative_to(old) + if any(part in _PRESERVE_SKIP or part.endswith(".pyc") for part in rel.parts): + continue + if local is None: + # A no-git subdir install cannot distinguish removed upstream code from user files. + # Never resurrect an old Python module/package into a new plugin revision. + if rel.suffix == ".py" or os.path.lexists(new / rel): + continue + elif keep.isdisjoint((rel, *rel.parents)): + continue + + dst = new / rel + # The replacement tree owns file/dir shape changes. Carrying across a type clash can + # either copy into the wrong directory or make parent mkdir fail and abort the update. + if dst.is_dir() and not dst.is_symlink(): + continue + try: + dst.parent.mkdir(parents=True, exist_ok=True) + except (FileExistsError, NotADirectoryError): + continue + if dst.is_symlink() or dst.is_file(): + dst.unlink() + shutil.copy2(src, dst, follow_symlinks=False) + + class RepinResult(NamedTuple): sha: str changed: bool @@ -408,7 +452,8 @@ def repin_catalog_plugin( if at_catalog_pin(sidecar, entry.sha): return RepinResult(entry.sha, False, target.name, []) - local, modified = _local_changes(target) + changes = _local_changes(target) + local, modified = changes if changes is not None else (None, []) old_sha8 = str(sidecar.get("sha") or "old")[:8] installed_surface = plugin_surface(_read_manifest(target), target) @@ -427,32 +472,34 @@ def repin_catalog_plugin( preview_target = _resolve_subdir_within(preview_root, subdir) if subdir else preview_root _consent_gate(_read_manifest_for_install(preview_target), preview_target) - with tempfile.TemporaryDirectory(prefix=".repin-", dir=_plugins_dir()) as tmp: - stash = Path(tmp) / "local" - _stash_local_files(target, local, stash) - # Outside the plugins dir: the discovery scanners recurse into every subdirectory there. - backup = _plugins_dir().parent / "plugins-backup" / f"{target.name}-{old_sha8}" - _stash_local_files(target, modified, backup) - from hermes_cli.plugins_transaction import update_plugin + # Outside the plugins dir: the discovery scanners recurse into every subdirectory there. + backup = _plugins_dir().parent / "plugins-backup" / f"{target.name}-{old_sha8}" + _stash_local_files(target, modified, backup) + from hermes_cli.plugins_transaction import update_plugin - update_plugin(target, catalog_entry=entry, interactive=interactive, preserved_files=stash) - matches = [] - for installed_name, row in _read_install_metadata().items(): - if not isinstance(row, dict): - continue - block = row.get("catalog") - if ( - isinstance(block, dict) - and block.get("name") == entry.name - and at_catalog_pin(block, entry.sha) - ): - matches.append(installed_name) - if len(matches) != 1: - raise PluginOperationError( - f"Catalog update published but its install record is ambiguous: {matches or 'missing'}." - ) - installed_name = matches[0] - new_target = target.parent / installed_name + update_plugin( + target, + catalog_entry=entry, + interactive=interactive, + carry_user_files=lambda staged: _carry_user_files(target, staged, local), + ) + matches = [] + for installed_name, row in _read_install_metadata().items(): + if not isinstance(row, dict): + continue + block = row.get("catalog") + if ( + isinstance(block, dict) + and block.get("name") == entry.name + and at_catalog_pin(block, entry.sha) + ): + matches.append(installed_name) + if len(matches) != 1: + raise PluginOperationError( + f"Catalog update published but its install record is ambiguous: {matches or 'missing'}." + ) + installed_name = matches[0] + new_target = target.parent / installed_name warnings: list[str] = [] if modified: warnings.append(f"Local edits to {len(modified)} tracked file(s) were not carried over; copies are under " diff --git a/hermes_cli/plugins_cmd_update.py b/hermes_cli/plugins_cmd_update.py index b7ff1179f2..8ea12530e2 100644 --- a/hermes_cli/plugins_cmd_update.py +++ b/hermes_cli/plugins_cmd_update.py @@ -39,7 +39,7 @@ def _pull_plugin_update(target: Path, pinned_msg, not_git_msg, before_pull=None, raise _pc().PluginOperationError(not_git_msg()) if before_pull is not None: before_pull() - return _reclone_plugin_update(source, install_record.get("revision")) + return _reclone_plugin_update(target, source, install_record.get("revision")) if before_pull is not None: before_pull() from hermes_cli.plugins_transaction import update_plugin @@ -47,12 +47,19 @@ def _pull_plugin_update(target: Path, pinned_msg, not_git_msg, before_pull=None, return update_plugin(target, interactive=interactive) -def _reclone_plugin_update(source: str, previous_revision: object) -> str: - """Update a plugin whose tree is not a git checkout: a subdirectory install ships only - ``/``, so the ``.git`` stays in the temp clone (#65314). Re-run the install - from the recorded source (same URL, same subdir) and swap the fresh tree in; the metadata - revision is rewritten by the installer. Returns pull-shaped output for the callers.""" - new_target, _manifest, _name = _pc()._install_plugin_core(source, force=True) +def _reclone_plugin_update(target: Path, source: str, previous_revision: object) -> str: + """Update a subdirectory install by re-cloning its source and atomically replacing the tree. + + These installs carry no local ``.git``, so the staged replacement also carries user-owned + config/data from *target* before publication instead of silently deleting it (#122006). + """ + from hermes_cli.plugins_cmd_catalog import _carry_user_files + + new_target, _manifest, _name = _pc()._install_plugin_core( + source, + force=True, + before_swap=lambda _manifest, tree: _carry_user_files(target, tree, None), + ) revision = str(_pc()._read_install_metadata().get(new_target.name, {}).get("revision") or "") previous = previous_revision if isinstance(previous_revision, str) else "" if revision and revision == previous: diff --git a/hermes_cli/plugins_transaction.py b/hermes_cli/plugins_transaction.py index f95936e7c8..5e939ec597 100644 --- a/hermes_cli/plugins_transaction.py +++ b/hermes_cli/plugins_transaction.py @@ -1,6 +1,7 @@ """Publish plugin code and its dependency selection through one recoverable handoff.""" from __future__ import annotations +from collections.abc import Callable from pathlib import Path import shutil @@ -80,12 +81,14 @@ def update_plugin( *, catalog_entry=None, interactive: bool = False, - preserved_files: Path | None = None, + carry_user_files: Callable[[Path], None] | None = None, ) -> str: """Prepare a catalog re-pin or custom Git pull without changing the live tree. *interactive*: a terminal user is present to consent to newly declared dependencies; - the dashboard and the gateway's auto-apply pass False and get a refusal instead.""" + the dashboard and the gateway's auto-apply pass False and get a refusal instead. + *carry_user_files(staged)* may merge user-owned state into the staged tree before + manifest validation, example-file generation, dependency preparation and publication.""" import tempfile from hermes_cli import plugins_cmd as pc @@ -158,8 +161,8 @@ def update_plugin( if not ok: raise pc.PluginOperationError(output) revision = pc._git_head_revision(staged, pc._resolve_git_executable()) - if preserved_files is not None and preserved_files.exists(): - shutil.copytree(preserved_files, staged, dirs_exist_ok=True) + if carry_user_files is not None: + carry_user_files(staged) manifest = pc._read_manifest_for_install(staged) installed_name = str(manifest.get("name") or target.name) if catalog_entry is None and installed_name != target.name: