fix: keep snapshot cleanup outside transaction outcomes

Report filesystem cleanup failures with the leaked snapshot path without masking committed success, provenance errors, incomplete rollback or snapshot capture failures. Reuse the same cleanup boundary during failed capture. Exercise both real managers through install/update and removal with sixteen failure-first cases.

Assisted-by: GitHub Copilot (model: GPT-6 Astra, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: marcelsafin

docs/reference/bundles.md

@@ -61,6 +61,8 @@ Re-resolves a bundle and **refreshes** its components through each primitive's u
 
 Before refreshing or removing an owned component, the bundler snapshots its installed files and registry metadata. If a component operation or provenance write fails, it attempts to restore those local snapshots, including disabled state, user configuration, extension hook settings and pre-existing configuration backups, and generated command files and skill resources for current and previously active integrations, without downloading an older version. Previously absent outputs and configuration backups are also restored to absence. Custom steps are restored before dependent workflows. Recovery is best-effort and reports incomplete restoration; these temporary snapshots cover failures during the command, not process crashes or unrelated project files.
 
+If temporary snapshot cleanup fails, a warning identifies the path for manual removal without changing the committed result or masking the original rollback error.
+
 > **Pin enforcement is install-time only.** Idempotency checks are id-based, not version-aware: a component that is already present is skipped during `install` without comparing its on-disk version to the manifest pin. Version pins are therefore guaranteed to be applied only when the bundler actually installs a component for the first time or refreshes it. Run `specify bundle update` to re-apply every owned component at its pinned version.
 
 ## Remove a Bundle

src/specify_cli/bundler/models/snapshot.py

@@ -1,13 +1,16 @@
 """Ephemeral installed-component state, never serialized into bundle records."""
 from __future__ import annotations
 
+import logging
 from dataclasses import dataclass, field
 from pathlib import Path
 from tempfile import TemporaryDirectory
 from typing import Any
 
 from .manifest import ComponentRef
 
+logger = logging.getLogger(__name__)
+
 
 @dataclass
 class ArtifactSnapshot:
@@ -28,4 +31,10 @@ class ComponentSnapshot:
 
     def close(self) -> None:
         if self.backup is not None:
-            self.backup.cleanup()
+            try:
+                self.backup.cleanup()
+            except OSError as exc:
+                logger.warning(
+                    "Could not clean up rollback snapshot at %s; remove it manually: %s",
+                    self.backup.name, exc,
+                )

src/specify_cli/bundler/services/primitives.py

@@ -181,19 +181,20 @@ def _snapshot_directory(
 ) -> ComponentSnapshot:
     backup = TemporaryDirectory(prefix="speckit-bundle-rollback-")
     destination = Path(backup.name) / component.id
-    try:
-        shutil.copytree(directory, destination, symlinks=True)
-    except OSError:
-        backup.cleanup()
-        raise
-    return ComponentSnapshot(
+    snapshot = ComponentSnapshot(
         component=_snapshot_ref(
             component, version=metadata.get("version"), metadata=metadata
         ),
         metadata=copy.deepcopy(metadata),
         directory=destination,
         backup=backup,
     )
+    try:
+        shutil.copytree(directory, destination, symlinks=True)
+    except OSError:
+        snapshot.close()
+        raise
+    return snapshot
 
 
 def _snapshot_source(snapshot: ComponentSnapshot) -> Path:

tests/integration/test_bundler_state_rollback.py

@@ -233,6 +233,80 @@ def fail_save(*_args):
         assert not backup.exists()
 
 
+@pytest.mark.parametrize("operation", ["refresh", "remove"])
+@pytest.mark.parametrize(
+    "scenario", ["success", "save-failure", "rollback-failure", "capture-failure"]
+)
+def test_snapshot_cleanup_failure_preserves_transaction_outcome(
+    installed_components, monkeypatch, caplog, operation, scenario,
+):
+    import shutil
+    from pathlib import Path
+    from tempfile import TemporaryDirectory
+
+    project, kind, manager_type, installer, plan, metadata = installed_components
+    original_record = records_path(project).read_bytes()
+    cleanup = TemporaryDirectory.cleanup
+    copy_tree = shutil.copytree
+    attempted = []
+
+    def deny_cleanup(directory):
+        if Path(directory.name).name.startswith("speckit-bundle-rollback-"):
+            attempted.append(directory)
+            raise PermissionError("snapshot cleanup denied")
+        return cleanup(directory)
+
+    def fail_copy(source, destination, *args, **kwargs):
+        if Path(source) == project / ".specify" / kind / "owned":
+            raise PermissionError("snapshot copy refused")
+        return copy_tree(source, destination, *args, **kwargs)
+
+    def fail_save(*_args):
+        raise OSError("provenance write refused")
+
+    def fail_restore(*_args):
+        raise OSError("restoration refused")
+
+    def change():
+        if operation == "remove":
+            return remove_bundle(project, "demo-bundle", installer)
+        return install_bundle(project, plan(["owned", "keeper"]), installer, refresh=True)
+
+    monkeypatch.setattr(TemporaryDirectory, "cleanup", deny_cleanup)
+    if scenario in ("save-failure", "rollback-failure"):
+        monkeypatch.setattr("specify_cli.bundler.services.installer.save_records", fail_save)
+    if scenario == "rollback-failure":
+        monkeypatch.setattr(installer, "restore", fail_restore)
+    if scenario == "capture-failure":
+        monkeypatch.setattr(shutil, "copytree", fail_copy)
+    try:
+        if scenario == "success":
+            assert change().changed
+            assert (manager_type(project).registry.get("owned") is not None) == (
+                operation == "refresh"
+            )
+        else:
+            message = (
+                "snapshot copy refused" if scenario == "capture-failure"
+                else "provenance write refused"
+            )
+            with pytest.raises(BundlerError, match=message) as error:
+                change()
+            assert ("Rollback was incomplete" in str(error.value)) == (
+                scenario == "rollback-failure"
+            )
+            assert records_path(project).read_bytes() == original_record
+            if scenario != "rollback-failure":
+                assert manager_type(project).registry.get("owned") == metadata
+    finally:
+        for directory in attempted:
+            cleanup(directory)
+    assert attempted
+    assert "snapshot cleanup denied" in caplog.text
+    for directory in attempted:
+        assert directory.name in caplog.text
+
+
 @pytest.mark.parametrize("kind", ["steps", "workflows"])
 def test_dropped_component_restores_local_payload_and_exact_registry(
     tmp_path, monkeypatch, kind