Merge db8ac56ea2 into 4fade39c90

tests/tools/test_skills_sync.py

@@ -583,6 +583,58 @@ class TestSyncSkills:
         assert manifest["old-skill"] != old_hash
 
 
+    def test_description_md_not_copied_to_ghost_dirs(self, tmp_path):
+        """DESCRIPTION.md should not be copied into empty category dirs with no SKILL.md."""
+        bundled = tmp_path / "bundled_skills"
+        # Bundled: a category with both SKILL.md and DESCRIPTION.md
+        (bundled / "apple" / "findmy").mkdir(parents=True)
+        (bundled / "apple" / "findmy" / "SKILL.md").write_text("# FindMy")
+        (bundled / "apple" / "DESCRIPTION.md").write_text("Apple stuff")
+        # Bundled: a stub-only category (only DESCRIPTION.md, no SKILL.md anywhere)
+        (bundled / "empty-cat").mkdir(parents=True)
+        (bundled / "empty-cat" / "DESCRIPTION.md").write_text("Empty")
+
+        skills_dir = tmp_path / "user_skills"
+        manifest_file = skills_dir / ".bundled_manifest"
+        # User deleted all skills from apple — only the DESCRIPTION.md ghost remains
+        # (or was never installed). The category dir does NOT exist on disk.
+        # empty-cat also doesn't exist.
+
+        with self._patches(bundled, skills_dir, manifest_file):
+            sync_skills(quiet=True)
+
+        # findmy was synced (new install) → apple/ now has SKILL.md → DESCRIPTION.md should be copied
+        assert (skills_dir / "apple" / "findmy" / "SKILL.md").exists()
+        assert (skills_dir / "apple" / "DESCRIPTION.md").exists()
+
+        # empty-cat has no SKILL.md anywhere → DESCRIPTION.md should NOT be copied
+        # and the directory should NOT be created
+        assert not (skills_dir / "empty-cat").exists()
+
+    def test_description_md_not_recreated_after_user_deletion(self, tmp_path):
+        """After user deletes all skills from a category, DESCRIPTION.md should not come back."""
+        bundled = tmp_path / "bundled_skills"
+        (bundled / "apple" / "findmy").mkdir(parents=True)
+        (bundled / "apple" / "findmy" / "SKILL.md").write_text("# FindMy")
+        (bundled / "apple" / "DESCRIPTION.md").write_text("Apple stuff")
+
+        skills_dir = tmp_path / "user_skills"
+        manifest_file = skills_dir / ".bundled_manifest"
+        skills_dir.mkdir(parents=True)
+
+        # Simulate: user previously had findmy installed, then deleted it.
+        # Manifest still has findmy (origin hash from last sync).
+        old_hash = _dir_hash(bundled / "apple" / "findmy")
+        manifest_file.write_text(f"findmy:{old_hash}\n")
+        # No apple/ dir on disk — user deleted everything.
+
+        with self._patches(bundled, skills_dir, manifest_file):
+            sync_skills(quiet=True)
+
+        # findmy should NOT be re-added (user deleted it)
+        assert not (skills_dir / "apple").exists()
+
+
 class TestGetBundledDir:
     def test_env_var_override(self, tmp_path, monkeypatch):
         """HERMES_BUNDLED_SKILLS env var overrides the default path resolution."""

tools/skills_sync.py

@@ -294,16 +294,21 @@ def sync_skills(quiet: bool = False) -> dict:
     for name in cleaned:
         del manifest[name]
 
-    # Also copy DESCRIPTION.md files for categories (if not already present)
+    # Also copy DESCRIPTION.md files for categories (if not already present).
+    # Only copy when the category already has at least one SKILL.md on disk —
+    # this prevents recreating empty "ghost" directories that the user removed.
     for desc_md in bundled_dir.rglob("DESCRIPTION.md"):
         rel = desc_md.relative_to(bundled_dir)
         dest_desc = SKILLS_DIR / rel
-        if not dest_desc.exists():
-            try:
-                dest_desc.parent.mkdir(parents=True, exist_ok=True)
-                shutil.copy2(desc_md, dest_desc)
-            except (OSError, IOError) as e:
-                logger.debug("Could not copy %s: %s", desc_md, e)
+        if dest_desc.exists():
+            continue
+        cat_dir = dest_desc.parent
+        if not cat_dir.exists() or not list(cat_dir.rglob("SKILL.md")):
+            continue
+        try:
+            shutil.copy2(desc_md, dest_desc)
+        except (OSError, IOError) as e:
+            logger.debug("Could not copy %s: %s", desc_md, e)
 
     _write_manifest(manifest)