Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/specify_cli/bundler/services/adapters.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ def install(self, project_root: Path, component: ComponentRef) -> None:
manager = self._manager_for(component, project_root)
manager.install(component)

def refresh(self, project_root: Path, component: ComponentRef) -> None:
manager = self._manager_for(component, project_root)
manager.refresh(component)

def remove(self, project_root: Path, component: ComponentRef) -> None:
manager = self._manager_for(component, project_root)
manager.remove(component)
Expand Down
70 changes: 63 additions & 7 deletions src/specify_cli/bundler/services/primitives.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,17 @@ def _bundled_manifest_version(manifest_path: Path, root_key: str) -> str | None:


class _KindManager(Protocol):
def is_installed(self, component: ComponentRef) -> bool: ...
def is_installed(self, component: ComponentRef) -> bool:
pass

def install(self, component: ComponentRef) -> None: ...
def install(self, component: ComponentRef) -> None:
pass

def remove(self, component: ComponentRef) -> None: ...
def refresh(self, component: ComponentRef) -> None:
pass

def remove(self, component: ComponentRef) -> None:
pass


def primitive_manager(
Expand Down Expand Up @@ -151,6 +157,12 @@ def is_installed(self, component: ComponentRef) -> bool:
return False

def install(self, component: ComponentRef) -> None:
self._do_install(component, force=False)

def refresh(self, component: ComponentRef) -> None:
self._do_install(component, force=True)

def _do_install(self, component: ComponentRef, *, force: bool) -> None:
from ... import get_speckit_version
from ..._assets import _locate_bundled_preset

Expand All @@ -168,7 +180,9 @@ def install(self, component: ComponentRef) -> None:
component.version,
_bundled_manifest_version(bundled / "preset.yml", "preset"),
)
self._manager.install_from_directory(bundled, speckit_version, priority)
self._manager.install_from_directory(
bundled, speckit_version, priority, **({"force": True} if force else {})
)
return

if not self._allow_network:
Expand All @@ -194,7 +208,9 @@ def install(self, component: ComponentRef) -> None:
)
zip_path = catalog.download_pack(component.id)
try:
self._manager.install_from_zip(zip_path, speckit_version, priority)
self._manager.install_from_zip(
zip_path, speckit_version, priority, **({"force": True} if force else {})
)
finally:
with contextlib.suppress(Exception):
if zip_path.exists():
Expand Down Expand Up @@ -224,6 +240,12 @@ def is_installed(self, component: ComponentRef) -> bool:
return False

def install(self, component: ComponentRef) -> None:
self._do_install(component, force=False)

def refresh(self, component: ComponentRef) -> None:
self._do_install(component, force=True)

def _do_install(self, component: ComponentRef, *, force: bool) -> None:
from ... import get_speckit_version
from ..._assets import _locate_bundled_extension

Expand All @@ -242,7 +264,7 @@ def install(self, component: ComponentRef) -> None:
_bundled_manifest_version(bundled / "extension.yml", "extension"),
)
self._manager.install_from_directory(
bundled, speckit_version, priority=priority
bundled, speckit_version, priority=priority, force=force
)
return

Expand Down Expand Up @@ -272,7 +294,7 @@ def install(self, component: ComponentRef) -> None:
zip_path = catalog.download_extension(component.id)
try:
self._manager.install_from_zip(
zip_path, speckit_version, priority=priority
zip_path, speckit_version, priority=priority, force=force
)
finally:
with contextlib.suppress(Exception):
Expand Down Expand Up @@ -318,6 +340,11 @@ def install(self, component: ComponentRef) -> None:
lambda: workflow_add(component.id),
)

def refresh(self, component: ComponentRef) -> None:
# workflow_add is idempotent for already-installed workflows; delegate
# to the standard install path which handles version refresh correctly.
self.install(component)

def _assert_pinned_version(self, component: ComponentRef) -> None:
if not component.version:
return
Expand Down Expand Up @@ -378,6 +405,35 @@ def install(self, component: ComponentRef) -> None:
lambda: workflow_step_add(component.id),
)

