Răsfoiți Sursa

fix(config): harden library repo migration

xcad 2 luni în urmă
părinte
comite
5fa4a6f211

+ 8 - 0
AGENTS.md

@@ -11,10 +11,18 @@ A sophisticated collection of infrastructure templates (boilerplates) with a Pyt
 ### Running and Testing
 
 ```bash
+# Create and activate a local virtual environment first
+python3 -m venv .venv
+source .venv/bin/activate
+python3 -m pip install --upgrade pip
+python3 -m pip install -e ".[test]"
+
 # Run the CLI application
 python3 -m cli
 # Debugging and Testing commands
 python3 -m cli --log-level DEBUG compose list
+# Run tests
+python3 -m pytest
 ```
 
 ### Production-Ready Testing

+ 2 - 2
cli/core/config/__init__.py

@@ -4,6 +4,6 @@ This package provides the ConfigManager class for managing application configura
 including defaults, preferences, and library configurations.
 """
 
-from .config_manager import ConfigManager, LibraryConfig
+from .config_manager import ConfigManager, LibraryConfig, is_legacy_default_library_url, normalize_git_url
 
-__all__ = ["ConfigManager", "LibraryConfig"]
+__all__ = ["ConfigManager", "LibraryConfig", "is_legacy_default_library_url", "normalize_git_url"]

+ 52 - 16
cli/core/config/config_manager.py

@@ -6,6 +6,7 @@ import tempfile
 from dataclasses import dataclass
 from pathlib import Path
 from typing import Any, ClassVar
+from urllib.parse import urlparse
 
 import yaml
 
@@ -19,6 +20,40 @@ DEFAULT_LIBRARY_BRANCH = "main"
 DEFAULT_LIBRARY_URL = "https://github.com/christianlempa/boilerplates-library.git"
 LEGACY_DEFAULT_LIBRARY_DIRECTORY = "library"
 LEGACY_DEFAULT_LIBRARY_URL = "https://github.com/christianlempa/boilerplates.git"
+LEGACY_DEFAULT_LIBRARY_REPO = "github.com/christianlempa/boilerplates"
+
+
+def normalize_git_url(url: str | None) -> str:
+    """Normalize git URLs so SSH/HTTPS variants can be compared safely."""
+    if not url:
+        return ""
+
+    candidate = url.strip()
+    if not candidate:
+        return ""
+
+    if candidate.startswith("git@"):
+        host_and_path = candidate[4:]
+        host, _, path = host_and_path.partition(":")
+        normalized = f"{host}/{path}"
+    elif "://" in candidate:
+        parsed = urlparse(candidate)
+        normalized = f"{parsed.netloc}{parsed.path}"
+    else:
+        normalized = candidate
+
+    normalized = normalized.rstrip("/")
+    if normalized.endswith(".git"):
+        normalized = normalized[:-4]
+    if normalized.startswith("www."):
+        normalized = normalized[4:]
+
+    return normalized.lower()
+
+
+def is_legacy_default_library_url(url: str | None) -> bool:
+    """Return True when a URL points at the legacy christianlempa/boilerplates repo."""
+    return normalize_git_url(url) == LEGACY_DEFAULT_LIBRARY_REPO
 
 
 @dataclass
@@ -133,8 +168,8 @@ class ConfigManager:
                         library["type"] = "git"
                         needs_migration = True
 
-            default_library_migrated = self._migrate_default_library(config)
-            needs_migration = needs_migration or default_library_migrated
+            boilerplates_repo_migrated = self._migrate_boilerplates_repo_libraries(config)
+            needs_migration = needs_migration or boilerplates_repo_migrated
 
             # Write back if migration was needed
             if needs_migration:
@@ -146,22 +181,17 @@ class ConfigManager:
             logger.error(f"Config migration failed: {e}")
             raise ConfigError(f"Failed to migrate configuration '{self.config_path}': {e}") from e
 
-    def _migrate_default_library(self, config: dict[str, Any]) -> bool:
-        """Rewrite the built-in default git library entry to the 0.2.0 library repo."""
+    def _migrate_boilerplates_repo_libraries(self, config: dict[str, Any]) -> bool:
+        """Rewrite any legacy christianlempa/boilerplates library entry to the new repo."""
         libraries = config.get("libraries", [])
         migrated = False
+        migrated_libraries: list[str] = []
 
         for library in libraries:
-            if library.get("name") != DEFAULT_LIBRARY_NAME:
-                continue
-
             if library.get("type", "git") != "git":
                 continue
 
-            if library.get("url") != LEGACY_DEFAULT_LIBRARY_URL:
-                continue
-
-            if library.get("directory", LEGACY_DEFAULT_LIBRARY_DIRECTORY) != LEGACY_DEFAULT_LIBRARY_DIRECTORY:
+            if not is_legacy_default_library_url(library.get("url")):
                 continue
 
             target_state = {
@@ -171,26 +201,32 @@ class ConfigManager:
 
             changed = any(library.get(key) != value for key, value in target_state.items())
             if not changed:
-                break
+                continue
 
+            library_name = library.get("name", DEFAULT_LIBRARY_NAME)
             previous_location = library.get("url") or library.get("path") or "<unknown>"
             library.update(target_state)
             migrated = True
+            migrated_libraries.append(library_name)
             logger.info(
-                "Migrated default library from %s to %s (%s)",
+                "Migrated library '%s' from %s to %s (%s)",
+                library_name,
                 previous_location,
                 DEFAULT_LIBRARY_URL,
                 DEFAULT_LIBRARY_DIRECTORY,
             )
+
+        if migrated:
+            migrated_names = ", ".join(sorted(migrated_libraries))
             self._add_migration_notice(
                 kind="default_library_repo",
                 message=(
-                    "Your default template library was migrated to "
+                    "Migrated template library configuration"
+                    f" ({migrated_names}) from christianlempa/boilerplates to "
                     "christianlempa/boilerplates-library. "
-                    "Run 'boilerplates repo update' to sync the new templates."
+                    "Run 'boilerplates repo update' to resync the managed library checkout."
                 ),
             )
-            break
 
         return migrated
 

+ 83 - 2
cli/core/repo.py

@@ -11,7 +11,7 @@ from rich.progress import SpinnerColumn, TextColumn
 from rich.table import Table
 from typer import Argument, Option, Typer
 
-from ..core.config import ConfigManager, LibraryConfig
+from ..core.config import ConfigManager, LibraryConfig, normalize_git_url
 from ..core.display import DisplayManager, IconManager
 from ..core.exceptions import ConfigError
 
@@ -69,10 +69,91 @@ def _clone_or_pull_repo(
         Tuple of (success, message)
     """
     if target_path.exists() and (target_path / ".git").exists():
