fix(skills): keep built-in provenance for skills the catalog dropped

Co-authored-by: fangliquanflq <fangliquan@qq.com>

hermes_cli/web_routers/skills.py

@@ -345,7 +345,7 @@ async def get_skills(profile: Optional[str] = None):
     from tools.skills_tool import _find_all_skills
     from hermes_cli.skills_config import get_disabled_skills
     from tools.skill_usage import (
-        _read_bundled_manifest_names, _read_hub_installed_names, activity_count, load_usage)
+        _read_bundled_names, _read_hub_installed_names, activity_count, load_usage)
 
     def _run():
         with _profile_scope(profile):
@@ -357,7 +357,7 @@ async def get_skills(profile: Optional[str] = None):
             # without a per-skill manifest read): hub > bundled > agent, where
             # "agent" covers agent-authored AND local hand-made skills — the ones
             # the user may edit/delete from the UI.
-            bundled_names = _read_bundled_manifest_names()
+            bundled_names = _read_bundled_names()
             hub_names = _read_hub_installed_names()
         for s in skills:
             s["enabled"] = s["name"] not in disabled

tests/tools/test_skills_sync.py

@@ -513,6 +513,52 @@ class TestSyncSkills:
             assert (skills_dir / "category" / "new-skill" / "SKILL.md").exists()
 
 
+class TestDroppedBuiltinProvenance:
+    """#95415: a built-in the catalog dropped keeps built-in provenance while any copy of it is left;
+    otherwise /api/skills badges it "Learned" and the Desktop offers edit/archive for it."""
+
+    def _bundled(self, tmp_path, *names):
+        bundled = tmp_path / "bundled_skills"
+        shutil.rmtree(bundled, ignore_errors=True)
+        for name in names:
+            (bundled / "cat" / name).mkdir(parents=True)
+            (bundled / "cat" / name / "SKILL.md").write_text(f"---\nname: {name}\n---\n# {name}\n")
+        return bundled
+
+    def _sync(self, bundled):
+        with patch("tools.skills_sync._get_bundled_dir", return_value=bundled), \
+                patch("tools.skills_sync._get_optional_dir", return_value=bundled.parent / "optional-skills"):
+            return sync_skills(quiet=True)
+
+    def test_catalog_drop_keeps_builtin_provenance_while_a_copy_exists(self, tmp_path):
+        from hermes_constants import get_hermes_home
+        from tools.skill_usage import provenance
+
+        skills = get_hermes_home() / "skills"
+        self._sync(self._bundled(tmp_path, "kept", "dropped", "dropped-archived", "dropped-deleted"))
+        (skills / ".archive").mkdir()
+        shutil.move(str(skills / "cat" / "dropped-archived"), str(skills / ".archive" / "dropped-archived"))
+        shutil.rmtree(skills / "cat" / "dropped-deleted")
+
+        result = self._sync(self._bundled(tmp_path, "kept"))
+
+        assert result["cleaned"] == ["dropped-deleted"]
+        assert {"dropped", "dropped-archived"} <= set(_read_manifest())
+        assert provenance("dropped") == "bundled" and provenance("dropped-archived") == "bundled"
+
+    def test_suppressed_builtin_is_not_agent_authored_after_manifest_cleanup(self, tmp_path):
+        """Profiles an older sync already cleaned: the curator suppression list only records built-ins."""
+        from hermes_constants import get_hermes_home
+        from tools.skill_usage import provenance
+
+        skills = get_hermes_home() / "skills"
+        (skills / "resurrected").mkdir(parents=True)
+        (skills / "resurrected" / "SKILL.md").write_text("---\nname: resurrected\n---\n")
+        (skills / ".curator_suppressed").write_text("resurrected\n")
+
+        assert provenance("resurrected") == "bundled"
+
+
 class TestGetBundledDir:
     def test_env_var_override_with_default_fallback(self, tmp_path, monkeypatch):
         custom_dir = tmp_path / "custom_skills"

tools/skill_usage.py

@@ -143,10 +143,12 @@ def activity_count(record: Dict[str, Any]) -> int:
 
 
 # --- Provenance — which skills are agent-created (and thus eligible for curation) ---
-def _read_bundled_manifest_names() -> Set[str]:
-    """Names from ``.bundled_manifest`` ("name:hash" per line); empty if missing/unreadable."""
+def _read_bundled_names() -> Set[str]:
+    """Built-in names: ``.bundled_manifest`` ("name:hash" per line) plus the curator suppression list, which
+    only ever records built-ins; a pruned built-in whose manifest entry an older sync cleaned after the
+    catalog dropped it is still not agent-authored (#95415). Empty if both are missing/unreadable."""
     lines = _read_lines(_skills_dir() / ".bundled_manifest", "Failed to read bundled manifest: %s")
