[bug-fix] Fix bundle-update-force-mislead: add refresh() to DefaultPrimitiveInstaller (#3452)

* Fix bundle-update-force-mislead: add refresh() to DefaultPrimitiveInstaller

Apply the remediation from the bug assessment on issue #3424.

DefaultPrimitiveInstaller lacked a refresh() method, causing
_refresh_component() to fall back to install(), which calls
ExtensionManager.install_from_directory() with force=False. This
raised ExtensionError with a leaked --force hint that bundle update
does not support, leaving users with no valid recovery path.

Fix: add refresh() to each kind manager (ExtensionKindManager and
PresetKindManager delegate to _do_install(force=True); WorkflowKindManager
and StepKindManager delegate to install() as their callables are
idempotent). DefaultPrimitiveInstaller.refresh() dispatches to the kind
manager's refresh(). PresetManager.install_from_directory() and
install_from_zip() gain a force parameter that removes the existing
preset before reinstalling, mirroring ExtensionManager's force semantics.

Refs #3424

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* 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)

* fix: add missing role/effective_integration to InstallPlan in _plan() and remove redundant import

- Remove duplicate `DefaultPrimitiveInstaller` import inside test body
  (already imported at module scope on line 15)
- Add required `role` and `effective_integration` fields to `InstallPlan`
  constructor in `_plan()` helper to prevent TypeError at runtime

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

* fix: address latest PR review comments

Assisted-by: GitHub Copilot (model: gpt-5.6-terra, autonomous)

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

src/specify_cli/bundler/services/adapters.py

@@ -227,6 +227,10 @@ class DefaultPrimitiveInstaller:
         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)

src/specify_cli/bundler/services/primitives.py

@@ -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(
@@ -151,6 +157,12 @@ class _PresetKindManager:
             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
 
@@ -168,7 +180,9 @@ class _PresetKindManager:
                 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:
@@ -194,7 +208,9 @@ class _PresetKindManager:
         )
         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():
@@ -224,6 +240,12 @@ class _ExtensionKindManager:
             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
 
@@ -242,7 +264,7 @@ class _ExtensionKindManager:
                 _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
 
@@ -272,7 +294,7 @@ class _ExtensionKindManager:
         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):
@@ -318,6 +340,11 @@ class _WorkflowKindManager:
                 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
@@ -378,6 +405,35 @@ class _StepKindManager:
                 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)):
+            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)
+                raise
+        finally:
+            shutil.rmtree(backup_dir.parent, ignore_errors=True)
+
     def remove(self, component: ComponentRef) -> None:
         from ... import workflow_step_remove
 

src/specify_cli/presets/__init__.py

@@ -3359,6 +3359,7 @@ class PresetManager:
         source_dir: Path,
         speckit_version: str,
         priority: int = 10,
+        force: bool = False,
     ) -> PresetManifest:
         """Install preset from a local directory.
 
@@ -3366,6 +3367,7 @@ class PresetManager:
             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
@@ -3384,10 +3386,12 @@ class PresetManager:
         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():
@@ -3535,6 +3539,7 @@ class PresetManager:
         zip_path: Path,
         speckit_version: str,
         priority: int = 10,
+        force: bool = False,
     ) -> PresetManifest:
         """Install preset from ZIP file.
 
@@ -3542,6 +3547,7 @@ class PresetManager:
             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
@@ -3573,7 +3579,7 @@ class PresetManager:
                     "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.

tests/unit/test_bundler_primitives.py

@@ -20,6 +20,7 @@ from specify_cli.bundler.services.primitives import (
     _WorkflowKindManager,
     primitive_manager,
 )
+from tests.bundler_helpers import valid_manifest_dict
 
 
 def _component(kind: str, cid: str = "x") -> ComponentRef:
@@ -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,
+    )