Skip to content

Commit e0408b0

Browse files
takemi-ohamaclaude
andauthored
fix(plugin): copy_plugin で rmtree+copytree を rsync 同期に置換 (#17)
* fix(plugin): copy_plugin で rmtree+copytree を rsync 同期に置換 ユーザが `projects/<name>` シンボリックリンク経由で `plugins/<plugin>/projects/<name>/` 内部に `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) <noreply@anthropic.com> * fix(plugin): _sync_dir をユーザ編集保護セマンティクスに変更 前コミットの rsync 同期は inode は保つが、ユーザがプラグイン配下で編集 したファイル (`compose.yml`, `env` 等) や独自に置いたファイル (`.env`, ログ等) をそのまま上書き / 削除していた。これは旧 rmtree+copytree と 同等の data-loss セマンティクスで、UX 改善としては不十分。 挙動を以下の保守的なものに置き換える: | src | dst | content | action | |---|---|---|---| | 存在 | 無し | - | コピー (added) | | 存在 | 存在 | 同じ | no-op | | 存在 | 存在 | 違う | dst 維持、src を `<name>.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) <noreply@anthropic.com> * fix(plugin): plugin.yml は _sync_dir で常時上書きする `_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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent aca7927 commit e0408b0

1 file changed

Lines changed: 167 additions & 6 deletions

File tree

lib/devbase/plugin/installer.py

Lines changed: 167 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
"""Plugin installer - handles install/uninstall operations"""
22

3+
import hashlib
4+
import os
35
import shutil
46
import subprocess
57
import tempfile
68
import yaml
9+
from dataclasses import dataclass, field
710
from pathlib import Path
811
from typing import Optional
912

@@ -259,26 +262,184 @@ def _install_from_repo(
259262
raise PluginError("No plugin name specified")
260263

261264

265+
def _replace_entry(path: Path) -> None:
266+
"""Remove ``path`` (file, symlink, or directory) so it can be replaced."""
267+
if path.is_symlink() or not path.is_dir():
268+
path.unlink()
269+
else:
270+
shutil.rmtree(path)
271+
272+
273+
def _hash_file(path: Path) -> str:
274+
"""Return the SHA-256 hex digest of a regular file's contents."""
275+
h = hashlib.sha256()
276+
with path.open('rb') as f:
277+
for chunk in iter(lambda: f.read(65536), b''):
278+
h.update(chunk)
279+
return h.hexdigest()
280+
281+
282+
@dataclass
283+
class _SyncReport:
284+
"""Summary of an in-place plugin sync, surfaced to users after update."""
285+
added: list[Path] = field(default_factory=list)
286+
updated: list[Path] = field(default_factory=list)
287+
kept_local: list[Path] = field(default_factory=list)
288+
preserved_orphans: list[Path] = field(default_factory=list)
289+
290+
291+
# Files at the plugin root that are upstream-owned metadata: always overwritten
292+
# so registry version/description never desync from upstream even if a user
293+
# happened to edit them locally.
294+
_ALWAYS_OVERWRITE_AT_ROOT = frozenset({'plugin.yml'})
295+
296+
297+
def _sync_dir(src: Path, dst: Path, report: _SyncReport, rel: Path = Path('.')) -> None:
298+
"""Conservatively sync ``src`` → ``dst``, preserving user edits.
299+
300+
Semantics (per file in src/dst):
301+
302+
| src | dst | content | action |
303+
|---|---|---|---|
304+
| exists | missing | - | copy from src (record as ``added``) |
305+
| exists | exists | same | no-op |
306+
| exists | exists | differ | keep dst, write src as ``<name>.new`` (``kept_local``) |
307+
| missing | exists | - | leave dst alone (``preserved_orphans``) |
308+
309+
Exception: files named in ``_ALWAYS_OVERWRITE_AT_ROOT`` at the plugin root
310+
are always overwritten with upstream content (treated as plugin metadata,
311+
not user-editable).
312+
313+
Preserves the inode of ``dst`` and of subdirectories present in both — a
314+
user whose CWD lives inside ``dst`` (typically via a ``projects/<name>``
315+
symlink resolving into the plugin tree) keeps a valid CWD across updates.
316+
317+
User-only files (orphans) and user-edited files are never destroyed.
318+
"""
319+
dst.mkdir(parents=True, exist_ok=True)
320+
321+
src_entries = {e.name: e for e in src.iterdir()}
322+
dst_entries = {e.name: e for e in dst.iterdir()}
323+
324+
for name, dst_entry in dst_entries.items():
325+
if name in src_entries:
326+
continue
327+
if name.endswith('.new'):
328+
# `.new` is our own conflict marker — refresh, don't preserve.
329+
_replace_entry(dst_entry)
330+
continue
331+
report.preserved_orphans.append(rel / name)
332+
333+
for name, src_entry in src_entries.items():
334+
dst_entry = dst / name
335+
sub_rel = rel / name
336+
if (
337+
rel == Path('.')
338+
and name in _ALWAYS_OVERWRITE_AT_ROOT
339+
and not src_entry.is_symlink()
340+
and not src_entry.is_dir()
341+
):
342+
if dst_entry.is_symlink() or dst_entry.exists():
343+
_replace_entry(dst_entry)
344+
shutil.copy2(src_entry, dst_entry)
345+
report.updated.append(sub_rel)
346+
continue
347+
if src_entry.is_symlink():
348+
link_target = os.readlink(src_entry)
349+
if dst_entry.is_symlink() and os.readlink(dst_entry) == link_target:
350+
continue
351+
if dst_entry.is_symlink() or dst_entry.exists():
352+
# Conflict: leave user's, drop upstream alongside as `.new` symlink.
353+
new_dst = dst_entry.with_name(dst_entry.name + '.new')
354+
if new_dst.is_symlink() or new_dst.exists():
355+
_replace_entry(new_dst)
356+
os.symlink(link_target, new_dst)
357+
report.kept_local.append(sub_rel)
358+
else:
359+
os.symlink(link_target, dst_entry)
360+
report.added.append(sub_rel)
361+
elif src_entry.is_dir():
362+
if dst_entry.is_symlink() or (dst_entry.exists() and not dst_entry.is_dir()):
363+
# Type mismatch: user has a file/symlink where upstream has a dir.
364+
# Drop upstream alongside as `<name>.new/`.
365+
new_dst = dst_entry.with_name(dst_entry.name + '.new')
366+
if new_dst.is_symlink() or (new_dst.exists() and not new_dst.is_dir()):
367+
_replace_entry(new_dst)
368+
_sync_dir(src_entry, new_dst, report, sub_rel)
369+
report.kept_local.append(sub_rel)
370+
else:
371+
already_existed = dst_entry.is_dir()
372+
_sync_dir(src_entry, dst_entry, report, sub_rel)
373+
if not already_existed:
374+
report.added.append(sub_rel)
375+
else:
376+
if not dst_entry.exists() and not dst_entry.is_symlink():
377+
shutil.copy2(src_entry, dst_entry)
378+
report.added.append(sub_rel)
379+
continue
380+
if dst_entry.is_symlink() or dst_entry.is_dir():
381+
# Type mismatch: user has a symlink/dir where upstream has a file.
382+
new_dst = dst_entry.with_name(dst_entry.name + '.new')
383+
if new_dst.is_symlink() or new_dst.exists():
384+
_replace_entry(new_dst)
385+
shutil.copy2(src_entry, new_dst)
386+
report.kept_local.append(sub_rel)
387+
continue
388+
# Both are regular files — compare content.
389+
if _hash_file(src_entry) == _hash_file(dst_entry):
390+
continue
391+
new_dst = dst_entry.with_name(dst_entry.name + '.new')
392+
if new_dst.is_symlink() or new_dst.exists():
393+
_replace_entry(new_dst)
394+
shutil.copy2(src_entry, new_dst)
395+
report.kept_local.append(sub_rel)
396+
397+
262398
def copy_plugin(
263399
registry: PluginRegistry,
264400
name: str,
265401
plugin_path: Path,
266402
source_display: str,
267403
plugins_dir: Path,
268404
) -> None:
269-
"""Copy a plugin from cloned repo to plugins/.
405+
"""Install or update a plugin from a cloned repo into ``plugins/``.
406+
407+
For updates, contents are synced in place (preserving directory inodes
408+
and user-edited files) instead of rmtree+copytree. User-edited files
409+
are kept as-is; the upstream version of a conflicting file is dropped
410+
alongside with a ``.new`` suffix for the user to diff/merge manually.
411+
Files present only in the user's working tree (orphans) are preserved.
270412
271413
Raises PluginError on failure.
272414
"""
273415
if not plugin_path.is_dir():
274416
raise PluginError(f"Plugin directory not found: {plugin_path}")
275417

276418
dest = plugins_dir / name
277-
if dest.exists():
278-
logger.warning("Removing existing plugin '%s'", name)
279-
shutil.rmtree(dest)
280-
281-
shutil.copytree(plugin_path, dest)
419+
if dest.is_symlink():
420+
logger.warning("Removing existing plugin '%s' (symlink)", name)
421+
dest.unlink()
422+
shutil.copytree(plugin_path, dest)
423+
elif dest.exists():
424+
logger.info("Updating existing plugin '%s'", name)
425+
report = _SyncReport()
426+
_sync_dir(plugin_path, dest, report)
427+
if report.kept_local:
428+
logger.warning(
429+
" %d local edit(s) kept; upstream saved as .new alongside:",
430+
len(report.kept_local),
431+
)
432+
for p in report.kept_local[:10]:
433+
logger.warning(" - %s (upstream: %s.new)", p, p.name)
434+
if len(report.kept_local) > 10:
435+
logger.warning(" ... and %d more", len(report.kept_local) - 10)
436+
if report.preserved_orphans:
437+
logger.info(
438+
" %d local-only file(s) preserved (not in upstream)",
439+
len(report.preserved_orphans),
440+
)
441+
else:
442+
shutil.copytree(plugin_path, dest)
282443

283444
info = load_plugin_info(dest)
284445
version = info.version if info else '0.1.0'

0 commit comments

Comments
 (0)