mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
feat(extensions): add --force flag to extension add for overwrite reinstall (#2530)
* feat(extensions): add --force flag to extension add for overwrite reinstall Add --force support to `specify extension add` that allows overwriting an already-installed extension without manually removing it first. - install_from_directory() and install_from_zip() accept force=True, automatically calling remove() before installation - The --force CLI flag works with all install modes (--dev, --from URL, bundled, and catalog) - Config files (*-config.yml) are preserved across force reinstall - Error message suggests --force when extension is already installed - 6 new tests covering unit and CLI force reinstall flows * fix: address PR review feedback on --force implementation - Remove unused `backup_config_dir` variable assignment (Ruff F841) - Defer `remove()` until after `_validate_install_conflicts()` to prevent data loss if validation fails mid-reinstall - Use `TemporaryDirectory` instead of `NamedTemporaryFile` in ZIP test to avoid Windows file-locking failures * fix: only restore config backup when --force actually triggers a remove When --force is used but the extension is not already installed, the backup restore/cleanup should not run. Previously it could resurrect stale config files from a previous removal and delete the backup directory unnecessarily. * fix: address Copilot review feedback on --force implementation - Clear stale backup dir before remove() so only fresh backups are restored - Restore only config files (*-config.yml, *-config.local.yml) from backup - Remove trailing \n from --force console message (console.print adds newline) * fix: handle non-directory paths in backup cleanup/restore - Use is_dir() before rmtree/iterdir on backup path to avoid crashes when .backup/<id> exists as a file or symlink - Remove unused manifest1 variable in test_install_force_reinstall * fix: handle symlinks in backup cleanup/restore and correct CLI message - Check is_symlink() before is_dir() in backup cleanup and restore: Path.is_dir() follows symlinks (returns True for symlink-to-dir) but shutil.rmtree() raises OSError on symlinks. Handle symlinks by unlinking them instead. - Skip symlink entries during config file restore. - Change --force dev-install message from "Reinstalling" to "Installing [...] (will overwrite if already installed)" because --force also works for first-time installs.
This commit is contained in:
@@ -793,6 +793,102 @@ class TestExtensionManager:
|
||||
with pytest.raises(ExtensionError, match="already installed"):
|
||||
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
|
||||
|
||||
def test_install_force_reinstall(self, extension_dir, project_dir):
|
||||
"""Test force-reinstalling an already-installed extension."""
|
||||
manager = ExtensionManager(project_dir)
|
||||
|
||||
# Install once
|
||||
manager.install_from_directory(
|
||||
extension_dir, "0.1.0", register_commands=False
|
||||
)
|
||||
assert manager.registry.is_installed("test-ext")
|
||||
|
||||
# Force-reinstall
|
||||
manifest2 = manager.install_from_directory(
|
||||
extension_dir, "0.1.0", register_commands=False, force=True
|
||||
)
|
||||
|
||||
assert manifest2.id == "test-ext"
|
||||
assert manager.registry.is_installed("test-ext")
|
||||
# Check extension directory was recreated
|
||||
ext_dir = project_dir / ".specify" / "extensions" / "test-ext"
|
||||
assert ext_dir.exists()
|
||||
assert (ext_dir / "extension.yml").exists()
|
||||
assert (ext_dir / "commands" / "hello.md").exists()
|
||||
|
||||
def test_install_force_config_preserved(self, extension_dir, project_dir):
|
||||
"""Test that config files are preserved when force-reinstalling."""
|
||||
manager = ExtensionManager(project_dir)
|
||||
|
||||
# Install once
|
||||
manager.install_from_directory(
|
||||
extension_dir, "0.1.0", register_commands=False
|
||||
)
|
||||
|
||||
# Create a config file in the installed extension directory
|
||||
ext_dir = project_dir / ".specify" / "extensions" / "test-ext"
|
||||
config_file = ext_dir / "test-ext-config.yml"
|
||||
config_file.write_text("test: config")
|
||||
|
||||
# Force-reinstall
|
||||
manager.install_from_directory(
|
||||
extension_dir, "0.1.0", register_commands=False, force=True
|
||||
)
|
||||
|
||||
# Config file should still exist after reinstall
|
||||
new_config = ext_dir / "test-ext-config.yml"
|
||||
assert new_config.exists()
|
||||
assert new_config.read_text() == "test: config"
|
||||
|
||||
def test_install_force_without_existing(self, extension_dir, project_dir):
|
||||
"""Test force-install when extension is NOT already installed (works normally)."""
|
||||
manager = ExtensionManager(project_dir)
|
||||
|
||||
manifest = manager.install_from_directory(
|
||||
extension_dir, "0.1.0", register_commands=False, force=True
|
||||
)
|
||||
|
||||
assert manifest.id == "test-ext"
|
||||
assert manager.registry.is_installed("test-ext")
|
||||
|
||||
def test_install_zip_force_reinstall(self, extension_dir, project_dir):
|
||||
"""Test force-reinstalling from ZIP when already installed."""
|
||||
import zipfile
|
||||
import tempfile
|
||||
|
||||
manager = ExtensionManager(project_dir)
|
||||
|
||||
# Install once from directory
|
||||
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
|
||||
|
||||
# Create a ZIP of the extension in a temp directory (not NamedTemporaryFile,
|
||||
# which can fail on Windows due to file locking).
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
zip_path = Path(tmpdir) / "test-ext.zip"
|
||||
with zipfile.ZipFile(zip_path, "w") as zf:
|
||||
for f in extension_dir.rglob("*"):
|
||||
if f.is_file():
|
||||
zf.write(f, f.relative_to(extension_dir))
|
||||
|
||||
# Force-reinstall from ZIP
|
||||
manifest = manager.install_from_zip(
|
||||
zip_path, "0.1.0", force=True
|
||||
)
|
||||
|
||||
assert manifest.id == "test-ext"
|
||||
assert manager.registry.is_installed("test-ext")
|
||||
ext_dir = project_dir / ".specify" / "extensions" / "test-ext"
|
||||
assert ext_dir.exists()
|
||||
|
||||
def test_install_duplicate_error_mentions_force(self, extension_dir, project_dir):
|
||||
"""Test that duplicate install error message suggests --force."""
|
||||
manager = ExtensionManager(project_dir)
|
||||
|
||||
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
|
||||
|
||||
with pytest.raises(ExtensionError, match="--force"):
|
||||
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
|
||||
|
||||
def test_install_rejects_extension_id_in_core_namespace(self, temp_dir, project_dir):
|
||||
"""Install should reject extension IDs that shadow core commands."""
|
||||
import yaml
|
||||
@@ -5114,3 +5210,69 @@ $ARGUMENTS
|
||||
# Verify body references are still dotted for non-Cline
|
||||
assert "speckit.mock-ext.greet" in hello_body
|
||||
assert "speckit-mock-ext-greet" not in hello_body
|
||||
|
||||
|
||||
class TestExtensionForceCLI:
|
||||
"""CLI tests for `specify extension add --dev --force`."""
|
||||
|
||||
def _create_minimal_extension(self, base_dir: str | Path, ext_id: str = "test-ext") -> Path:
|
||||
"""Create a minimal extension directory with manifest."""
|
||||
import yaml
|
||||
|
||||
ext_dir = Path(base_dir) / ext_id
|
||||
ext_dir.mkdir(parents=True, exist_ok=True)
|
||||
(ext_dir / "commands").mkdir()
|
||||
|
||||
manifest = {
|
||||
"schema_version": "1.0",
|
||||
"extension": {
|
||||
"id": ext_id,
|
||||
"name": "Test Extension",
|
||||
"version": "1.0.0",
|
||||
"description": "Test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"commands": [
|
||||
{
|
||||
"name": f"speckit.{ext_id}.hello",
|
||||
"file": "commands/hello.md",
|
||||
"description": "Test command",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
|
||||
(ext_dir / "extension.yml").write_text(yaml.dump(manifest))
|
||||
(ext_dir / "commands" / "hello.md").write_text(
|
||||
"---\ndescription: Test\n---\n\nHello $ARGUMENTS\n"
|
||||
)
|
||||
return ext_dir
|
||||
|
||||
def test_add_dev_force_reinstall(self, tmp_path):
|
||||
"""extension add --dev --force should reinstall without error."""
|
||||
from typer.testing import CliRunner
|
||||
from unittest.mock import patch
|
||||
from specify_cli import app
|
||||
|
||||
project_dir = tmp_path / "project"
|
||||
project_dir.mkdir()
|
||||
(project_dir / ".specify").mkdir()
|
||||
|
||||
ext_src = self._create_minimal_extension(tmp_path)
|
||||
|
||||
runner = CliRunner()
|
||||
with patch.object(Path, "cwd", return_value=project_dir):
|
||||
# First install
|
||||
result1 = runner.invoke(
|
||||
app, ["extension", "add", str(ext_src), "--dev"], catch_exceptions=False
|
||||
)
|
||||
assert result1.exit_code == 0, strip_ansi(result1.output)
|
||||
assert "installed" in strip_ansi(result1.output)
|
||||
|
||||
# Force reinstall
|
||||
result2 = runner.invoke(
|
||||
app, ["extension", "add", str(ext_src), "--dev", "--force"], catch_exceptions=False
|
||||
)
|
||||
assert result2.exit_code == 0, strip_ansi(result2.output)
|
||||
assert "installed" in strip_ansi(result2.output)
|
||||
|
||||
Reference in New Issue
Block a user