-        return _pull_repo_updates(name, target_path, branch)
+        remote_url = _get_repo_remote_url(target_path)
+        if not remote_url:
+            return _replace_repo_checkout(
+                name,
+                url,
+                target_path,
+                branch,
+                sparse_dir,
+                reason="existing checkout has no readable origin remote",
+            )
+
+        if normalize_git_url(remote_url) != normalize_git_url(url):
+            return _replace_repo_checkout(
+                name,
+                url,
+                target_path,
+                branch,
+                sparse_dir,
+                reason=f"configured remote changed from {remote_url} to {url}",
+            )
+
+        success, message = _pull_repo_updates(name, target_path, branch)
+        if success:
+            return success, message
+
+        if _is_recoverable_pull_failure(message):
+            return _replace_repo_checkout(
+                name,
+                url,
+                target_path,
+                branch,
+                sparse_dir,
+                reason="managed checkout diverged from origin",
+            )
+
+        return success, message
     return _clone_new_repo(name, url, target_path, branch, sparse_dir)
 
 
+def _get_repo_remote_url(target_path: Path) -> str | None:
+    """Return the current origin remote URL for an existing checkout."""
+    success, stdout, stderr = _run_git_command(["remote", "get-url", "origin"], cwd=target_path)
+    if not success:
+        logger.warning("Failed to read origin remote for '%s': %s", target_path, stderr or stdout)
+        return None
+    return stdout.strip() or None
+
+
+def _is_recoverable_pull_failure(message: str) -> bool:
+    """Identify pull failures that can be fixed by replacing the managed checkout."""
+    recoverable_markers = (
+        "Not possible to fast-forward",
+        "Diverging branches can't be fast-forwarded",
+        "refusing to merge unrelated histories",
+        "fatal: bad object",
+        "couldn't find remote ref",
+    )
+    return any(marker in message for marker in recoverable_markers)
+
+
+def _replace_repo_checkout(
+    name: str,
+    url: str,
+    target_path: Path,
+    branch: str | None,
+    sparse_dir: str | None,
+    *,
+    reason: str,
+) -> tuple[bool, str]:
+    """Replace a managed checkout with a clean clone from the configured remote."""
+    logger.warning("Replacing managed library checkout '%s' at %s: %s", name, target_path, reason)
+
+    try:
+        shutil.rmtree(target_path)
+    except OSError as exc:
+        logger.error("Failed to remove managed library checkout '%s': %s", name, exc)
+        return False, f"{reason}; failed to remove old checkout: {exc}"
+
+    success, message = _clone_new_repo(name, url, target_path, branch, sparse_dir)
+    if not success:
+        return False, f"{reason}; {message}"
+
+    return True, f"Re-cloned successfully after {reason}"
+
+
 def _pull_repo_updates(name: str, target_path: Path, branch: str | None) -> tuple[bool, str]:
     """Pull updates for an existing repository."""
     logger.debug(f"Pulling updates for library '{name}' at {target_path}")

