fix(extensions): do not overwrite pre-existing user skills on install

src/specify_cli/agents.py

@@ -615,6 +615,44 @@ def _generic_owned_output(path: Path, source_id: str, project_root: Path) -> boo
         relative = path.relative_to(project_root.resolve()).as_posix()
         return hashes.get(relative) == hashlib.sha256(path.read_bytes()).hexdigest()
 
+    @staticmethod
+    def _skill_file_owned_by_source(path: Path, source_id: str) -> bool:
+        """Return True when an existing SKILL.md was generated by *source_id*.
+
+        Native registration writes ``metadata.source`` as
+        ``"<source_id>:<source file>"`` (see :meth:`render_skill_command`);
+        the supplemental skills route writes ``"extension:<source_id>"``
+        and the preset skills route writes ``"preset:<source_id>"``.
+        A dev-mode symlink pointing at a ``.specify-dev`` cache file is
+        likewise this tooling's own artifact and may be refreshed.
+        Anything else — a user-authored skill, another extension's skill,
+        or an unreadable/unparseable file — is not ours: fail closed and
+        treat it as unowned so registration skips it instead of
+        overwriting it (#5380).
+        """
+        if path.is_symlink():
+            try:
+                target = path.resolve(strict=False)
+            except (OSError, RuntimeError):
+                target = None
+            if target is not None and ".specify-dev" in target.parts:
+                return True
+        try:
+            raw = path.read_text(encoding="utf-8")
+        except (OSError, UnicodeDecodeError):
+            return False
+        fm, _ = CommandRegistrar.parse_frontmatter(raw)
+        source = (
+            fm.get("metadata", {}).get("source", "") if isinstance(fm, dict) else ""
+        )
+        if not isinstance(source, str):
+            return False
+        return (
+            source == f"extension:{source_id}"
+            or source == f"preset:{source_id}"
+            or source.startswith(f"{source_id}:")
+        )
+
     @staticmethod
     def _same_lexical_path(left: Path, right: Path) -> bool:
         """Compare paths after lexical normalization without resolving symlinks."""
@@ -888,6 +926,24 @@ def register_commands(
             if agent_name == "generic" and (dest_file.exists() or dest_file.is_symlink()):
                 if not self._generic_owned_output(dest_file, source_id, project_root):
                     continue
+            if (
+                agent_config["extension"] == "/SKILL.md"
+                and (dest_file.exists() or dest_file.is_symlink())
+                and not self._skill_file_owned_by_source(dest_file, source_id)
+            ):
+                # Never overwrite a pre-existing user skill: the supplemental
+                # skills route already skips unowned directories, and native
+                # registration must do the same (#5380). The command is left
+                # unregistered so a later removal cannot delete it either.
+                import warnings
+
+                warnings.warn(
+                    f"Skipping skill '{output_name}': '{dest_file}' already "
+                    f"exists and was not created by '{source_id}'.",
+                    UserWarning,
+                    stacklevel=2,
+                )
+                continue
             dest_file.parent.mkdir(parents=True, exist_ok=True)
             self._write_registered_output(
                 dest_file,
@@ -973,6 +1029,21 @@ def register_commands(
                 if agent_name == "generic" and (alias_file.exists() or alias_file.is_symlink()):
                     if not self._generic_owned_output(alias_file, source_id, project_root):
                         continue
+                if (
+                    agent_config["extension"] == "/SKILL.md"
+                    and (alias_file.exists() or alias_file.is_symlink())
+                    and not self._skill_file_owned_by_source(alias_file, source_id)
+                ):
+                    # Same ownership guard as the primary skill above (#5380).
+                    import warnings
+
+                    warnings.warn(
+                        f"Skipping skill '{alias_output_name}': '{alias_file}' "
+                        f"already exists and was not created by '{source_id}'.",
+                        UserWarning,
+                        stacklevel=2,
+                    )
+                    continue
                 alias_file.parent.mkdir(parents=True, exist_ok=True)
                 self._write_registered_output(
                     alias_file,

tests/test_extension_skills.py

@@ -718,6 +718,41 @@ def test_existing_skill_directory_without_marker_is_not_modified(
         assert "speckit-test-ext-hello" not in metadata["registered_skills"]
         assert "speckit-test-ext-world" in metadata["registered_skills"]
 
+    def test_native_registration_does_not_overwrite_user_skill(
+        self, skills_project, extension_dir
+    ):
+        """Native registration must not overwrite a pre-existing user skill (#5380).
+
+        The supplemental skills route already skips pre-existing unowned
+        directories, but native command registration wrote SKILL.md
+        unconditionally, so a later ``extension remove`` deleted the user's
+        content outright.
+        """
+        project_dir, skills_dir = skills_project
+        user_dir = skills_dir / "speckit-test-ext-hello"
+        user_dir.mkdir(parents=True)
+        (user_dir / "SKILL.md").write_text("user skill", encoding="utf-8")
+        (user_dir / "notes.txt").write_text("user", encoding="utf-8")
+
+        manager = ExtensionManager(project_dir)
+        manifest = manager.install_from_directory(extension_dir, "0.1.0")
+
+        # Install must leave the user's SKILL.md untouched ...
+        assert (user_dir / "SKILL.md").read_text(encoding="utf-8") == "user skill"
+        # ... and must not register the skipped skill (native registration
+        # writes the surviving skill, so it lands in registered_commands
+        # while the supplemental route leaves registered_skills empty).
+        metadata = manager.registry.get(manifest.id)
+        registered_commands = metadata.get("registered_commands", {})
+        assert "speckit.test-ext.hello" not in registered_commands.get("claude", [])
+        assert "speckit.test-ext.world" in registered_commands.get("claude", [])
+
+        manager.remove(manifest.id, keep_config=False)
+
+        # ... so removal cannot delete content the extension never created.
+        assert (user_dir / "SKILL.md").read_text(encoding="utf-8") == "user skill"
+        assert (user_dir / "notes.txt").read_text(encoding="utf-8") == "user"
+
     def test_dev_skill_symlink_refreshes_existing_cache(
         self, skills_project, extension_dir, temp_dir
     ):