Skip to content

Commit 8186783

Browse files
authored
Address review feedback on primitives.py and test_bundler_primitives.py
- Replace ... with pass in _KindManager Protocol method stubs - Conditionally pass force= keyword only when force=True in _PresetKindManager - Fix _StepKindManager.refresh() to remove step before re-installing - Rename test to reflect actual assertion (refresh succeeds + force=True) - Remove duplicate install_bundle import Assisted-by: GitHub Copilot (model: claude-sonnet-4.5, autonomous)
1 parent c36667e commit 8186783

2 files changed

Lines changed: 21 additions & 11 deletions

File tree

src/specify_cli/bundler/services/primitives.py

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,17 @@ def _bundled_manifest_version(manifest_path: Path, root_key: str) -> str | None:
8585

8686

8787
class _KindManager(Protocol):
88-
def is_installed(self, component: ComponentRef) -> bool: ...
88+
def is_installed(self, component: ComponentRef) -> bool:
89+
pass
8990

90-
def install(self, component: ComponentRef) -> None: ...
91+
def install(self, component: ComponentRef) -> None:
92+
pass
9193

92-
def refresh(self, component: ComponentRef) -> None: ...
94+
def refresh(self, component: ComponentRef) -> None:
95+
pass
9396

94-
def remove(self, component: ComponentRef) -> None: ...
97+
def remove(self, component: ComponentRef) -> None:
98+
pass
9599

96100

97101
def primitive_manager(
@@ -176,7 +180,9 @@ def _do_install(self, component: ComponentRef, *, force: bool) -> None:
176180
component.version,
177181
_bundled_manifest_version(bundled / "preset.yml", "preset"),
178182
)
179-
self._manager.install_from_directory(bundled, speckit_version, priority, force=force)
183+
self._manager.install_from_directory(
184+
bundled, speckit_version, priority, **({"force": True} if force else {})
185+
)
180186
return
181187

182188
if not self._allow_network:
@@ -202,7 +208,9 @@ def _do_install(self, component: ComponentRef, *, force: bool) -> None:
202208
)
203209
zip_path = catalog.download_pack(component.id)
204210
try:
205-
self._manager.install_from_zip(zip_path, speckit_version, priority, force=force)
211+
self._manager.install_from_zip(
212+
zip_path, speckit_version, priority, **({"force": True} if force else {})
213+
)
206214
finally:
207215
with contextlib.suppress(Exception):
208216
if zip_path.exists():
@@ -398,8 +406,11 @@ def install(self, component: ComponentRef) -> None:
398406
)
399407

400408
def refresh(self, component: ComponentRef) -> None:
401-
# workflow_step_add is idempotent for already-installed steps; delegate
402-
# to the standard install path which handles version refresh correctly.
409+
# workflow_step_add errors when the step is already installed; remove it
410+
# first (when network is permitted so the re-install can follow) before
411+
# delegating to install().
412+
if self._allow_network and self.is_installed(component):
413+
self.remove(component)
403414
self.install(component)
404415

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

tests/unit/test_bundler_primitives.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,9 +274,9 @@ def test_default_installer_refresh_dispatches_to_kind_manager(tmp_path: Path, mo
274274
assert force_values == [True], "DefaultPrimitiveInstaller.refresh() must use force=True"
275275

276276

277-
def test_no_force_hint_in_bundler_error_on_refresh(tmp_path: Path, monkeypatch):
277+
def test_refresh_succeeds_and_passes_force_true(tmp_path: Path, monkeypatch):
278278
"""Regression: bundle update (refresh=True) of an already-installed extension
279-
must not surface the extension-level '--force' hint inside the BundlerError."""
279+
must succeed and pass force=True to install_from_directory."""
280280
from specify_cli.bundler.services.installer import install_bundle
281281
from specify_cli.bundler.models.manifest import BundleManifest
282282
import specify_cli._assets as assets
@@ -299,7 +299,6 @@ def test_no_force_hint_in_bundler_error_on_refresh(tmp_path: Path, monkeypatch):
299299
"components": [{"kind": "extensions", "id": "my-ext"}],
300300
}
301301
manifest = BundleManifest(raw, source_id="test")
302-
from specify_cli.bundler.services.installer import install_bundle
303302
from specify_cli.bundler.services.adapters import DefaultPrimitiveInstaller
304303

305304
installer = DefaultPrimitiveInstaller(allow_network=False)

0 commit comments

Comments
 (0)