Skip to content

Commit b5eca74

Browse files
takemi-ohamaclaude
andcommitted
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>
1 parent 431fce8 commit b5eca74

1 file changed

Lines changed: 108 additions & 21 deletions

File tree

lib/devbase/plugin/installer.py

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

3+
import hashlib
34
import os
45
import shutil
56
import subprocess
67
import tempfile
78
import yaml
9+
from dataclasses import dataclass, field
810
from pathlib import Path
911
from typing import Optional
1012

@@ -268,39 +270,108 @@ def _replace_entry(path: Path) -> None:
268270
shutil.rmtree(path)
269271

270272

271-
def _sync_dir(src: Path, dst: Path) -> None:
272-
"""rsync-like merge: make ``dst``'s contents match ``src``'s.
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()
273280

274-
Preserves the inode of ``dst`` and of any subdirectories that exist in
275-
both ``src`` and ``dst``. Compared to ``rmtree(dst) + copytree(src, dst)``,
276-
a process whose CWD is inside ``dst`` (or a subdir present in both) keeps
277-
a valid CWD across this operation — important when the user is working
278-
inside a ``projects/<name>`` symlink that resolves into the plugin tree.
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+
def _sync_dir(src: Path, dst: Path, report: _SyncReport, rel: Path = Path('.')) -> None:
292+
"""Conservatively sync ``src`` → ``dst``, preserving user edits.
293+
294+
Semantics (per file in src/dst):
295+
296+
| src | dst | content | action |
297+
|---|---|---|---|
298+
| exists | missing | - | copy from src (record as ``added``) |
299+
| exists | exists | same | no-op |
300+
| exists | exists | differ | keep dst, write src as ``<name>.new`` (``kept_local``) |
301+
| missing | exists | - | leave dst alone (``preserved_orphans``) |
302+
303+
Preserves the inode of ``dst`` and of subdirectories present in both — a
304+
user whose CWD lives inside ``dst`` (typically via a ``projects/<name>``
305+
symlink resolving into the plugin tree) keeps a valid CWD across updates.
306+
307+
User-only files (orphans) and user-edited files are never destroyed.
279308
"""
280309
dst.mkdir(parents=True, exist_ok=True)
281310

282311
src_entries = {e.name: e for e in src.iterdir()}
283312
dst_entries = {e.name: e for e in dst.iterdir()}
284313

285314
for name, dst_entry in dst_entries.items():
286-
if name not in src_entries:
315+
if name in src_entries:
316+
continue
317+
if name.endswith('.new'):
318+
# `.new` is our own conflict marker — refresh, don't preserve.
287319
_replace_entry(dst_entry)
320+
continue
321+
report.preserved_orphans.append(rel / name)
288322

289323
for name, src_entry in src_entries.items():
290324
dst_entry = dst / name
325+
sub_rel = rel / name
291326
if src_entry.is_symlink():
292327
link_target = os.readlink(src_entry)
328+
if dst_entry.is_symlink() and os.readlink(dst_entry) == link_target:
329+
continue
293330
if dst_entry.is_symlink() or dst_entry.exists():
294-
_replace_entry(dst_entry)
295-
os.symlink(link_target, dst_entry)
331+
# Conflict: leave user's, drop upstream alongside as `.new` symlink.
332+
new_dst = dst_entry.with_name(dst_entry.name + '.new')
333+
if new_dst.is_symlink() or new_dst.exists():
334+
_replace_entry(new_dst)
335+
os.symlink(link_target, new_dst)
336+
report.kept_local.append(sub_rel)
337+
else:
338+
os.symlink(link_target, dst_entry)
339+
report.added.append(sub_rel)
296340
elif src_entry.is_dir():
297-
if dst_entry.exists() and not dst_entry.is_dir():
298-
_replace_entry(dst_entry)
299-
_sync_dir(src_entry, dst_entry)
341+
if dst_entry.is_symlink() or (dst_entry.exists() and not dst_entry.is_dir()):
342+
# Type mismatch: user has a file/symlink where upstream has a dir.
343+
# Drop upstream alongside as `<name>.new/`.
344+
new_dst = dst_entry.with_name(dst_entry.name + '.new')
345+
if new_dst.is_symlink() or (new_dst.exists() and not new_dst.is_dir()):
346+
_replace_entry(new_dst)
347+
_sync_dir(src_entry, new_dst, report, sub_rel)
348+
report.kept_local.append(sub_rel)
349+
else:
350+
already_existed = dst_entry.is_dir()
351+
_sync_dir(src_entry, dst_entry, report, sub_rel)
352+
if not already_existed:
353+
report.added.append(sub_rel)
300354
else:
301-
if dst_entry.is_symlink() or (dst_entry.exists() and dst_entry.is_dir()):
302-
_replace_entry(dst_entry)
303-
shutil.copy2(src_entry, dst_entry)
355+
if not dst_entry.exists() and not dst_entry.is_symlink():
356+
shutil.copy2(src_entry, dst_entry)
357+
report.added.append(sub_rel)
358+
continue
359+
if dst_entry.is_symlink() or dst_entry.is_dir():
360+
# Type mismatch: user has a symlink/dir where upstream has a file.
361+
new_dst = dst_entry.with_name(dst_entry.name + '.new')
362+
if new_dst.is_symlink() or new_dst.exists():
363+
_replace_entry(new_dst)
364+
shutil.copy2(src_entry, new_dst)
365+
report.kept_local.append(sub_rel)
366+
continue
367+
# Both are regular files — compare content.
368+
if _hash_file(src_entry) == _hash_file(dst_entry):
369+
continue
370+
new_dst = dst_entry.with_name(dst_entry.name + '.new')
371+
if new_dst.is_symlink() or new_dst.exists():
372+
_replace_entry(new_dst)
373+
shutil.copy2(src_entry, new_dst)
374+
report.kept_local.append(sub_rel)
304375

305376

306377
def copy_plugin(
@@ -312,10 +383,11 @@ def copy_plugin(
312383
) -> None:
313384
"""Install or update a plugin from a cloned repo into ``plugins/``.
314385
315-
For updates, the existing plugin directory inode is preserved by
316-
syncing contents in place (rsync-like) instead of rmtree+copytree.
317-
This avoids invalidating any shell or editor whose CWD is inside the
318-
plugin (typically via a ``projects/<name>`` symlink).
386+
For updates, contents are synced in place (preserving directory inodes
387+
and user-edited files) instead of rmtree+copytree. User-edited files
388+
are kept as-is; the upstream version of a conflicting file is dropped
389+
alongside with a ``.new`` suffix for the user to diff/merge manually.
390+
Files present only in the user's working tree (orphans) are preserved.
319391
320392
Raises PluginError on failure.
321393
"""
@@ -329,7 +401,22 @@ def copy_plugin(
329401
shutil.copytree(plugin_path, dest)
330402
elif dest.exists():
331403
logger.info("Updating existing plugin '%s'", name)
332-
_sync_dir(plugin_path, dest)
404+
report = _SyncReport()
405+
_sync_dir(plugin_path, dest, report)
406+
if report.kept_local:
407+
logger.warning(
408+
" %d local edit(s) kept; upstream saved as .new alongside:",
409+
len(report.kept_local),
410+
)
411+
for p in report.kept_local[:10]:
412+
logger.warning(" - %s (upstream: %s.new)", p, p.name)
413+
if len(report.kept_local) > 10:
414+
logger.warning(" ... and %d more", len(report.kept_local) - 10)
415+
if report.preserved_orphans:
416+
logger.info(
417+
" %d local-only file(s) preserved (not in upstream)",
418+
len(report.preserved_orphans),
419+
)
333420
else:
334421
shutil.copytree(plugin_path, dest)
335422

0 commit comments

Comments
 (0)