Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
49 changes: 42 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:
pass

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

def remove(self, component: ComponentRef) -> None: ...
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,14 @@ def install(self, component: ComponentRef) -> None:
lambda: workflow_step_add(component.id),
)

def refresh(self, component: ComponentRef) -> None:
# workflow_step_add errors when the step is already installed; remove it
# first (when network is permitted so the re-install can follow) before
# delegating to install().
if self._allow_network and self.is_installed(component):
self.remove(component)
self.install(component)

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
105 changes: 105 additions & 0 deletions tests/unit/test_bundler_primitives.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,108 @@ 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 = []
monkeypatch.setattr(
ExtensionManager, "install_from_directory",
lambda self, *a, **k: force_seen.append(k.get("force", False)),
)
Comment on lines +289 to +296

raw = {
"schema_version": "1.0",
"bundle_id": "test-bundle",
"name": "Test",
"version": "1.0.0",
"components": [{"kind": "extensions", "id": "my-ext"}],
}
manifest = BundleManifest(raw, source_id="test")
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.version,
role=manifest.bundle.role,
effective_integration=None,
components=components,
)
Loading