def refresh(self, component: ComponentRef) -> None:
# Preserve an existing step until we've validated we can perform refresh.
# For already-installed steps, keep a backup and restore it if the
# remove+reinstall path fails.
if not (self._allow_network and self.is_installed(component)):
Comment on lines +408 to +412
self.install(component)
return

import shutil
import tempfile

step_dir = self._registry.steps_dir / component.id
metadata = self._registry.get(component.id)
backup_dir = Path(tempfile.mkdtemp(prefix="speckit-step-refresh-")) / component.id
try:
if step_dir.exists():
shutil.copytree(step_dir, backup_dir)
self.remove(component)
try:
self.install(component)
except BundlerError:
if backup_dir.exists():
shutil.copytree(backup_dir, step_dir, dirs_exist_ok=True)
if metadata is not None and not self._registry.is_installed(component.id):
self._registry.add(component.id, metadata)
Comment on lines +431 to +432
raise
finally:
shutil.rmtree(backup_dir.parent, ignore_errors=True)

def remove(self, component: ComponentRef) -> None:
from ... import workflow_step_remove

Expand Down
16 changes: 11 additions & 5 deletions src/specify_cli/presets/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1503,13 +1503,15 @@ def install_from_directory(
source_dir: Path,
speckit_version: str,
priority: int = 10,
force: bool = False,
) -> PresetManifest:
"""Install preset from a local directory.

Args:
source_dir: Path to preset directory
speckit_version: Current spec-kit version
priority: Resolution priority (lower = higher precedence, default 10)
force: If True and the preset is already installed, remove it first

Returns:
Installed preset manifest
Expand All @@ -1528,10 +1530,12 @@ def install_from_directory(
self.check_compatibility(manifest, speckit_version)

if self.registry.is_installed(manifest.id):
raise PresetError(
f"Preset '{manifest.id}' is already installed. "
f"Use 'specify preset remove {manifest.id}' first."
)
if not force:
raise PresetError(
f"Preset '{manifest.id}' is already installed. "
f"Use 'specify preset remove {manifest.id}' first."
)
self.remove(manifest.id)

dest_dir = self.presets_dir / manifest.id
if dest_dir.exists():
Expand Down Expand Up @@ -1622,13 +1626,15 @@ def install_from_zip(
zip_path: Path,
speckit_version: str,
priority: int = 10,
force: bool = False,
) -> PresetManifest:
"""Install preset from ZIP file.

Args:
zip_path: Path to preset ZIP file
speckit_version: Current spec-kit version
priority: Resolution priority (lower = higher precedence, default 10)
force: If True and the preset is already installed, remove it first

Returns:
Installed preset manifest
Expand Down Expand Up @@ -1671,7 +1677,7 @@ def install_from_zip(
"No preset.yml found in ZIP file"
)

return self.install_from_directory(pack_dir, speckit_version, priority)
return self.install_from_directory(pack_dir, speckit_version, priority, force=force)

def remove(self, pack_id: str) -> bool:
"""Remove an installed preset.
Expand Down
119 changes: 119 additions & 0 deletions tests/unit/test_bundler_primitives.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
_WorkflowKindManager,
primitive_manager,
)
from tests.bundler_helpers import valid_manifest_dict


def _component(kind: str, cid: str = "x") -> ComponentRef:
Expand Down Expand Up @@ -215,3 +216,121 @@ def test_bundled_preset_pin_match_installs(tmp_path: Path, monkeypatch):
manager.install(ComponentRef(kind="presets", id="my-preset", version="1.0.0"))
manager.install(ComponentRef(kind="presets", id="my-preset", version=None))
assert len(called) == 2


def test_extension_refresh_calls_install_with_force(tmp_path: Path, monkeypatch):
"""_ExtensionKindManager.refresh() must pass force=True to install_from_directory
so an already-installed extension is overwritten instead of raising an error."""
import specify_cli._assets as assets
from specify_cli.extensions import ExtensionManager

bundled = _write_manifest(tmp_path / "ext", "extension", "1.0.0")
monkeypatch.setattr(assets, "_locate_bundled_extension", lambda cid: bundled)
force_values: list = []
monkeypatch.setattr(
ExtensionManager, "install_from_directory",
lambda self, *a, **k: force_values.append(k.get("force", False)),
)

manager = primitive_manager("extensions", tmp_path, allow_network=False)
manager.refresh(ComponentRef(kind="extensions", id="my-ext"))
assert force_values == [True], "refresh() must pass force=True"


def test_preset_refresh_calls_install_with_force(tmp_path: Path, monkeypatch):
"""_PresetKindManager.refresh() must pass force=True to install_from_directory
so an already-installed preset is overwritten instead of raising an error."""
import specify_cli._assets as assets
from specify_cli.presets import PresetManager

bundled = _write_manifest(tmp_path / "preset", "preset", "1.0.0")
monkeypatch.setattr(assets, "_locate_bundled_preset", lambda cid: bundled)
force_values: list = []
monkeypatch.setattr(
PresetManager, "install_from_directory",
lambda self, *a, **k: force_values.append(k.get("force", False)),
)

manager = primitive_manager("presets", tmp_path, allow_network=False)
manager.refresh(ComponentRef(kind="presets", id="my-preset"))
assert force_values == [True], "refresh() must pass force=True"


def test_default_installer_refresh_dispatches_to_kind_manager(tmp_path: Path, monkeypatch):
"""DefaultPrimitiveInstaller.refresh() must call the kind manager's refresh(),
which is the hook _refresh_component() will find — fixing the --force leak."""
import specify_cli._assets as assets
from specify_cli.extensions import ExtensionManager

bundled = _write_manifest(tmp_path / "ext", "extension", "1.0.0")
monkeypatch.setattr(assets, "_locate_bundled_extension", lambda cid: bundled)
force_values: list = []
monkeypatch.setattr(
ExtensionManager, "install_from_directory",
lambda self, *a, **k: force_values.append(k.get("force", False)),
)

installer = DefaultPrimitiveInstaller(allow_network=False)
installer.refresh(tmp_path, _component("extensions", "my-ext"))
assert force_values == [True], "DefaultPrimitiveInstaller.refresh() must use force=True"


def test_refresh_succeeds_and_passes_force_true(tmp_path: Path, monkeypatch):
"""Regression: bundle update (refresh=True) of an already-installed extension
must succeed and pass force=True to install_from_directory."""
from specify_cli.bundler.services.installer import install_bundle
from specify_cli.bundler.models.manifest import BundleManifest
import specify_cli._assets as assets
from specify_cli.extensions import ExtensionManager

bundled = _write_manifest(tmp_path / "ext", "extension", "1.0.0")
monkeypatch.setattr(assets, "_locate_bundled_extension", lambda cid: bundled)
# Simulate refresh succeeding (force=True removes the duplicate-install guard)
force_seen: list = []
def _fake_install_from_directory(self, *a, **k):
force_seen.append(k.get("force", False))
self.registry.add("my-ext", {"version": "1.0.0"})

monkeypatch.setattr(
ExtensionManager, "install_from_directory", _fake_install_from_directory
)

raw = valid_manifest_dict(
bundle={
"id": "test-bundle",
"name": "Test",
"version": "1.0.0",
"role": "developer",
"description": "Test bundle",
"author": "Spec Kit",
"license": "MIT",
},
provides={
"extensions": [{"id": "my-ext", "version": "1.0.0"}],
"presets": [],
"steps": [],
"workflows": [],
},
)
manifest = BundleManifest.from_dict(raw)
installer = DefaultPrimitiveInstaller(allow_network=False)
# First install
install_bundle(tmp_path, _plan(manifest), installer, manifest=manifest)
# Refresh (bundle update) — must not raise with --force hint
install_bundle(tmp_path, _plan(manifest), installer, manifest=manifest, refresh=True)
# force=True must have been passed during the refresh call
assert True in force_seen, "refresh path should have called install_from_directory with force=True"


def _plan(manifest):
from specify_cli.bundler.services.installer import InstallPlan
from specify_cli.bundler.models.manifest import ComponentRef as CR

components = [CR(kind=c.kind, id=c.id) for c in manifest.components]
return InstallPlan(
bundle_id=manifest.bundle.id,
version=manifest.bundle.version,
role=manifest.bundle.role,
effective_integration=None,
components=components,
)
Loading