Skip to content
Draft
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
32 changes: 28 additions & 4 deletions src/specify_cli/bundler/services/primitives.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ def is_installed(self, component: ComponentRef) -> bool: ...

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

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

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


Expand Down Expand Up @@ -151,6 +153,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 +176,7 @@ 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=force)
return

if not self._allow_network:
Expand All @@ -194,7 +202,7 @@ 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=force)
finally:
with contextlib.suppress(Exception):
if zip_path.exists():
Expand Down Expand Up @@ -224,6 +232,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 +256,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 +286,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 +332,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 +397,11 @@ def install(self, component: ComponentRef) -> None:
lambda: workflow_step_add(component.id),
)

def refresh(self, component: ComponentRef) -> None:
# workflow_step_add is idempotent for already-installed steps; delegate
# to the standard install path which handles version refresh correctly.
self.install(component)
Comment on lines +400 to +403

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
102 changes: 102 additions & 0 deletions tests/unit/test_bundler_primitives.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,105 @@ 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_no_force_hint_in_bundler_error_on_refresh(tmp_path: Path, monkeypatch):
"""Regression: bundle update (refresh=True) of an already-installed extension
must not surface the extension-level '--force' hint inside the BundlerError."""
Comment on lines +277 to +279
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)),
)

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")
from specify_cli.bundler.services.installer import install_bundle
from specify_cli.bundler.services.adapters import DefaultPrimitiveInstaller

Comment on lines +302 to +304
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, components=components)