mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
fix(curator): protect load-bearing built-in skills from archival/consolidation (#41817)
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.
This commit is contained in:
parent
777dc9da62
commit
8e223b36ed
4 changed files with 87 additions and 1 deletions
|
|
@ -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 "
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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, (
|
||||
|
|
|
|||
|
|
@ -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/<name>/SKILL.md` directly with your editor. The pin guards tool-driven deletion, not your own filesystem access.
|
||||
|
||||
## Usage telemetry
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue