From 8e223b36ed01fb3b5c9f99cfa1c7273a57cdbc47 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 7 Jun 2026 22:23:29 -0700 Subject: [PATCH] fix(curator): protect load-bearing built-in skills from archival/consolidation (#41817) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The curator's idle-archival path (apply_automatic_transitions under prune_builtins) could archive the bundled `plan` skill, killing the /plan slash command silently — typing /plan then returned 'Unknown command' with no signal that a skill had vanished. The archived skill's hash stays in .bundled_manifest, so 'hermes update' wouldn't re-seed it. Add PROTECTED_BUILTIN_SKILLS ({plan}) enforced at the master gate is_curation_eligible() (covers archive_skill + the transition walk) and in the candidate enumerator (so the LLM consolidation pass never sees them). Immune to prune_builtins, pin state, and LLM judgment. --- agent/curator.py | 5 +++ tests/agent/test_curator.py | 44 +++++++++++++++++++++ tools/skill_usage.py | 37 ++++++++++++++++- website/docs/user-guide/features/curator.md | 2 + 4 files changed, 87 insertions(+), 1 deletion(-) diff --git a/agent/curator.py b/agent/curator.py index aae8ec0044a..93986da7a75 100644 --- a/agent/curator.py +++ b/agent/curator.py @@ -375,6 +375,11 @@ CURATOR_REVIEW_PROMPT = ( "into ~/.hermes/skills/.archive/) is the maximum destructive action. " "Archives are recoverable; deletion is not.\n" "3. DO NOT touch skills shown as pinned=yes. Skip them entirely.\n" + "3b. DO NOT archive, delete, consolidate, move, or otherwise modify any " + "skill named in the protected built-ins list (currently: plan). These " + "back load-bearing UX (slash-command entry points referenced in docs and " + "tips) and are filtered out of the candidate list below — never resurrect " + "one as an archive or absorb target.\n" "4. DO NOT use usage counters as a reason to skip consolidation. The " "counters are new and often mostly zero. Judge overlap on CONTENT, " "not on use_count. 'use=0' is not evidence a skill is valuable; it's " diff --git a/tests/agent/test_curator.py b/tests/agent/test_curator.py index cf9a002880a..401b941f98d 100644 --- a/tests/agent/test_curator.py +++ b/tests/agent/test_curator.py @@ -390,6 +390,50 @@ def test_prune_builtins_restore_clears_suppression(curator_env, monkeypatch): assert "bundled" not in u.read_suppressed_names() +def test_protected_builtin_never_archived_even_when_stale(curator_env, monkeypatch): + """A protected built-in (e.g. `plan`) is never archived, even when it is a + stale bundled skill under prune_builtins — it backs a load-bearing slash + command and must survive every curator pass.""" + u = curator_env["usage"] + c = curator_env["curator"] + skills_dir = curator_env["home"] / "skills" + name = next(iter(u.PROTECTED_BUILTIN_SKILLS)) # the real protected name(s) + _write_skill(skills_dir, name) + (skills_dir / ".bundled_manifest").write_text(f"{name}:abc\n", encoding="utf-8") + _enable_prune_builtins(curator_env, monkeypatch) + + # Force a record that is far past the archive cutoff. + super_old = (datetime.now(timezone.utc) - timedelta(days=500)).isoformat() + data = u.load_usage() + data[name] = u._empty_record() + data[name]["last_used_at"] = super_old + u.save_usage(data) + + counts = c.apply_automatic_transitions() + assert counts["archived"] == 0 + # Not even enumerated as a candidate → not "checked". + assert name not in u.list_agent_created_skill_names() + assert (skills_dir / name).exists() + assert name not in u.read_suppressed_names() + + +def test_protected_builtin_is_not_curation_eligible(curator_env, monkeypatch): + """is_curation_eligible() returns False for protected built-ins regardless + of prune_builtins, and archive_skill() refuses them directly.""" + u = curator_env["usage"] + skills_dir = curator_env["home"] / "skills" + name = next(iter(u.PROTECTED_BUILTIN_SKILLS)) + _write_skill(skills_dir, name) + (skills_dir / ".bundled_manifest").write_text(f"{name}:abc\n", encoding="utf-8") + _enable_prune_builtins(curator_env, monkeypatch) + + assert u.is_protected_builtin(name) is True + assert u.is_curation_eligible(name) is False + ok, msg = u.archive_skill(name) + assert ok is False + assert (skills_dir / name).exists() + + def test_prune_builtins_never_touches_hub_skills(curator_env, monkeypatch): u = curator_env["usage"] skills_dir = curator_env["home"] / "skills" diff --git a/tools/skill_usage.py b/tools/skill_usage.py index 1e1cc5c7c92..b0bd32f3985 100644 --- a/tools/skill_usage.py +++ b/tools/skill_usage.py @@ -55,6 +55,28 @@ STATE_STALE = "stale" STATE_ARCHIVED = "archived" _VALID_STATES = {STATE_ACTIVE, STATE_STALE, STATE_ARCHIVED} +# Load-bearing bundled built-ins the curator must NEVER archive or consolidate, +# regardless of ``curator.prune_builtins``, pin state, or LLM judgment. These +# back advertised UX paths (e.g. ``plan`` powers the ``/plan`` slash-command +# flow and is referenced in tips/docs/fresh-profile seeding); silently archiving +# one turns its slash command into "Unknown command" with no signal to the user. +# Protection is by skill ``name`` (frontmatter ``name:``), matching the keys used +# throughout this module. Keep this list tiny and intentional — it is not a +# substitute for ``curator.prune_builtins: false``, which exempts ALL built-ins. +PROTECTED_BUILTIN_SKILLS: Set[str] = { + "plan", +} + + +def is_protected_builtin(skill_name: str) -> bool: + """Whether *skill_name* is a load-bearing built-in the curator never touches. + + Protected built-ins are exempt from archival and consolidation on every + path: the automatic state-transition walk, the LLM consolidation pass (they + are dropped from the candidate list), and direct ``archive_skill`` calls. + """ + return skill_name in PROTECTED_BUILTIN_SKILLS + def _skills_dir() -> Path: return get_hermes_home() / "skills" @@ -338,6 +360,10 @@ def list_agent_created_skill_names() -> List[str]: # Hub-installed skills are always off-limits. if name in hub: continue + # Protected built-ins are never curation candidates — exempt from the + # automatic transition walk AND the LLM consolidation pass. + if is_protected_builtin(name): + continue if name in bundled: # Built-ins are only candidates when pruning is enabled. They never # carry a curator-managed record, so the record gate is skipped. @@ -407,8 +433,12 @@ def is_curation_eligible(skill_name: str) -> bool: Agent-created skills are always eligible. Bundled built-ins become eligible only when ``curator.prune_builtins`` is enabled. Hub-installed skills are - NEVER eligible — they have an external upstream owner. + NEVER eligible — they have an external upstream owner. Protected built-ins + (``PROTECTED_BUILTIN_SKILLS``) are NEVER eligible regardless of any flag — + they back load-bearing UX and must never be archived or consolidated. """ + if is_protected_builtin(skill_name): + return False if is_hub_installed(skill_name): return False if is_bundled(skill_name): @@ -648,6 +678,11 @@ def archive_skill(skill_name: str) -> Tuple[bool, str]: update-time re-seeder leaves it archived instead of restoring it. """ if not is_curation_eligible(skill_name): + if is_protected_builtin(skill_name): + return False, ( + f"skill '{skill_name}' is a protected built-in; it backs " + "load-bearing UX and is never archived or consolidated" + ) if is_hub_installed(skill_name): return False, f"skill '{skill_name}' is hub-installed; never archive" return False, ( diff --git a/website/docs/user-guide/features/curator.md b/website/docs/user-guide/features/curator.md index 6e65f4e226b..aac5bb86b60 100644 --- a/website/docs/user-guide/features/curator.md +++ b/website/docs/user-guide/features/curator.md @@ -192,6 +192,8 @@ The flag is stored as `"pinned": true` on the skill's entry in `~/.hermes/skills Only **agent-created** skills can be pinned — `hermes curator pin` refuses on bundled and hub-installed skills with an explanatory message if you try. Hub-installed skills are never subject to curator mutation. Bundled built-in skills are only touched when `curator.prune_builtins: true` (the default), and even then only archived after `archive_after_days` of non-use — never patched, consolidated, or deleted. Set `curator.prune_builtins: false` to exempt bundled skills entirely. +A small set of **protected built-ins** is hardcoded as never-archivable and never-consolidatable, regardless of `curator.prune_builtins`, pin state, or LLM judgment. These back load-bearing UX — for example, `plan` powers the `/plan` slash-command flow — so silently archiving one would turn its slash command into an "Unknown command" error with no signal to you. Protected built-ins are filtered out of the curator's candidate list entirely, so the consolidation pass never sees them. + If you want a stronger guarantee than "no deletion" — for instance, freezing a skill's content entirely while the agent still reads it — edit `~/.hermes/skills//SKILL.md` directly with your editor. The pin guards tool-driven deletion, not your own filesystem access. ## Usage telemetry