From 665645e2cef4e226db219cf36b1ec50dc0772222 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Wed, 27 May 2026 05:24:28 +0000 Subject: [PATCH 1/8] =?UTF-8?q?chore:=20PLAN04=20release=20branch=20?= =?UTF-8?q?=E4=BD=9C=E6=88=90?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From 79b661fee8abd588b38c7146d3a13b84300e73d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=A4=A7=E6=B5=9C=E6=AF=85=E7=BE=8E?= Date: Thu, 28 May 2026 11:22:53 +0900 Subject: [PATCH 2/8] =?UTF-8?q?feat(plugin):=20repos/=20=E6=B0=B8=E7=B6=9A?= =?UTF-8?q?=E3=82=AF=E3=83=AD=E3=83=BC=E3=83=B3=E3=81=AB=E3=82=88=E3=82=8B?= =?UTF-8?q?=E3=83=97=E3=83=A9=E3=82=B0=E3=82=A4=E3=83=B3=E7=AE=A1=E7=90=86?= =?UTF-8?q?=E3=81=A8=20projects/=20=E7=9B=B4=E6=8E=A5=E3=82=B7=E3=83=B3?= =?UTF-8?q?=E3=83=9C=E3=83=AA=E3=83=83=E3=82=AF=E3=83=AA=E3=83=B3=E3=82=AF?= =?UTF-8?q?=20(#29)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: PLAN04-repos-core Draft PR 作成 * feat(plugin): repos/ 永続クローン + 直接リンク install (PLAN04-repos-core) plugins/ 中間層を廃止し、repos/ に git clone を永続保持して projects/ からシンボリックリンクで直接参照する構造に変更。 - models.py: RegisteredRepository に local_path フィールド追加 - registry.py: get_repos_dir() 追加 - repo_manager.py: repos/ 永続クローン、git pull refresh、dirty check 付き remove - installer.py: repos/ ベースのシンボリックリンク install、repos/ 保護 uninstall、 copy_plugin / _sync_dir 等のコピー系ロジック削除 - syncer.py: InstalledPlugin.path ベース走査、同名衝突時の . suffix リンク - updater.py: git pull ベース update - cli.py: repo remove に --force オプション追加 - .gitignore: repos/ 追加 Co-Authored-By: Claude Opus 4.7 (1M context) * fix(plugin): サブディレクトリ配置・dirty検知・suffix衝突の3件修正 - installer.py / updater.py: rel_path を plugin名ではなく plugin_path.relative_to(devbase_root) で算出し、 registry.yml の path が name と異なるサブディレクトリ配置に対応 - repo_manager.py: upstream未設定時に @{u}..HEAD が失敗して dirty=false となりデータ損失の恐れがあった問題を修正。 upstream未設定時は dirty 扱いにして安全側に倒す - syncer.py: collision suffix を owner のみ → owner--repo に変更し、 同一 owner の複数 repo で同名 project が衝突する問題を修正。 既存 symlink 存在チェックも追加 Co-Authored-By: Claude Opus 4.7 (1M context) * fix(plugin): pull前スナップショット・repo全体更新・clone失敗クリーンアップ - updater: git pull 前に旧 plugin の projects をスナップショットし、 pull 後の migration で旧ディレクトリが消えても移行先を検出可能に - updater: name 指定の update でも同一 repo の全 installed plugin の metadata (version/path) を pull 後に再読み込みして整合性を維持 - repo_manager: repo add で clone 後の registry.yml parse や名前衝突 失敗時に clone_dir を自動削除し、リトライ時の詰まりを防止 - syncer: path.split('/') を Path().parts に変更 (OS 非依存化) Co-Authored-By: Claude Opus 4.7 (1M context) * fix(plugin): round 3 レビュー指摘対応 — 直接 install の自動 repo 登録 + cleanup - installer.py: user/repo:plugin-name 形式で未登録リポジトリを指定した際に 自動で repo add を実行し、既存の直接指定形式を維持 (codex round 3 major) - updater.py: _update_repo_plugins の未使用引数 repo_local_path を削除 (gemini round 3 minor) - repo_manager.py: git_clone を try/except ブロック内に移動し、 部分 clone 失敗時もディレクトリを自動クリーンアップ (gemini round 3 minor) 全 210 テスト PASSED Co-Authored-By: Claude Opus 4.7 (1M context) * fix(plugin): round 4 レビュー指摘対応 — @ref 拒否・refresh メタデータ同期・pull エラー改善 - installer.py: 未登録リポジトリの自動登録時に @ref を明示的に拒否 (永続 clone はデフォルトブランチを追跡するため、pinned ref と矛盾する) - repo_manager.py: refresh_repository で git pull 後に installed plugin の metadata (version/path) を再計算し sync_projects() を実行 - repo_manager.py: _git_pull で upstream tracking branch の有無を事前検査し、 未設定時に具体的な修正手順を含むエラーメッセージを返す Co-Authored-By: Claude Opus 4.7 (1M context) * fix(plugin): round 5 レビュー指摘対応 — NameError修正・エラーメッセージ改善・レガシーrepo移行・batch refresh効率化 - installer.py:131 — @ref 拒否時の未定義変数 `url` を `repo_url` に修正 (NameError 解消) - repo_manager.py:133 — _git_pull の upstream 未設定エラーで detached HEAD/remote未設定を個別判定、remote名を動的取得 - repo_manager.py:379 — refresh_repository に sync パラメータ追加、batch refresh 時は最後に1回だけ sync_projects 実行 - installer.py:223 — legacy repo (local_path 未設定) の自動移行: 初回 install 時に永続 clone を作成して local_path を設定 - テスト追加: @ref 拒否の PluginError テスト、legacy repo migration テスト (計 212 tests PASSED) Co-Authored-By: Claude Opus 4.7 (1M context) * fix(plugin): レガシー移行時の整合性修正 + UX改善 (round 6 review) - installer.py: レガシーrepo移行時に parse_registry_yml で検証してから plugins.yml へ保存するように変更。plugins リストも registry.yml から 最新情報を取得して更新 (major x2 対応) - repo_manager.py: 複数リモート時に origin を優先選択 (minor) - repo_manager.py: detached HEAD エラーに具体的な復帰コマンド例を追加 (minor) Co-Authored-By: Claude Opus 4.7 (1M context) * fix(plugin): print→logger 統一 + SSH/HTTPS URL 重複登録検知 (deferred nit) - installer.py: _install_from_repo 内の print() を logger.info() に統一 - repo_manager.py: add_repository で SSH/HTTPS 形式の URL バリアント重複を _url_to_repos_dirname 正規化により検知し、RepositoryError で拒否 Co-Authored-By: Claude Opus 4.7 (1M context) * fix(plugin): round 1 レビュー指摘対応 — @ref拒否・host付きdirname・refresh snapshot・print→logger・migration テスト - 登録済みrepoへの @ref 指定を PluginError で拒否（codex + gemini 指摘） - _url_to_repos_dirname に host を含め、異なるホストの同名 repo の衝突を防止 - refresh_repository で git pull 前に _snapshot_plugin_projects を取得し _update_repo_plugins に渡すことで、pull 後のディレクトリ変更時も移行可能に - repo_manager.py の残存 print() を logger.info() に統一 - _migrate_removed_plugin / _snapshot_plugin_projects のテスト追加（3件） - refresh の pre_pull_projects 受け渡し検証テスト追加 Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .gitignore | 1 + lib/devbase/cli.py | 2 + lib/devbase/commands/plugin.py | 9 +- lib/devbase/plugin/installer.py | 432 ++++++------- lib/devbase/plugin/models.py | 15 +- lib/devbase/plugin/registry.py | 6 +- lib/devbase/plugin/repo_manager.py | 464 ++++++++++---- lib/devbase/plugin/syncer.py | 98 ++- lib/devbase/plugin/updater.py | 193 ++++-- tests/plugin/__init__.py | 0 tests/plugin/test_repos_core.py | 971 +++++++++++++++++++++++++++++ 11 files changed, 1743 insertions(+), 448 deletions(-) create mode 100644 tests/plugin/__init__.py create mode 100644 tests/plugin/test_repos_core.py diff --git a/.gitignore b/.gitignore index b11348e..f96d56b 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ __pycache__/ .gemini/ .docker-compose.scale.yml plugins.yml +repos/ plugins/*/ !plugins/.gitkeep projects/* diff --git a/lib/devbase/cli.py b/lib/devbase/cli.py index e2fd4be..9cd1ce1 100644 --- a/lib/devbase/cli.py +++ b/lib/devbase/cli.py @@ -244,6 +244,8 @@ def _add_plugin_parser(subparsers): r_remove = pl_repo_sub.add_parser('remove', help='Unregister a repository') r_remove.add_argument('name', help='Repository name') + r_remove.add_argument('--force', action='store_true', + help='Force removal even if repo has uncommitted/unpushed changes') pl_repo_sub.add_parser('list', help='List repositories') diff --git a/lib/devbase/commands/plugin.py b/lib/devbase/commands/plugin.py index 0f1be34..b20d31d 100644 --- a/lib/devbase/commands/plugin.py +++ b/lib/devbase/commands/plugin.py @@ -149,7 +149,8 @@ def cmd_repo(devbase_root: Path, args) -> int: handlers = { 'add': lambda: add_repository(registry, args.url, name=args.name), - 'remove': lambda: remove_repository(registry, args.name), + 'remove': lambda: remove_repository(registry, args.name, + force=getattr(args, 'force', False)), 'list': lambda: show_repositories(registry), 'refresh': lambda: _repo_refresh(registry, args), } @@ -181,9 +182,13 @@ def _repo_refresh(registry, args): errors = [] for repo in repos: try: - refresh_repository(registry, repo.name) + refresh_repository(registry, repo.name, sync=False) except DevbaseError as e: logger.error("%s", e) errors.append(str(e)) + + # Sync once after all repos are refreshed (instead of per-repo) + sync_projects(registry) + if errors: raise DevbaseError(f"{len(errors)} repository refresh(es) failed") diff --git a/lib/devbase/plugin/installer.py b/lib/devbase/plugin/installer.py index 79ca000..747c548 100644 --- a/lib/devbase/plugin/installer.py +++ b/lib/devbase/plugin/installer.py @@ -1,12 +1,9 @@ """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 @@ -49,12 +46,19 @@ def parse_registry_yml(path: Path) -> Optional[RegistryInfo]: ) -def git_clone(url: str, dest: Path, ref: Optional[str] = None) -> None: +def git_clone( + url: str, + dest: Path, + ref: Optional[str] = None, + shallow: bool = True, +) -> None: """Clone a git repository. Raises PluginError on failure. """ - cmd = ['git', 'clone', '--depth', '1'] + cmd = ['git', 'clone'] + if shallow: + cmd.extend(['--depth', '1']) if ref: cmd.extend(['--branch', ref]) cmd.extend([url, str(dest)]) @@ -89,9 +93,7 @@ def install_plugin( """ source = PluginSource.parse(source_str, link=link) plugins_dir = registry.get_plugins_dir() - plugins_dir.mkdir(exist_ok=True) - # Name-only: look up in registered repositories if not source.repo and source.plugin_name: result = registry.find_plugin_in_repos(source.plugin_name) if result: @@ -101,7 +103,7 @@ def install_plugin( ref=source.ref, linked=False, ) _install_from_repo( - registry, repo_source, plugins_dir, install_all=False, + registry, repo_source, install_all=False, ) return raise PluginError( @@ -110,20 +112,49 @@ def install_plugin( "Use 'devbase plugin repo list' to see registered repositories and available plugins." ) - # Resolve repo URL repo_url = resolve_repo_url(source.repo) - # Local path with --link if link and (Path(source.repo).is_dir()): + plugins_dir.mkdir(exist_ok=True) _install_from_local(registry, source, plugins_dir) return - # Git repository (user/repo:plugin-name or URL:plugin-name) + # Auto-register the repository if not already registered, so that + # `devbase plugin install user/repo:plugin-name` keeps working without + # a prior `repo add`. + if not registry.get_repository_by_url(repo_url): + if source.ref: + raise PluginError( + f"Cannot use @{source.ref} with unregistered repository '{repo_url}'.\n" + "Permanent clones track the default branch and do not support pinned refs.\n" + "Register the repository first, then install without @ref:\n" + f" devbase plugin repo add {repo_url}\n" + f" devbase plugin install {repo_url}:{source.plugin_name}" + ) + from .repo_manager import add_repository + try: + add_repository(registry, repo_url) + except Exception as e: + raise PluginError( + f"Repository '{repo_url}' is not registered and auto-registration failed: {e}\n" + "Use 'devbase plugin repo add ' to register manually." + ) + + # Reject @ref on already-registered repos — the permanent clone tracks + # the default branch and does not support pinned refs. This matches the + # validation for *unregistered* repos (line 126-133 above). + if source.ref: + raise PluginError( + f"Cannot use @{source.ref} with registered repository '{repo_url}'.\n" + "Permanent clones track the default branch and do not support pinned refs.\n" + f"Install without @ref:\n" + f" devbase plugin install {repo_url}:{source.plugin_name}" + ) + _install_from_repo( registry, PluginSource( - repo=repo_url, plugin_name=source.plugin_name, ref=source.ref, linked=False, + repo=repo_url, plugin_name=source.plugin_name, ref=None, linked=False, ), - plugins_dir, install_all=install_all, ) @@ -140,10 +171,8 @@ def _install_from_local( local_path = Path(source.repo) if source.plugin_name: - # Specific plugin within the repo plugin_path = local_path / source.plugin_name if not plugin_path.is_dir(): - # Try looking at registry.yml for path mapping reg_info = parse_registry_yml(local_path) if reg_info: for entry in reg_info.plugins: @@ -165,7 +194,7 @@ def _link_plugin( source_display: str, plugins_dir: Path, ) -> None: - """Create a symlink for a local plugin""" + """Create a symlink for a local plugin (--link install only)""" dest = plugins_dir / name if dest.exists() or dest.is_symlink(): logger.warning("Removing existing plugin '%s'", name) @@ -195,281 +224,190 @@ def _link_plugin( def _install_from_repo( registry: PluginRegistry, source: PluginSource, - plugins_dir: Path, install_all: bool = False, ) -> None: - """Install plugin(s) from a git repository. + """Install plugin(s) from a registered repository via symlink to repos/. Raises PluginError on failure. """ - with tempfile.TemporaryDirectory() as tmpdir: - clone_dir = Path(tmpdir) / 'repo' - git_clone(source.repo, clone_dir, source.ref) - - reg_info = parse_registry_yml(clone_dir) - if not reg_info: - raise PluginError("No registry.yml found in repository") - - if install_all: - # Install all plugins from the repo - errors = [] - for entry in reg_info.plugins: - try: - copy_plugin( - registry, entry.name, - clone_dir / entry.path.rstrip('/'), - source.repo, plugins_dir - ) - except PluginError as e: - errors.append(str(e)) - sync_projects(registry) - if errors: - raise PluginError( - "Some plugins failed to install:\n" + "\n".join(errors) - ) - return + repo_reg = registry.get_repository_by_url(source.repo) + if not repo_reg: + raise PluginError( + f"Repository '{source.repo}' is not registered.\n" + "Use 'devbase plugin repo add ' first." + ) - if source.plugin_name: - # Find specific plugin - target_entry = None - for entry in reg_info.plugins: - if entry.name == source.plugin_name: - target_entry = entry - break - - if not target_entry: - available = "\n".join( - f" - {e.name}: {e.description}" for e in reg_info.plugins - ) + if not repo_reg.local_path: + # Legacy repository registered before persistent-clone support. + # Auto-migrate by creating a persistent clone in repos/. + logger.info( + "Migrating repository '%s' to persistent clone...", repo_reg.name, + ) + from .repo_manager import _url_to_repos_dirname + dir_name = _url_to_repos_dirname(repo_reg.url) + repos_dir = registry.get_repos_dir() + repos_dir.mkdir(exist_ok=True) + clone_dir = repos_dir / dir_name + + if not clone_dir.is_dir(): + try: + git_clone(repo_reg.url, clone_dir, shallow=False) + except PluginError as e: raise PluginError( - f"Plugin '{source.plugin_name}' not found in repository\n" - f"Available plugins:\n{available}" + f"Failed to create persistent clone for '{repo_reg.name}': {e}\n" + "Remove and re-add the repository:\n" + f" devbase plugin repo remove {repo_reg.name}\n" + f" devbase plugin repo add {repo_reg.url}" ) - plugin_path = clone_dir / target_entry.path.rstrip('/') - copy_plugin( - registry, target_entry.name, plugin_path, source.repo, plugins_dir + # Validate the clone by parsing registry.yml BEFORE saving to + # plugins.yml. This prevents a broken clone from polluting the + # persisted state. If parsing fails, the old repository entry + # (without local_path) is kept so the user can retry. + reg_info = parse_registry_yml(clone_dir) + if not reg_info: + raise PluginError( + f"No registry.yml found in cloned repository '{repo_reg.name}'.\n" + "Remove and re-add the repository:\n" + f" devbase plugin repo remove {repo_reg.name}\n" + f" devbase plugin repo add {repo_reg.url}" ) - sync_projects(registry) - else: - # No plugin specified - show available - print(f"Available plugins in {source.repo}:") - for entry in reg_info.plugins: - installed = registry.get(entry.name) - status = " (installed)" if installed else "" - print(f" {entry.name}: {entry.description}{status}") - print(f"\nUse 'devbase plugin install {source.repo}:PLUGIN_NAME' to install") - 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 _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() - - -@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) - - -# 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'}) + from .models import RegisteredRepository, AvailablePlugin + local_path = f"repos/{dir_name}" + # Build an up-to-date plugin list from the freshly cloned + # registry.yml instead of carrying over stale metadata. + migrated_plugins = [ + AvailablePlugin( + name=e.name, + description=e.description, + path=e.path, + ) + for e in reg_info.plugins + ] + updated_repo = RegisteredRepository( + name=repo_reg.name, + url=repo_reg.url, + added_at=repo_reg.added_at, + local_path=local_path, + plugins=migrated_plugins, + ) + registry.add_repository(updated_repo) + repo_reg = updated_repo + logger.info("Repository '%s' migrated to %s", repo_reg.name, local_path) -def _sync_dir(src: Path, dst: Path, report: _SyncReport, rel: Path = Path('.')) -> None: - """Conservatively sync ``src`` → ``dst``, preserving user edits. + clone_dir = registry.devbase_root / repo_reg.local_path + if not clone_dir.is_dir(): + raise PluginError( + f"Clone directory not found: {clone_dir}\n" + "Use 'devbase plugin repo remove' and 'repo add' to re-clone." + ) - Semantics (per file in src/dst): + reg_info = parse_registry_yml(clone_dir) + if not reg_info: + raise PluginError("No registry.yml found in repository") + + if install_all: + errors = [] + for entry in reg_info.plugins: + try: + _register_repo_plugin( + registry, entry.name, + clone_dir / entry.path.rstrip('/'), + source.repo, repo_reg.local_path, + ) + except PluginError as e: + errors.append(str(e)) + sync_projects(registry) + if errors: + raise PluginError( + "Some plugins failed to install:\n" + "\n".join(errors) + ) + return - | 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``) | + if source.plugin_name: + target_entry = None + for entry in reg_info.plugins: + if entry.name == source.plugin_name: + target_entry = entry + break + + if not target_entry: + available = "\n".join( + f" - {e.name}: {e.description}" for e in reg_info.plugins + ) + raise PluginError( + f"Plugin '{source.plugin_name}' not found in repository\n" + f"Available plugins:\n{available}" + ) - 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). + _register_repo_plugin( + registry, target_entry.name, + clone_dir / target_entry.path.rstrip('/'), + source.repo, repo_reg.local_path, + ) + sync_projects(registry) + else: + logger.info("Available plugins in %s:", source.repo) + for entry in reg_info.plugins: + installed = registry.get(entry.name) + status = " (installed)" if installed else "" + logger.info(" %s: %s%s", entry.name, entry.description, status) + logger.info( + "Use 'devbase plugin install %s:PLUGIN_NAME' to install", + source.repo, + ) + raise PluginError("No plugin name specified") - 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) - - 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 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 ( - 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: - continue - if dst_entry.is_symlink() or dst_entry.exists(): - # 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.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 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( +def _register_repo_plugin( registry: PluginRegistry, name: str, plugin_path: Path, - source_display: str, - plugins_dir: Path, + source_url: str, + repo_local_path: str, ) -> None: - """Install or update a plugin from a cloned repo into ``plugins/``. - - 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. - """ + """Register a plugin from repos/ (no file copy, just metadata).""" if not plugin_path.is_dir(): raise PluginError(f"Plugin directory not found: {plugin_path}") - dest = plugins_dir / name - 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) - 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) - - info = load_plugin_info(dest) + info = load_plugin_info(plugin_path) version = info.version if info else '0.1.0' + # Use the actual plugin_path relative to devbase_root so that + # subdirectory plugins (registry.yml path != name) resolve correctly. + rel_path = str(plugin_path.relative_to(registry.devbase_root)) + registry.add(InstalledPlugin( name=name, version=version, - source=source_display, + source=source_url, installed_at=registry.now_iso(), - path=f"plugins/{name}", + path=rel_path, linked=False, )) - logger.info("Installed plugin '%s' (v%s)", name, version) + logger.info("Installed plugin '%s' (v%s) from repos/", name, version) def uninstall_plugin(registry: PluginRegistry, name: str) -> None: """Uninstall a plugin. + For repos/-based plugins: removes registry entry and syncs symlinks only. + For --link plugins: removes the symlink in plugins/. + Raises PluginError if not installed. """ plugin = registry.get(name) if not plugin: raise PluginError(f"Plugin '{name}' is not installed") - plugin_dir = registry.devbase_root / plugin.path - if plugin_dir.is_symlink(): - plugin_dir.unlink() - elif plugin_dir.is_dir(): - shutil.rmtree(plugin_dir) + if plugin.linked: + plugin_dir = registry.devbase_root / plugin.path + if plugin_dir.is_symlink(): + plugin_dir.unlink() + elif plugin_dir.is_dir(): + shutil.rmtree(plugin_dir) registry.remove(name) logger.info("Uninstalled plugin '%s'", name) diff --git a/lib/devbase/plugin/models.py b/lib/devbase/plugin/models.py index 597a5e4..24b87e9 100644 --- a/lib/devbase/plugin/models.py +++ b/lib/devbase/plugin/models.py @@ -151,18 +151,22 @@ class RegisteredRepository: name: str url: str added_at: str = "" + local_path: str = "" plugins: list[AvailablePlugin] = field(default_factory=list) def to_dict(self) -> dict: - return { + d: dict = { 'name': self.name, 'url': self.url, 'added_at': self.added_at, - 'plugins': [ - {'name': p.name, 'description': p.description, 'path': p.path} - for p in self.plugins - ], } + if self.local_path: + d['local_path'] = self.local_path + d['plugins'] = [ + {'name': p.name, 'description': p.description, 'path': p.path} + for p in self.plugins + ] + return d @classmethod def from_dict(cls, data: dict) -> 'RegisteredRepository': @@ -178,6 +182,7 @@ def from_dict(cls, data: dict) -> 'RegisteredRepository': name=data.get('name', ''), url=data.get('url', ''), added_at=data.get('added_at', ''), + local_path=data.get('local_path', ''), plugins=plugins, ) diff --git a/lib/devbase/plugin/registry.py b/lib/devbase/plugin/registry.py index b0940ec..f852fcb 100644 --- a/lib/devbase/plugin/registry.py +++ b/lib/devbase/plugin/registry.py @@ -85,9 +85,13 @@ def remove(self, name: str) -> bool: return False def get_plugins_dir(self) -> Path: - """Get the plugins directory path""" + """Get the plugins directory path (used for --link installs only)""" return self.devbase_root / 'plugins' + def get_repos_dir(self) -> Path: + """Get the repos directory path (persistent git clones)""" + return self.devbase_root / 'repos' + def get_projects_dir(self) -> Path: """Get the projects directory path""" return self.devbase_root / 'projects' diff --git a/lib/devbase/plugin/repo_manager.py b/lib/devbase/plugin/repo_manager.py index 211784a..0222caa 100644 --- a/lib/devbase/plugin/repo_manager.py +++ b/lib/devbase/plugin/repo_manager.py @@ -1,6 +1,6 @@ """Repository management - handles repo add/remove/list/refresh operations""" -import tempfile +import subprocess import yaml from pathlib import Path from typing import Optional @@ -33,18 +33,176 @@ def _get_official_registry_url() -> str: return DEFAULT_OFFICIAL_REGISTRY +def _derive_repo_name(url: str) -> str: + """Derive a repository name from a URL using owner/repo format. + + Examples: + https://github.com/devbasex/devbase-samples.git -> devbasex/devbase-samples + git@github.com:user/my-repo.git -> user/my-repo + """ + name = url.rstrip('/') + if name.endswith('.git'): + name = name[:-4] + if ':' in name and '@' in name: + return name.rsplit(':', 1)[-1] + from urllib.parse import urlparse + path = urlparse(name).path.strip('/') + segments = path.split('/') + if len(segments) >= 2: + return f"{segments[-2]}/{segments[-1]}" + return segments[-1] if segments else name + + +def _extract_host(url: str) -> str: + """Extract the hostname from a git URL (HTTPS or SSH). + + Examples: + https://github.com/devbasex/repo.git -> github.com + git@gitlab.com:user/repo.git -> gitlab.com + """ + stripped = url.rstrip('/') + if '@' in stripped and ':' in stripped and not stripped.startswith('http'): + # SSH form: git@host:owner/repo.git + after_at = stripped.split('@', 1)[1] + return after_at.split(':', 1)[0] + from urllib.parse import urlparse + parsed = urlparse(stripped) + return parsed.hostname or "unknown" + + +def _url_to_repos_dirname(url: str) -> str: + """Convert a repo URL to a repos/ directory name using host--owner--repo format. + + Includes the hostname so that repos from different hosts (e.g. github.com + vs gitlab.com) with the same owner/repo do not collide. SSH and HTTPS + variants of the same host produce the same dirname, enabling duplicate + detection. + + Examples: + https://github.com/devbasex/devbase-samples.git -> github.com--devbasex--devbase-samples + git@github.com:user/my-repo.git -> github.com--user--my-repo + https://gitlab.com/user/my-repo.git -> gitlab.com--user--my-repo + """ + host = _extract_host(url) + owner_repo = _derive_repo_name(url) + return f"{host}--{owner_repo.replace('/', '--')}" + + +def _is_repo_dirty(repo_dir: Path) -> tuple[bool, str]: + """Check if a git repository has uncommitted or unpushed changes. + + Returns (is_dirty, description). + """ + issues = [] + + try: + result = subprocess.run( + ['git', 'status', '--porcelain'], + capture_output=True, text=True, cwd=str(repo_dir), + ) + if result.returncode == 0 and result.stdout.strip(): + issues.append("uncommitted changes") + except subprocess.CalledProcessError: + pass + + try: + # Check if upstream tracking branch exists + upstream_check = subprocess.run( + ['git', 'rev-parse', '--abbrev-ref', '@{u}'], + capture_output=True, text=True, cwd=str(repo_dir), + ) + if upstream_check.returncode == 0: + # Upstream exists — check for unpushed commits + result = subprocess.run( + ['git', 'log', '--oneline', '@{u}..HEAD'], + capture_output=True, text=True, cwd=str(repo_dir), + ) + if result.returncode == 0 and result.stdout.strip(): + issues.append("unpushed commits") + else: + # No upstream tracking branch — local commits may be lost + # if deleted, so treat as dirty to be safe + issues.append("no upstream tracking branch (local-only commits may exist)") + except subprocess.CalledProcessError: + pass + + if issues: + return True, ", ".join(issues) + return False, "" + + +def _git_pull(repo_dir: Path) -> None: + """Run git pull in a repository directory. + + Raises PluginError on failure. Detects missing upstream tracking branch + and provides an actionable error message. + """ + # Pre-check: verify an upstream tracking branch is set. + # Without it, `git pull` will fail with a confusing message. + upstream = subprocess.run( + ['git', 'rev-parse', '--abbrev-ref', '@{u}'], + capture_output=True, text=True, cwd=str(repo_dir), + ) + if upstream.returncode != 0: + # Detect current branch name + branch_result = subprocess.run( + ['git', 'branch', '--show-current'], + capture_output=True, text=True, cwd=str(repo_dir), + ) + current_branch = branch_result.stdout.strip() if branch_result.returncode == 0 else "" + + # Detect the first available remote (usually "origin") + remote_result = subprocess.run( + ['git', 'remote'], + capture_output=True, text=True, cwd=str(repo_dir), + ) + remote_name = "" + if remote_result.returncode == 0 and remote_result.stdout.strip(): + remotes = remote_result.stdout.strip().splitlines() + # Prefer "origin" when multiple remotes exist + remote_name = "origin" if "origin" in remotes else remotes[0] + + if not current_branch: + raise PluginError( + f"git pull failed in {repo_dir}: HEAD is detached.\n" + "This can happen if the branch was changed manually in repos/.\n" + f"Check out a branch first, then retry:\n" + f" git -C {repo_dir} checkout main" + ) + if not remote_name: + raise PluginError( + f"git pull failed in {repo_dir}: no remote configured.\n" + f"Current branch '{current_branch}' has no remote to pull from." + ) + raise PluginError( + f"git pull failed in {repo_dir}: no upstream tracking branch.\n" + f"Current branch '{current_branch}' has no remote to pull from.\n" + "This can happen if the branch was changed manually in repos/.\n" + f"Fix with: git -C {repo_dir} branch --set-upstream-to={remote_name}/{current_branch}" + ) + + try: + subprocess.run( + ['git', 'pull'], + check=True, capture_output=True, text=True, cwd=str(repo_dir), + ) + except subprocess.CalledProcessError as e: + raise PluginError( + f"git pull failed in {repo_dir}: {e.stderr.strip()}" + ) + + def add_repository( registry: PluginRegistry, url: str, name: Optional[str] = None, ) -> None: - """Register a repository: clone -> read registry.yml -> save to plugins.yml. + """Register a repository: clone to repos/ -> read registry.yml -> save to plugins.yml. Raises RepositoryError on failure. """ repo_url = resolve_repo_url(url) - # Check if already registered by URL existing = registry.get_repository_by_url(repo_url) if existing: raise RepositoryError( @@ -52,75 +210,122 @@ def add_repository( "Use 'devbase plugin repo refresh' to update the plugin list." ) - with tempfile.TemporaryDirectory() as tmpdir: - clone_dir = Path(tmpdir) / 'repo' - try: - git_clone(repo_url, clone_dir) - except PluginError as e: - raise RepositoryError(str(e)) - - try: - reg_info = parse_registry_yml(clone_dir) - except PluginError as e: - raise RepositoryError(str(e)) + # Detect duplicate registration via SSH/HTTPS URL variants. + # _url_to_repos_dirname normalizes both forms to the same dirname, + # so we compare against existing repos to prevent redundant clones. + new_dirname = _url_to_repos_dirname(repo_url) + for repo in registry.list_repositories(): + if _url_to_repos_dirname(repo.url) == new_dirname and repo.url != repo_url: + logger.warning( + "Repository '%s' (%s) appears to be the same as '%s' " + "(URL variant: SSH vs HTTPS). Skipping duplicate registration.", + repo.name, repo.url, repo_url, + ) + raise RepositoryError( + f"Repository already registered under a different URL variant: " + f"{repo.name} ({repo.url})\n" + "Both SSH and HTTPS URLs resolve to the same repository.\n" + "Use 'devbase plugin repo refresh' to update the existing entry." + ) + + repos_dir = registry.get_repos_dir() + repos_dir.mkdir(exist_ok=True) + + dir_name = _url_to_repos_dirname(repo_url) + clone_dir = repos_dir / dir_name + + if clone_dir.exists(): + raise RepositoryError( + f"Directory already exists: {clone_dir}\n" + "The repository may have been previously added. " + "Remove the directory manually or use a different --name." + ) + + try: + git_clone(repo_url, clone_dir, shallow=False) + + reg_info = parse_registry_yml(clone_dir) if not reg_info: raise RepositoryError(f"No registry.yml found in {repo_url}") - # Determine repo name: explicit --name > registry.yml name > owner/repo from URL derived_name = _derive_repo_name(repo_url) candidate_name = name or reg_info.name or derived_name - # On name collision, fall back to owner/repo format if not already used if registry.get_repository(candidate_name) and candidate_name != derived_name: candidate_name = derived_name repo_name = candidate_name - # Check name collision if registry.get_repository(repo_name): raise RepositoryError( f"Repository name '{repo_name}' already exists.\n" "Use --name to specify a different name." ) - - plugins = [ - AvailablePlugin( - name=e.name, - description=e.description, - path=e.path, - ) - for e in reg_info.plugins - ] - - repo = RegisteredRepository( - name=repo_name, - url=repo_url, - added_at=registry.now_iso(), - plugins=plugins, + except Exception: + # Clean up the cloned directory so a retry won't fail with + # "Directory already exists". This also handles partial clones + # (e.g. disk full, network interruption mid-clone). + import shutil as _shutil + if clone_dir.is_dir(): + _shutil.rmtree(clone_dir) + raise + + plugins = [ + AvailablePlugin( + name=e.name, + description=e.description, + path=e.path, ) - registry.add_repository(repo) - - logger.info("Repository registered: %s (%s)", repo_name, repo_url) - if plugins: - print("Available plugins:") - for p in plugins: - installed = registry.get(p.name) - status = " (installed)" if installed else "" - print(f" - {p.name}: {p.description}{status}") - - -def remove_repository(registry: PluginRegistry, name: str) -> None: - """Remove a repository registration and uninstall all plugins from it. + for e in reg_info.plugins + ] + + local_path = f"repos/{dir_name}" + + repo = RegisteredRepository( + name=repo_name, + url=repo_url, + added_at=registry.now_iso(), + local_path=local_path, + plugins=plugins, + ) + registry.add_repository(repo) + + logger.info("Repository registered: %s (%s)", repo_name, repo_url) + if plugins: + logger.info("Available plugins:") + for p in plugins: + installed = registry.get(p.name) + status = " (installed)" if installed else "" + logger.info(" - %s: %s%s", p.name, p.description, status) + + +def remove_repository( + registry: PluginRegistry, + name: str, + force: bool = False, +) -> None: + """Remove a repository registration, uninstall plugins, and delete repos/ clone. - Raises RepositoryError if not found. + Raises RepositoryError if not found or if repos/ is dirty (without --force). """ + import shutil from .installer import uninstall_plugin repo = registry.get_repository(name) if not repo: raise RepositoryError(f"Repository '{name}' not found.") - # Uninstall all plugins installed from this repository + repos_dir = registry.get_repos_dir() + repo_clone_dir = registry.devbase_root / repo.local_path if repo.local_path else None + + if repo_clone_dir and repo_clone_dir.is_dir() and not force: + is_dirty, description = _is_repo_dirty(repo_clone_dir) + if is_dirty: + raise RepositoryError( + f"Repository '{name}' has {description} in {repo_clone_dir}.\n" + "Commit/push your changes first, or use --force to delete anyway." + ) + installed = registry.list_installed() plugins_to_remove = [p for p in installed if p.source == repo.url] for plugin in plugins_to_remove: @@ -128,6 +333,11 @@ def remove_repository(registry: PluginRegistry, name: str) -> None: uninstall_plugin(registry, plugin.name) registry.remove_repository(name) + + if repo_clone_dir and repo_clone_dir.is_dir(): + shutil.rmtree(repo_clone_dir) + logger.info("Removed clone directory: %s", repo_clone_dir) + logger.info("Repository removed: %s", name) @@ -142,23 +352,31 @@ def show_repositories(registry: PluginRegistry) -> None: installed_names = {p.name for p in registry.list_installed()} for repo in repos: - print(f"{repo.name} ({repo.url})") + local_info = f" [{repo.local_path}]" if repo.local_path else "" + logger.info("%s (%s)%s", repo.name, repo.url, local_info) if repo.plugins: for p in repo.plugins: status = " [installed]" if p.name in installed_names else "" - print(f" - {p.name}: {p.description}{status}") + logger.info(" - %s: %s%s", p.name, p.description, status) else: - print(" (no plugins)") - print() + logger.info(" (no plugins)") + logger.info("") - print(f"Total: {len(repos)} repository(ies)") + logger.info("Total: %d repository(ies)", len(repos)) def refresh_repository( registry: PluginRegistry, name: str, + *, + sync: bool = True, ) -> None: - """Refresh plugin list for a registered repository (re-clone -> update cache). + """Refresh plugin list for a registered repository (git pull + re-read registry.yml). + + Args: + sync: If True (default), call sync_projects after updating metadata. + Set to False when refreshing multiple repositories in a batch to + avoid redundant sync calls — the caller should sync once at the end. Raises RepositoryError if not found. """ @@ -166,44 +384,91 @@ def refresh_repository( if not repo: raise RepositoryError(f"Repository '{name}' not found.") - with tempfile.TemporaryDirectory() as tmpdir: - clone_dir = Path(tmpdir) / 'repo' - try: - git_clone(repo.url, clone_dir) - except PluginError as e: - raise RepositoryError(str(e)) - - try: - reg_info = parse_registry_yml(clone_dir) - except PluginError as e: - raise RepositoryError(str(e)) - if not reg_info: - raise RepositoryError(f"No registry.yml found in {repo.url}") + if not repo.local_path: + raise RepositoryError( + f"Repository '{name}' has no local clone path. " + "Remove and re-add the repository to create a persistent clone." + ) - plugins = [ - AvailablePlugin( - name=e.name, - description=e.description, - path=e.path, - ) - for e in reg_info.plugins - ] - - updated_repo = RegisteredRepository( - name=repo.name, - url=repo.url, - added_at=repo.added_at, - plugins=plugins, + clone_dir = registry.devbase_root / repo.local_path + if not clone_dir.is_dir(): + raise RepositoryError( + f"Clone directory not found: {clone_dir}\n" + "Remove and re-add the repository to re-clone." ) - registry.add_repository(updated_repo) - logger.info("Repository refreshed: %s", repo.name) - if plugins: - print("Available plugins:") - for p in plugins: - installed = registry.get(p.name) - status = " (installed)" if installed else "" - print(f" - {p.name}: {p.description}{status}") + # Snapshot installed plugin project names BEFORE git pull so that + # migration can still detect where old projects moved even after + # the working tree is updated by pull. + from .updater import _snapshot_plugin_projects + installed = registry.list_installed() + repo_installed = [p for p in installed if p.source == repo.url and not p.linked] + pre_pull_projects = _snapshot_plugin_projects(registry, repo_installed) + + try: + _git_pull(clone_dir) + except PluginError as e: + raise RepositoryError(str(e)) + + try: + reg_info = parse_registry_yml(clone_dir) + except PluginError as e: + raise RepositoryError(str(e)) + if not reg_info: + raise RepositoryError(f"No registry.yml found in {repo.url}") + + old_plugin_names = {p.name for p in repo.plugins} + + plugins = [ + AvailablePlugin( + name=e.name, + description=e.description, + path=e.path, + ) + for e in reg_info.plugins + ] + new_plugin_names = {p.name for p in plugins} + + installed_names = {p.name for p in installed} + removed_installed = (old_plugin_names - new_plugin_names) & installed_names + if removed_installed: + for pname in sorted(removed_installed): + logger.warning( + "Installed plugin '%s' no longer exists in registry.yml of '%s'", + pname, name, + ) + + updated_repo = RegisteredRepository( + name=repo.name, + url=repo.url, + added_at=repo.added_at, + local_path=repo.local_path, + plugins=plugins, + ) + registry.add_repository(updated_repo) + + # After pull, update installed plugin metadata (version, path) and + # re-sync project symlinks so that registry.yml changes (e.g. renamed + # paths) are reflected in the installed state. + from .updater import _update_repo_plugins + from .syncer import sync_projects + repo_errors = _update_repo_plugins( + registry, repo.url, clone_dir, + pre_pull_projects=pre_pull_projects, + ) + if repo_errors: + for err in repo_errors: + logger.warning(" %s", err) + if sync: + sync_projects(registry) + + logger.info("Repository refreshed: %s", repo.name) + if plugins: + logger.info("Available plugins:") + for p in plugins: + installed_p = registry.get(p.name) + status = " (installed)" if installed_p else "" + logger.info(" - %s: %s%s", p.name, p.description, status) def add_official_repository(registry: PluginRegistry) -> bool: @@ -214,7 +479,6 @@ def add_official_repository(registry: PluginRegistry) -> bool: """ official_url = _get_official_registry_url() - # Already registered? if registry.get_repository_by_url(official_url): return True @@ -224,25 +488,3 @@ def add_official_repository(registry: PluginRegistry) -> bool: except Exception as e: logger.warning("Could not register official repository: %s", e) return False - - -def _derive_repo_name(url: str) -> str: - """Derive a repository name from a URL using owner/repo format. - - Examples: - https://github.com/devbasex/devbase-samples.git -> devbasex/devbase-samples - git@github.com:user/my-repo.git -> user/my-repo - """ - name = url.rstrip('/') - if name.endswith('.git'): - name = name[:-4] - # Handle git@ SSH URLs (git@github.com:owner/repo) - if ':' in name and '@' in name: - return name.rsplit(':', 1)[-1] - # HTTPS URLs: extract owner/repo from URL path - from urllib.parse import urlparse - path = urlparse(name).path.strip('/') - segments = path.split('/') - if len(segments) >= 2: - return f"{segments[-2]}/{segments[-1]}" - return segments[-1] if segments else name diff --git a/lib/devbase/plugin/syncer.py b/lib/devbase/plugin/syncer.py index 9ac3762..771b4ed 100644 --- a/lib/devbase/plugin/syncer.py +++ b/lib/devbase/plugin/syncer.py @@ -40,84 +40,126 @@ def discover_projects(plugin_dir: Path) -> list[str]: ] +def _extract_owner(plugin: InstalledPlugin) -> str: + """Extract a unique suffix identifier from a plugin for collision resolution. + + For repos/-based plugins: full owner--repo dirname from repos/--/... + This ensures uniqueness when the same owner has multiple repos. + For --link plugins: basename of the source path + """ + if plugin.linked: + return Path(plugin.source).name if plugin.source else plugin.name + + parts = Path(plugin.path).parts + if len(parts) >= 2 and parts[0] == 'repos': + # Return full dir_name (owner--repo) to avoid collision + # between repos from the same owner + return parts[1] + return plugin.name + + def sync_projects(registry: PluginRegistry, verbose: bool = True) -> int: """Synchronize project symlinks from all installed plugins. - Creates symlinks in projects/ pointing to plugins/*/projects/* + Creates symlinks in projects/ pointing to plugin directories: + - repos/-based plugins: projects/ -> ../repos/--//projects/ + - --link plugins: projects/ -> ../plugins//projects/ + + On name collision, the winner (highest priority) gets the bare name, + losers get . suffix symlinks. Returns: Number of symlinks created """ - plugins_dir = registry.get_plugins_dir() projects_dir = registry.get_projects_dir() - - # Ensure projects/ exists projects_dir.mkdir(exist_ok=True) - # Collect all existing non-symlink entries in projects/ (real directories) real_projects = set() for entry in projects_dir.iterdir(): if not entry.is_symlink() and entry.is_dir(): real_projects.add(entry.name) - # Remove existing symlinks (clean slate) for entry in projects_dir.iterdir(): if entry.is_symlink(): entry.unlink() - if not plugins_dir.is_dir(): + installed = registry.list_installed() + if not installed: if verbose: - logger.info("plugins/ directory does not exist yet") + logger.info("No plugins installed") return 0 - # Build priority-sorted list of (project_name, plugin_name, plugin_priority, plugin_dir) - project_candidates: dict[str, list[tuple[str, int, Path]]] = {} + project_candidates: dict[str, list[tuple[InstalledPlugin, int, Path]]] = {} - installed = registry.list_installed() - installed_names = {p.name for p in installed} - - for plugin_entry in sorted(plugins_dir.iterdir()): - if not plugin_entry.is_dir() or plugin_entry.name.startswith('.'): - continue - if plugin_entry.name not in installed_names: + for plugin in installed: + plugin_dir = registry.devbase_root / plugin.path + if not plugin_dir.is_dir(): + if verbose: + logger.warning("Plugin directory missing: %s", plugin.path) continue - info = load_plugin_info(plugin_entry) + info = load_plugin_info(plugin_dir) priority = info.priority if info else 0 - for proj_name in discover_projects(plugin_entry): + for proj_name in discover_projects(plugin_dir): if proj_name not in project_candidates: project_candidates[proj_name] = [] project_candidates[proj_name].append( - (plugin_entry.name, priority, plugin_entry) + (plugin, priority, plugin_dir) ) - # Create symlinks with priority resolution created = 0 for proj_name, candidates in sorted(project_candidates.items()): - # Skip if real directory exists if proj_name in real_projects: if verbose: logger.info(" Skip: %s (real directory exists)", proj_name) continue - # Sort by priority (highest first), then by name (alphabetical) - candidates.sort(key=lambda c: (-c[1], c[0])) + candidates.sort(key=lambda c: (-c[1], c[0].name)) winner_plugin, winner_priority, winner_dir = candidates[0] if len(candidates) > 1 and verbose: logger.warning( "Project '%s' exists in multiple plugins — using '%s' (priority: %d)", - proj_name, winner_plugin, winner_priority, + proj_name, winner_plugin.name, winner_priority, ) - - # Create relative symlink - target = Path('..') / 'plugins' / winner_plugin / 'projects' / proj_name + for loser_plugin, _, _ in candidates[1:]: + owner = _extract_owner(loser_plugin) + logger.info( + " Also available as: projects/%s.%s", + proj_name, owner, + ) + + target = _make_relative_target(winner_plugin, proj_name) link_path = projects_dir / proj_name link_path.symlink_to(target) created += 1 + if len(candidates) > 1: + for loser_plugin, _, loser_dir in candidates[1:]: + owner = _extract_owner(loser_plugin) + suffix_name = f"{proj_name}.{owner}" + + if suffix_name in real_projects: + if verbose: + logger.info(" Skip: %s (real directory exists)", suffix_name) + continue + + suffix_target = _make_relative_target(loser_plugin, proj_name) + suffix_link = projects_dir / suffix_name + if suffix_link.exists() or suffix_link.is_symlink(): + if verbose: + logger.warning(" Skip: %s (symlink already exists)", suffix_name) + continue + suffix_link.symlink_to(suffix_target) + created += 1 + if verbose: logger.info("Synced %d project(s) from %d plugin(s)", created, len(installed)) return created + + +def _make_relative_target(plugin: InstalledPlugin, proj_name: str) -> Path: + """Build the relative symlink target from projects/ to a plugin's project.""" + return Path('..') / plugin.path / 'projects' / proj_name diff --git a/lib/devbase/plugin/updater.py b/lib/devbase/plugin/updater.py index 4f1045b..4fdd163 100644 --- a/lib/devbase/plugin/updater.py +++ b/lib/devbase/plugin/updater.py @@ -1,16 +1,15 @@ """Plugin updater - handles update and migration operations""" -import shutil -import tempfile from pathlib import Path from typing import Optional from devbase.errors import PluginError from devbase.log import get_logger -from .installer import git_clone, resolve_repo_url, parse_registry_yml, copy_plugin +from .installer import parse_registry_yml from .models import InstalledPlugin, RegistryInfo from .registry import PluginRegistry +from .repo_manager import _git_pull from .syncer import sync_projects, discover_projects logger = get_logger("devbase.plugin.updater") @@ -33,30 +32,33 @@ def _migrate_removed_plugin( plugin: InstalledPlugin, clone_dir: Path, reg_info: RegistryInfo, - plugins_dir: Path, + pre_pull_projects: Optional[set[str]] = None, ) -> bool: """Migrate a plugin that no longer exists in the source. Detects which new plugins contain the old plugin's projects and replaces the old plugin with them. + + Args: + pre_pull_projects: Project names captured BEFORE git pull. + If provided, used instead of reading the (now-updated) working tree + so that renamed/moved plugin directories are still detected. """ - old_plugin_dir = registry.devbase_root / plugin.path - old_projects: set[str] = set() - if old_plugin_dir.is_dir(): - old_projects = set(discover_projects(old_plugin_dir)) + if pre_pull_projects is not None: + old_projects = pre_pull_projects + else: + old_plugin_dir = registry.devbase_root / plugin.path + old_projects = set() + if old_plugin_dir.is_dir(): + old_projects = set(discover_projects(old_plugin_dir)) if not old_projects: logger.info(" Plugin '%s' has no projects — removing", plugin.name) - plugin_dir = plugins_dir / plugin.name - if plugin_dir.is_dir(): - shutil.rmtree(plugin_dir) registry.remove(plugin.name) return True - # Build project → new_plugin mapping from source project_to_plugin = _discover_source_projects(clone_dir, reg_info) - # Find which new plugins contain the old projects replacement_plugins: dict[str, list[str]] = {} unmapped_projects: list[str] = [] for proj in sorted(old_projects): @@ -78,29 +80,112 @@ def _migrate_removed_plugin( for p in unmapped_projects: logger.warning(" - %s", p) - # Remove old plugin - old_dir = plugins_dir / plugin.name - if old_dir.is_dir(): - shutil.rmtree(old_dir) registry.remove(plugin.name) - # Install replacement plugins (skip already installed ones) + repo_reg = registry.get_repository_by_url(plugin.source) + repo_local_path = repo_reg.local_path if repo_reg else "" + for new_name in sorted(replacement_plugins): if registry.get(new_name): logger.info(" Skip: '%s' already installed", new_name) continue entry = next((e for e in reg_info.plugins if e.name == new_name), None) - if entry: + if entry and repo_local_path: + from .installer import _register_repo_plugin plugin_path = clone_dir / entry.path.rstrip('/') - copy_plugin( - registry, entry.name, plugin_path, plugin.source, plugins_dir, + _register_repo_plugin( + registry, entry.name, plugin_path, + plugin.source, repo_local_path, ) return True +def _snapshot_plugin_projects( + registry: PluginRegistry, + plugins: list[InstalledPlugin], +) -> dict[str, set[str]]: + """Snapshot project names for each plugin BEFORE git pull. + + Returns {plugin_name: {project_names}} so migration can detect + where old projects moved even after the working tree is updated. + """ + result: dict[str, set[str]] = {} + for plugin in plugins: + plugin_dir = registry.devbase_root / plugin.path + if plugin_dir.is_dir(): + result[plugin.name] = set(discover_projects(plugin_dir)) + else: + result[plugin.name] = set() + return result + + +def _update_repo_plugins( + registry: PluginRegistry, + repo_url: str, + clone_dir: Path, + pre_pull_projects: Optional[dict[str, set[str]]] = None, +) -> list[str]: + """Re-read registry.yml and update ALL installed plugins from the given repo. + + After git pull updates the working tree, every installed plugin from the + same repository must have its plugins.yml metadata (version, path) refreshed + — not just the one the user asked for. + + Args: + pre_pull_projects: {plugin_name: {project_names}} captured before pull. + Passed to _migrate_removed_plugin so it can detect project moves + even when the old directory was renamed/deleted by pull. + + Returns a list of error messages (empty on full success). + """ + reg_info = parse_registry_yml(clone_dir) + if not reg_info: + return [f"No registry.yml in source for repo '{repo_url}'"] + + errors: list[str] = [] + installed = registry.list_installed() + repo_plugins = [p for p in installed if p.source == repo_url and not p.linked] + + for plugin in repo_plugins: + target_entry = next( + (e for e in reg_info.plugins if e.name == plugin.name), None, + ) + + if not target_entry: + logger.info(" Plugin '%s' no longer exists in source", plugin.name) + snapshot = ( + pre_pull_projects.get(plugin.name) + if pre_pull_projects else None + ) + if not _migrate_removed_plugin( + registry, plugin, clone_dir, reg_info, + pre_pull_projects=snapshot, + ): + errors.append(f"Migration failed for '{plugin.name}'") + continue + + plugin_path = clone_dir / target_entry.path.rstrip('/') + from .syncer import load_plugin_info + info = load_plugin_info(plugin_path) + version = info.version if info else '0.1.0' + + rel_path = str(plugin_path.relative_to(registry.devbase_root)) + registry.add(InstalledPlugin( + name=plugin.name, + version=version, + source=plugin.source, + installed_at=plugin.installed_at, + path=rel_path, + linked=False, + )) + logger.info("Updated plugin '%s' (v%s)", plugin.name, version) + + return errors + + def update_plugin(registry: PluginRegistry, name: Optional[str] = None) -> None: - """Update a plugin (or all if name is None). + """Update a plugin (or all if name is None) via git pull. Raises PluginError on failure. """ @@ -116,7 +201,13 @@ def update_plugin(registry: PluginRegistry, name: Optional[str] = None) -> None: if name and not targets: raise PluginError(f"Plugin '{name}' is not installed") + # Snapshot project lists BEFORE pull so migration can detect moves + # even after the working tree is overwritten by git pull. + _pre_pull_projects = _snapshot_plugin_projects(registry, installed) + + updated_repos: set[str] = set() errors = [] + for plugin in targets: if plugin.linked: logger.info("Skip: '%s' is locally linked (update manually)", plugin.name) @@ -129,45 +220,39 @@ def update_plugin(registry: PluginRegistry, name: Optional[str] = None) -> None: ) continue - logger.info("Updating '%s' from %s...", plugin.name, plugin.source) + repo_reg = registry.get_repository_by_url(plugin.source) + if not repo_reg or not repo_reg.local_path: + errors.append( + f"Plugin '{plugin.name}': repository not found or has no local clone. " + "Use 'devbase plugin repo add' to re-register." + ) + continue - repo_url = resolve_repo_url(plugin.source) - plugins_dir = registry.get_plugins_dir() + clone_dir = registry.devbase_root / repo_reg.local_path + if not clone_dir.is_dir(): + errors.append( + f"Plugin '{plugin.name}': clone directory not found: {clone_dir}" + ) + continue - with tempfile.TemporaryDirectory() as tmpdir: - clone_dir = Path(tmpdir) / 'repo' + if repo_reg.url not in updated_repos: + logger.info("Updating '%s' via git pull in %s...", plugin.name, clone_dir) try: - git_clone(repo_url, clone_dir) + _git_pull(clone_dir) except PluginError as e: errors.append(str(e)) continue + updated_repos.add(repo_reg.url) - reg_info = parse_registry_yml(clone_dir) - if not reg_info: - errors.append(f"No registry.yml in source for '{plugin.name}'") - continue - - target_entry = None - for entry in reg_info.plugins: - if entry.name == plugin.name: - target_entry = entry - break - - if not target_entry: - logger.info(" Plugin '%s' no longer exists in source", plugin.name) - if not _migrate_removed_plugin( - registry, plugin, clone_dir, reg_info, plugins_dir, - ): - errors.append(f"Migration failed for '{plugin.name}'") - continue - - plugin_path = clone_dir / target_entry.path.rstrip('/') - try: - copy_plugin( - registry, plugin.name, plugin_path, plugin.source, plugins_dir - ) - except PluginError as e: - errors.append(str(e)) + # After pull, update ALL installed plugins from this repo + # (not just the named target) so metadata stays in sync. + repo_errors = _update_repo_plugins( + registry, repo_reg.url, clone_dir, + pre_pull_projects=_pre_pull_projects, + ) + errors.extend(repo_errors) + else: + logger.info("Skip: '%s' (repo already pulled and refreshed)", plugin.name) sync_projects(registry) diff --git a/tests/plugin/__init__.py b/tests/plugin/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/plugin/test_repos_core.py b/tests/plugin/test_repos_core.py new file mode 100644 index 0000000..cb8e59a --- /dev/null +++ b/tests/plugin/test_repos_core.py @@ -0,0 +1,971 @@ +"""Tests for PLAN04 repos/ persistent clone + direct link install""" + +from __future__ import annotations + +import os +import subprocess +import textwrap +from pathlib import Path +from unittest.mock import patch, MagicMock + +import pytest + +from devbase.errors import PluginError, RepositoryError +from devbase.plugin.models import ( + AvailablePlugin, + InstalledPlugin, + RegisteredRepository, +) +from devbase.plugin.registry import PluginRegistry +from devbase.plugin.repo_manager import ( + _derive_repo_name, + _is_repo_dirty, + _url_to_repos_dirname, + add_repository, + refresh_repository, + remove_repository, +) +from devbase.plugin.installer import ( + git_clone, + install_plugin, + uninstall_plugin, +) +from devbase.plugin.syncer import ( + _extract_owner, + _make_relative_target, + sync_projects, +) + + +# ── Fixtures ──────────────────────────────────────────────────── + + +@pytest.fixture +def devbase_root(tmp_path): + """Create a minimal devbase directory structure.""" + (tmp_path / "projects").mkdir() + return tmp_path + + +@pytest.fixture +def registry(devbase_root): + return PluginRegistry(devbase_root) + + +def _make_repo_dir( + devbase_root: Path, owner_repo: str, plugins: list[dict], + host: str = "github.com", +) -> Path: + """Create a fake repos/----/ with registry.yml and plugin dirs.""" + dir_name = f"{host}--{owner_repo.replace('/', '--')}" + repo_dir = devbase_root / "repos" / dir_name + repo_dir.mkdir(parents=True, exist_ok=True) + + # Create a fake .git directory + (repo_dir / ".git").mkdir(exist_ok=True) + + plugin_entries = [] + for p in plugins: + pdir = repo_dir / p["path"] + pdir.mkdir(parents=True, exist_ok=True) + # plugin.yml + import yaml + with open(pdir / "plugin.yml", "w") as f: + yaml.dump({ + "name": p["name"], + "version": p.get("version", "1.0.0"), + "priority": p.get("priority", 0), + }, f) + # projects/ + for proj in p.get("projects", []): + (pdir / "projects" / proj).mkdir(parents=True, exist_ok=True) + plugin_entries.append({ + "name": p["name"], + "path": p["path"], + "description": p.get("description", ""), + }) + + import yaml + with open(repo_dir / "registry.yml", "w") as f: + yaml.dump({ + "name": owner_repo.split("/")[-1], + "plugins": plugin_entries, + }, f) + + return repo_dir + + +def _register_repo( + registry: PluginRegistry, owner_repo: str, url: str, plugins: list[dict], + host: str = "github.com", +): + """Register a repository in plugins.yml with local_path.""" + dir_name = f"{host}--{owner_repo.replace('/', '--')}" + repo = RegisteredRepository( + name=owner_repo.split("/")[-1], + url=url, + added_at=registry.now_iso(), + local_path=f"repos/{dir_name}", + plugins=[ + AvailablePlugin(name=p["name"], description=p.get("description", ""), path=p["path"]) + for p in plugins + ], + ) + registry.add_repository(repo) + + +# ── models.py ─────────────────────────────────────────────────── + + +class TestRegisteredRepositoryLocalPath: + def test_to_dict_includes_local_path(self): + repo = RegisteredRepository( + name="test", + url="https://github.com/test/repo.git", + local_path="repos/test--repo", + ) + d = repo.to_dict() + assert d["local_path"] == "repos/test--repo" + + def test_to_dict_omits_empty_local_path(self): + repo = RegisteredRepository( + name="test", + url="https://github.com/test/repo.git", + ) + d = repo.to_dict() + assert "local_path" not in d + + def test_from_dict_reads_local_path(self): + repo = RegisteredRepository.from_dict({ + "name": "test", + "url": "https://github.com/test/repo.git", + "local_path": "repos/test--repo", + }) + assert repo.local_path == "repos/test--repo" + + def test_from_dict_defaults_empty_local_path(self): + repo = RegisteredRepository.from_dict({ + "name": "test", + "url": "https://github.com/test/repo.git", + }) + assert repo.local_path == "" + + +# ── registry.py ───────────────────────────────────────────────── + + +class TestGetReposDir: + def test_returns_repos_path(self, registry, devbase_root): + assert registry.get_repos_dir() == devbase_root / "repos" + + +# ── repo_manager.py ───────────────────────────────────────────── + + +class TestUrlToReposDirname: + def test_github_https(self): + assert _url_to_repos_dirname("https://github.com/devbasex/devbase-samples.git") == "github.com--devbasex--devbase-samples" + + def test_github_ssh(self): + assert _url_to_repos_dirname("git@github.com:user/my-repo.git") == "github.com--user--my-repo" + + def test_owner_with_hyphens(self): + assert _url_to_repos_dirname("https://github.com/takemi-ohama/devbase-ext.git") == "github.com--takemi-ohama--devbase-ext" + + def test_different_hosts_produce_different_dirnames(self): + """github.com and gitlab.com repos with same owner/name must not collide.""" + gh = _url_to_repos_dirname("https://github.com/org/repo.git") + gl = _url_to_repos_dirname("https://gitlab.com/org/repo.git") + assert gh != gl + assert "github.com" in gh + assert "gitlab.com" in gl + + def test_ssh_and_https_same_host_match(self): + """SSH and HTTPS variants of the same host produce the same dirname.""" + https = _url_to_repos_dirname("https://github.com/user/repo.git") + ssh = _url_to_repos_dirname("git@github.com:user/repo.git") + assert https == ssh + + +class TestDeriveRepoName: + def test_github_https(self): + assert _derive_repo_name("https://github.com/devbasex/devbase-samples.git") == "devbasex/devbase-samples" + + def test_github_ssh(self): + assert _derive_repo_name("git@github.com:user/my-repo.git") == "user/my-repo" + + +class TestAddRepository: + def test_add_creates_persistent_clone(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + repos_dir = devbase_root / "repos" + + with patch("devbase.plugin.repo_manager.git_clone") as mock_clone, \ + patch("devbase.plugin.repo_manager.parse_registry_yml") as mock_parse: + + def fake_clone(u, dest, **kwargs): + dest.mkdir(parents=True, exist_ok=True) + (dest / ".git").mkdir() + + mock_clone.side_effect = fake_clone + + from devbase.plugin.models import RegistryInfo, RegistryEntry + mock_parse.return_value = RegistryInfo( + name="testrepo", + plugins=[RegistryEntry(name="myplugin", path="myplugin", description="test")], + ) + + add_repository(registry, url) + + mock_clone.assert_called_once() + call_kwargs = mock_clone.call_args + assert call_kwargs.kwargs.get("shallow") is False or ( + len(call_kwargs.args) >= 3 or "shallow" in str(call_kwargs) + ) + + repo = registry.get_repository("testrepo") + assert repo is not None + assert repo.local_path == "repos/github.com--testorg--testrepo" + assert repo.url == url + + def test_add_duplicate_url_raises(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + _register_repo(registry, "testorg/testrepo", url, []) + + with pytest.raises(RepositoryError, match="already registered"): + add_repository(registry, url) + + +class TestRemoveRepository: + def test_remove_deletes_clone_dir(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + repo_dir = _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1", "projects": ["proj1"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + ]) + + remove_repository(registry, "testrepo", force=True) + + assert not repo_dir.exists() + assert registry.get_repository("testrepo") is None + + def test_remove_dirty_repo_raises_without_force(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", []) + _register_repo(registry, "testorg/testrepo", url, []) + + with patch("devbase.plugin.repo_manager._is_repo_dirty", return_value=(True, "uncommitted changes")): + with pytest.raises(RepositoryError, match="uncommitted changes"): + remove_repository(registry, "testrepo") + + def test_remove_dirty_repo_succeeds_with_force(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + repo_dir = _make_repo_dir(devbase_root, "testorg/testrepo", []) + _register_repo(registry, "testorg/testrepo", url, []) + + with patch("devbase.plugin.repo_manager._is_repo_dirty", return_value=(True, "uncommitted changes")): + remove_repository(registry, "testrepo", force=True) + + assert not repo_dir.exists() + + def test_remove_uninstalls_plugins_and_syncs(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1", "projects": ["proj1"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + ]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + )) + + projects_dir = devbase_root / "projects" + link = projects_dir / "proj1" + link.symlink_to(Path("..") / "repos" / "github.com--testorg--testrepo" / "p1" / "projects" / "proj1") + + remove_repository(registry, "testrepo", force=True) + + assert registry.get("p1") is None + assert not link.exists() + + +class TestRefreshRepository: + def test_refresh_pulls_and_updates_metadata(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1"}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + ]) + + with patch("devbase.plugin.repo_manager._git_pull"): + refresh_repository(registry, "testrepo") + + repo = registry.get_repository("testrepo") + assert repo is not None + assert any(p.name == "p1" for p in repo.plugins) + + def test_refresh_warns_removed_installed_plugin(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + repo_dir = _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p2", "path": "p2"}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + {"name": "p2", "path": "p2"}, + ]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + )) + + with patch("devbase.plugin.repo_manager._git_pull"), \ + patch("devbase.plugin.repo_manager.logger") as mock_logger: + refresh_repository(registry, "testrepo") + mock_logger.warning.assert_any_call( + "Installed plugin '%s' no longer exists in registry.yml of '%s'", + "p1", "testrepo", + ) + + +# ── installer.py ──────────────────────────────────────────────── + + +class TestGitClone: + def test_shallow_true_adds_depth(self): + with patch("subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0) + dest = Path("/tmp/test-clone") + dest.parent.mkdir(parents=True, exist_ok=True) + git_clone("https://example.com/repo.git", dest, shallow=True) + cmd = mock_run.call_args[0][0] + assert "--depth" in cmd + + def test_shallow_false_no_depth(self): + with patch("subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0) + dest = Path("/tmp/test-clone2") + dest.parent.mkdir(parents=True, exist_ok=True) + git_clone("https://example.com/repo.git", dest, shallow=False) + cmd = mock_run.call_args[0][0] + assert "--depth" not in cmd + + +class TestInstallPlugin: + def test_install_creates_symlinks_via_repos(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "myplugin", "path": "myplugin", "projects": ["myproj"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "myplugin", "path": "myplugin"}, + ]) + + install_plugin(registry, "myplugin") + + plugin = registry.get("myplugin") + assert plugin is not None + assert plugin.path == "repos/github.com--testorg--testrepo/myplugin" + assert not plugin.linked + + proj_link = devbase_root / "projects" / "myproj" + assert proj_link.is_symlink() + target = os.readlink(str(proj_link)) + assert "repos/github.com--testorg--testrepo/myplugin/projects/myproj" in target + + def test_install_all_plugins(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1", "projects": ["proj1"]}, + {"name": "p2", "path": "p2", "projects": ["proj2"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + {"name": "p2", "path": "p2"}, + ]) + + install_plugin(registry, url, install_all=True) + + assert registry.get("p1") is not None + assert registry.get("p2") is not None + + def test_install_not_registered_raises(self, registry): + with pytest.raises(PluginError, match="not found in registered"): + install_plugin(registry, "nonexistent") + + def test_install_ref_rejected_for_unregistered_repo(self, registry): + """@ref with unregistered repo raises PluginError (not NameError).""" + with pytest.raises(PluginError, match="Cannot use @v1.0"): + install_plugin(registry, "testorg/testrepo:myplugin@v1.0") + + def test_install_ref_rejected_for_registered_repo(self, registry, devbase_root): + """@ref with already-registered repo also raises PluginError.""" + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "myplugin", "path": "myplugin", "projects": ["myproj"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "myplugin", "path": "myplugin"}, + ]) + + # Use shorthand form — PluginSource.parse only extracts @ref for + # non-URL forms (testorg/repo:plugin@ref, not https://...@ref). + with pytest.raises(PluginError, match="Cannot use @v2.0"): + install_plugin(registry, "testorg/testrepo:myplugin@v2.0") + + def test_install_legacy_repo_without_local_path(self, registry, devbase_root): + """Legacy repos (no local_path) are auto-migrated to persistent clone.""" + url = "https://github.com/testorg/testrepo.git" + # Register a legacy repo (no local_path) + repo = RegisteredRepository( + name="testrepo", url=url, + added_at=registry.now_iso(), + local_path="", + plugins=[AvailablePlugin(name="p1", description="test", path="p1")], + ) + registry.add_repository(repo) + + with patch("devbase.plugin.installer.git_clone") as mock_clone: + def fake_clone(u, dest, **kwargs): + dest.mkdir(parents=True, exist_ok=True) + # Create plugin dir + registry.yml in the clone + pdir = dest / "p1" + pdir.mkdir() + import yaml + (pdir / "plugin.yml").write_text("name: p1\nversion: 2.0.0\n") + (pdir / "projects").mkdir() + (pdir / "projects" / "proj1").mkdir() + with open(dest / "registry.yml", "w") as f: + yaml.dump({ + "name": "testrepo", + "plugins": [{"name": "p1", "path": "p1", "description": "test"}], + }, f) + + mock_clone.side_effect = fake_clone + + install_plugin(registry, "p1") + + # Repo should now have local_path set + updated = registry.get_repository_by_url(url) + assert updated.local_path == "repos/github.com--testorg--testrepo" + + # Plugin should be installed + plugin = registry.get("p1") + assert plugin is not None + assert "repos/github.com--testorg--testrepo" in plugin.path + + +class TestUninstallPlugin: + def test_uninstall_repos_plugin_preserves_files(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + repo_dir = _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "myplugin", "path": "myplugin", "projects": ["myproj"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "myplugin", "path": "myplugin"}, + ]) + registry.add(InstalledPlugin( + name="myplugin", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/myplugin", + )) + + proj_link = devbase_root / "projects" / "myproj" + proj_link.symlink_to( + Path("..") / "repos" / "github.com--testorg--testrepo" / "myplugin" / "projects" / "myproj" + ) + + uninstall_plugin(registry, "myplugin") + + assert registry.get("myplugin") is None + assert not proj_link.exists() + assert (repo_dir / "myplugin").is_dir() + + def test_uninstall_linked_plugin_removes_symlink(self, registry, devbase_root): + plugins_dir = devbase_root / "plugins" + plugins_dir.mkdir() + + local_src = devbase_root / "local-repo" / "myplugin" + local_src.mkdir(parents=True) + (local_src / "plugin.yml").write_text("name: myplugin\nversion: 1.0.0\n") + + link_dest = plugins_dir / "myplugin" + link_dest.symlink_to(local_src) + + registry.add(InstalledPlugin( + name="myplugin", version="1.0.0", source=str(local_src.parent), + installed_at=registry.now_iso(), + path="plugins/myplugin", + linked=True, + )) + + uninstall_plugin(registry, "myplugin") + + assert not link_dest.exists() + assert local_src.is_dir() + + def test_uninstall_nonexistent_raises(self, registry): + with pytest.raises(PluginError, match="not installed"): + uninstall_plugin(registry, "nonexistent") + + +# ── syncer.py ─────────────────────────────────────────────────── + + +class TestSyncProjects: + def test_basic_sync_creates_symlinks(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1", "projects": ["proj1", "proj2"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + ]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + )) + + count = sync_projects(registry, verbose=False) + + assert count == 2 + assert (devbase_root / "projects" / "proj1").is_symlink() + assert (devbase_root / "projects" / "proj2").is_symlink() + + def test_collision_creates_suffix_links(self, registry, devbase_root): + url1 = "https://github.com/orgA/repo1.git" + url2 = "https://github.com/orgB/repo2.git" + _make_repo_dir(devbase_root, "orgA/repo1", [ + {"name": "p1", "path": "p1", "projects": ["shared"], "priority": 10}, + ]) + _make_repo_dir(devbase_root, "orgB/repo2", [ + {"name": "p2", "path": "p2", "projects": ["shared"], "priority": 0}, + ]) + _register_repo(registry, "orgA/repo1", url1, [{"name": "p1", "path": "p1"}]) + _register_repo(registry, "orgB/repo2", url2, [{"name": "p2", "path": "p2"}]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url1, + installed_at=registry.now_iso(), + path="repos/github.com--orgA--repo1/p1", + )) + registry.add(InstalledPlugin( + name="p2", version="1.0.0", source=url2, + installed_at=registry.now_iso(), + path="repos/github.com--orgB--repo2/p2", + )) + + count = sync_projects(registry, verbose=False) + + bare_link = devbase_root / "projects" / "shared" + assert bare_link.is_symlink() + target = os.readlink(str(bare_link)) + assert "github.com--orgA--repo1" in target + + suffix_link = devbase_root / "projects" / "shared.github.com--orgB--repo2" + assert suffix_link.is_symlink() + suffix_target = os.readlink(str(suffix_link)) + assert "github.com--orgB--repo2" in suffix_target + + def test_no_collision_no_suffix(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1", "projects": ["proj1"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [{"name": "p1", "path": "p1"}]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + )) + + sync_projects(registry, verbose=False) + + assert not (devbase_root / "projects" / "proj1.testorg").exists() + + def test_winner_has_no_suffix(self, registry, devbase_root): + url1 = "https://github.com/orgA/repo1.git" + url2 = "https://github.com/orgB/repo2.git" + _make_repo_dir(devbase_root, "orgA/repo1", [ + {"name": "p1", "path": "p1", "projects": ["shared"], "priority": 10}, + ]) + _make_repo_dir(devbase_root, "orgB/repo2", [ + {"name": "p2", "path": "p2", "projects": ["shared"], "priority": 0}, + ]) + _register_repo(registry, "orgA/repo1", url1, [{"name": "p1", "path": "p1"}]) + _register_repo(registry, "orgB/repo2", url2, [{"name": "p2", "path": "p2"}]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url1, + installed_at=registry.now_iso(), + path="repos/github.com--orgA--repo1/p1", + )) + registry.add(InstalledPlugin( + name="p2", version="1.0.0", source=url2, + installed_at=registry.now_iso(), + path="repos/github.com--orgB--repo2/p2", + )) + + sync_projects(registry, verbose=False) + + assert not (devbase_root / "projects" / "shared.github.com--orgA--repo1").exists() + + def test_real_directory_skipped(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1", "projects": ["existing"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [{"name": "p1", "path": "p1"}]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + )) + + (devbase_root / "projects" / "existing").mkdir() + + count = sync_projects(registry, verbose=False) + assert count == 0 + assert not (devbase_root / "projects" / "existing").is_symlink() + + def test_missing_plugin_dir_warns(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + _register_repo(registry, "testorg/testrepo", url, [{"name": "p1", "path": "p1"}]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + )) + + count = sync_projects(registry, verbose=True) + assert count == 0 + + def test_link_plugin_collision_uses_source_basename(self, registry, devbase_root): + """--link plugin と repos/ plugin の衝突時に . suffix""" + url = "https://github.com/orgA/repo1.git" + _make_repo_dir(devbase_root, "orgA/repo1", [ + {"name": "p1", "path": "p1", "projects": ["shared"], "priority": 10}, + ]) + _register_repo(registry, "orgA/repo1", url, [{"name": "p1", "path": "p1"}]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--orgA--repo1/p1", + )) + + plugins_dir = devbase_root / "plugins" + plugins_dir.mkdir() + local_plugin = devbase_root / "my-local-repo" / "p2" + local_plugin.mkdir(parents=True) + (local_plugin / "plugin.yml").write_text("name: p2\nversion: 1.0.0\npriority: 0\n") + (local_plugin / "projects" / "shared").mkdir(parents=True) + link = plugins_dir / "p2" + link.symlink_to(local_plugin) + + registry.add(InstalledPlugin( + name="p2", version="1.0.0", source=str(devbase_root / "my-local-repo"), + installed_at=registry.now_iso(), + path="plugins/p2", + linked=True, + )) + + sync_projects(registry, verbose=False) + + suffix_link = devbase_root / "projects" / "shared.my-local-repo" + assert suffix_link.is_symlink() + + +class TestExtractOwner: + def test_repos_based(self): + plugin = InstalledPlugin( + name="p1", version="1.0.0", source="url", + installed_at="", path="repos/github.com--orgA--repo1/p1", + ) + assert _extract_owner(plugin) == "github.com--orgA--repo1" + + def test_linked(self): + plugin = InstalledPlugin( + name="p1", version="1.0.0", source="/path/to/my-local-repo", + installed_at="", path="plugins/p1", linked=True, + ) + assert _extract_owner(plugin) == "my-local-repo" + + +class TestMakeRelativeTarget: + def test_repos_based(self): + plugin = InstalledPlugin( + name="p1", version="1.0.0", source="url", + installed_at="", path="repos/github.com--orgA--repo1/p1", + ) + target = _make_relative_target(plugin, "myproj") + assert target == Path("..") / "repos" / "github.com--orgA--repo1" / "p1" / "projects" / "myproj" + + def test_linked(self): + plugin = InstalledPlugin( + name="p1", version="1.0.0", source="/path/to/repo", + installed_at="", path="plugins/p1", linked=True, + ) + target = _make_relative_target(plugin, "myproj") + assert target == Path("..") / "plugins" / "p1" / "projects" / "myproj" + + +# ── updater.py ────────────────────────────────────────────────── + + +class TestUpdatePlugin: + def test_update_calls_git_pull(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1", "projects": ["proj1"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + ]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + )) + + from devbase.plugin.updater import update_plugin + + with patch("devbase.plugin.updater._git_pull") as mock_pull: + update_plugin(registry, "p1") + mock_pull.assert_called_once() + + def test_update_skips_linked(self, registry, devbase_root): + registry.add(InstalledPlugin( + name="linked-plugin", version="1.0.0", source="/local/path", + installed_at=registry.now_iso(), + path="plugins/linked-plugin", + linked=True, + )) + + from devbase.plugin.updater import update_plugin + + with patch("devbase.plugin.updater._git_pull") as mock_pull: + update_plugin(registry, "linked-plugin") + mock_pull.assert_not_called() + + def test_update_deduplicates_git_pull(self, registry, devbase_root): + """Same repo pulled only once even when multiple plugins share it.""" + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1", "projects": ["proj1"]}, + {"name": "p2", "path": "p2", "projects": ["proj2"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + {"name": "p2", "path": "p2"}, + ]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + )) + registry.add(InstalledPlugin( + name="p2", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p2", + )) + + from devbase.plugin.updater import update_plugin + + with patch("devbase.plugin.updater._git_pull") as mock_pull: + update_plugin(registry) + assert mock_pull.call_count == 1 + + +class TestSnapshotPluginProjects: + """Tests for _snapshot_plugin_projects""" + + def test_snapshot_captures_project_names(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1", "projects": ["proj1", "proj2"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + ]) + plugin = InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + ) + registry.add(plugin) + + from devbase.plugin.updater import _snapshot_plugin_projects + snapshot = _snapshot_plugin_projects(registry, [plugin]) + + assert "p1" in snapshot + assert snapshot["p1"] == {"proj1", "proj2"} + + def test_snapshot_missing_dir_returns_empty(self, registry, devbase_root): + """Plugin with missing directory returns empty set.""" + url = "https://github.com/testorg/testrepo.git" + plugin = InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + ) + + from devbase.plugin.updater import _snapshot_plugin_projects + snapshot = _snapshot_plugin_projects(registry, [plugin]) + + assert "p1" in snapshot + assert snapshot["p1"] == set() + + +class TestMigrateRemovedPlugin: + """Tests for _migrate_removed_plugin — detects project moves after rename/restructure.""" + + def test_migration_replaces_renamed_plugin(self, registry, devbase_root): + """Plugin renamed (old-name -> new-name) with same projects migrates correctly.""" + url = "https://github.com/testorg/testrepo.git" + + # Initial state: old plugin 'p1' with projects + old_dir = _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1", "projects": ["proj1", "proj2"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + ]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + )) + + # Snapshot projects BEFORE simulating pull + from devbase.plugin.updater import _snapshot_plugin_projects, _migrate_removed_plugin + plugin = registry.get("p1") + pre_pull = _snapshot_plugin_projects(registry, [plugin]) + + # Simulate git pull that renames p1 -> p1-renamed (delete old, create new) + import shutil + shutil.rmtree(old_dir / "p1") + new_plugin_dir = old_dir / "p1-renamed" / "projects" + new_plugin_dir.mkdir(parents=True) + (old_dir / "p1-renamed" / "plugin.yml").write_text("name: p1-renamed\nversion: 2.0.0\n") + (new_plugin_dir / "proj1").mkdir() + (new_plugin_dir / "proj2").mkdir() + # Update registry.yml to reflect new name + import yaml + with open(old_dir / "registry.yml", "w") as f: + yaml.dump({ + "name": "testrepo", + "plugins": [{"name": "p1-renamed", "path": "p1-renamed", "description": "renamed"}], + }, f) + + from devbase.plugin.installer import parse_registry_yml + reg_info = parse_registry_yml(old_dir) + + result = _migrate_removed_plugin( + registry, plugin, old_dir, reg_info, + pre_pull_projects=pre_pull["p1"], + ) + + assert result is True + assert registry.get("p1") is None # old removed + assert registry.get("p1-renamed") is not None # new installed + + def test_migration_no_replacement_returns_false(self, registry, devbase_root): + """When projects cannot be found in new plugins, migration fails gracefully.""" + url = "https://github.com/testorg/testrepo.git" + + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1", "projects": ["proj1"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + ]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + )) + + from devbase.plugin.updater import _snapshot_plugin_projects, _migrate_removed_plugin + plugin = registry.get("p1") + pre_pull = _snapshot_plugin_projects(registry, [plugin]) + + # Simulate pull that removes all plugins (empty registry) + import shutil + clone_dir = devbase_root / "repos" / "github.com--testorg--testrepo" + shutil.rmtree(clone_dir / "p1") + import yaml + with open(clone_dir / "registry.yml", "w") as f: + yaml.dump({"name": "testrepo", "plugins": []}, f) + + from devbase.plugin.installer import parse_registry_yml + reg_info = parse_registry_yml(clone_dir) + + result = _migrate_removed_plugin( + registry, plugin, clone_dir, reg_info, + pre_pull_projects=pre_pull["p1"], + ) + + assert result is False + + def test_migration_empty_projects_removes_plugin(self, registry, devbase_root): + """Plugin with no projects is simply removed.""" + url = "https://github.com/testorg/testrepo.git" + + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1", "projects": []}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + ]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + )) + + from devbase.plugin.updater import _migrate_removed_plugin + from devbase.plugin.installer import parse_registry_yml + plugin = registry.get("p1") + clone_dir = devbase_root / "repos" / "github.com--testorg--testrepo" + reg_info = parse_registry_yml(clone_dir) + + # No pre_pull_projects means read from disk — dir exists but no projects + result = _migrate_removed_plugin(registry, plugin, clone_dir, reg_info) + + assert result is True + assert registry.get("p1") is None + + +class TestRefreshWithPrePullSnapshot: + """Verify refresh_repository passes pre_pull snapshot to _update_repo_plugins.""" + + def test_refresh_passes_pre_pull_projects(self, registry, devbase_root): + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1", "projects": ["proj1"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + ]) + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path="repos/github.com--testorg--testrepo/p1", + )) + + with patch("devbase.plugin.repo_manager._git_pull"), \ + patch("devbase.plugin.updater._update_repo_plugins", return_value=[]) as mock_update: + refresh_repository(registry, "testrepo") + + mock_update.assert_called_once() + call_kwargs = mock_update.call_args + assert "pre_pull_projects" in call_kwargs.kwargs + pre = call_kwargs.kwargs["pre_pull_projects"] + assert "p1" in pre + assert "proj1" in pre["p1"] From 5a6158a335315bf73ec6a116ff155ed149ab72cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=A4=A7=E6=B5=9C=E6=AF=85=E7=BE=8E?= Date: Fri, 29 May 2026 01:46:49 +0900 Subject: [PATCH 3/8] =?UTF-8?q?feat(plugin):=20=E6=97=A7=20plugins/=20?= =?UTF-8?q?=E3=82=B3=E3=83=94=E3=83=BC=E3=82=A4=E3=83=B3=E3=82=B9=E3=83=88?= =?UTF-8?q?=E3=83=BC=E3=83=AB=E3=82=92=20repos/=20=E6=B0=B8=E7=B6=9A?= =?UTF-8?q?=E3=82=AF=E3=83=AD=E3=83=BC=E3=83=B3=E3=81=B8=E7=A7=BB=E8=A1=8C?= =?UTF-8?q?=E3=81=99=E3=82=8B=20devbase=20plugin=20migrate=20=E3=82=92?= =?UTF-8?q?=E8=BF=BD=E5=8A=A0=20(#31)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: PLAN04-migration Draft PR 作成 * feat(plugin): 既存 plugins/ コピーインストールを repos/ 永続クローンへ移行 PLAN04 PR2。PR1 (#29) で repos/ 永続クローン方式に切り替えたが、PR1 以前に plugins// へファイルコピーされた既存インストールは移行されないため、その 移行ロジックを追加する。 - migrator.py (新規): - needs_migration / _is_legacy_plugin: legacy plugins/ インストールの検出 (linked は --link 専用として除外) - _dirs_differ: コピーとクローンの差分検出 (内容変更・追加ファイルを保守的に差分扱い) - migrate: 未クローン repo の永続クローン作成、InstalledPlugin.path の repos/ 書き換え、 差分なしは plugins/ 削除・差分ありは .bak 保全、sync_projects 再実行、 --link/.bak/skip が無ければ plugins/ を .gitkeep のみに正規化 - plugin migrate サブコマンド (cli.py / commands/plugin.py) - install/update 初回実行時に _auto_migrate で自動移行 Co-Authored-By: Claude Opus 4.7 * fix(plugin): migration の symlink 差分検知漏れ / .bak 上書き / registry 先行更新を修正 cross-review round 1 の major 指摘 4 件 (codex 2 / gemini 2) に対応。 - _dirs_differ: regular file のみ比較していたため legacy copy のみに存在する symlink / 空ディレクトリ / 型不一致を差分として検知できず、後続の shutil.rmtree で silently 削除される恐れがあった。全エントリを対象に 型 + 内容 (file は byte, symlink は target) を比較するよう厳密化 - _unique_bak_path: 既存の .bak を無条件に rmtree していたため、 前回 migration で保全した未整理バックアップが消失する恐れがあった。 存在時は .bak-2, .bak-3 ... と一意名に退避するよう変更 - migrate: filesystem の退避/削除が成功してから registry.add で plugins.yml を repos/ path に書き換えるよう順序を入れ替え。失敗時に registry だけ先行更新され retry も効かなくなる partial state を防止 - _cleanup_plugins_dir: .bak-N 形式も保全 .bak として検知するよう調整 - 上記挙動を網羅するテスト 6 件追加 Co-Authored-By: Claude Opus 4.7 (1M context) * fix(plugin): migration の差分判定厳密化 / partial clone 復旧 / cleanup 報告修正 cross-review round 2 の指摘対応。 - _dirs_differ: upstream 専用追加 (clone にのみ存在) を差分扱いしないよう 変更。コピー側にのみ存在するエントリ・共通エントリの型/target/内容差分の みを preserved 判定に使い、通常の upstream 更新で不要な .bak 退避が発生 しないようにした (codex#91 / gemini#91 重複指摘)。 - _files_equal: read_bytes() の全読み込みを 64KB チャンクのストリーム比較に 置き換え、巨大ファイルでのメモリ枯渇リスクを排除 (gemini#105)。 - _ensure_repo_cloned: 前回 clone 失敗で残った partial dir (.git 無し / registry.yml 不正) を検知して削除・再 clone するよう修正。無限に parse 失敗を繰り返す経路を解消 (codex#132)。 - _cleanup_plugins_dir: .gitkeep でも .bak でもない想定外エントリが残る場合 は cleaned=True と報告せず False を返すよう修正 (gemini#176)。 - docs: devbase plugin migrate の CLI リファレンスを追加 (codex review body)。 テスト 4 件追加 (upstream 専用追加は差分なし / 同サイズ内容差 / partial clone 再 clone / cleanup の想定外エントリ保持)。全 252 件 pass。 Co-Authored-By: Claude Opus 4.7 (1M context) * fix(plugin): migration の保全判定・clone 健全性・registry 保存効率を改善 (PR2 round3) cross-review round 3 の指摘に対応: - _cleanup_plugins_dir の `.bak` 判定を `'.bak' in name` から `.bak[-N]` 末尾一致 (_is_bak_name) に修正。my.bakery 等の誤マッチを排除 - migrate ループ内の per-plugin `registry.add` を loop 末尾の単一 `registry.add_many` に集約し plugins.yml の保存頻発を解消 (各 plugin の fs 移動と entry 構築は同一 try 内のため失敗時の retry 性は維持) - _ensure_repo_cloned で local_path 設定済みでも .git/registry.yml を 検証 (_clone_is_healthy)。壊れた既存 dir は除去して再 clone - _files_equal で S_IMODE を比較。旧コピーの実行ビット変更を差分扱いし保全 - _auto_migrate の preserved/skipped 再通知を loud な per-plugin WARNING から 簡潔な INFO ヒント 1 行に抑制 (詳細は devbase plugin migrate 側で出力) migrator テスト 12 件追加 (.bak 末尾判定 / clone 健全性 / 実行ビット差分 / batched save / broken local_path 再 clone / 警告抑制)。全 264 件 pass。 Co-Authored-By: Claude Opus 4.7 (1M context) * fix(plugin): migration の registry 重複排除 / exec ビット限定比較 / 健全 clone 保全 (PR2 round4) - registry.add_many: 引数内で名前が重複する場合 last-wins で一意化してから 反映し、plugins.yml に矛盾エントリが残らないようにした - _files_equal: 全権限ビット比較を exec ビット (+x) 限定に変更し、umask / group 設定差による誤った .bak 退避を防止 - _ensure_repo_cloned: local_path 記録済みだが unhealthy な既存 dir でも .git があれば未コミット/未 push のローカル変更を失わないよう rmtree せず PluginError を送出し、.git 欠落 (真に壊れている) 場合のみ再 clone する test: add_many 重複排除 / exec ビット限定 / .git 付き clone 保全の 6 件を追加 Co-Authored-By: Claude Opus 4.7 (1M context) * fix(plugin): migration の derived clone 保全 / registry 先行永続化 / 特殊ファイル差分検知 (PR2 round5) cross-review round 5 で指摘された 4 件に対応: - derived clone 経路 (.git 保護): repo.local_path 未設定でも repos/ に .git 付き既存 clone があり registry.yml だけ欠ける場合、無条件 rmtree で 未コミット/未 push のローカル変更を失っていた。_reclaim_or_protect_existing を新設し local_path 経路と同じく .git 有りは削除せず PluginError で復旧案内 する (freshly clone した分のみ破棄)。 - registry 先行永続化: 旧 plugins/ コピーの削除/.bak 退避 → add_many の順序を 逆転し、検証済み path rewrite を破壊的 fs 操作の前に 1 回保存する二相構成へ。 保存失敗時はコピー無傷で abort (次回 retry 可能)、phase2 の retire 失敗は registry が既に有効な repos/ clone を指すため lingering copy として _cleanup_plugins_dir が surface する (silent data loss を排除)。 - clone_dir がファイル/symlink で squat: clone_dir.is_dir() のみでは git_clone が失敗するため、ファイル/symlink は unlink して再 clone (git tree を持たない ため損失なし)。 - _entry_kind == 'other' (socket/pipe/device): 内容比較できず identical を 証明できないため diverged 扱いとし .bak 保全に倒す。 migrator テスト 5 件追加 (derived .git 保護 / clone_dir ファイル squat / registry 保存失敗でコピー無傷 / retire は保存後 / fifo は差分扱い)。 全 275 件 pass。ruff (E9,F63,F7,F82) / compileall pass。 Co-Authored-By: Claude Opus 4.7 (1M context) * fix(plugin): derived clone 経路で健全 clone を reuse する (round 6) round 5 で derived 経路 (local_path 未設定) の `.git` 付き既存 dir を 無条件に保護 (PluginError) していたが過剰だった。`repos/` に .git + registry.yml が両方そろった健全 clone が残っている場合は PluginError で migration を skip せず、そのまま reuse して local_path を 永続化するよう修正。 - 健全 clone (.git + registry.yml) → reuse + local_path 永続化 - .git ありだが unhealthy (registry.yml 欠落) → 従来どおり保護 (PluginError) - .git 無し / file・symlink squat → reclaim して再 clone local_path 経路と derived 経路で挙動を揃え、fresh clone 後と healthy reuse 後の local_path 永続化を `_persist_repo_local_path` に共通化した。 健全 derived clone が reuse され migration が skip されないことを検証する テスト `test_derived_path_with_healthy_clone_is_reused` を追加。 Co-Authored-By: Claude Opus 4.7 (1M context) * perf(plugin): migration の clone 永続化を単一保存に集約 (round 7) _ensure_repo_cloned が clone のたびに add_repository で plugins.yml を 保存していたため、多数リポジトリ移行時に保存回数が repo 数に比例していた。 clone 済み repo 行を pending_repos に貯め、path rewrite と合わせて Phase2 (破壊的 cleanup) の直前に save_migration で 1 回だけ保存するよう変更。 二相アトミシティは維持: 旧 copy 削除より前に registry が必ず flush 済みで あること (clone を指す local_path / plugin path の両方) を不変条件として 保持。save_migration は repos + plugins を単一 load+save で upsert する。 Co-Authored-By: Claude Opus 4.7 (1M context) * perf(plugin): migration の registry.yml パースをリポジトリあたり 1 回に集約 (PR2 round8) gemini review (migrator.py:428 [minor/performance]) 対応。 _ensure_repo_cloned が clone/reuse 時に parse_registry_yml した RegistryInfo を戻り値で返すようにし、migrate ループ側で再パースしていた重複を解消した。 さらに _build_persisted_repo もパース済み reg_info を受け取る形に変更し、 fresh-clone 経路での二重パース (helper 内 + ループ) も排除。結果として registry.yml の読み込みはリポジトリあたり最大 1 回 (local_path fast path は lazy fallback) に削減。未使用になった _build_persisted_repo の registry 引数も 除去。挙動・テスト (plugin 203 / migrator 60) は不変。 Co-Authored-By: Claude Opus 4.7 (1M context) * perf(plugin): migration の repo 解決をループ前に 1 回へ集約 + migrate を prefix 解決対象に追加 (PR2 round9) - migrate ループ内の registry.get_repository_by_url (毎回 plugins.yml を再読込) を ループ前の URL→repo 辞書索引 1 回に置換し、O(N) ディスク I/O を O(1) に集約 - SUBCMD_MAP[('plugin','pl')] に 'migrate' を追加し、devbase plugin mi / pl mi の prefix 解決が効くよう修正 (従来は argparse エラー) - 再読込が plugin 数に比例しないことを検証する回帰テストを追加 Co-Authored-By: Claude Opus 4.7 (1M context) * test(plugin): migration の repo 解決が plugin 数に比例して plugins.yml を再読込しない回帰テストを追加 (PR2 round9) round9 で migrate ループ内の get_repository_by_url (毎回 plugins.yml 再読込) を ループ前の URL→repo 辞書索引 1 回に置換した変更の回帰防止。_load 呼び出し回数を 計数し plugin 数より少ないことを検証する。 Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 --- docs/user/cli-reference.md | 18 + lib/devbase/cli.py | 5 +- lib/devbase/commands/plugin.py | 32 + lib/devbase/plugin/installer.py | 28 + lib/devbase/plugin/migrator.py | 532 +++++++++++++++ lib/devbase/plugin/registry.py | 79 ++- lib/devbase/plugin/updater.py | 3 + tests/plugin/test_migrator.py | 1094 +++++++++++++++++++++++++++++++ 8 files changed, 1783 insertions(+), 8 deletions(-) create mode 100644 lib/devbase/plugin/migrator.py create mode 100644 tests/plugin/test_migrator.py diff --git a/docs/user/cli-reference.md b/docs/user/cli-reference.md index 74d0bf6..d099d0a 100644 --- a/docs/user/cli-reference.md +++ b/docs/user/cli-reference.md @@ -404,6 +404,24 @@ devbase plugin info devbase plugin sync ``` +### `devbase plugin migrate` + +旧形式 (`plugins/` へのコピー) でインストールされたプラグインを、`repos/` 配下の永続クローンへ移行します。`install` / `update` 実行時にも自動で呼び出されるため、通常は手動実行不要です。 + +``` +devbase plugin migrate +``` + +移行の挙動: + +| 状況 | 動作 | +|---|---| +| コピーがクローンと一致 | 旧コピーを削除し `repos/` へ移行 (migrated) | +| コピーにローカル変更あり | 旧コピーを `plugins/.bak` として保全 (preserved、手動で reconcile) | +| 移行できない (ソース未登録 等) | スキップしてエラーを表示 (skipped) | + +`--link` でインストールしたプラグインは移行対象外です。 + ### `devbase plugin repo add` プラグインリポジトリを登録します。 diff --git a/lib/devbase/cli.py b/lib/devbase/cli.py index 9cd1ce1..f185419 100644 --- a/lib/devbase/cli.py +++ b/lib/devbase/cli.py @@ -37,7 +37,7 @@ SUBCMD_MAP = { ('container', 'ct'): ['up', 'down', 'ps', 'login', 'logs', 'scale', 'build'], ('env',): ['init', 'sync', 'list', 'set', 'get', 'delete', 'edit', 'project', 'export', 'import'], - ('plugin', 'pl'): ['list', 'install', 'uninstall', 'update', 'info', 'sync', 'repo'], + ('plugin', 'pl'): ['list', 'install', 'uninstall', 'update', 'info', 'sync', 'repo', 'migrate'], ('snapshot', 'ss'): ['create', 'list', 'restore', 'copy', 'delete', 'rotate'], } @@ -234,6 +234,9 @@ def _add_plugin_parser(subparsers): pl_sub.add_parser('sync', help='Resync project symlinks') + pl_sub.add_parser('migrate', + help='Migrate legacy plugins/ installs to repos/ clones') + # Plugin repo sub-subcommands pl_repo = pl_sub.add_parser('repo', help='Manage plugin repositories') pl_repo_sub = pl_repo.add_subparsers(dest='repo_command') diff --git a/lib/devbase/commands/plugin.py b/lib/devbase/commands/plugin.py index b20d31d..f6acf61 100644 --- a/lib/devbase/commands/plugin.py +++ b/lib/devbase/commands/plugin.py @@ -9,6 +9,7 @@ from devbase.plugin.updater import update_plugin from devbase.plugin.info import show_plugin_info, show_available_plugins from devbase.plugin.syncer import sync_projects +from devbase.plugin.migrator import migrate from devbase.plugin.repo_manager import ( add_repository, remove_repository, @@ -34,6 +35,7 @@ def cmd_plugin(devbase_root: Path, args) -> int: 'update': lambda: cmd_plugin_update(devbase_root, getattr(args, 'name', None)), 'info': lambda: cmd_plugin_info(devbase_root, getattr(args, 'name', '')), 'sync': lambda: cmd_sync(devbase_root), + 'migrate': lambda: cmd_plugin_migrate(devbase_root), 'repo': lambda: cmd_repo(devbase_root, args), } @@ -138,6 +140,36 @@ def cmd_sync(devbase_root: Path) -> int: return 0 +def cmd_plugin_migrate(devbase_root: Path) -> int: + """Migrate legacy plugins/ copy installs to repos/ persistent clones""" + registry = PluginRegistry(devbase_root) + try: + result = migrate(registry) + except DevbaseError as e: + logger.error("%s", e) + return 1 + + if not (result.migrated or result.preserved or result.skipped): + logger.info("No legacy plugins/ installs to migrate.") + return 0 + + if result.migrated: + logger.info("Migrated %d plugin(s) to repos/: %s", + len(result.migrated), ", ".join(result.migrated)) + if result.preserved: + logger.warning( + "Preserved %d plugin(s) with local changes as plugins/.bak " + "(reconcile manually): %s", + len(result.preserved), ", ".join(result.preserved)) + if result.skipped: + logger.warning("Could not migrate %d plugin(s): %s", + len(result.skipped), ", ".join(result.skipped)) + for err in result.errors: + logger.warning(" %s", err) + return 1 + return 0 + + def cmd_repo(devbase_root: Path, args) -> int: """Dispatch repo subcommands""" registry = PluginRegistry(devbase_root) diff --git a/lib/devbase/plugin/installer.py b/lib/devbase/plugin/installer.py index 747c548..96ffb6d 100644 --- a/lib/devbase/plugin/installer.py +++ b/lib/devbase/plugin/installer.py @@ -81,6 +81,32 @@ def resolve_repo_url(repo: str) -> str: return f"https://github.com/{repo}.git" +def _auto_migrate(registry: PluginRegistry) -> None: + """Migrate any legacy plugins/ copy installs to repos/ before proceeding. + + Triggered on the first install/update after upgrading to repos/-based + plugin management so users do not have to run `devbase plugin migrate` + manually. No-op when nothing legacy remains. + """ + from .migrator import migrate, needs_migration + if not needs_migration(registry): + return + logger.info("Legacy plugins/ installs detected — migrating to repos/...") + result = migrate(registry) + if result.migrated: + logger.info(" Migrated: %s", ", ".join(result.migrated)) + # preserved/skipped recur on every install/update until the user + # reconciles, so avoid re-emitting a loud per-plugin WARNING each time: + # surface a single concise hint pointing at the explicit command, which + # prints the full per-plugin detail when run. + if result.preserved or result.skipped: + pending = len(result.preserved) + len(result.skipped) + logger.info( + " %d plugin(s) still need attention — run 'devbase plugin migrate' " + "for details.", pending, + ) + + def install_plugin( registry: PluginRegistry, source_str: str, @@ -91,6 +117,8 @@ def install_plugin( Raises PluginError on failure. """ + _auto_migrate(registry) + source = PluginSource.parse(source_str, link=link) plugins_dir = registry.get_plugins_dir() diff --git a/lib/devbase/plugin/migrator.py b/lib/devbase/plugin/migrator.py new file mode 100644 index 0000000..7c37d26 --- /dev/null +++ b/lib/devbase/plugin/migrator.py @@ -0,0 +1,532 @@ +"""Plugin migrator - migrates legacy plugins/ copy installs to repos/ clones""" + +import re +import shutil +import stat +from dataclasses import dataclass, field +from pathlib import Path +from typing import TYPE_CHECKING, Optional + +from devbase.errors import PluginError +from devbase.log import get_logger + +from .models import AvailablePlugin, InstalledPlugin, RegisteredRepository +from .registry import PluginRegistry + +if TYPE_CHECKING: + from .models import RegistryInfo + +logger = get_logger("devbase.plugin.migrator") + + +@dataclass +class MigrationResult: + """Outcome of a plugins/ -> repos/ migration run.""" + migrated: list[str] = field(default_factory=list) # cleanly moved + copy deleted + preserved: list[str] = field(default_factory=list) # copy differed -> kept as .bak + skipped: list[str] = field(default_factory=list) # could not migrate + errors: list[str] = field(default_factory=list) + plugins_dir_cleaned: bool = False # plugins/ emptied to .gitkeep + + +def _is_legacy_plugin(plugin: InstalledPlugin) -> bool: + """True if a plugin still uses the pre-PLAN04 plugins/ copy format. + + --link installs also live under plugins/ but are intentional and must not + be migrated, so they are excluded. + """ + if plugin.linked: + return False + parts = Path(plugin.path).parts + return len(parts) >= 1 and parts[0] == 'plugins' + + +def needs_migration(registry: PluginRegistry) -> bool: + """True if any installed plugin still uses the legacy plugins/ copy format.""" + return any(_is_legacy_plugin(p) for p in registry.list_installed()) + + +# Names produced by _unique_bak_path: ".bak" or ".bak-". +_BAK_NAME_RE = re.compile(r'\.bak(-\d+)?$') + + +def _is_bak_name(name: str) -> bool: + """True if name matches the preserved-copy convention (.bak[-N]). + + A substring check like ``'.bak' in name`` would wrongly flag unrelated + entries such as ``my.bakery`` or ``notes.bak.txt``; anchor to the suffix. + """ + return _BAK_NAME_RE.search(name) is not None + + +def _unique_bak_path(old_dir: Path) -> Path: + """Return a non-existing .bak path, suffixing -2, -3, … if needed. + + A previous migration run may have already preserved .bak awaiting + manual reconciliation; never overwrite it, so each diverged copy lands in + its own directory. + """ + bak = old_dir.with_name(old_dir.name + '.bak') + if not bak.exists(): + return bak + n = 2 + while True: + candidate = old_dir.with_name(f"{old_dir.name}.bak-{n}") + if not candidate.exists(): + return candidate + n += 1 + + +def _entry_kind(p: Path) -> str: + """Classify a path for diff purposes: symlink / dir / file / other. + + Symlinks are checked first (a symlink to a dir would otherwise read as a + dir) so that a copy/clone type mismatch is always detected. + """ + if p.is_symlink(): + return 'symlink' + if p.is_dir(): + return 'dir' + if p.is_file(): + return 'file' + return 'other' + + +_EXEC_BITS = stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH + + +def _files_equal(fa: Path, fb: Path) -> bool: + """Compare two regular files by exec bits, size, then byte content. + + Reads in fixed-size chunks rather than slurping the whole file so a large + plugin asset can't exhaust memory during the migration scan. Only the + execute bits are compared (not the full S_IMODE): a local exec-bit change + (e.g. an entry script the user made executable) is functionally meaningful + and must be preserved, but read/write permission differences caused by the + environment's umask or group settings should not spuriously force a .bak. + """ + sa, sb = fa.stat(), fb.stat() + if (sa.st_mode & _EXEC_BITS) != (sb.st_mode & _EXEC_BITS): + return False + if sa.st_size != sb.st_size: + return False + chunk = 64 * 1024 + with fa.open('rb') as a, fb.open('rb') as b: + while True: + ba, bb = a.read(chunk), b.read(chunk) + if ba != bb: + return False + if not ba: + return True + + +def _dirs_differ(copy_dir: Path, repo_dir: Path) -> bool: + """True if deleting the legacy copy would discard data not in the clone. + + Walks *every* entry (regular files, symlinks, and directories — including + empty ones). An entry is treated as divergence only when it represents + data the copy holds but the clone does not: a copy-only entry (user-added + file/symlink/empty dir), or a common entry whose type, symlink target, or + file content differs. Upstream-only additions (present in the clone but + not the copy) are *not* a difference — deleting the copy loses nothing — so + a routine upstream change no longer forces a manual-reconcile .bak. + """ + def _entries(base: Path) -> set[Path]: + return {p.relative_to(base) for p in base.rglob('*')} + + copy_entries = _entries(copy_dir) + repo_entries = _entries(repo_dir) + + # User-added entries live only in the copy and would be lost on delete. + if copy_entries - repo_entries: + return True + + for rel in copy_entries: + fa, fb = copy_dir / rel, repo_dir / rel + kind = _entry_kind(fa) + if kind != _entry_kind(fb): + return True + if kind == 'symlink': + if fa.readlink() != fb.readlink(): + return True + elif kind == 'file': + if not _files_equal(fa, fb): + return True + elif kind == 'other': + # A socket / pipe / device the copy holds can't be content-compared, + # so it can't be proven identical — treat it as divergence and fall + # back to preserving the copy (.bak) rather than risk deleting data + # we couldn't inspect. + return True + return False + + +def _clone_is_healthy(clone_dir: Path) -> bool: + """True if clone_dir looks like a usable repo clone (has .git + registry.yml). + + A repos/ dir that lost its .git (or whose registry.yml went missing) points + migrated plugins at a tree that can never be pulled/updated again, so it + must be re-cloned rather than reused. + """ + return ( + clone_dir.is_dir() + and (clone_dir / '.git').exists() + and (clone_dir / 'registry.yml').is_file() + ) + + +def _reclaim_or_protect_existing(clone_dir: Path) -> None: + """Clear a reusable leftover at clone_dir, or protect a .git-bearing tree. + + Called before (re-)cloning into clone_dir. Three cases: + + - clone_dir is a symlink (broken, to a file, or even to a dir) -> remove + the link; it is never a real persistent clone and git_clone would fail. + - clone_dir does not exist -> nothing to do. + - clone_dir exists but is not a directory (a stray file squatting on the + path) -> remove it so git_clone can create the directory; a regular file + cannot hold a git working tree, so nothing is lost. + - clone_dir is a directory without .git -> a broken/partial clone that can + never be pulled; remove it so a fresh clone repairs it. + - clone_dir is a directory *with* .git -> it may hold uncommitted or + unpushed local work, so refuse to delete it and raise asking the user to + repair/remove it manually rather than silently destroying their changes. + """ + # Check the symlink first: is_dir()/exists() both follow symlinks, so a + # symlink-to-dir would otherwise slip through as a "directory". + if clone_dir.is_symlink(): + clone_dir.unlink() + return + if not clone_dir.exists(): + return + if not clone_dir.is_dir(): + # A regular file is squatting on the path; git_clone would fail. It can + # hold no git working tree, so removing it loses nothing. + clone_dir.unlink() + return + if not (clone_dir / '.git').exists(): + shutil.rmtree(clone_dir) + return + raise PluginError( + f"Existing clone '{clone_dir}' has a .git but is missing " + f"registry.yml; refusing to delete it to avoid losing local " + f"changes. Restore registry.yml (e.g. 'git checkout -- " + f"registry.yml') or remove the directory manually, then retry." + ) + + +def _build_persisted_repo( + repo: RegisteredRepository, + dir_name: str, + reg_info: Optional["RegistryInfo"], +) -> RegisteredRepository: + """Build a repo row with local_path = repos/ + a refreshed plugin + list, WITHOUT saving. + + Used after a fresh clone *and* when reusing a healthy clone left by an + earlier run (a pre-persistent-clone registration with no local_path), so the + plugins.yml entry is repaired identically in both cases and future runs take + the local_path fast path. + + `reg_info` is the clone's already-parsed registry.yml (the caller parses it + exactly once and threads it through here and on to migrate()), so this no + longer re-reads/re-parses the file itself. When it is None the repo's prior + plugin list is kept. + + The caller is responsible for persisting the returned row: during migration + every repo update is accumulated and flushed in a single plugins.yml save + (see migrate()), so this no longer writes per repo. + """ + plugins = [ + AvailablePlugin(name=e.name, description=e.description, path=e.path) + for e in reg_info.plugins + ] if reg_info else list(repo.plugins) + return RegisteredRepository( + name=repo.name, url=repo.url, added_at=repo.added_at, + local_path=f"repos/{dir_name}", plugins=plugins, + ) + + +def _ensure_repo_cloned( + registry: PluginRegistry, + repo: RegisteredRepository, + pending_repos: list[RegisteredRepository], +) -> tuple[Path, RegisteredRepository, Optional["RegistryInfo"]]: + """Return the repos/ clone dir for a repo, cloning it if necessary. + + If the repo was registered before persistent-clone support (no local_path), + the clone dir is missing, or the existing clone is broken (missing .git / + registry.yml), perform a full clone and stage local_path + a refreshed + plugin list for persistence. A healthy repos/ clone left by an + earlier run is reused (and its missing local_path staged) rather than + re-cloned or protected. + + Returns ``(clone_dir, repo, reg_info)`` where ``reg_info`` is the clone's + parsed registry.yml whenever this function already had to parse it (the + derived-reuse and fresh-clone paths), so migrate() can reuse it instead of + re-reading the same file. It is None only on the local_path fast path, + where no parse happened; migrate() parses lazily there. + + Any repo row that needs (re)persisting is appended to `pending_repos` + instead of being saved here; migrate() flushes them in a single + plugins.yml save before any destructive cleanup, so the registry still + durably points at the clone before old copies are retired, but the save + count stays O(1) rather than one save per cloned repo. + """ + from .installer import git_clone, parse_registry_yml + from .repo_manager import _url_to_repos_dirname + + if repo.local_path: + clone_dir = registry.devbase_root / repo.local_path + if _clone_is_healthy(clone_dir): + return clone_dir, repo, None + _reclaim_or_protect_existing(clone_dir) + + dir_name = _url_to_repos_dirname(repo.url) + repos_dir = registry.get_repos_dir() + repos_dir.mkdir(exist_ok=True) + clone_dir = repos_dir / dir_name + + # A pre-persistent-clone registration (no local_path) may already have a + # healthy repos/ clone from an earlier run; reuse it instead of + # protecting (raising) on its .git, just like the local_path branch above — + # only stage the missing local_path so future runs take the fast path. + if _clone_is_healthy(clone_dir): + reg_info = parse_registry_yml(clone_dir) + updated = _build_persisted_repo(repo, dir_name, reg_info) + pending_repos.append(updated) + return clone_dir, updated, reg_info + + # A leftover from a previously interrupted clone (e.g. disk full or network + # drop) would otherwise be reused forever — re-cloning is skipped while + # parse_registry_yml() keeps failing, so migration can never self-heal. + # Remove a leftover that is *not* a valid clone (missing .git, or a stray + # non-directory squatting on the path) so the clone below re-creates it + # cleanly; but a dir that still has .git may hold uncommitted/unpushed work + # and is protected (raises) rather than destroyed. + _reclaim_or_protect_existing(clone_dir) + + freshly_cloned = False + if not clone_dir.is_dir(): + try: + git_clone(repo.url, clone_dir, shallow=False) + freshly_cloned = True + except Exception: + # Drop a partial clone so the next run starts from a clean slate. + if clone_dir.is_dir(): + shutil.rmtree(clone_dir) + raise + + reg_info = parse_registry_yml(clone_dir) + if not reg_info: + # registry.yml is missing/invalid. Only discard a clone we just made + # (guaranteed to hold no local work); an existing .git-bearing dir is + # protected so we never delete uncommitted/unpushed changes — surface a + # recoverable error instead, mirroring the local_path branch. + if freshly_cloned: + shutil.rmtree(clone_dir) + raise PluginError( + f"No registry.yml found in cloned repository '{repo.name}'." + ) + raise PluginError( + f"Existing clone '{clone_dir}' has a .git but is missing " + f"registry.yml; refusing to delete it to avoid losing local " + f"changes. Restore registry.yml (e.g. 'git checkout -- " + f"registry.yml') or remove the directory manually, then retry." + ) + + updated = _build_persisted_repo(repo, dir_name, reg_info) + pending_repos.append(updated) + logger.info("Repository '%s' cloned to %s", repo.name, updated.local_path) + return clone_dir, updated, reg_info + + +def _cleanup_plugins_dir(registry: PluginRegistry) -> bool: + """Normalize plugins/ to just .gitkeep once it holds no live copy installs. + + Conservatively kept untouched when anything still depends on it: + --link installs, skipped legacy copies still referenced by plugins.yml, or + preserved .bak directories awaiting manual reconciliation. Returns + True only when plugins/ was reduced to .gitkeep. + """ + plugins_dir = registry.get_plugins_dir() + if not plugins_dir.is_dir(): + return False + + if any(p.linked for p in registry.list_installed()): + return False + + # A skipped legacy install still points into plugins/ — keep its files. + if needs_migration(registry): + return False + + entries = [e for e in plugins_dir.iterdir() if e.name != '.gitkeep'] + bak_dirs = [e for e in entries if _is_bak_name(e.name)] + if bak_dirs: + logger.info( + "plugins/ retained: %d preserved .bak dir(s) await manual reconciliation", + len(bak_dirs), + ) + return False + + # Anything left over that is neither .gitkeep nor a preserved .bak means + # plugins/ is not actually clean; leave it untouched and report uncleaned. + leftover = [e for e in entries if e not in bak_dirs] + if leftover: + logger.info( + "plugins/ retained: %d unexpected entr(y/ies) remain (%s)", + len(leftover), ", ".join(sorted(e.name for e in leftover)), + ) + return False + + gitkeep = plugins_dir / '.gitkeep' + if not gitkeep.exists(): + gitkeep.touch() + return True + + +def migrate(registry: PluginRegistry, *, run_sync: bool = True) -> MigrationResult: + """Migrate legacy plugins/ copy installs to repos/ clones. + + For each legacy plugin: ensure its source repo is cloned to repos/, rewrite + InstalledPlugin.path to the repos/ location, then delete the old copy when + byte-identical or preserve it as .bak when it diverged. Finally + re-sync project symlinks and empty plugins/ to .gitkeep when safe. + """ + from .installer import parse_registry_yml + from .syncer import load_plugin_info, sync_projects + + result = MigrationResult() + legacy = [p for p in registry.list_installed() if _is_legacy_plugin(p)] + if not legacy: + return result + + # Two-phase migration so a registry-save failure can never leave a copy + # deleted while plugins.yml still points at the stale plugins/ path: + # + # Phase 1 (no destructive fs ops): validate the repo/clone/entry and + # decide each copy's fate (delete vs preserve as .bak), collecting the + # cloned-repo rows in `pending_repos`, the repos/ path rewrites in + # `pending`, and the retire actions in `retire`. Cloning stages the + # repo row in `pending_repos` rather than saving per clone, so the save + # count stays O(1) regardless of how many repos are cloned. + # Persist: write every repo row + path rewrite in a single plugins.yml + # save (save_migration), flushing the staged clones BEFORE any cleanup. + # Phase 2 (destructive): only after plugins.yml is durably at repos/ do we + # delete/rename the old copies. + # + # Ordering rationale: if the save raises, no copy has been touched yet, so + # the registry stays legacy and the next run retries cleanly with the copies + # intact (recoverable). Conversely the validated repos/ clone is known good + # before we commit, so committing the rewrite first cannot strand a plugin + # on a missing tree; a stray copy left by a phase-2 hiccup is merely surfaced + # by _cleanup_plugins_dir, never silent data loss. + pending: list[InstalledPlugin] = [] + pending_repos: list[RegisteredRepository] = [] # cloned-repo rows to persist + retire: list[tuple[str, Path, Path]] = [] # (plugin_name, old_dir, repo_dir) + + # Index every registered repo by URL once. registry.get_repository_by_url + # re-reads (and re-parses) plugins.yml on every call, so calling it per + # legacy plugin is O(N) disk reads over the loop; a single up-front read + # collapses that to O(1). Last-wins on duplicate URLs mirrors the name-keyed + # upsert model used elsewhere in the registry. + repos_by_url = { + repo.url: repo for repo in registry.list_repositories() if repo.url + } + + for plugin in legacy: + try: + repo = repos_by_url.get(plugin.source) if plugin.source else None + if not repo: + result.skipped.append(plugin.name) + result.errors.append( + f"{plugin.name}: source repository not registered " + f"({plugin.source or 'no source URL'})" + ) + continue + + clone_dir, repo, reg_info = _ensure_repo_cloned( + registry, repo, pending_repos, + ) + + # _ensure_repo_cloned already parsed registry.yml on the clone/reuse + # paths; only the healthy local_path fast path returns None, so parse + # lazily there instead of unconditionally re-reading the same file. + if reg_info is None: + reg_info = parse_registry_yml(clone_dir) + entry = None + if reg_info: + entry = next( + (e for e in reg_info.plugins if e.name == plugin.name), None, + ) + if not entry: + result.skipped.append(plugin.name) + result.errors.append( + f"{plugin.name}: not found in registry.yml of '{repo.name}'" + ) + continue + + repo_plugin_dir = clone_dir / entry.path.rstrip('/') + if not repo_plugin_dir.is_dir(): + result.skipped.append(plugin.name) + result.errors.append( + f"{plugin.name}: plugin dir missing in clone: {repo_plugin_dir}" + ) + continue + + rel_path = str(repo_plugin_dir.relative_to(registry.devbase_root)) + info = load_plugin_info(repo_plugin_dir) + version = info.version if info else plugin.version + + old_dir = registry.devbase_root / plugin.path + pending.append(InstalledPlugin( + name=plugin.name, + version=version, + source=plugin.source, + installed_at=plugin.installed_at, + path=rel_path, + linked=False, + )) + retire.append((plugin.name, old_dir, repo_plugin_dir)) + except Exception as e: + result.skipped.append(plugin.name) + result.errors.append(f"{plugin.name}: {e}") + + # Persist every staged cloned-repo row AND validated path rewrite in a + # single save BEFORE retiring any copy. This both (a) keeps the registry + # durably pointing at the repos/ clones before destructive cleanup — the + # two-phase atomicity invariant — and (b) collapses what used to be one save + # per cloned repo plus the path-rewrite save into a single O(1) write. A + # failure here aborts with the copies untouched (recoverable). + registry.save_migration(pending_repos, pending) + + # Now that plugins.yml durably points at repos/, retire the old copies. + for name, old_dir, repo_plugin_dir in retire: + try: + if old_dir.is_dir() and not old_dir.is_symlink(): + if _dirs_differ(old_dir, repo_plugin_dir): + bak = _unique_bak_path(old_dir) + old_dir.rename(bak) + result.preserved.append(name) + logger.warning( + "Plugin '%s' had local changes — preserved at %s " + "(reconcile manually, then remove)", + name, bak, + ) + else: + shutil.rmtree(old_dir) + result.migrated.append(name) + else: + result.migrated.append(name) + except Exception as e: + # Registry is already at repos/ (valid clone), so the plugin works; + # a copy we failed to retire just lingers under plugins/ and is + # surfaced by _cleanup_plugins_dir rather than lost. + result.migrated.append(name) + result.errors.append(f"{name}: copy not retired: {e}") + + if run_sync: + sync_projects(registry) + + result.plugins_dir_cleaned = _cleanup_plugins_dir(registry) + return result diff --git a/lib/devbase/plugin/registry.py b/lib/devbase/plugin/registry.py index f852fcb..b5eb6ed 100644 --- a/lib/devbase/plugin/registry.py +++ b/lib/devbase/plugin/registry.py @@ -65,11 +65,79 @@ def get(self, name: str) -> Optional[InstalledPlugin]: def add(self, plugin: InstalledPlugin) -> None: """Add a plugin to the registry""" - data = self._load() + self.add_many([plugin]) + + @staticmethod + def _apply_plugins(data: dict, plugins: list[InstalledPlugin]) -> None: + """Upsert `plugins` into `data['installed_plugins']` (in place). + + Duplicate names within `plugins` are de-duplicated (last one wins) so a + caller passing the same plugin twice can't leave two conflicting entries + in plugins.yml. Does not touch disk — callers persist `data` once. + """ + if not plugins: + return + # Keep only the last entry per name; dict preserves insertion order so + # the surviving entries stay in the order they were last supplied. + unique = list({p.name: p for p in plugins}.values()) + names = {p.name for p in unique} data['installed_plugins'] = [ - p for p in data['installed_plugins'] if p['name'] != plugin.name + p for p in data['installed_plugins'] if p['name'] not in names + ] + data['installed_plugins'].extend(p.to_dict() for p in unique) + + @staticmethod + def _apply_repositories( + data: dict, repos: list[RegisteredRepository], + ) -> None: + """Upsert `repos` into `data['repositories']` (in place). + + Duplicate names (last one wins) are de-duplicated so accumulating the + same repo twice can't leave two conflicting rows. Does not touch disk. + """ + if not repos: + return + unique = list({r.name: r for r in repos}.values()) + names = {r.name for r in unique} + data['repositories'] = [ + r for r in data['repositories'] if r['name'] not in names ] - data['installed_plugins'].append(plugin.to_dict()) + data['repositories'].extend(r.to_dict() for r in unique) + + def add_many(self, plugins: list[InstalledPlugin]) -> None: + """Add/replace multiple plugins with a single load+save. + + Saving plugins.yml once for a batch avoids the repeated read+atomic + rewrite that calling add() per plugin would incur (e.g. during + migration of many legacy installs). + + Duplicate names within `plugins` are de-duplicated (last one wins) so a + caller passing the same plugin twice can't leave two conflicting entries + in plugins.yml.""" + if not plugins: + return + data = self._load() + self._apply_plugins(data, plugins) + self._save(data) + + def save_migration( + self, + repositories: list[RegisteredRepository], + plugins: list[InstalledPlugin], + ) -> None: + """Persist repository + plugin updates from a migration in ONE save. + + migrate() clones each legacy repo and rewrites each plugin's path; both + the refreshed repository rows (local_path + plugin list) and the plugin + path rewrites must land durably *before* any old copy is deleted. This + applies both sets of upserts to a single loaded snapshot and writes + plugins.yml exactly once, so the save count stays O(1) regardless of how + many repos were cloned (rather than one save per cloned repo).""" + if not repositories and not plugins: + return + data = self._load() + self._apply_repositories(data, repositories) + self._apply_plugins(data, plugins) self._save(data) def remove(self, name: str) -> bool: @@ -125,10 +193,7 @@ def get_repository_by_url(self, url: str) -> Optional[RegisteredRepository]: def add_repository(self, repo: RegisteredRepository) -> None: """Add or update a repository in the registry""" data = self._load() - data['repositories'] = [ - r for r in data['repositories'] if r['name'] != repo.name - ] - data['repositories'].append(repo.to_dict()) + self._apply_repositories(data, [repo]) self._save(data) def remove_repository(self, name: str) -> bool: diff --git a/lib/devbase/plugin/updater.py b/lib/devbase/plugin/updater.py index 4fdd163..d66d18a 100644 --- a/lib/devbase/plugin/updater.py +++ b/lib/devbase/plugin/updater.py @@ -189,6 +189,9 @@ def update_plugin(registry: PluginRegistry, name: Optional[str] = None) -> None: Raises PluginError on failure. """ + from .installer import _auto_migrate + _auto_migrate(registry) + installed = registry.list_installed() if not installed: logger.info("No plugins installed") diff --git a/tests/plugin/test_migrator.py b/tests/plugin/test_migrator.py new file mode 100644 index 0000000..c833fa0 --- /dev/null +++ b/tests/plugin/test_migrator.py @@ -0,0 +1,1094 @@ +"""Tests for PLAN04 PR2: legacy plugins/ -> repos/ migration""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import patch + +import yaml +import pytest + +from devbase.errors import PluginError +from devbase.plugin.models import ( + AvailablePlugin, + InstalledPlugin, + RegisteredRepository, +) +from devbase.plugin.registry import PluginRegistry +from devbase.plugin.migrator import ( + _cleanup_plugins_dir, + _clone_is_healthy, + _dirs_differ, + _is_bak_name, + _is_legacy_plugin, + migrate, + needs_migration, +) + +URL = "https://github.com/testorg/testrepo.git" +DIRNAME = "github.com--testorg--testrepo" + + +def _make_repo_clone(devbase_root: Path, plugins: list[dict]) -> Path: + """Create repos// with .git, registry.yml and plugin dirs.""" + repo_dir = devbase_root / "repos" / DIRNAME + repo_dir.mkdir(parents=True, exist_ok=True) + (repo_dir / ".git").mkdir(exist_ok=True) + + entries = [] + for p in plugins: + pdir = repo_dir / p["path"] + pdir.mkdir(parents=True, exist_ok=True) + (pdir / "plugin.yml").write_text( + yaml.dump({"name": p["name"], "version": p.get("version", "1.0.0")}) + ) + for proj in p.get("projects", []): + proj_dir = pdir / "projects" / proj + proj_dir.mkdir(parents=True, exist_ok=True) + (proj_dir / "compose.yml").write_text("services: {}\n") + entries.append({"name": p["name"], "path": p["path"], "description": ""}) + + (repo_dir / "registry.yml").write_text( + yaml.dump({"name": "testrepo", "plugins": entries}) + ) + return repo_dir + + +def _register_repo(registry: PluginRegistry, plugins: list[dict], + local_path: str | None = f"repos/{DIRNAME}") -> None: + registry.add_repository(RegisteredRepository( + name="testrepo", url=URL, added_at=registry.now_iso(), + local_path=local_path or "", + plugins=[ + AvailablePlugin(name=p["name"], description="", path=p["path"]) + for p in plugins + ], + )) + + +def _make_legacy_copy(devbase_root: Path, name: str, plugin: dict, + extra: dict[str, str] | None = None) -> Path: + """Create plugins// mirroring the repo plugin dir (optionally diverged).""" + pdir = devbase_root / "plugins" / name + pdir.mkdir(parents=True, exist_ok=True) + (pdir / "plugin.yml").write_text( + yaml.dump({"name": plugin["name"], "version": plugin.get("version", "1.0.0")}) + ) + for proj in plugin.get("projects", []): + proj_dir = pdir / "projects" / proj + proj_dir.mkdir(parents=True, exist_ok=True) + (proj_dir / "compose.yml").write_text("services: {}\n") + for rel, content in (extra or {}).items(): + f = pdir / rel + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(content) + return pdir + + +# ── Fixtures ──────────────────────────────────────────────────── + + +@pytest.fixture +def devbase_root(tmp_path): + (tmp_path / "projects").mkdir() + return tmp_path + + +@pytest.fixture +def registry(devbase_root): + return PluginRegistry(devbase_root) + + +def _installed(name: str, path: str, linked: bool = False) -> InstalledPlugin: + return InstalledPlugin( + name=name, + version="1.0.0", + source="https://github.com/testorg/testrepo.git", + installed_at="2026-01-01T00:00:00+00:00", + path=path, + linked=linked, + ) + + +# ── detection ─────────────────────────────────────────────────── + + +class TestIsLegacyPlugin: + def test_copy_install_under_plugins_is_legacy(self): + assert _is_legacy_plugin(_installed("adminer", "plugins/adminer")) is True + + def test_repos_based_is_not_legacy(self): + plugin = _installed( + "adminer", "repos/github.com--testorg--testrepo/adminer", + ) + assert _is_legacy_plugin(plugin) is False + + def test_linked_under_plugins_is_not_legacy(self): + plugin = _installed("local", "plugins/local", linked=True) + assert _is_legacy_plugin(plugin) is False + + +class TestDirsDiffer: + def _write(self, base: Path, rel: str, content: str) -> None: + p = base / rel + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(content) + + def test_identical_dirs_do_not_differ(self, tmp_path): + a, b = tmp_path / "a", tmp_path / "b" + self._write(a, "plugin.yml", "name: x\n") + self._write(a, "projects/p/compose.yml", "services: {}\n") + self._write(b, "plugin.yml", "name: x\n") + self._write(b, "projects/p/compose.yml", "services: {}\n") + assert _dirs_differ(a, b) is False + + def test_changed_file_content_differs(self, tmp_path): + a, b = tmp_path / "a", tmp_path / "b" + self._write(a, "plugin.yml", "name: x\nversion: 9\n") + self._write(b, "plugin.yml", "name: x\n") + assert _dirs_differ(a, b) is True + + def test_same_size_different_content_differs(self, tmp_path): + # Same byte length but different content — exercises the streamed + # chunk comparison rather than the size fast-path. + a, b = tmp_path / "a", tmp_path / "b" + self._write(a, "plugin.yml", "name: aaaa\n") + self._write(b, "plugin.yml", "name: bbbb\n") + assert _dirs_differ(a, b) is True + + def test_extra_file_in_a_differs(self, tmp_path): + a, b = tmp_path / "a", tmp_path / "b" + self._write(a, "plugin.yml", "name: x\n") + self._write(a, "projects/p/.env", "SECRET=1\n") + self._write(b, "plugin.yml", "name: x\n") + assert _dirs_differ(a, b) is True + + def test_upstream_only_addition_does_not_differ(self, tmp_path): + # A file present only upstream (in the clone) is not data the legacy + # copy holds, so deleting the copy loses nothing — a routine upstream + # addition must NOT force a manual-reconcile .bak. + a, b = tmp_path / "a", tmp_path / "b" + self._write(a, "plugin.yml", "name: x\n") + self._write(b, "plugin.yml", "name: x\n") + self._write(b, "README.md", "new upstream doc\n") + assert _dirs_differ(a, b) is False + + def test_extra_symlink_in_copy_differs(self, tmp_path): + a, b = tmp_path / "a", tmp_path / "b" + self._write(a, "plugin.yml", "name: x\n") + self._write(b, "plugin.yml", "name: x\n") + # User-added symlink present only in the legacy copy + (a / "link").symlink_to("plugin.yml") + assert _dirs_differ(a, b) is True + + def test_empty_dir_only_in_copy_differs(self, tmp_path): + a, b = tmp_path / "a", tmp_path / "b" + self._write(a, "plugin.yml", "name: x\n") + self._write(b, "plugin.yml", "name: x\n") + # User-added empty directory present only in the legacy copy + (a / "logs").mkdir(parents=True) + assert _dirs_differ(a, b) is True + + def test_symlink_target_change_differs(self, tmp_path): + a, b = tmp_path / "a", tmp_path / "b" + self._write(a, "plugin.yml", "name: x\n") + self._write(b, "plugin.yml", "name: x\n") + (a / "link").symlink_to("a-target") + (b / "link").symlink_to("b-target") + assert _dirs_differ(a, b) is True + + def test_file_vs_symlink_type_mismatch_differs(self, tmp_path): + a, b = tmp_path / "a", tmp_path / "b" + self._write(a, "plugin.yml", "name: x\n") + self._write(b, "plugin.yml", "name: x\n") + # Same name, but a regular file in the copy vs a symlink in the clone + self._write(a, "shared", "data\n") + (b / "shared").symlink_to("plugin.yml") + assert _dirs_differ(a, b) is True + + def test_identical_symlinks_do_not_differ(self, tmp_path): + a, b = tmp_path / "a", tmp_path / "b" + self._write(a, "plugin.yml", "name: x\n") + self._write(b, "plugin.yml", "name: x\n") + (a / "link").symlink_to("plugin.yml") + (b / "link").symlink_to("plugin.yml") + assert _dirs_differ(a, b) is False + + def test_exec_bit_change_differs(self, tmp_path): + # Same name, size and bytes, but the copy made the script executable. + # Deleting the copy would lose that mode change, so it must count as + # divergence (preserved as .bak) rather than a clean migration. + a, b = tmp_path / "a", tmp_path / "b" + self._write(a, "entry.sh", "#!/bin/sh\necho hi\n") + self._write(b, "entry.sh", "#!/bin/sh\necho hi\n") + (b / "entry.sh").chmod(0o644) + (a / "entry.sh").chmod(0o755) + # Sanity: bytes identical, only mode differs. + assert (a / "entry.sh").read_bytes() == (b / "entry.sh").read_bytes() + assert _dirs_differ(a, b) is True + # And once the modes match, the copy is treated as identical again. + (a / "entry.sh").chmod(0o644) + assert _dirs_differ(a, b) is False + + +class TestIsBakName: + def test_plain_bak_matches(self): + assert _is_bak_name("carmo.bak") is True + + def test_numbered_bak_matches(self): + assert _is_bak_name("carmo.bak-2") is True + assert _is_bak_name("carmo.bak-17") is True + + def test_substring_bak_does_not_match(self): + # The previous `'.bak' in name` check wrongly flagged these. + assert _is_bak_name("my.bakery") is False + assert _is_bak_name("notes.bak.txt") is False + assert _is_bak_name("backup") is False + + +class TestCloneIsHealthy: + def test_valid_clone_is_healthy(self, devbase_root): + plugins = [{"name": "adminer", "path": "adminer"}] + clone = _make_repo_clone(devbase_root, plugins) + assert _clone_is_healthy(clone) is True + + def test_missing_git_is_unhealthy(self, devbase_root): + plugins = [{"name": "adminer", "path": "adminer"}] + clone = _make_repo_clone(devbase_root, plugins) + shutil_rmtree_git(clone) + assert _clone_is_healthy(clone) is False + + def test_missing_registry_yml_is_unhealthy(self, devbase_root): + plugins = [{"name": "adminer", "path": "adminer"}] + clone = _make_repo_clone(devbase_root, plugins) + (clone / "registry.yml").unlink() + assert _clone_is_healthy(clone) is False + + +def shutil_rmtree_git(clone: Path) -> None: + import shutil + shutil.rmtree(clone / ".git") + + +class TestNeedsMigration: + def test_true_when_legacy_present(self, registry): + registry.add(_installed("adminer", "plugins/adminer")) + assert needs_migration(registry) is True + + def test_false_when_only_repos_and_linked(self, registry): + registry.add(_installed( + "adminer", "repos/github.com--testorg--testrepo/adminer", + )) + registry.add(_installed("local", "plugins/local", linked=True)) + assert needs_migration(registry) is False + + def test_false_when_empty(self, registry): + assert needs_migration(registry) is False + + +# ── migrate() ─────────────────────────────────────────────────── + + +class TestMigrateClean: + def test_clean_migration_updates_path_and_deletes_copy(self, registry, devbase_root): + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + + result = migrate(registry) + + # path rewritten to repos/-based location + migrated_plugin = registry.get("adminer") + assert migrated_plugin.path == f"repos/{DIRNAME}/adminer" + # old copy removed + assert not (devbase_root / "plugins" / "adminer").exists() + # result bookkeeping + assert result.migrated == ["adminer"] + assert result.preserved == [] + assert result.errors == [] + + def test_clean_migration_creates_repos_symlink(self, registry, devbase_root): + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + + migrate(registry) + + link = devbase_root / "projects" / "adminer" + assert link.is_symlink() + target = (link.parent / link.readlink()).resolve() + assert target == (devbase_root / "repos" / DIRNAME / "adminer" / "projects" / "adminer").resolve() + + def test_clean_migration_empties_plugins_dir_to_gitkeep(self, registry, devbase_root): + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + + result = migrate(registry) + + plugins_dir = devbase_root / "plugins" + remaining = sorted(p.name for p in plugins_dir.iterdir()) + assert remaining == [".gitkeep"] + assert result.plugins_dir_cleaned is True + + +class TestMigrateWithLocalChanges: + def test_diverged_copy_preserved_as_bak(self, registry, devbase_root): + plugins = [{"name": "carmo", "path": "carmo", "projects": ["carmo"]}] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + # User added a local .env that does not exist upstream + _make_legacy_copy(devbase_root, "carmo", plugins[0], + extra={"projects/carmo/.env": "LOCAL=1\n"}) + registry.add(_installed("carmo", "plugins/carmo")) + + result = migrate(registry) + + assert result.preserved == ["carmo"] + assert result.migrated == [] + bak = devbase_root / "plugins" / "carmo.bak" + assert bak.is_dir() + assert (bak / "projects" / "carmo" / ".env").read_text() == "LOCAL=1\n" + # original copy path no longer present + assert not (devbase_root / "plugins" / "carmo").exists() + + def test_bak_retained_plugins_dir_not_cleaned(self, registry, devbase_root): + plugins = [{"name": "carmo", "path": "carmo", "projects": ["carmo"]}] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + _make_legacy_copy(devbase_root, "carmo", plugins[0], + extra={"extra.txt": "x\n"}) + registry.add(_installed("carmo", "plugins/carmo")) + + result = migrate(registry) + + assert result.plugins_dir_cleaned is False + # path is still rewritten to repos/ even when copy is preserved + assert registry.get("carmo").path == f"repos/{DIRNAME}/carmo" + + def test_existing_bak_is_not_overwritten(self, registry, devbase_root): + plugins = [{"name": "carmo", "path": "carmo", "projects": ["carmo"]}] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + _make_legacy_copy(devbase_root, "carmo", plugins[0], + extra={"projects/carmo/.env": "LOCAL=1\n"}) + registry.add(_installed("carmo", "plugins/carmo")) + # A previous migration run already preserved carmo.bak with its own data + prev_bak = devbase_root / "plugins" / "carmo.bak" + prev_bak.mkdir(parents=True) + (prev_bak / "old.txt").write_text("PREVIOUS\n") + + result = migrate(registry) + + assert result.preserved == ["carmo"] + # the old .bak survives untouched + assert (prev_bak / "old.txt").read_text() == "PREVIOUS\n" + # the new diverged copy lands in a distinct .bak-2 dir + new_bak = devbase_root / "plugins" / "carmo.bak-2" + assert new_bak.is_dir() + assert (new_bak / "projects" / "carmo" / ".env").read_text() == "LOCAL=1\n" + + +class TestMigrateClonesMissingRepo: + def test_repo_without_local_path_is_cloned(self, registry, devbase_root): + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + # Repo registered WITHOUT local_path (legacy registration) + _register_repo(registry, plugins, local_path=None) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + + def fake_clone(url, dest, **kwargs): + _make_repo_clone(dest.parent.parent, plugins) + + with patch("devbase.plugin.installer.git_clone", side_effect=fake_clone): + result = migrate(registry) + + assert result.migrated == ["adminer"] + repo = registry.get_repository_by_url(URL) + assert repo.local_path == f"repos/{DIRNAME}" + assert registry.get("adminer").path == f"repos/{DIRNAME}/adminer" + + +class TestMigrateKeepsLinked: + def test_linked_install_keeps_plugins_dir(self, registry, devbase_root): + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + # A separate --link install lives under plugins/ and must be preserved + (devbase_root / "plugins" / "locallink").mkdir(parents=True) + registry.add(_installed("locallink", "plugins/locallink", linked=True)) + + result = migrate(registry) + + assert result.migrated == ["adminer"] + assert result.plugins_dir_cleaned is False + assert (devbase_root / "plugins" / "locallink").is_dir() + + +class TestMigrateSkips: + def test_unregistered_source_is_skipped(self, registry, devbase_root): + _make_legacy_copy( + devbase_root, "orphan", + {"name": "orphan", "path": "orphan", "projects": ["orphan"]}, + ) + registry.add(_installed("orphan", "plugins/orphan")) + + result = migrate(registry) + + assert result.skipped == ["orphan"] + assert result.migrated == [] + # untouched: copy stays, path unchanged + assert (devbase_root / "plugins" / "orphan").is_dir() + assert registry.get("orphan").path == "plugins/orphan" + + +class TestCleanupPluginsDir: + def test_unexpected_leftover_keeps_plugins_dir_uncleaned(self, registry, devbase_root): + # plugins/ holds a stray entry that is neither .gitkeep nor a .bak + # (e.g. left behind by an external tool); cleanup must not claim it + # cleaned and must leave the entry in place. + plugins_dir = devbase_root / "plugins" + plugins_dir.mkdir() + (plugins_dir / "stray").mkdir() + + assert _cleanup_plugins_dir(registry) is False + assert (plugins_dir / "stray").is_dir() + + def test_bak_lookalike_is_not_treated_as_preserved(self, registry, devbase_root): + # An entry whose name merely contains ".bak" as a substring (e.g. + # "my.bakery") is NOT a preserved copy; it must be reported as an + # unexpected leftover rather than silently retained as a .bak. + plugins_dir = devbase_root / "plugins" + plugins_dir.mkdir() + (plugins_dir / "my.bakery").mkdir() + + assert _cleanup_plugins_dir(registry) is False + # Still present (cleanup never deletes leftovers) and not mistaken for + # a .bak dir. + assert (plugins_dir / "my.bakery").is_dir() + + def test_numbered_bak_dir_is_retained(self, registry, devbase_root): + # A real preserved copy from a prior run (carmo.bak-2) keeps plugins/ + # uncleaned just like carmo.bak does. + plugins_dir = devbase_root / "plugins" + plugins_dir.mkdir() + (plugins_dir / "carmo.bak-2").mkdir() + + assert _cleanup_plugins_dir(registry) is False + assert (plugins_dir / "carmo.bak-2").is_dir() + + +class TestMigratePartialCloneRecovery: + def test_partial_clone_without_git_is_recloned(self, registry, devbase_root): + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + # Repo registered without local_path so migrate() takes the clone path. + _register_repo(registry, plugins, local_path=None) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + # Leftover partial clone from a prior interrupted run: a directory with + # no .git and no registry.yml that previously caused an infinite loop. + partial = devbase_root / "repos" / DIRNAME + partial.mkdir(parents=True) + (partial / "junk.txt").write_text("partial\n") + + def fake_clone(url, dest, **kwargs): + _make_repo_clone(dest.parent.parent, plugins) + + with patch("devbase.plugin.installer.git_clone", side_effect=fake_clone) as mock: + result = migrate(registry) + + # The broken dir was removed and a fresh clone performed. + mock.assert_called_once() + assert result.migrated == ["adminer"] + assert registry.get("adminer").path == f"repos/{DIRNAME}/adminer" + assert not (devbase_root / "repos" / DIRNAME / "junk.txt").exists() + + def test_registered_local_path_without_git_is_recloned(self, registry, devbase_root): + # local_path is recorded (repo migrated before) but the clone lost its + # .git — e.g. an interrupted operation. Reusing it would leave the + # migrated plugin pointing at an un-pullable tree, so it must re-clone. + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _register_repo(registry, plugins) # local_path = repos/ + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + # Existing repos/ dir that is NOT a valid clone (no .git, no registry.yml). + broken = devbase_root / "repos" / DIRNAME + broken.mkdir(parents=True) + (broken / "leftover.txt").write_text("broken\n") + + def fake_clone(url, dest, **kwargs): + _make_repo_clone(dest.parent.parent, plugins) + + with patch("devbase.plugin.installer.git_clone", side_effect=fake_clone) as mock: + result = migrate(registry) + + mock.assert_called_once() + assert result.migrated == ["adminer"] + assert (devbase_root / "repos" / DIRNAME / ".git").exists() + assert not (devbase_root / "repos" / DIRNAME / "leftover.txt").exists() + + +class TestMigrateBatchesRegistryWrites: + def test_multiple_plugins_all_persisted_with_single_save(self, registry, devbase_root): + plugins = [ + {"name": "adminer", "path": "adminer", "projects": ["adminer"]}, + {"name": "carmo", "path": "carmo", "projects": ["carmo"]}, + {"name": "redis", "path": "redis", "projects": ["redis"]}, + ] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + for p in plugins: + _make_legacy_copy(devbase_root, p["name"], p) + registry.add(_installed(p["name"], f"plugins/{p['name']}")) + + with patch.object( + PluginRegistry, "_save", autospec=True, side_effect=PluginRegistry._save, + ) as save_spy: + result = migrate(registry) + + assert sorted(result.migrated) == ["adminer", "carmo", "redis"] + # All three path rewrites land in a single plugins.yml save rather than + # one save per plugin. + assert save_spy.call_count == 1 + for p in plugins: + assert registry.get(p["name"]).path == f"repos/{DIRNAME}/{p['name']}" + + def test_repo_lookup_does_not_reload_plugins_yml_per_plugin( + self, registry, devbase_root, + ): + """Several legacy plugins share one repo, so the per-plugin source->repo + lookup must hit an in-memory index, not reload plugins.yml each time. + + migrate() previously called registry.get_repository_by_url per legacy + plugin; that re-reads (and re-parses) plugins.yml on every call, so N + plugins cost O(N) reads. It now builds a URL->repo index once up front, + so the plugins.yml read count must stay independent of the plugin count. + We count _load() calls and require it to stay well below one read per + plugin (a per-iteration regression would push it to >= N). + """ + plugins = [ + {"name": f"demo{i}", "path": f"demo{i}", "projects": [f"demo{i}"]} + for i in range(8) + ] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + for p in plugins: + _make_legacy_copy(devbase_root, p["name"], p) + registry.add(_installed(p["name"], f"plugins/{p['name']}")) + + load_calls = 0 + orig_load = registry._load + + def _counting_load(): + nonlocal load_calls + load_calls += 1 + return orig_load() + + with patch.object(registry, "_load", side_effect=_counting_load): + result = migrate(registry) + + assert sorted(result.migrated) == sorted(p["name"] for p in plugins) + # The per-plugin get_repository_by_url is gone, so the read count does + # not scale with the 8 plugins; guard the regression by requiring it to + # stay strictly below one read per plugin. + assert load_calls < len(plugins) + + def test_many_cloned_repos_persist_with_single_save(self, registry, devbase_root): + """O(1) saves even when every repo must be freshly cloned. + + Previously _ensure_repo_cloned saved plugins.yml once per cloned repo + (via add_repository), so migrating N repos cost N repo-saves + 1 + path-rewrite save. migrate() now stages every cloned repo row and + flushes them together with the path rewrites in one save_migration, so + the save count is O(1) regardless of repo count — while still persisting + the clones BEFORE any copy is retired (two-phase atomicity). + """ + from devbase.plugin.repo_manager import _url_to_repos_dirname + + repos = [ + ("https://github.com/testorg/repo-a.git", "alpha"), + ("https://github.com/testorg/repo-b.git", "beta"), + ("https://github.com/testorg/repo-c.git", "gamma"), + ] + dirnames = {url: _url_to_repos_dirname(url) for url, _ in repos} + + for url, pname in repos: + # Register each repo WITHOUT local_path so _ensure_repo_cloned takes + # the fresh-clone path (which used to save per repo). + registry.add_repository(RegisteredRepository( + name=pname, url=url, added_at=registry.now_iso(), local_path="", + plugins=[AvailablePlugin(name=pname, description="", path=pname)], + )) + # Legacy plugins/ copy + installed entry pointing at it. + pdir = devbase_root / "plugins" / pname + pdir.mkdir(parents=True, exist_ok=True) + (pdir / "plugin.yml").write_text(yaml.dump({"name": pname, "version": "1.0.0"})) + (pdir / "projects" / pname).mkdir(parents=True, exist_ok=True) + (pdir / "projects" / pname / "compose.yml").write_text("services: {}\n") + registry.add(InstalledPlugin( + name=pname, version="1.0.0", source=url, + installed_at="2026-01-01T00:00:00+00:00", + path=f"plugins/{pname}", linked=False, + )) + + def fake_clone(url, dest, **kwargs): + # Build a healthy clone identical (byte-for-byte) to the legacy copy + # so it is cleanly retired, proving the registry was persisted first. + pname = dest.name.split("--")[-1].replace("repo-", "") + # Map dest dirname back to the plugin name via the registered repos. + for u, p in repos: + if dest.name == dirnames[u]: + pname = p + break + dest.mkdir(parents=True, exist_ok=True) + (dest / ".git").mkdir(exist_ok=True) + pdir = dest / pname + pdir.mkdir(parents=True, exist_ok=True) + (pdir / "plugin.yml").write_text(yaml.dump({"name": pname, "version": "1.0.0"})) + (pdir / "projects" / pname).mkdir(parents=True, exist_ok=True) + (pdir / "projects" / pname / "compose.yml").write_text("services: {}\n") + (dest / "registry.yml").write_text(yaml.dump( + {"name": pname, "plugins": [{"name": pname, "path": pname, "description": ""}]} + )) + + with patch("devbase.plugin.installer.git_clone", side_effect=fake_clone): + with patch.object( + PluginRegistry, "_save", autospec=True, + side_effect=PluginRegistry._save, + ) as save_spy: + result = migrate(registry) + + assert sorted(result.migrated) == ["alpha", "beta", "gamma"] + # 3 cloned repos + 3 path rewrites, yet a single plugins.yml write. + assert save_spy.call_count == 1 + for url, pname in repos: + # Each repo row got its local_path persisted (registry durably points + # at the clone)... + repo = registry.get_repository_by_url(url) + assert repo.local_path == f"repos/{dirnames[url]}" + # ...and each plugin path was rewritten to the repos/ clone. + assert registry.get(pname).path == f"repos/{dirnames[url]}/{pname}" + # The legacy copy was retired only because the registry was persisted + # before Phase 2 (the clone is byte-identical, so it is deleted). + assert not (devbase_root / "plugins" / pname).exists() + + +class TestCmdPluginMigrate: + def test_command_runs_migration(self, devbase_root): + from devbase.commands.plugin import cmd_plugin_migrate + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _make_repo_clone(devbase_root, plugins) + registry = PluginRegistry(devbase_root) + _register_repo(registry, plugins) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + + rc = cmd_plugin_migrate(devbase_root) + + assert rc == 0 + assert PluginRegistry(devbase_root).get("adminer").path == f"repos/{DIRNAME}/adminer" + + def test_command_noop_when_nothing_to_migrate(self, devbase_root): + from devbase.commands.plugin import cmd_plugin_migrate + rc = cmd_plugin_migrate(devbase_root) + assert rc == 0 + + +class TestAutoMigrateOnInstall: + def test_install_triggers_migration_of_legacy(self, registry, devbase_root): + plugins = [ + {"name": "adminer", "path": "adminer", "projects": ["adminer"]}, + {"name": "carmo", "path": "carmo", "projects": ["carmo"]}, + ] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + + from devbase.plugin.installer import install_plugin + install_plugin(registry, "carmo") + + # pre-existing legacy install migrated as a side effect + assert registry.get("adminer").path == f"repos/{DIRNAME}/adminer" + # the explicitly requested install also succeeds + assert registry.get("carmo").path == f"repos/{DIRNAME}/carmo" + + +class TestAutoMigrateWarningSuppression: + def test_preserved_does_not_emit_loud_per_plugin_warning( + self, registry, devbase_root, caplog, + ): + # A diverged legacy copy is preserved as .bak. _auto_migrate runs on + # every install/update; it must not re-emit a loud per-plugin WARNING + # each time — a concise INFO hint pointing at `devbase plugin migrate` + # is enough (the explicit command prints the full detail). + plugins = [ + {"name": "carmo", "path": "carmo", "projects": ["carmo"]}, + {"name": "redis", "path": "redis", "projects": ["redis"]}, + ] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + # carmo diverges (user-added file) -> will be preserved, not migrated. + _make_legacy_copy(devbase_root, "carmo", plugins[0], + extra={"projects/carmo/.env": "LOCAL=1\n"}) + registry.add(_installed("carmo", "plugins/carmo")) + + from devbase.plugin.installer import _auto_migrate + import logging + with caplog.at_level(logging.INFO, logger="devbase.plugin.installer"): + _auto_migrate(registry) + + installer_logs = [ + r for r in caplog.records + if r.name == "devbase.plugin.installer" + ] + # No WARNING-level record from the auto path. + assert all(r.levelno < logging.WARNING for r in installer_logs) + # The concise hint is present. + joined = " ".join(r.getMessage() for r in installer_logs) + assert "devbase plugin migrate" in joined + + +class TestAutoMigrateOnUpdate: + def test_update_triggers_migration_of_legacy(self, registry, devbase_root): + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + + from devbase.plugin.updater import update_plugin + with patch("devbase.plugin.updater._git_pull"): + update_plugin(registry) + + assert registry.get("adminer").path == f"repos/{DIRNAME}/adminer" + # only auto-migration removes the stale plugins/ copy; a plain pull + # would rewrite the path but leave the old copy on disk. + assert not (devbase_root / "plugins" / "adminer").exists() + + +class TestAddManyDedup: + def test_duplicate_names_keep_last(self, registry): + # add_many is a public batch API; passing the same name twice must not + # leave two conflicting entries in plugins.yml — the last wins. + first = _installed("adminer", "repos/x/adminer") + second = InstalledPlugin( + name="adminer", version="2.0.0", + source="https://github.com/testorg/testrepo.git", + installed_at="2026-02-02T00:00:00+00:00", + path="repos/y/adminer", linked=False, + ) + registry.add_many([first, second]) + installed = registry.list_installed() + assert [p.name for p in installed] == ["adminer"] + # Last entry won. + assert registry.get("adminer").path == "repos/y/adminer" + assert registry.get("adminer").version == "2.0.0" + + def test_duplicate_does_not_duplicate_existing(self, registry): + # An existing entry replaced by a batch containing duplicates of that + # name still results in exactly one row. + registry.add(_installed("adminer", "plugins/adminer")) + registry.add_many([ + _installed("adminer", "repos/a/adminer"), + _installed("adminer", "repos/b/adminer"), + ]) + installed = [p for p in registry.list_installed() if p.name == "adminer"] + assert len(installed) == 1 + assert installed[0].path == "repos/b/adminer" + + +class TestSaveMigration: + """save_migration upserts repos + plugins in a single load+save.""" + + def test_repos_and_plugins_applied_in_one_save(self, registry): + repo = RegisteredRepository( + name="testrepo", url=URL, added_at=registry.now_iso(), + local_path=f"repos/{DIRNAME}", + plugins=[AvailablePlugin(name="adminer", description="", path="adminer")], + ) + plugin = _installed("adminer", f"repos/{DIRNAME}/adminer") + + with patch.object( + PluginRegistry, "_save", autospec=True, side_effect=PluginRegistry._save, + ) as save_spy: + registry.save_migration([repo], [plugin]) + + assert save_spy.call_count == 1 + assert registry.get_repository_by_url(URL).local_path == f"repos/{DIRNAME}" + assert registry.get("adminer").path == f"repos/{DIRNAME}/adminer" + + def test_repo_upsert_replaces_existing_row(self, registry): + # A repo registered without local_path is replaced (not duplicated) by + # the migration row carrying local_path. + registry.add_repository(RegisteredRepository( + name="testrepo", url=URL, added_at=registry.now_iso(), local_path="", + )) + updated = RegisteredRepository( + name="testrepo", url=URL, added_at=registry.now_iso(), + local_path=f"repos/{DIRNAME}", + ) + registry.save_migration([updated], []) + + repos = registry.list_repositories() + assert len(repos) == 1 + assert repos[0].local_path == f"repos/{DIRNAME}" + + def test_empty_inputs_do_not_save(self, registry): + with patch.object(PluginRegistry, "_save", autospec=True) as save_spy: + registry.save_migration([], []) + save_spy.assert_not_called() + + +class TestFilesEqualExecBitOnly: + """_files_equal should only care about exec bits, not full perms.""" + + def test_rw_perm_diff_does_not_differ(self, tmp_path): + # Identical bytes, both non-executable, but differing read/write bits + # (e.g. from a different umask) must NOT count as divergence. + a, b = tmp_path / "a", tmp_path / "b" + a.mkdir(); b.mkdir() + (a / "f.txt").write_text("same\n") + (b / "f.txt").write_text("same\n") + (a / "f.txt").chmod(0o644) + (b / "f.txt").chmod(0o640) + assert _dirs_differ(a, b) is False + + def test_exec_bit_diff_still_differs(self, tmp_path): + # Functionally meaningful exec-bit change is still detected. + a, b = tmp_path / "a", tmp_path / "b" + a.mkdir(); b.mkdir() + (a / "f.sh").write_text("#!/bin/sh\n") + (b / "f.sh").write_text("#!/bin/sh\n") + (a / "f.sh").chmod(0o755) + (b / "f.sh").chmod(0o644) + assert _dirs_differ(a, b) is True + + +class TestEnsureRepoClonedProtectsGit: + """A recorded local_path whose dir keeps .git must not be deleted.""" + + def test_local_path_with_git_missing_registry_is_not_deleted( + self, registry, devbase_root, + ): + # local_path recorded; clone has .git but registry.yml is gone. The dir + # may hold uncommitted/unpushed local work, so migration must refuse to + # rmtree it and raise instead of silently destroying it. + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _register_repo(registry, plugins) # local_path = repos/ + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + clone = devbase_root / "repos" / DIRNAME + clone.mkdir(parents=True) + (clone / ".git").mkdir() + (clone / "local-work.txt").write_text("uncommitted\n") + # No registry.yml -> _clone_is_healthy is False. + + def fake_clone(url, dest, **kwargs): # pragma: no cover - must not run + raise AssertionError("re-clone must not happen when .git is present") + + with patch("devbase.plugin.installer.git_clone", side_effect=fake_clone): + result = migrate(registry) + + # The plugin was skipped (not migrated) and the dir survives intact. + assert "adminer" in result.skipped + assert (clone / ".git").is_dir() + assert (clone / "local-work.txt").read_text() == "uncommitted\n" + # registry entry still points at the legacy path so it retries later. + assert registry.get("adminer").path == "plugins/adminer" + + def test_local_path_without_git_is_still_recloned(self, registry, devbase_root): + # Sanity: when .git is gone the dir is genuinely broken and re-cloning + # (the existing behaviour) still applies. + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _register_repo(registry, plugins) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + broken = devbase_root / "repos" / DIRNAME + broken.mkdir(parents=True) + (broken / "leftover.txt").write_text("broken\n") + + def fake_clone(url, dest, **kwargs): + _make_repo_clone(dest.parent.parent, plugins) + + with patch( + "devbase.plugin.installer.git_clone", side_effect=fake_clone, + ) as mock: + result = migrate(registry) + + mock.assert_called_once() + assert result.migrated == ["adminer"] + assert (devbase_root / "repos" / DIRNAME / ".git").exists() + assert not (devbase_root / "repos" / DIRNAME / "leftover.txt").exists() + + def test_derived_path_with_git_missing_registry_is_not_deleted( + self, registry, devbase_root, + ): + # local_path is NOT recorded (pre-persistent-clone registration) but a + # repos/ clone already exists with .git and only registry.yml + # missing. It may hold uncommitted/unpushed local work, so the derived + # clone path must refuse to rmtree it just as the local_path branch does. + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _register_repo(registry, plugins, local_path=None) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + clone = devbase_root / "repos" / DIRNAME + clone.mkdir(parents=True) + (clone / ".git").mkdir() + (clone / "local-work.txt").write_text("uncommitted\n") + # No registry.yml -> parse_registry_yml fails on the existing dir. + + def fake_clone(url, dest, **kwargs): # pragma: no cover - must not run + raise AssertionError("re-clone must not happen when .git is present") + + with patch("devbase.plugin.installer.git_clone", side_effect=fake_clone): + result = migrate(registry) + + assert "adminer" in result.skipped + assert (clone / ".git").is_dir() + assert (clone / "local-work.txt").read_text() == "uncommitted\n" + # registry entry still legacy so a later run can retry. + assert registry.get("adminer").path == "plugins/adminer" + + def test_derived_path_with_healthy_clone_is_reused( + self, registry, devbase_root, + ): + # local_path is NOT recorded (pre-persistent-clone registration) but a + # *healthy* repos/ clone (.git AND registry.yml) was left by an + # earlier run. It must be reused — migration proceeds, the missing + # local_path is persisted — not protected (skipped) on its .git nor + # re-cloned. + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _register_repo(registry, plugins, local_path=None) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + # Healthy clone already present (sentinel proves it is reused, not wiped). + clone = _make_repo_clone(devbase_root, plugins) + (clone / ".git" / "sentinel").write_text("kept\n") + + def fake_clone(url, dest, **kwargs): # pragma: no cover - must not run + raise AssertionError("must reuse a healthy clone, never re-clone") + + with patch("devbase.plugin.installer.git_clone", side_effect=fake_clone): + result = migrate(registry) + + # Migration succeeded (NOT skipped) and the existing clone survived. + assert result.migrated == ["adminer"] + assert "adminer" not in result.skipped + assert (clone / ".git" / "sentinel").read_text() == "kept\n" + # The previously-missing local_path was persisted, and the plugin now + # points at the reused repos/ clone. + repo = registry.get_repository_by_url(URL) + assert repo.local_path == f"repos/{DIRNAME}" + assert registry.get("adminer").path == f"repos/{DIRNAME}/adminer" + + def test_clone_dir_existing_as_file_is_replaced(self, registry, devbase_root): + # repos/ is squatted on by a regular *file* (not a directory). + # git_clone would fail; the file holds no git tree so it is removed and + # a fresh clone created in its place. + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _register_repo(registry, plugins, local_path=None) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + repos_dir = devbase_root / "repos" + repos_dir.mkdir(parents=True) + stray = repos_dir / DIRNAME + stray.write_text("not a directory\n") + assert stray.is_file() + + def fake_clone(url, dest, **kwargs): + _make_repo_clone(dest.parent.parent, plugins) + + with patch( + "devbase.plugin.installer.git_clone", side_effect=fake_clone, + ) as mock: + result = migrate(registry) + + mock.assert_called_once() + assert result.migrated == ["adminer"] + assert (devbase_root / "repos" / DIRNAME).is_dir() + assert (devbase_root / "repos" / DIRNAME / ".git").exists() + + +class TestMigratePersistsRegistryBeforeRetiringCopy: + """A registry-save failure must not leave a copy deleted with a stale path. + + Round 3 batched the path rewrites into a single save AFTER retiring the + copies; if that save raised, the copies were already gone/renamed while + plugins.yml still pointed at plugins/. migrate() now persists the rewrites + (and any freshly cloned repo rows) via a single save_migration call BEFORE + any destructive filesystem op, so a save failure aborts with every copy + intact. + """ + + def test_save_failure_leaves_copy_intact(self, registry, devbase_root): + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + + # save_migration blows up exactly when migrate() tries to persist. + with patch.object( + PluginRegistry, "save_migration", side_effect=OSError("disk full"), + ): + with pytest.raises(OSError): + migrate(registry) + + # The copy was NOT deleted or renamed to .bak: it is still right where + # it was, so the next run can retry cleanly. + assert (devbase_root / "plugins" / "adminer" / "plugin.yml").is_file() + assert not (devbase_root / "plugins" / "adminer.bak").exists() + + def test_copy_retired_only_after_registry_persisted(self, registry, devbase_root): + # The copy delete/rename must happen strictly after save_migration returns. + plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + _make_legacy_copy(devbase_root, "adminer", plugins[0]) + registry.add(_installed("adminer", "plugins/adminer")) + + copy = devbase_root / "plugins" / "adminer" + orig_save_migration = PluginRegistry.save_migration + + def spy_save_migration(self, repos_arg, plugins_arg): + # At save time the copy must still exist (not yet retired). + assert copy.is_dir(), "copy retired before registry was persisted" + return orig_save_migration(self, repos_arg, plugins_arg) + + with patch.object(PluginRegistry, "save_migration", autospec=True, + side_effect=spy_save_migration): + result = migrate(registry) + + assert result.migrated == ["adminer"] + # After a successful save the clean copy is gone. + assert not copy.exists() + assert registry.get("adminer").path == f"repos/{DIRNAME}/adminer" + + +class TestDirsDifferOtherEntryKind: + """An entry of kind 'other' (fifo/socket/device) can't be content-compared + and must be treated as divergence so the copy is preserved, not deleted.""" + + def test_fifo_in_copy_differs(self, tmp_path): + import os + a, b = tmp_path / "a", tmp_path / "b" + a.mkdir(); b.mkdir() + (a / "plugin.yml").write_text("name: x\n") + (b / "plugin.yml").write_text("name: x\n") + try: + os.mkfifo(a / "pipe") + os.mkfifo(b / "pipe") + except (AttributeError, NotImplementedError, OSError): + pytest.skip("mkfifo not supported on this platform") + # Both sides hold a fifo at the same path; it can't be proven identical + # so deleting the copy is unsafe -> divergence. + assert _dirs_differ(a, b) is True From 65be8ac43d09475fe2e40d752800df550de54d55 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Thu, 28 May 2026 17:42:09 +0000 Subject: [PATCH 4/8] =?UTF-8?q?fix(plugin):=20repo=20refresh=20=E3=81=AE?= =?UTF-8?q?=E5=A3=8A=E3=82=8C=E3=81=9F=E7=A7=BB=E8=A1=8C=E3=82=92=E6=88=90?= =?UTF-8?q?=E5=8A=9F=E6=89=B1=E3=81=84=E3=81=AB=E3=81=9B=E3=81=9A=E7=A7=BB?= =?UTF-8?q?=E8=A1=8C=E6=99=82=E3=81=AE=20registry.yml=20=E9=87=8D=E8=A4=87?= =?UTF-8?q?=E3=83=91=E3=83=BC=E3=82=B9=E3=82=92=E6=8A=91=E5=88=B6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - refresh_repository: _update_repo_plugins が repo_errors を返した場合は warning で握りつぶさず RepositoryError として伝播。pull 後に削除された プラグインの移行失敗を成功扱いにしない (major) - migrate: 同一リポジトリの複数プラグイン移行時、local_path fast path で 返る registry.yml の遅延パースを URL 単位でキャッシュしリポジトリあたり 1 回に抑制 (minor / performance) - 両挙動の回帰テストを追加 Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/devbase/plugin/migrator.py | 15 ++++++++++++--- lib/devbase/plugin/repo_manager.py | 11 ++++++++++- tests/plugin/test_migrator.py | 30 ++++++++++++++++++++++++++++++ tests/plugin/test_repos_core.py | 23 +++++++++++++++++++++++ 4 files changed, 75 insertions(+), 4 deletions(-) diff --git a/lib/devbase/plugin/migrator.py b/lib/devbase/plugin/migrator.py index 7c37d26..a8432a1 100644 --- a/lib/devbase/plugin/migrator.py +++ b/lib/devbase/plugin/migrator.py @@ -434,6 +434,12 @@ def migrate(registry: PluginRegistry, *, run_sync: bool = True) -> MigrationResu repo.url: repo for repo in registry.list_repositories() if repo.url } + # Cache the lazily-parsed registry.yml per repo URL. On the healthy + # local_path fast path _ensure_repo_cloned returns reg_info=None, so a repo + # with multiple legacy plugins would otherwise re-parse the same registry.yml + # once per plugin here; the cache collapses that to one parse per repo. + reg_info_by_url: dict[str, Optional["RegistryInfo"]] = {} + for plugin in legacy: try: repo = repos_by_url.get(plugin.source) if plugin.source else None @@ -450,10 +456,13 @@ def migrate(registry: PluginRegistry, *, run_sync: bool = True) -> MigrationResu ) # _ensure_repo_cloned already parsed registry.yml on the clone/reuse - # paths; only the healthy local_path fast path returns None, so parse - # lazily there instead of unconditionally re-reading the same file. + # paths; only the healthy local_path fast path returns None. Parse + # lazily there, but reuse a cached parse for subsequent plugins of + # the same repo instead of re-reading the same file each iteration. if reg_info is None: - reg_info = parse_registry_yml(clone_dir) + if repo.url not in reg_info_by_url: + reg_info_by_url[repo.url] = parse_registry_yml(clone_dir) + reg_info = reg_info_by_url[repo.url] entry = None if reg_info: entry = next( diff --git a/lib/devbase/plugin/repo_manager.py b/lib/devbase/plugin/repo_manager.py index 0222caa..a0b1fbb 100644 --- a/lib/devbase/plugin/repo_manager.py +++ b/lib/devbase/plugin/repo_manager.py @@ -457,8 +457,17 @@ def refresh_repository( pre_pull_projects=pre_pull_projects, ) if repo_errors: + # A failed removal-migration leaves the stale plugins/ entry in + # plugins.yml; reporting "refreshed" here would mask that broken + # install state. Surface it as a hard error (mirrors update_plugin, + # which raises on _update_repo_plugins errors) instead of warning and + # exiting 0. for err in repo_errors: - logger.warning(" %s", err) + logger.error(" %s", err) + raise RepositoryError( + f"Repository '{repo.name}' refresh left plugins in a broken state:\n" + + "\n".join(f" - {e}" for e in repo_errors) + ) if sync: sync_projects(registry) diff --git a/tests/plugin/test_migrator.py b/tests/plugin/test_migrator.py index c833fa0..cf5a01e 100644 --- a/tests/plugin/test_migrator.py +++ b/tests/plugin/test_migrator.py @@ -415,6 +415,36 @@ def fake_clone(url, dest, **kwargs): assert registry.get("adminer").path == f"repos/{DIRNAME}/adminer" + def test_registry_yml_parsed_once_for_multiple_plugins_same_repo( + self, registry, devbase_root + ): + # Two legacy plugins from the same repo with a healthy local_path clone: + # _ensure_repo_cloned takes the fast path (returns reg_info=None) for + # both, so migrate()'s per-URL reg_info cache must collapse the lazy + # registry.yml parse to one per repo instead of one per plugin. + plugins = [ + {"name": "p1", "path": "p1", "projects": ["p1"]}, + {"name": "p2", "path": "p2", "projects": ["p2"]}, + ] + _make_repo_clone(devbase_root, plugins) + _register_repo(registry, plugins) + for p in plugins: + _make_legacy_copy(devbase_root, p["name"], p) + registry.add(_installed(p["name"], f"plugins/{p['name']}")) + + import devbase.plugin.installer as installer_mod + real_parse = installer_mod.parse_registry_yml + with patch( + "devbase.plugin.installer.parse_registry_yml", + side_effect=real_parse, + ) as spy_parse: + result = migrate(registry) + + assert sorted(result.migrated) == ["p1", "p2"] + # Without the cache this would be 2 (one lazy parse per plugin). + assert spy_parse.call_count == 1 + + class TestMigrateKeepsLinked: def test_linked_install_keeps_plugins_dir(self, registry, devbase_root): plugins = [{"name": "adminer", "path": "adminer", "projects": ["adminer"]}] diff --git a/tests/plugin/test_repos_core.py b/tests/plugin/test_repos_core.py index cb8e59a..34b6fa2 100644 --- a/tests/plugin/test_repos_core.py +++ b/tests/plugin/test_repos_core.py @@ -334,6 +334,29 @@ def test_refresh_warns_removed_installed_plugin(self, registry, devbase_root): "p1", "testrepo", ) + def test_refresh_raises_when_update_repo_plugins_errors( + self, registry, devbase_root + ): + # A failed removal-migration leaves the stale plugins/ entry in + # plugins.yml; refresh must surface that as RepositoryError rather than + # warning-and-exiting-0 (otherwise `devbase plugin repo refresh` reports + # success on a broken install state). + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "p1", "path": "p1"}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "p1", "path": "p1"}, + ]) + + with patch("devbase.plugin.repo_manager._git_pull"), \ + patch( + "devbase.plugin.updater._update_repo_plugins", + return_value=["Migration failed for 'p1'"], + ): + with pytest.raises(RepositoryError, match="broken state"): + refresh_repository(registry, "testrepo") + # ── installer.py ──────────────────────────────────────────────── From 7b77659f4a6b89069e9d03a01b1d7be1cbcd27f5 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Thu, 28 May 2026 17:54:19 +0000 Subject: [PATCH 5/8] =?UTF-8?q?fix(plugin):=20update=20=E6=99=82=E3=81=AB?= =?UTF-8?q?=20registry.yml=20=E3=81=AE=E6=8C=87=E3=81=99=20plugin=20direct?= =?UTF-8?q?ory=20=E5=AD=98=E5=9C=A8=E3=82=92=E6=A4=9C=E8=A8=BC=E3=81=99?= =?UTF-8?q?=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _update_repo_plugins が registry.yml の path をそのまま plugins.yml に書き 込むため、path が実在しないディレクトリを指していても repos/.../missing で 成功扱いになっていた。_register_repo_plugin と同様に plugin_path.is_dir() を 検証し、存在しない場合は registry を更新せず errors に積むよう修正。 回帰テスト test_update_errors_when_registry_path_missing を追加。 Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/devbase/plugin/updater.py | 10 +++++++++ tests/plugin/test_repos_core.py | 38 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/lib/devbase/plugin/updater.py b/lib/devbase/plugin/updater.py index d66d18a..cdb46ec 100644 --- a/lib/devbase/plugin/updater.py +++ b/lib/devbase/plugin/updater.py @@ -166,6 +166,16 @@ def _update_repo_plugins( continue plugin_path = clone_dir / target_entry.path.rstrip('/') + if not plugin_path.is_dir(): + errors.append( + f"Plugin directory not found for '{plugin.name}': {plugin_path}" + ) + logger.warning( + "Skipping '%s': registry.yml path '%s' does not exist", + plugin.name, target_entry.path, + ) + continue + from .syncer import load_plugin_info info = load_plugin_info(plugin_path) version = info.version if info else '0.1.0' diff --git a/tests/plugin/test_repos_core.py b/tests/plugin/test_repos_core.py index 34b6fa2..2b6409f 100644 --- a/tests/plugin/test_repos_core.py +++ b/tests/plugin/test_repos_core.py @@ -804,6 +804,44 @@ def test_update_deduplicates_git_pull(self, registry, devbase_root): update_plugin(registry) assert mock_pull.call_count == 1 + def test_update_errors_when_registry_path_missing( + self, registry, devbase_root + ): + """registry.yml pointing at a non-existent dir must NOT be treated as + a successful update: the metadata must not be rewritten to a missing + path, and the failure must be surfaced via the error list.""" + import yaml + + url = "https://github.com/testorg/testrepo.git" + dir_name = "github.com--testorg--testrepo" + clone_dir = devbase_root / "repos" / dir_name + clone_dir.mkdir(parents=True) + (clone_dir / ".git").mkdir() + # registry.yml advertises p1 at "p1" but that directory does NOT exist. + with open(clone_dir / "registry.yml", "w") as f: + yaml.dump({ + "name": "testrepo", + "plugins": [{"name": "p1", "path": "p1", "description": ""}], + }, f) + + registry.add(InstalledPlugin( + name="p1", version="1.0.0", source=url, + installed_at=registry.now_iso(), + path=f"repos/{dir_name}/p1", + )) + + from devbase.plugin.updater import _update_repo_plugins + + errors = _update_repo_plugins(registry, url, clone_dir) + + assert errors, "missing plugin directory must produce an error" + assert any("p1" in e for e in errors) + # Registry entry must be left intact (not rewritten to a missing path + # and not silently succeeding). + plugin = registry.get("p1") + assert plugin is not None + assert plugin.path == f"repos/{dir_name}/p1" + class TestSnapshotPluginProjects: """Tests for _snapshot_plugin_projects""" From d74631759de9403a58ed90bd61e75c1868823075 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Thu, 28 May 2026 18:06:25 +0000 Subject: [PATCH 6/8] =?UTF-8?q?fix(status):=20repos/=20=E3=83=99=E3=83=BC?= =?UTF-8?q?=E3=82=B9=E3=83=97=E3=83=A9=E3=82=B0=E3=82=A4=E3=83=B3=E3=81=AE?= =?UTF-8?q?=20project=5Fcount=20=E3=82=92=20plugin.path=20=E5=9F=BA?= =?UTF-8?q?=E6=BA=96=E3=81=A7=E7=AE=97=E5=87=BA=E3=81=99=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit devbase status の _get_plugin_info が project_count を旧レイアウト plugins//projects から数えていたため、PLAN04 で repos// へ移行したプラグインの project_count が常に 0 表示されていた。 registry.devbase_root / plugin.path / projects を基準に数えるよう変更し、 plugin.py の表示ロジックと整合させた（repos/ と --link 両方を解決）。 回帰テスト test_status_project_count.py を追加（repos ベース / --link / projects 無しの 3 ケース）。 Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/devbase/commands/status.py | 6 +- tests/plugin/test_status_project_count.py | 87 +++++++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 tests/plugin/test_status_project_count.py diff --git a/lib/devbase/commands/status.py b/lib/devbase/commands/status.py index d0cddd1..6fac2e2 100644 --- a/lib/devbase/commands/status.py +++ b/lib/devbase/commands/status.py @@ -100,10 +100,12 @@ def _get_plugin_info(registry: PluginRegistry) -> list[dict]: """インストール済みプラグインとプロジェクト数を取得する""" results = [] plugins = registry.list_installed() - plugins_dir = registry.get_plugins_dir() for plugin in plugins: - plugin_projects_dir = plugins_dir / plugin.name / "projects" + # plugin.path は devbase_root からの相対パス。 + # repos/ ベース (repos//) と --link ベース + # (plugins/) の両方を同じロジックで解決する。 + plugin_projects_dir = registry.devbase_root / plugin.path / "projects" if plugin_projects_dir.is_dir(): project_count = sum( 1 for p in plugin_projects_dir.iterdir() if p.is_dir() diff --git a/tests/plugin/test_status_project_count.py b/tests/plugin/test_status_project_count.py new file mode 100644 index 0000000..18e6ec3 --- /dev/null +++ b/tests/plugin/test_status_project_count.py @@ -0,0 +1,87 @@ +"""Regression tests for devbase status の project_count 算出。 + +PLAN04 で plugin の実体配置が plugins/ から repos// +へ移行したため、status の project_count も InstalledPlugin.path +(devbase_root からの相対パス) を基準に数える必要がある。 +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from devbase.commands.status import _get_plugin_info +from devbase.plugin.models import InstalledPlugin +from devbase.plugin.registry import PluginRegistry + + +@pytest.fixture +def devbase_root(tmp_path): + (tmp_path / "projects").mkdir() + return tmp_path + + +@pytest.fixture +def registry(devbase_root): + return PluginRegistry(devbase_root) + + +def _make_projects(base: Path, names: list[str]) -> None: + proj = base / "projects" + proj.mkdir(parents=True, exist_ok=True) + for n in names: + (proj / n).mkdir() + + +def test_project_count_for_repos_based_plugin(registry, devbase_root): + """repos/ ベースのプラグインで project_count が正しく数えられる。""" + plugin_dir = devbase_root / "repos" / "github.com-owner-repo" / "myplugin" + _make_projects(plugin_dir, ["proj-a", "proj-b"]) + + registry.add(InstalledPlugin( + name="myplugin", + version="1.0.0", + source="https://github.com/owner/repo", + installed_at=registry.now_iso(), + path="repos/github.com-owner-repo/myplugin", + linked=False, + )) + + info = _get_plugin_info(registry) + assert info == [{"name": "myplugin", "project_count": 2}] + + +def test_project_count_for_linked_plugin(registry, devbase_root): + """--link インストール (plugins/) でも従来どおり数えられる。""" + plugin_dir = devbase_root / "plugins" / "linkedplugin" + _make_projects(plugin_dir, ["only-one"]) + + registry.add(InstalledPlugin( + name="linkedplugin", + version="0.1.0", + source="/local/path", + installed_at=registry.now_iso(), + path="plugins/linkedplugin", + linked=True, + )) + + info = _get_plugin_info(registry) + assert info == [{"name": "linkedplugin", "project_count": 1}] + + +def test_project_count_zero_when_no_projects_dir(registry, devbase_root): + """projects/ ディレクトリが無い場合は 0。""" + (devbase_root / "repos" / "github.com-owner-repo" / "noproj").mkdir(parents=True) + + registry.add(InstalledPlugin( + name="noproj", + version="1.0.0", + source="https://github.com/owner/repo", + installed_at=registry.now_iso(), + path="repos/github.com-owner-repo/noproj", + linked=False, + )) + + info = _get_plugin_info(registry) + assert info == [{"name": "noproj", "project_count": 0}] From b9ad5d9fc75d5fab98d9d16c041ce99458c7b596 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Thu, 28 May 2026 18:18:48 +0000 Subject: [PATCH 7/8] =?UTF-8?q?fix(plugin):=20=E5=90=8D=E5=89=8D=E6=8C=87?= =?UTF-8?q?=E5=AE=9A=E3=82=A4=E3=83=B3=E3=82=B9=E3=83=88=E3=83=BC=E3=83=AB?= =?UTF-8?q?=E3=81=AE=20@ref=20=E6=8B=92=E5=90=A6=E3=81=A8=20status=20?= =?UTF-8?q?=E3=81=AE=E7=A9=BA=20path=20=E3=82=AC=E3=83=BC=E3=83=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - installer.py: `devbase plugin install myplugin@v1` の名前指定インストール分岐で source.ref を _install_from_repo() に渡し既定ブランチを黙ってインストールしていた 問題を修正。未登録/登録済みリポジトリと同様に @ref を PluginError で拒否する (major) - status.py: plugin.path が空文字列の場合に環境ルートの projects/ を誤参照する 可能性を防ぐため事前ガードを追加し 0 件扱いとする (minor / 堅牢性) - 回帰テスト追加: test_install_ref_rejected_for_name_only / test_project_count_zero_when_path_empty Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/devbase/commands/status.py | 5 +++++ lib/devbase/plugin/installer.py | 14 +++++++++++++- tests/plugin/test_repos_core.py | 21 +++++++++++++++++++++ tests/plugin/test_status_project_count.py | 18 ++++++++++++++++++ 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/lib/devbase/commands/status.py b/lib/devbase/commands/status.py index 6fac2e2..1416483 100644 --- a/lib/devbase/commands/status.py +++ b/lib/devbase/commands/status.py @@ -105,6 +105,11 @@ def _get_plugin_info(registry: PluginRegistry) -> list[dict]: # plugin.path は devbase_root からの相対パス。 # repos/ ベース (repos//) と --link ベース # (plugins/) の両方を同じロジックで解決する。 + # path が空の場合 (旧/破損エントリ) は devbase_root/projects を + # 誤参照してしまうため、先にガードして 0 件扱いとする。 + if not plugin.path: + results.append({"name": plugin.name, "project_count": 0}) + continue plugin_projects_dir = registry.devbase_root / plugin.path / "projects" if plugin_projects_dir.is_dir(): project_count = sum( diff --git a/lib/devbase/plugin/installer.py b/lib/devbase/plugin/installer.py index 96ffb6d..f03fb73 100644 --- a/lib/devbase/plugin/installer.py +++ b/lib/devbase/plugin/installer.py @@ -123,12 +123,24 @@ def install_plugin( plugins_dir = registry.get_plugins_dir() if not source.repo and source.plugin_name: + # Reject @ref on name-only installs too — the permanent clone tracks + # the default branch and does not support pinned refs. Without this + # guard, `devbase plugin install myplugin@v1` would silently drop the + # ref in _install_from_repo() and install the default branch instead. + # This matches the validation for unregistered/registered repos below. + if source.ref: + raise PluginError( + f"Cannot use @{source.ref} with plugin '{source.plugin_name}'.\n" + "Permanent clones track the default branch and do not support pinned refs.\n" + f"Install without @ref:\n" + f" devbase plugin install {source.plugin_name}" + ) result = registry.find_plugin_in_repos(source.plugin_name) if result: repo, avail_plugin = result repo_source = PluginSource( repo=repo.url, plugin_name=source.plugin_name, - ref=source.ref, linked=False, + ref=None, linked=False, ) _install_from_repo( registry, repo_source, install_all=False, diff --git a/tests/plugin/test_repos_core.py b/tests/plugin/test_repos_core.py index 2b6409f..558b81b 100644 --- a/tests/plugin/test_repos_core.py +++ b/tests/plugin/test_repos_core.py @@ -443,6 +443,27 @@ def test_install_ref_rejected_for_registered_repo(self, registry, devbase_root): with pytest.raises(PluginError, match="Cannot use @v2.0"): install_plugin(registry, "testorg/testrepo:myplugin@v2.0") + def test_install_ref_rejected_for_name_only(self, registry, devbase_root): + """@ref on a name-only install is rejected too. + + Without the guard, `devbase plugin install myplugin@v1` would parse + to (repo='', plugin_name='myplugin', ref='v1'), enter the + find_plugin_in_repos branch and silently drop the ref in + _install_from_repo(), installing the default branch instead. + """ + url = "https://github.com/testorg/testrepo.git" + _make_repo_dir(devbase_root, "testorg/testrepo", [ + {"name": "myplugin", "path": "myplugin", "projects": ["myproj"]}, + ]) + _register_repo(registry, "testorg/testrepo", url, [ + {"name": "myplugin", "path": "myplugin"}, + ]) + + with pytest.raises(PluginError, match="Cannot use @v1"): + install_plugin(registry, "myplugin@v1") + # registry must not have installed the plugin from the default branch + assert registry.get("myplugin") is None + def test_install_legacy_repo_without_local_path(self, registry, devbase_root): """Legacy repos (no local_path) are auto-migrated to persistent clone.""" url = "https://github.com/testorg/testrepo.git" diff --git a/tests/plugin/test_status_project_count.py b/tests/plugin/test_status_project_count.py index 18e6ec3..fb5596c 100644 --- a/tests/plugin/test_status_project_count.py +++ b/tests/plugin/test_status_project_count.py @@ -85,3 +85,21 @@ def test_project_count_zero_when_no_projects_dir(registry, devbase_root): info = _get_plugin_info(registry) assert info == [{"name": "noproj", "project_count": 0}] + + +def test_project_count_zero_when_path_empty(registry, devbase_root): + """path が空 (旧/破損エントリ) のとき環境ルートの projects を誤参照しない。""" + # 環境ルートに projects/ が存在し、中身があっても 0 と数えること。 + _make_projects(devbase_root, ["root-proj-a", "root-proj-b"]) + + registry.add(InstalledPlugin( + name="brokenpath", + version="0.1.0", + source="", + installed_at=registry.now_iso(), + path="", + linked=False, + )) + + info = _get_plugin_info(registry) + assert info == [{"name": "brokenpath", "project_count": 0}] From 9d5b7128ba947e51fc123462e430ae0313d89ddf Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Thu, 28 May 2026 18:30:43 +0000 Subject: [PATCH 8/8] =?UTF-8?q?perf(plugin):=20migrate=20=E3=83=AB?= =?UTF-8?q?=E3=83=BC=E3=83=97=E3=81=A7=E3=82=AF=E3=83=AD=E3=83=BC=E3=83=B3?= =?UTF-8?q?=E6=B8=88=E3=81=BF=20repo=20=E3=81=A8=20registry.yml=20?= =?UTF-8?q?=E3=83=91=E3=83=BC=E3=82=B9=E3=82=92=E5=90=8C=E4=B8=80=E3=83=AA?= =?UTF-8?q?=E3=83=9D=E3=82=B8=E3=83=88=E3=83=AA=E3=81=AE=E3=83=97=E3=83=A9?= =?UTF-8?q?=E3=82=B0=E3=82=A4=E3=83=B3=E9=96=93=E3=81=A7=E5=86=8D=E5=88=A9?= =?UTF-8?q?=E7=94=A8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 更新後の repo を repos_by_url に書き戻し、後続プラグインが local_path fast path を通るようにして clone-reuse 分岐の再入 (registry.yml 再パース + pending_repos 重複登録) を回避。clone/reuse パスの reg_info も reg_info_by_url にキャッシュし、同一リポジトリの複数プラグイン移行時の registry.yml パースを 1 回に抑制。 --- lib/devbase/plugin/migrator.py | 11 ++++++++++ tests/plugin/test_migrator.py | 38 ++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/lib/devbase/plugin/migrator.py b/lib/devbase/plugin/migrator.py index a8432a1..5e91e4a 100644 --- a/lib/devbase/plugin/migrator.py +++ b/lib/devbase/plugin/migrator.py @@ -455,6 +455,17 @@ def migrate(registry: PluginRegistry, *, run_sync: bool = True) -> MigrationResu registry, repo, pending_repos, ) + # _ensure_repo_cloned may hand back an updated repo row (local_path + # now staged after a clone/reuse). Write it back so subsequent + # legacy plugins of the same repo take the local_path fast path + # instead of re-entering the clone-reuse branch, which would + # re-parse registry.yml and append a duplicate pending_repos row. + repos_by_url[repo.url] = repo + # Cache the parse from the clone/reuse path too, so the fast path + # for sibling plugins reuses it instead of re-reading registry.yml. + if reg_info is not None: + reg_info_by_url[repo.url] = reg_info + # _ensure_repo_cloned already parsed registry.yml on the clone/reuse # paths; only the healthy local_path fast path returns None. Parse # lazily there, but reuse a cached parse for subsequent plugins of diff --git a/tests/plugin/test_migrator.py b/tests/plugin/test_migrator.py index cf5a01e..e6df501 100644 --- a/tests/plugin/test_migrator.py +++ b/tests/plugin/test_migrator.py @@ -414,6 +414,44 @@ def fake_clone(url, dest, **kwargs): assert repo.local_path == f"repos/{DIRNAME}" assert registry.get("adminer").path == f"repos/{DIRNAME}/adminer" + def test_clone_and_registry_parse_reused_across_plugins_same_repo( + self, registry, devbase_root + ): + # Two legacy plugins from the same repo registered WITHOUT local_path. + # The first plugin clones the repo and stages local_path on a fresh repo + # row; migrate() writes that updated row back to its per-URL index so the + # second plugin takes the local_path fast path instead of re-entering the + # clone-reuse branch (which would re-parse registry.yml and append a + # duplicate repo row). Caching the clone-path parse lets that fast path + # reuse it, so registry.yml is parsed exactly once for the repo. + plugins = [ + {"name": "p1", "path": "p1", "projects": ["p1"]}, + {"name": "p2", "path": "p2", "projects": ["p2"]}, + ] + _register_repo(registry, plugins, local_path=None) + for p in plugins: + _make_legacy_copy(devbase_root, p["name"], p) + registry.add(_installed(p["name"], f"plugins/{p['name']}")) + + def fake_clone(url, dest, **kwargs): + _make_repo_clone(dest.parent.parent, plugins) + + import devbase.plugin.installer as installer_mod + real_parse = installer_mod.parse_registry_yml + with patch( + "devbase.plugin.installer.git_clone", side_effect=fake_clone, + ) as spy_clone, patch( + "devbase.plugin.installer.parse_registry_yml", side_effect=real_parse, + ) as spy_parse: + result = migrate(registry) + + assert sorted(result.migrated) == ["p1", "p2"] + # Clone once, then reuse via the staged local_path fast path. + assert spy_clone.call_count == 1 + # Without the write-back + cache the second plugin re-enters the + # clone-reuse branch and parses registry.yml again (would be 2). + assert spy_parse.call_count == 1 + def test_registry_yml_parsed_once_for_multiple_plugins_same_repo( self, registry, devbase_root