+ 5 - 0
pyproject.toml

@@ -25,6 +25,11 @@ dependencies = [
     "email-validator>=2.0.0",
 ]
 
+[project.optional-dependencies]
+test = [
+    "pytest>=8.0.0",
+]
+
 [project.scripts]
 boilerplates = "cli.__main__:run"
 

+ 49 - 6
tests/test_config_manager.py

@@ -12,9 +12,9 @@ from cli.core.config.config_manager import (
     DEFAULT_LIBRARY_DIRECTORY,
     DEFAULT_LIBRARY_NAME,
     DEFAULT_LIBRARY_URL,
-    LEGACY_DEFAULT_LIBRARY_DIRECTORY,
     LEGACY_DEFAULT_LIBRARY_URL,
     ConfigManager,
+    is_legacy_default_library_url,
 )
 from cli.core.exceptions import ConfigError
 
@@ -39,7 +39,7 @@ def test_default_config_uses_new_library_repo(tmp_path: Path) -> None:
 
 
 def test_migrate_legacy_default_library_and_queue_notice(tmp_path: Path) -> None:
-    """Only the legacy built-in default library entry should be rewritten."""
+    """Legacy christianlempa/boilerplates entries should be rewritten."""
     config_path = tmp_path / "config.yaml"
     ConfigManager.consume_migration_notices()
     _write_config(
@@ -53,7 +53,7 @@ def test_migrate_legacy_default_library_and_queue_notice(tmp_path: Path) -> None
                     "type": "git",
                     "url": LEGACY_DEFAULT_LIBRARY_URL,
                     "branch": "feature/test",
-                    "directory": LEGACY_DEFAULT_LIBRARY_DIRECTORY,
+                    "directory": "library",
                     "enabled": True,
                 },
                 {
@@ -85,13 +85,14 @@ def test_migrate_legacy_default_library_and_queue_notice(tmp_path: Path) -> None
 
     assert len(notices) == 1
     assert "boilerplates-library" in notices[0].message
+    assert "default" in notices[0].message
 
     # Notices are consumed once.
     assert ConfigManager.consume_migration_notices() == []
 
 
-def test_does_not_migrate_custom_default_library(tmp_path: Path) -> None:
-    """A user-customized default entry should be left untouched."""
+def test_does_not_migrate_custom_non_boilerplates_library(tmp_path: Path) -> None:
+    """Non-matching custom library entries should be left untouched."""
     config_path = tmp_path / "config.yaml"
     ConfigManager.consume_migration_notices()
     _write_config(
@@ -122,6 +123,48 @@ def test_does_not_migrate_custom_default_library(tmp_path: Path) -> None:
     assert ConfigManager.consume_migration_notices() == []
 
 
+def test_migrates_any_library_pointing_to_legacy_boilerplates_repo(tmp_path: Path) -> None:
+    """Any library URL pointing to christianlempa/boilerplates should migrate."""
+    config_path = tmp_path / "config.yaml"
+    ConfigManager.consume_migration_notices()
+    _write_config(
+        config_path,
+        {
+            "defaults": {},
+            "preferences": {},
+            "libraries": [
+                {
+                    "name": "custom",
+                    "type": "git",
+                    "url": "git@github.com:ChristianLempa/boilerplates.git",
+                    "branch": "feature/test",
+                    "directory": "templates",
+                    "enabled": True,
+                }
+            ],
+        },
+    )
+
+    manager = ConfigManager(config_path=config_path)
+    libraries = manager.get_libraries()
+    notices = ConfigManager.consume_migration_notices()
+
+    assert libraries[0]["name"] == "custom"
+    assert libraries[0]["url"] == DEFAULT_LIBRARY_URL
+    assert libraries[0]["branch"] == "feature/test"
+    assert libraries[0]["directory"] == DEFAULT_LIBRARY_DIRECTORY
+    assert len(notices) == 1
+    assert "custom" in notices[0].message
+
+
+def test_legacy_library_url_matcher_handles_common_git_url_variants() -> None:
+    """Legacy repo detection should match HTTPS and SSH GitHub URL forms."""
+    assert is_legacy_default_library_url("https://github.com/christianlempa/boilerplates.git")
+    assert is_legacy_default_library_url("https://github.com/ChristianLempa/boilerplates")
+    assert is_legacy_default_library_url("git@github.com:ChristianLempa/boilerplates.git")
+    assert not is_legacy_default_library_url("https://github.com/christianlempa/boilerplates-library.git")
+
+
 def test_migration_write_failure_raises_config_error(tmp_path: Path) -> None:
     """Migration failures should surface as config errors instead of being swallowed."""
     config_path = tmp_path / "config.yaml"
@@ -136,7 +179,7 @@ def test_migration_write_failure_raises_config_error(tmp_path: Path) -> None:
                     "type": "git",
                     "url": LEGACY_DEFAULT_LIBRARY_URL,
                     "branch": "main",
-                    "directory": LEGACY_DEFAULT_LIBRARY_DIRECTORY,
+                    "directory": "library",
                     "enabled": True,
                 }
             ],

