fix: harden bundle and workflow step integrity

src/specify_cli/bundler/models/manifest.py

@@ -192,9 +192,17 @@ def structural_errors(self) -> list[str]:
                 "(lowercase letters, digits, '.', '_', '-'; no path separators)."
             )
 
+        seen_components: set[tuple[str, str]] = set()
         for ref in self.components:
             if not ref.id:
                 errors.append(f"A {ref.kind[:-1]} entry is missing its 'id'.")
+            key = (ref.kind, ref.id)
+            if ref.id and key in seen_components:
+                errors.append(
+                    f"Duplicate {ref.kind[:-1]} '{ref.id}' in "
+                    f"'provides.{ref.kind}'."
+                )
+            seen_components.add(key)
             if ref.kind != "steps" and not ref.version:
                 errors.append(
                     f"{ref.kind[:-1]} '{ref.id or '<unknown>'}' must be pinned to a 'version'."

src/specify_cli/bundler/services/adapters.py

@@ -223,6 +223,12 @@ def is_installed(self, project_root: Path, component: ComponentRef) -> bool:
         manager = self._manager_for(component, project_root)
         return manager.is_installed(component)
 
+    def snapshot(
+        self, project_root: Path, component: ComponentRef
+    ) -> ComponentRef | None:
+        manager = self._manager_for(component, project_root)
+        return manager.snapshot(component)
+
     def install(self, project_root: Path, component: ComponentRef) -> None:
         manager = self._manager_for(component, project_root)
         manager.install(component)

src/specify_cli/bundler/services/installer.py

@@ -35,6 +35,10 @@ class PrimitiveInstaller(Protocol):
 
     def is_installed(self, project_root: Path, component: ComponentRef) -> bool: ...
 
+    def snapshot(
+        self, project_root: Path, component: ComponentRef
+    ) -> ComponentRef | None: ...
+
     def install(self, project_root: Path, component: ComponentRef) -> None: ...
 
     def remove(self, project_root: Path, component: ComponentRef) -> None: ...
@@ -65,11 +69,9 @@ def install_bundle(
 ) -> InstallResult:
     """Execute *plan*, recording provenance. Idempotent, with bounded rollback.
 
-    Atomicity is scoped, not global: on failure only the components newly
-    installed during *this* call are rolled back, and the provenance record is
-    written solely on full success (a failure records nothing). Components that
-    were already installed beforehand — including those re-applied when *refresh*
-    is True — are never rolled back.
+    Atomicity is scoped, not global: completed component mutations are reversed
+    on failure, and the provenance record is written solely on full success.
+    Rollback is best-effort because primitive restoration can itself fail.
 
     When *refresh* is True (used by ``specify bundle update``), components that
     are already installed are re-applied through the primitive machinery so they
@@ -108,9 +110,8 @@ def install_bundle(
         if r.bundle_id != plan.bundle_id
         for c in r.contributed_components
     }
-
     contributed: list[ComponentRef] = []
-    done: list[ComponentRef] = []
+    rollback_actions: list[tuple[str, ComponentRef]] = []
     try:
         for component in plan.components:
             key = (component.kind, component.id)
@@ -122,15 +123,19 @@ def install_bundle(
                 # does not own (FR-022).
                 owned = key in prior_ours or key in other_tracked
                 if refresh and owned:
+                    prior_component = _snapshot_component(
+                        project_root, installer, component
+                    )
                     _refresh_component(project_root, installer, component)
+                    rollback_actions.append(("refresh", prior_component))
                     result.refreshed.append(component)
                 else:
                     result.skipped.append(component)
                 if owned:
                     contributed.append(component)
                 continue
             installer.install(project_root, component)
-            done.append(component)
+            rollback_actions.append(("remove", component))
             result.installed.append(component)
             contributed.append(component)
 
@@ -153,27 +158,41 @@ def install_bundle(
                 if key in still_needed:
                     continue
                 if installer.is_installed(project_root, component):
+                    prior_component = _snapshot_component(
+                        project_root, installer, component
+                    )
                     installer.remove(project_root, component)
+                    rollback_actions.append(("install", prior_component))
                     result.uninstalled.append(component)
-    except BundlerError:
-        _rollback(project_root, installer, done)
-        raise
+
+        record = InstalledBundleRecord.create(
+            bundle_id=plan.bundle_id,
+            version=plan.version,
+            components=contributed,
+            # Preserve the original install time across refresh/update so
+            # ``bundle list`` keeps reporting when the bundle was first installed.
+            installed_at=existing.installed_at if existing is not None else None,
+        )
+        save_records(project_root, upsert_record(records, record))
     except Exception as exc:  # noqa: BLE001
-        _rollback(project_root, installer, done)
-        raise BundlerError(
-            f"Failed to install bundle '{plan.bundle_id}': {exc}. "
-            "No changes were recorded."
-        ) from exc
+        rollback_complete = _rollback(
+            project_root, installer, rollback_actions
+        )
+        if isinstance(exc, BundlerError) and rollback_complete:
+            raise
+        detail = (
+            "Completed changes were rolled back and no provenance record was written."
+            if rollback_complete
+            else "Rollback was incomplete and no provenance record was written; "
+            "the project may be inconsistent."
+        )
+        message = (
+            str(exc)
+            if isinstance(exc, BundlerError)
+            else f"Failed to install bundle '{plan.bundle_id}': {exc}"
+        )
+        raise BundlerError(f"{message}. {detail}") from exc
 
-    record = InstalledBundleRecord.create(
-        bundle_id=plan.bundle_id,
-        version=plan.version,
-        components=contributed,
-        # Preserve the original install time across refresh/update so
-        # ``bundle list`` keeps reporting when the bundle was first installed.
-        installed_at=existing.installed_at if existing is not None else None,
-    )
-    save_records(project_root, upsert_record(records, record))
     return result
 
 
@@ -245,13 +264,54 @@ def _refresh_component(
         installer.install(project_root, component)
 
 
+def _snapshot_component(
+    project_root: Path,
+    installer: PrimitiveInstaller,
+    component: ComponentRef,
+) -> ComponentRef:
+    """Capture actual installed metadata before a destructive update."""
+    try:
+        snapshot = installer.snapshot(project_root, component)
+    except BundlerError:
+        raise
+    except Exception as exc:  # noqa: BLE001
+        raise BundlerError(
+            f"Cannot safely update {component.label()}: failed to snapshot "
+            f"the installed component: {exc}"
+        ) from exc
+
+    if snapshot is None:
+        raise BundlerError(
+            f"Cannot safely update {component.label()}: installed state "
+            "could not be snapshotted."
+        )
+    if (snapshot.kind, snapshot.id) != (component.kind, component.id):
+        raise BundlerError(
+            f"Cannot safely update {component.label()}: snapshot returned "
+            f"the wrong component ({snapshot.label()})."
+        )
+    if not isinstance(snapshot.version, str) or not snapshot.version.strip():
+        raise BundlerError(
+            f"Cannot safely update {component.label()}: installed version "
+            "could not be determined for rollback."
+        )
+    return snapshot
+
+
 def _rollback(
     project_root: Path,
     installer: PrimitiveInstaller,
-    done: list[ComponentRef],
-) -> None:
-    for component in reversed(done):
+    actions: list[tuple[str, ComponentRef]],
+) -> bool:
+    complete = True
+    for operation, component in reversed(actions):
         try:
-            installer.remove(project_root, component)
+            if operation == "remove":
+                installer.remove(project_root, component)
+            elif operation == "install":
+                installer.install(project_root, component)
+            else:
+                _refresh_component(project_root, installer, component)
         except Exception:  # noqa: BLE001 - best-effort rollback
-            continue
+            complete = False
+    return complete

src/specify_cli/bundler/services/primitives.py

@@ -88,6 +88,9 @@ class _KindManager(Protocol):
     def is_installed(self, component: ComponentRef) -> bool:
         pass
 
+    def snapshot(self, component: ComponentRef) -> ComponentRef | None:
+        pass
+
     def install(self, component: ComponentRef) -> None:
         pass
 
@@ -142,6 +145,29 @@ def _delegate_command(action: str, label: str, call) -> None:
             raise BundlerError(f"Failed to {action} {label}.") from exc
 
 
+def _snapshot_ref(
+    component: ComponentRef,
+    *,
+    version: object,
+    metadata: dict[str, object] | None = None,
+) -> ComponentRef:
+    metadata = metadata or {}
+    actual_version = version.strip() if isinstance(version, str) else None
+    source = metadata.get("source")
+    priority = metadata.get("priority")
+    return ComponentRef(
+        kind=component.kind,
+        id=component.id,
+        version=actual_version or None,
+        source=source if isinstance(source, str) else None,
+        priority=(
+            priority
+            if isinstance(priority, int) and not isinstance(priority, bool)
+            else None
+        ),
+    )
+
+
 class _PresetKindManager:
     def __init__(self, project_root: Path, allow_network: bool) -> None:
         from ...presets import PresetManager
@@ -156,6 +182,21 @@ def is_installed(self, component: ComponentRef) -> bool:
         except Exception:  # noqa: BLE001
             return False
 
+    def snapshot(self, component: ComponentRef) -> ComponentRef | None:
+        metadata = self._manager.registry.get(component.id)
+        manifest = self._manager.get_pack(component.id)
+        if metadata is None and manifest is None:
+            return None
+        return _snapshot_ref(
+            component,
+            version=(
+                metadata.get("version")
+                if metadata is not None
+                else manifest.version
+            ),
+            metadata=metadata,
+        )
+
     def install(self, component: ComponentRef) -> None:
         self._do_install(component, force=False)
 
@@ -239,6 +280,16 @@ def is_installed(self, component: ComponentRef) -> bool:
         except Exception:  # noqa: BLE001
             return False
 
+    def snapshot(self, component: ComponentRef) -> ComponentRef | None:
+        metadata = self._manager.registry.get(component.id)
+        if metadata is None:
+            return None
+        return _snapshot_ref(
+            component,
+            version=metadata.get("version"),
+            metadata=metadata,
+        )
+
     def install(self, component: ComponentRef) -> None:
         self._do_install(component, force=False)
 
@@ -326,6 +377,14 @@ def is_installed(self, component: ComponentRef) -> bool:
         except Exception:  # noqa: BLE001
             return False
 
+    def snapshot(self, component: ComponentRef) -> ComponentRef | None:
+        metadata = self._registry.get(component.id)
+        if metadata is None:
+            return None
+        return _snapshot_ref(
+            component, version=metadata.get("version"), metadata=metadata
+        )
+
     def install(self, component: ComponentRef) -> None:
         if not self._allow_network and not self._is_bundled(component.id):
             raise BundlerError(
@@ -392,13 +451,22 @@ def is_installed(self, component: ComponentRef) -> bool:
         except Exception:  # noqa: BLE001
             return False
 
+    def snapshot(self, component: ComponentRef) -> ComponentRef | None:
+        metadata = self._registry.get(component.id)
+        if metadata is None:
+            return None
+        return _snapshot_ref(
+            component, version=metadata.get("version"), metadata=metadata
+        )
+
     def install(self, component: ComponentRef) -> None:
         if not self._allow_network:
             raise BundlerError(
                 f"Step '{component.id}' installs from a catalog and network access "
                 f"is disabled; re-run without --offline or install it first with "
                 f"'specify workflow step add {component.id}'."
             )
+        self._assert_pinned_version(component)
         from ... import workflow_step_add
 
         with _chdir(self._root):
@@ -458,6 +526,32 @@ def refresh(self, component: ComponentRef) -> None:
         finally:
             shutil.rmtree(backup_dir.parent, ignore_errors=True)
 
+    def _assert_pinned_version(self, component: ComponentRef) -> None:
+        if not component.version:
+            return
+        from ...workflows.catalog import StepCatalog, StepCatalogError
+
+        try:
+            info = StepCatalog(self._root).get_step_info(component.id)
+        except StepCatalogError as exc:
+            raise BundlerError(
+                f"Cannot verify pinned version for step '{component.id}': {exc}"
+            ) from exc
+        if not info:
+            raise BundlerError(
+                f"Cannot verify pinned version for step '{component.id}': "
+                "the step was not found in the catalog."
+            )
+        advertised = info.get("version")
+        if advertised is None or not str(advertised).strip():
+            raise BundlerError(
+                f"Cannot verify pinned version for step '{component.id}': "
+                "the catalog does not advertise a version."
+            )
+        _assert_pinned_version(
+            "Step", component.id, component.version, advertised
+        )
+
     def remove(self, component: ComponentRef) -> None:
         from ... import workflow_step_remove
 

src/specify_cli/bundler/services/references.py

@@ -48,11 +48,10 @@ def _resolved_locally(root: Path, component: ComponentRef) -> bool:
             # ``_locate_bundled_step`` to mirror the three lookups above.
             # ``BUILTIN_STEP_TYPES`` is the bundled-with-Spec-Kit check for this
             # kind. Deliberately NOT ``STEP_REGISTRY``: ``load_custom_steps``
-            # adds project-installed ids to that process-global mapping and
-            # never removes them, so in a long-lived process a community step
-            # loaded for one project would be accepted as "bundled" when
-            # validating another. Without any bundled check at all, every
-            # built-in step type looked unresolved.
+            # adds the most recently scanned project's ids to that process-global
+            # mapping, so a community step could be accepted as "bundled" when
+            # validating a different root. Without any bundled check at all,
+            # every built-in step type looked unresolved.
             if component.id in BUILTIN_STEP_TYPES:
                 return True
             return StepRegistry(root).is_installed(component.id)