fix(extensions): roll back native outputs on failed install

src/specify_cli/extensions/__init__.py

@@ -2996,6 +2996,38 @@ def _restore_stranded_config_file(
         # the restored user config with packaged defaults.  Cleanup is deferred
         # until after registry.add() succeeds (see post-commit cleanup below).
 
+        def rollback_package_dir() -> None:
+            """Remove the installed package, preserving rescued user config.
+
+            Shared by the generic and native rollbacks: config files a prior
+            ``remove --keep-config`` preserved (or a --force reinstall backed
+            up) stay in place with a ``.keep-config`` marker; everything else
+            the attempt copied is removed.
+            """
+            preserved = set(stranded_configs)
+            if did_remove:
+                backup_dir = self.extensions_dir / ".backup" / manifest.id
+                if backup_dir.is_dir() and not backup_dir.is_symlink():
+                    for config_file in backup_dir.iterdir():
+                        if (
+                            config_file.is_file()
+                            and not config_file.is_symlink()
+                            and config_file.name.endswith(("-config.yml", "-config.local.yml"))
+                        ):
+                            shutil.copy2(config_file, dest_dir / config_file.name)
+                            preserved.add(config_file.name)
+            if preserved:
+                for child in dest_dir.iterdir():
+                    if child.name in preserved:
+                        continue
+                    if child.is_dir() and not child.is_symlink():
+                        shutil.rmtree(child)
+                    else:
+                        child.unlink()
+                (dest_dir / ".keep-config").write_text("", encoding="utf-8")
+            else:
+                shutil.rmtree(dest_dir)
+
         def rollback_generic_registration() -> None:
             from ..shared_infra import _validate_safe_shared_directory
 
@@ -3024,35 +3056,36 @@ def rollback_generic_registration() -> None:
                     except OSError:
                         pass
 
-            preserved = set(stranded_configs)
-            if did_remove:
-                backup_dir = self.extensions_dir / ".backup" / manifest.id
-                if backup_dir.is_dir() and not backup_dir.is_symlink():
-                    for config_file in backup_dir.iterdir():
-                        if (
-                            config_file.is_file()
-                            and not config_file.is_symlink()
-                            and config_file.name.endswith(("-config.yml", "-config.local.yml"))
-                        ):
-                            shutil.copy2(config_file, dest_dir / config_file.name)
-                            preserved.add(config_file.name)
-            if preserved:
-                for child in dest_dir.iterdir():
-                    if child.name in preserved:
-                        continue
-                    if child.is_dir() and not child.is_symlink():
-                        shutil.rmtree(child)
-                    else:
-                        child.unlink()
-                (dest_dir / ".keep-config").write_text("", encoding="utf-8")
-            else:
-                shutil.rmtree(dest_dir)
+            rollback_package_dir()
+
+        def rollback_native_registration() -> None:
+            """Retire native command/skill outputs and the installed package.
+
+            Mirrors ``rollback_generic_registration`` for the non-generic
+            route: a failed install must leave no outputs the attempt created
+            (#5379). ``registered_commands`` holds the command files the
+            registrar wrote (for skills-mode agents these are the SKILL.md
+            outputs) and ``registered_skills`` the skill files the skills
+            registrar wrote; both lists only ever contain outputs this
+            attempt actually wrote.
+            """
+            from ..agents import CommandRegistrar
+
+            safe_commands = dict(registered_commands)
+            safe_commands.pop("generic", None)
+            if safe_commands:
+                CommandRegistrar(
+                    self.project_root, include_generic=False
+                ).unregister_commands(safe_commands, self.project_root)
+            self._unregister_extension_skills(registered_skills, manifest.id)
+            rollback_package_dir()
 
         hooks_started = False
         registry_started = False
         try:
             # Register commands with AI agents (active integration only, #2948)
             registered_commands = {}
+            registered_skills: list = []
             if register_commands:
                 registered_commands = self._register_commands_for_active_agent(
                     manifest, dest_dir, link_outputs=link_commands
@@ -3152,6 +3185,32 @@ def rollback_generic_registration() -> None:
                         f"Generic registration failed ({exc}); rollback failed: "
                         + "; ".join(rollback_errors)
                     ) from exc
+            else:
+                # Native route has no generic artifacts, but a failure after
+                # command rendering must still retire everything the attempt
+                # created: registry entry, hooks, native command/skill
+                # outputs, and the installed package (#5379).
+                rollback_errors = []
+                if registry_started:
+                    try:
+                        self.registry.remove(manifest.id)
+                    except Exception as error:
+                        rollback_errors.append(f"registry: {error}")
+                if hooks_started:
+                    try:
+                        hook_executor.unregister_hooks(manifest.id)
+                    except Exception as error:
+                        rollback_errors.append(f"hooks: {error}")
+                try:
+                    with changing_file(dest_dir, removal=True):
+                        rollback_native_registration()
+                except Exception as error:
+                    rollback_errors.append(f"artifacts: {error}")
+                if rollback_errors:
+                    raise ExtensionError(
+                        f"Extension install failed ({exc}); rollback failed: "
+                        + "; ".join(rollback_errors)
+                    ) from exc
             raise
 
         if did_remove:

tests/specify_cli/extensions/test_command_add.py

@@ -17,13 +17,14 @@
 import yaml
 from typer.testing import CliRunner
 
-from specify_cli import app
+from specify_cli import app, save_init_options
 from specify_cli.extensions import (
     CompatibilityError,
     ExtensionCatalog,
     ExtensionError,
     ExtensionManager,
     ExtensionRegistry,
+    HookExecutor,
     ValidationError,
 )
 from tests.conftest import strip_ansi
@@ -1387,3 +1388,98 @@ def test_extension_install_listing_matches_registered_command_names(
     assert result.exit_code == 0, output
     assert f"• {listed} - " in output, output
     assert f"• {not_listed} - " not in output, output
+
+
+def _sample_extension_dir(tmp_path):
+    """Build the minimal extension used by the #5379 rollback tests."""
+    ext = tmp_path / "ext"
+    (ext / "commands").mkdir(parents=True)
+    (ext / "commands/run.md").write_text("---\ndescription: d\n---\nhello\n")
+    (ext / "extension.yml").write_text(
+        yaml.safe_dump(
+            {
+                "schema_version": "1.0",
+                "extension": {
+                    "id": "sample",
+                    "name": "S",
+                    "version": "1.0.0",
+                    "description": "d",
+                    "author": "t",
+                },
+                "requires": {"speckit_version": ">=0.1.0"},
+                "provides": {
+                    "commands": [
+                        {"name": "speckit.sample.run", "file": "commands/run.md"}
+                    ]
+                },
+                "hooks": {
+                    "after_tasks": {"command": "speckit.sample.run", "optional": True}
+                },
+            }
+        )
+    )
+    return ext
+
+
+def _claude_skills_project(tmp_path):
+    project = tmp_path / "p"
+    (project / ".specify").mkdir(parents=True)
+    (project / ".claude/skills").mkdir(parents=True)
+    save_init_options(project, {"ai": "claude", "ai_skills": True, "script": "sh"})
+    return project
+
+
+def test_failed_native_install_removes_skill_outputs(tmp_path, monkeypatch):
+    """A failed native install must leave no outputs the attempt created (#5379)."""
+    project = _claude_skills_project(tmp_path)
+    ext = _sample_extension_dir(tmp_path)
+    monkeypatch.setattr(
+        HookExecutor,
+        "register_hooks",
+        lambda *a: (_ for _ in ()).throw(OSError("x")),
+    )
+    with pytest.raises(OSError):
+        ExtensionManager(project).install_from_directory(ext, "1.0.0")
+    assert not ExtensionManager(project).registry.is_installed("sample")
+    assert not (project / ".claude/skills/speckit-sample-run/SKILL.md").exists()
+    assert not (project / ".specify/extensions/sample").exists()
+
+
+def test_failed_native_install_at_registry_write_removes_skill_outputs(
+    tmp_path, monkeypatch
+):
+    """A failure at the final registry write must also roll back skill outputs (#5379)."""
+    project = _claude_skills_project(tmp_path)
+    ext = _sample_extension_dir(tmp_path)
+    monkeypatch.setattr(
+        ExtensionRegistry, "add", lambda *a, **k: (_ for _ in ()).throw(OSError("x"))
+    )
+    with pytest.raises(OSError):
+        ExtensionManager(project).install_from_directory(ext, "1.0.0")
+    assert not ExtensionManager(project).registry.is_installed("sample")
+    assert not (project / ".claude/skills/speckit-sample-run/SKILL.md").exists()
+    assert not (project / ".specify/extensions/sample").exists()
+
+
+def test_failed_native_force_install_preserves_kept_config(tmp_path, monkeypatch):
+    """A failed native --force reinstall must preserve kept user config (#5379)."""
+    project = _claude_skills_project(tmp_path)
+    ext = _sample_extension_dir(tmp_path)
+    (ext / "sample-config.yml").write_text("default: true\n", encoding="utf-8")
+    manager = ExtensionManager(project)
+    manager.install_from_directory(ext, "1.0.0")
+    assert manager.remove("sample", keep_config=True)
+    config = project / ".specify/extensions/sample/sample-config.yml"
+    config.write_text("user: preserved\n", encoding="utf-8")
+    monkeypatch.setattr(
+        HookExecutor,
+        "register_hooks",
+        lambda *a: (_ for _ in ()).throw(OSError("x")),
+    )
+    with pytest.raises(OSError):
+        ExtensionManager(project).install_from_directory(ext, "1.0.0", force=True)
+    assert not ExtensionManager(project).registry.is_installed("sample")
+    assert config.read_text(encoding="utf-8") == "user: preserved\n"
+    assert (config.parent / ".keep-config").exists()
+    assert not (config.parent / "extension.yml").exists()
+    assert not (project / ".claude/skills/speckit-sample-run/SKILL.md").exists()