-    return {n for n in (line.split(":", 1)[0].strip() for line in lines) if n}
+    return {n for n in (line.split(":", 1)[0].strip() for line in lines) if n} | read_suppressed_names()
 
 
 def _read_hub_installed_names() -> Set[str]:
@@ -221,7 +223,7 @@ def _scan_local_skills(keep: Callable[[str, Path, Set[str], Dict[str, Any]], boo
     """Sorted local skill names passing *keep(name, skill_md, bundled, usage)*; hub/protected names never reach it."""
     if not (base := _skills_dir()).exists():
         return []
-    hub, bundled, usage = _read_hub_installed_names(), _read_bundled_manifest_names(), load_usage()
+    hub, bundled, usage = _read_hub_installed_names(), _read_bundled_names(), load_usage()
     return sorted({name for name, skill_md in _iter_skill_mds(base, local_only=True)
                    if name not in hub and not is_protected_builtin(name) and keep(name, skill_md, bundled, usage)})
 
@@ -265,7 +267,7 @@ def is_hub_installed(skill_name: str) -> bool:
 
 
 def is_bundled(skill_name: str) -> bool:
-    return skill_name in _read_bundled_manifest_names()
+    return skill_name in _read_bundled_names()
 
 
 def _external_read_only_message(skill_name: str) -> str:

tools/skills_sync.py

@@ -3,7 +3,8 @@
 ~/.hermes/skills/, tracking each synced skill's origin hash in .bundled_manifest (v2 "name:hash"
 lines; v1 plain names auto-migrate). NEW skills are copied and recorded; EXISTING skills update
 only when bundled changed AND the user copy still matches the origin hash (else user-customized
--> SKIP); user-DELETED skills are not re-added; upstream-REMOVED ones leave the manifest."""
+-> SKIP); user-DELETED skills are not re-added; upstream-REMOVED ones leave the manifest once no active or
+archived copy remains (the entry is that copy's only built-in provenance record)."""
 
 import hashlib
 import logging
@@ -430,9 +431,17 @@ def sync_skills(quiet: bool = False) -> dict:
             _update_existing_skill(st, skill_name, skill_src, dest, bundled_hash)
         else:
             st.skipped += 1  # in manifest but not on disk — user deleted it
-    # Clean manifest entries for skills removed upstream. Skipped when opted out: bundled_skills
+    # Clean manifest entries for skills removed upstream once no copy is left. A dropped built-in still
+    # on disk (active or archived) keeps its entry: it is the only provenance record, and without it the
+    # copy reads as agent-authored ("Learned", editable) (#95415). Skipped when opted out: bundled_skills
     # is only the essential set there, so cleaning would drop tracking for everything else.
-    cleaned = [] if essential_only else sorted(set(st.manifest) - {name for name, _ in bundled_skills})
+    removed = [] if essential_only else sorted(set(st.manifest) - {name for name, _ in bundled_skills})
+    present = set()
+    if removed:  # curator archive is flat: directory name == skill name
+        archive = _skills_dir() / ".archive"
+        present = {_read_skill_name(md, md.parent.name) for md in _iter_active_skill_mds()} | (
+            {p.name for p in archive.iterdir() if p.is_dir()} if archive.is_dir() else set())
+    cleaned = [name for name in removed if name not in present]
     for name in cleaned:
         del st.manifest[name]
     _seed_category_descriptions(

tui_gateway/server.py

@@ -3330,8 +3330,8 @@ def _skill_usage_lookup():
     "hub" / "bundled" / "local" (``/api/skills`` ``provenance``, "local" spelled "agent"). Failure → 0 / "local"."""
     try:
         from tools.skill_usage import (
-            _read_bundled_manifest_names, _read_hub_installed_names, activity_count, load_usage)
-        records, bundled, hub = load_usage(), _read_bundled_manifest_names(), _read_hub_installed_names()
+            _read_bundled_names, _read_hub_installed_names, activity_count, load_usage)
+        records, bundled, hub = load_usage(), _read_bundled_names(), _read_hub_installed_names()
     except Exception as e:
         logger.debug("skill usage lookup unavailable: %s", e)
         return (lambda _name: 0), (lambda _name: "local")