+ 99 - 0
tests/test_repo.py

@@ -0,0 +1,99 @@
+"""Tests for managed library repository sync behavior."""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+from cli.core import repo
+
+
+def test_clone_or_pull_repo_replaces_checkout_when_origin_changes(monkeypatch, tmp_path: Path) -> None:
+    """Existing managed checkouts should be replaced when config points at a new remote."""
+    target_path = tmp_path / "default"
+    (target_path / ".git").mkdir(parents=True)
+
+    clone_calls: list[tuple[str, str, Path, str | None, str | None, str]] = []
+
+    def fake_get_repo_remote_url(_target_path: Path) -> str:
+        return "https://github.com/christianlempa/boilerplates.git"
+
+    def fake_replace_repo_checkout(
+        name: str,
+        url: str,
+        target_path: Path,
+        branch: str | None,
+        sparse_dir: str | None,
+        *,
+        reason: str,
+    ) -> tuple[bool, str]:
+        clone_calls.append((name, url, target_path, branch, sparse_dir, reason))
+        return True, "recloned"
+
+    monkeypatch.setattr(repo, "_get_repo_remote_url", fake_get_repo_remote_url)
+    monkeypatch.setattr(repo, "_replace_repo_checkout", fake_replace_repo_checkout)
+
+    success, message = repo._clone_or_pull_repo(
+        "default",
+        "https://github.com/christianlempa/boilerplates-library.git",
+        target_path,
+        branch="main",
+        sparse_dir=".",
+    )
+
+    assert success is True
+    assert message == "recloned"
+    assert clone_calls == [
+        (
+            "default",
+            "https://github.com/christianlempa/boilerplates-library.git",
+            target_path,
+            "main",
+            ".",
+            "configured remote changed from https://github.com/christianlempa/boilerplates.git to "
+            "https://github.com/christianlempa/boilerplates-library.git",
+        )
+    ]
+
+
+def test_clone_or_pull_repo_replaces_checkout_after_diverged_pull(monkeypatch, tmp_path: Path) -> None:
+    """Fast-forward failures should fall back to a fresh managed clone."""
+    target_path = tmp_path / "default"
+    (target_path / ".git").mkdir(parents=True)
+
+    clone_calls: list[str] = []
+
+    monkeypatch.setattr(repo, "_get_repo_remote_url", lambda _target_path: "git@github.com:ChristianLempa/boilerplates-library.git")
+    monkeypatch.setattr(
+        repo,
+        "_pull_repo_updates",
+        lambda _name, _target_path, _branch: (
+            False,
+            "Pull failed: fatal: Not possible to fast-forward, aborting.",
+        ),
+    )
+
+    def fake_replace_repo_checkout(
+        name: str,
+        url: str,
+        target_path: Path,
+        branch: str | None,
+        sparse_dir: str | None,
+        *,
+        reason: str,
+    ) -> tuple[bool, str]:
+        clone_calls.append(reason)
+        return True, "recloned after divergence"
+
+    monkeypatch.setattr(repo, "_replace_repo_checkout", fake_replace_repo_checkout)
+
+    success, message = repo._clone_or_pull_repo(
+        "default",
+        "https://github.com/christianlempa/boilerplates-library.git",
+        target_path,
+        branch="main",
+        sparse_dir=".",
+    )
+
+    assert success is True
+    assert message == "recloned after divergence"
+    assert clone_calls == ["managed checkout diverged from origin"]