From 431fce843be8a665631574fbd01f8a41739b0ac4 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Fri, 22 May 2026 07:58:20 +0900 Subject: [PATCH 1/3] =?UTF-8?q?fix(plugin):=20copy=5Fplugin=20=E3=81=A7=20?= =?UTF-8?q?rmtree+copytree=20=E3=82=92=20rsync=20=E5=90=8C=E6=9C=9F?= =?UTF-8?q?=E3=81=AB=E7=BD=AE=E6=8F=9B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ユーザが `projects/` シンボリックリンク経由で `plugins//projects//` 内部に `cd` した状態で `devbase plugin update` を実行すると、`copy_plugin()` の `shutil.rmtree(dest); shutil.copytree(plugin_path, dest)` がプラグインディレクトリの inode ごと置換し、ユーザのシェル CWD が "宙ぶらりんの旧 inode" を掴んだままになって ファイルが消えたように見える問題があった。 `_sync_dir(src, dst)` を新設し、rmtree せずに既存ディレクトリの inode を保ったまま ファイル差分を反映する (rsync 風)。差分は: - src に存在しない dst エントリは削除 (orphan) - src の symlink は再生成 (target が変わっていれば追従) - src の dir は再帰的に同期 (両方に存在する dir は inode 維持) - src の file は shutil.copy2 で上書き これにより、ユーザの CWD が更新対象プラグイン配下にあっても、シェルは引き続き 同じ inode を参照し続け、`ll` で正しく中身を表示できる。 linked プラグインで dest が symlink の場合は従来どおり unlink + copytree。 新規インストールも従来どおり copytree。 Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/devbase/plugin/installer.py | 65 ++++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 6 deletions(-) diff --git a/lib/devbase/plugin/installer.py b/lib/devbase/plugin/installer.py index 8a1d715..b33b204 100644 --- a/lib/devbase/plugin/installer.py +++ b/lib/devbase/plugin/installer.py @@ -1,5 +1,6 @@ """Plugin installer - handles install/uninstall operations""" +import os import shutil import subprocess import tempfile @@ -259,6 +260,49 @@ def _install_from_repo( raise PluginError("No plugin name specified") +def _replace_entry(path: Path) -> None: + """Remove ``path`` (file, symlink, or directory) so it can be replaced.""" + if path.is_symlink() or not path.is_dir(): + path.unlink() + else: + shutil.rmtree(path) + + +def _sync_dir(src: Path, dst: Path) -> None: + """rsync-like merge: make ``dst``'s contents match ``src``'s. + + Preserves the inode of ``dst`` and of any subdirectories that exist in + both ``src`` and ``dst``. Compared to ``rmtree(dst) + copytree(src, dst)``, + a process whose CWD is inside ``dst`` (or a subdir present in both) keeps + a valid CWD across this operation — important when the user is working + inside a ``projects/`` symlink that resolves into the plugin tree. + """ + dst.mkdir(parents=True, exist_ok=True) + + src_entries = {e.name: e for e in src.iterdir()} + dst_entries = {e.name: e for e in dst.iterdir()} + + for name, dst_entry in dst_entries.items(): + if name not in src_entries: + _replace_entry(dst_entry) + + for name, src_entry in src_entries.items(): + dst_entry = dst / name + if src_entry.is_symlink(): + link_target = os.readlink(src_entry) + if dst_entry.is_symlink() or dst_entry.exists(): + _replace_entry(dst_entry) + os.symlink(link_target, dst_entry) + elif src_entry.is_dir(): + if dst_entry.exists() and not dst_entry.is_dir(): + _replace_entry(dst_entry) + _sync_dir(src_entry, dst_entry) + else: + if dst_entry.is_symlink() or (dst_entry.exists() and dst_entry.is_dir()): + _replace_entry(dst_entry) + shutil.copy2(src_entry, dst_entry) + + def copy_plugin( registry: PluginRegistry, name: str, @@ -266,7 +310,12 @@ def copy_plugin( source_display: str, plugins_dir: Path, ) -> None: - """Copy a plugin from cloned repo to plugins/. + """Install or update a plugin from a cloned repo into ``plugins/``. + + For updates, the existing plugin directory inode is preserved by + syncing contents in place (rsync-like) instead of rmtree+copytree. + This avoids invalidating any shell or editor whose CWD is inside the + plugin (typically via a ``projects/`` symlink). Raises PluginError on failure. """ @@ -274,11 +323,15 @@ def copy_plugin( raise PluginError(f"Plugin directory not found: {plugin_path}") dest = plugins_dir / name - if dest.exists(): - logger.warning("Removing existing plugin '%s'", name) - shutil.rmtree(dest) - - shutil.copytree(plugin_path, dest) + if dest.is_symlink(): + logger.warning("Removing existing plugin '%s' (symlink)", name) + dest.unlink() + shutil.copytree(plugin_path, dest) + elif dest.exists(): + logger.info("Updating existing plugin '%s'", name) + _sync_dir(plugin_path, dest) + else: + shutil.copytree(plugin_path, dest) info = load_plugin_info(dest) version = info.version if info else '0.1.0' From b5eca7453cfe74db59364bcec6deccc8eb245cd6 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Fri, 22 May 2026 08:37:53 +0900 Subject: [PATCH 2/3] =?UTF-8?q?fix(plugin):=20=5Fsync=5Fdir=20=E3=82=92?= =?UTF-8?q?=E3=83=A6=E3=83=BC=E3=82=B6=E7=B7=A8=E9=9B=86=E4=BF=9D=E8=AD=B7?= =?UTF-8?q?=E3=82=BB=E3=83=9E=E3=83=B3=E3=83=86=E3=82=A3=E3=82=AF=E3=82=B9?= =?UTF-8?q?=E3=81=AB=E5=A4=89=E6=9B=B4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 前コミットの rsync 同期は inode は保つが、ユーザがプラグイン配下で編集 したファイル (`compose.yml`, `env` 等) や独自に置いたファイル (`.env`, ログ等) をそのまま上書き / 削除していた。これは旧 rmtree+copytree と 同等の data-loss セマンティクスで、UX 改善としては不十分。 挙動を以下の保守的なものに置き換える: | src | dst | content | action | |---|---|---|---| | 存在 | 無し | - | コピー (added) | | 存在 | 存在 | 同じ | no-op | | 存在 | 存在 | 違う | dst 維持、src を `.new` に並置 (kept_local) | | 無し | 存在 | - | dst 残す (preserved_orphans) | これにより: - `.env` 等 upstream に無いファイルは保持 - ユーザが手で書き換えた `compose.yml` も保持、upstream 版は `compose.yml.new` として手元で diff/merge できる - 内容が変わっていなければ `.new` も生成されない (ノイズなし) - 既存の `.new` (前回の sync で残った) は次回 sync 時に再生成 (常に 最新の upstream を反映) - ファイル/dir/symlink の type mismatch は upstream 側を `.new` 経由で 並置 (ユーザ側を壊さない) `copy_plugin()` は同期後に `kept_local` / `preserved_orphans` 件数を ログ出力し、ユーザに `.new` の存在と "merge の余地" を伝える。 NOTE: `plugin.yml` もユーザ編集として扱われるため、ユーザが意図せず 触っていた場合 registry に古い version が記録され得る。実害は表示が 古くなるだけで、`.new` が並置されるので気付ける。 Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/devbase/plugin/installer.py | 129 ++++++++++++++++++++++++++------ 1 file changed, 108 insertions(+), 21 deletions(-) diff --git a/lib/devbase/plugin/installer.py b/lib/devbase/plugin/installer.py index b33b204..943018e 100644 --- a/lib/devbase/plugin/installer.py +++ b/lib/devbase/plugin/installer.py @@ -1,10 +1,12 @@ """Plugin installer - handles install/uninstall operations""" +import hashlib import os import shutil import subprocess import tempfile import yaml +from dataclasses import dataclass, field from pathlib import Path from typing import Optional @@ -268,14 +270,41 @@ def _replace_entry(path: Path) -> None: shutil.rmtree(path) -def _sync_dir(src: Path, dst: Path) -> None: - """rsync-like merge: make ``dst``'s contents match ``src``'s. +def _hash_file(path: Path) -> str: + """Return the SHA-256 hex digest of a regular file's contents.""" + h = hashlib.sha256() + with path.open('rb') as f: + for chunk in iter(lambda: f.read(65536), b''): + h.update(chunk) + return h.hexdigest() - Preserves the inode of ``dst`` and of any subdirectories that exist in - both ``src`` and ``dst``. Compared to ``rmtree(dst) + copytree(src, dst)``, - a process whose CWD is inside ``dst`` (or a subdir present in both) keeps - a valid CWD across this operation — important when the user is working - inside a ``projects/`` symlink that resolves into the plugin tree. + +@dataclass +class _SyncReport: + """Summary of an in-place plugin sync, surfaced to users after update.""" + added: list[Path] = field(default_factory=list) + updated: list[Path] = field(default_factory=list) + kept_local: list[Path] = field(default_factory=list) + preserved_orphans: list[Path] = field(default_factory=list) + + +def _sync_dir(src: Path, dst: Path, report: _SyncReport, rel: Path = Path('.')) -> None: + """Conservatively sync ``src`` → ``dst``, preserving user edits. + + Semantics (per file in src/dst): + + | src | dst | content | action | + |---|---|---|---| + | exists | missing | - | copy from src (record as ``added``) | + | exists | exists | same | no-op | + | exists | exists | differ | keep dst, write src as ``.new`` (``kept_local``) | + | missing | exists | - | leave dst alone (``preserved_orphans``) | + + Preserves the inode of ``dst`` and of subdirectories present in both — a + user whose CWD lives inside ``dst`` (typically via a ``projects/`` + symlink resolving into the plugin tree) keeps a valid CWD across updates. + + User-only files (orphans) and user-edited files are never destroyed. """ dst.mkdir(parents=True, exist_ok=True) @@ -283,24 +312,66 @@ def _sync_dir(src: Path, dst: Path) -> None: dst_entries = {e.name: e for e in dst.iterdir()} for name, dst_entry in dst_entries.items(): - if name not in src_entries: + if name in src_entries: + continue + if name.endswith('.new'): + # `.new` is our own conflict marker — refresh, don't preserve. _replace_entry(dst_entry) + continue + report.preserved_orphans.append(rel / name) for name, src_entry in src_entries.items(): dst_entry = dst / name + sub_rel = rel / name if src_entry.is_symlink(): link_target = os.readlink(src_entry) + if dst_entry.is_symlink() and os.readlink(dst_entry) == link_target: + continue if dst_entry.is_symlink() or dst_entry.exists(): - _replace_entry(dst_entry) - os.symlink(link_target, dst_entry) + # Conflict: leave user's, drop upstream alongside as `.new` symlink. + new_dst = dst_entry.with_name(dst_entry.name + '.new') + if new_dst.is_symlink() or new_dst.exists(): + _replace_entry(new_dst) + os.symlink(link_target, new_dst) + report.kept_local.append(sub_rel) + else: + os.symlink(link_target, dst_entry) + report.added.append(sub_rel) elif src_entry.is_dir(): - if dst_entry.exists() and not dst_entry.is_dir(): - _replace_entry(dst_entry) - _sync_dir(src_entry, dst_entry) + if dst_entry.is_symlink() or (dst_entry.exists() and not dst_entry.is_dir()): + # Type mismatch: user has a file/symlink where upstream has a dir. + # Drop upstream alongside as `.new/`. + new_dst = dst_entry.with_name(dst_entry.name + '.new') + if new_dst.is_symlink() or (new_dst.exists() and not new_dst.is_dir()): + _replace_entry(new_dst) + _sync_dir(src_entry, new_dst, report, sub_rel) + report.kept_local.append(sub_rel) + else: + already_existed = dst_entry.is_dir() + _sync_dir(src_entry, dst_entry, report, sub_rel) + if not already_existed: + report.added.append(sub_rel) else: - if dst_entry.is_symlink() or (dst_entry.exists() and dst_entry.is_dir()): - _replace_entry(dst_entry) - shutil.copy2(src_entry, dst_entry) + if not dst_entry.exists() and not dst_entry.is_symlink(): + shutil.copy2(src_entry, dst_entry) + report.added.append(sub_rel) + continue + if dst_entry.is_symlink() or dst_entry.is_dir(): + # Type mismatch: user has a symlink/dir where upstream has a file. + new_dst = dst_entry.with_name(dst_entry.name + '.new') + if new_dst.is_symlink() or new_dst.exists(): + _replace_entry(new_dst) + shutil.copy2(src_entry, new_dst) + report.kept_local.append(sub_rel) + continue + # Both are regular files — compare content. + if _hash_file(src_entry) == _hash_file(dst_entry): + continue + new_dst = dst_entry.with_name(dst_entry.name + '.new') + if new_dst.is_symlink() or new_dst.exists(): + _replace_entry(new_dst) + shutil.copy2(src_entry, new_dst) + report.kept_local.append(sub_rel) def copy_plugin( @@ -312,10 +383,11 @@ def copy_plugin( ) -> None: """Install or update a plugin from a cloned repo into ``plugins/``. - For updates, the existing plugin directory inode is preserved by - syncing contents in place (rsync-like) instead of rmtree+copytree. - This avoids invalidating any shell or editor whose CWD is inside the - plugin (typically via a ``projects/`` symlink). + For updates, contents are synced in place (preserving directory inodes + and user-edited files) instead of rmtree+copytree. User-edited files + are kept as-is; the upstream version of a conflicting file is dropped + alongside with a ``.new`` suffix for the user to diff/merge manually. + Files present only in the user's working tree (orphans) are preserved. Raises PluginError on failure. """ @@ -329,7 +401,22 @@ def copy_plugin( shutil.copytree(plugin_path, dest) elif dest.exists(): logger.info("Updating existing plugin '%s'", name) - _sync_dir(plugin_path, dest) + report = _SyncReport() + _sync_dir(plugin_path, dest, report) + if report.kept_local: + logger.warning( + " %d local edit(s) kept; upstream saved as .new alongside:", + len(report.kept_local), + ) + for p in report.kept_local[:10]: + logger.warning(" - %s (upstream: %s.new)", p, p.name) + if len(report.kept_local) > 10: + logger.warning(" ... and %d more", len(report.kept_local) - 10) + if report.preserved_orphans: + logger.info( + " %d local-only file(s) preserved (not in upstream)", + len(report.preserved_orphans), + ) else: shutil.copytree(plugin_path, dest) From 83c70ad7bb55462b3fdb49ac6f26179f583f18d5 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Fri, 22 May 2026 10:18:56 +0900 Subject: [PATCH 3/3] =?UTF-8?q?fix(plugin):=20plugin.yml=20=E3=81=AF=20=5F?= =?UTF-8?q?sync=5Fdir=20=E3=81=A7=E5=B8=B8=E6=99=82=E4=B8=8A=E6=9B=B8?= =?UTF-8?q?=E3=81=8D=E3=81=99=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_sync_dir` のユーザ編集保護セマンティクスにより、ユーザが 意図せず `plugin.yml` を編集していた場合に registry の version / description 表示が upstream と desync する可能性があった。 `plugin.yml` はプラグインのメタデータでありユーザ編集対象では ないため、プラグインルート直下の `plugin.yml` のみ常時 upstream で上書きする例外を `_ALWAYS_OVERWRITE_AT_ROOT` で表現する。 Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/devbase/plugin/installer.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lib/devbase/plugin/installer.py b/lib/devbase/plugin/installer.py index 943018e..79ca000 100644 --- a/lib/devbase/plugin/installer.py +++ b/lib/devbase/plugin/installer.py @@ -288,6 +288,12 @@ class _SyncReport: preserved_orphans: list[Path] = field(default_factory=list) +# Files at the plugin root that are upstream-owned metadata: always overwritten +# so registry version/description never desync from upstream even if a user +# happened to edit them locally. +_ALWAYS_OVERWRITE_AT_ROOT = frozenset({'plugin.yml'}) + + def _sync_dir(src: Path, dst: Path, report: _SyncReport, rel: Path = Path('.')) -> None: """Conservatively sync ``src`` → ``dst``, preserving user edits. @@ -300,6 +306,10 @@ def _sync_dir(src: Path, dst: Path, report: _SyncReport, rel: Path = Path('.')) | exists | exists | differ | keep dst, write src as ``.new`` (``kept_local``) | | missing | exists | - | leave dst alone (``preserved_orphans``) | + Exception: files named in ``_ALWAYS_OVERWRITE_AT_ROOT`` at the plugin root + are always overwritten with upstream content (treated as plugin metadata, + not user-editable). + Preserves the inode of ``dst`` and of subdirectories present in both — a user whose CWD lives inside ``dst`` (typically via a ``projects/`` symlink resolving into the plugin tree) keeps a valid CWD across updates. @@ -323,6 +333,17 @@ def _sync_dir(src: Path, dst: Path, report: _SyncReport, rel: Path = Path('.')) for name, src_entry in src_entries.items(): dst_entry = dst / name sub_rel = rel / name + if ( + rel == Path('.') + and name in _ALWAYS_OVERWRITE_AT_ROOT + and not src_entry.is_symlink() + and not src_entry.is_dir() + ): + if dst_entry.is_symlink() or dst_entry.exists(): + _replace_entry(dst_entry) + shutil.copy2(src_entry, dst_entry) + report.updated.append(sub_rel) + continue if src_entry.is_symlink(): link_target = os.readlink(src_entry) if dst_entry.is_symlink() and os.readlink(dst_entry) == link_target: