From 70e1571d890fc0552c398d6f443315b2f7a06ca4 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 1 Jun 2026 02:07:32 -0700 Subject: [PATCH] feat(curator): prune built-in skills after inactivity + track usage for all skills (#36701) Two related changes to the skill curator: 1. Built-in pruning. New curator.prune_builtins config (default on) lets the curator archive bundled built-in skills after the inactivity period, not just agent-created ones. A .curator_suppressed list tells the update-time re-seeder (tools/skills_sync) to leave pruned built-ins archived, so the prune is durable across `hermes update`. Built-ins are seeded with a baseline record on first sight, so the inactivity clock starts at upgrade time -- no mass-prune on the first run. Hub-installed skills are never pruned regardless of the flag. Restoring a built-in clears its suppression. 2. Usage tracking for all skills. Telemetry (view/use/patch) was wrongly gated behind curation-eligibility, so built-ins were tracked only when prunable and hub skills never. Telemetry is observability and is now decoupled from curation: every skill accrues usage counts regardless of provenance, while lifecycle mutators (set_state/set_pinned/mark_agent_created) stay curation-gated. New usage_report() + provenance() expose all skills with an agent/bundled/hub tag. --- agent/curator.py | 53 +++++- agent/curator_backup.py | 2 + hermes_cli/config.py | 11 ++ tests/agent/test_curator.py | 125 ++++++++++++ tests/tools/test_skill_usage.py | 116 +++++++++--- tests/tools/test_skills_sync.py | 22 ++- tools/skill_usage.py | 326 ++++++++++++++++++++++++++++---- tools/skills_sync.py | 40 +++- 8 files changed, 622 insertions(+), 73 deletions(-) diff --git a/agent/curator.py b/agent/curator.py index e7e595281..aae8ec004 100644 --- a/agent/curator.py +++ b/agent/curator.py @@ -183,6 +183,18 @@ def get_archive_after_days() -> int: return DEFAULT_ARCHIVE_AFTER_DAYS +def get_prune_builtins() -> bool: + """Whether the curator may prune (archive) bundled built-in skills too. + + ON by default. When on, built-ins become curation candidates and are + archived after the same inactivity period as agent-created skills, with a + suppression list keeping them archived across `hermes update` re-seeds. + Hub-installed skills are never pruned regardless of this flag. + """ + cfg = _load_config() + return bool(cfg.get("prune_builtins", True)) + + # --------------------------------------------------------------------------- # Idle / interval check # --------------------------------------------------------------------------- @@ -254,9 +266,17 @@ def should_run_now(now: Optional[datetime] = None) -> bool: # --------------------------------------------------------------------------- def apply_automatic_transitions(now: Optional[datetime] = None) -> Dict[str, int]: - """Walk every agent-created skill and move active/stale/archived based on + """Walk every curator-managed skill and move active/stale/archived based on the latest real activity timestamp. Pinned skills are never touched. - Returns a counter dict describing what changed.""" + + Built-ins (eligible only when ``curator.prune_builtins`` is on) are seeded + with a baseline record the first time they're seen so their inactivity + clock starts NOW rather than at epoch — a long-unused built-in is therefore + archived only after a fresh ``archive_after_days`` of non-use, not on the + first pass after the flag flips on. + + Returns a counter dict describing what changed. + """ from tools import skill_usage as _u if now is None: @@ -264,7 +284,7 @@ def apply_automatic_transitions(now: Optional[datetime] = None) -> Dict[str, int stale_cutoff = now - timedelta(days=get_stale_after_days()) archive_cutoff = now - timedelta(days=get_archive_after_days()) - counts = {"marked_stale": 0, "archived": 0, "reactivated": 0, "checked": 0} + counts = {"marked_stale": 0, "archived": 0, "reactivated": 0, "checked": 0, "seeded": 0} for row in _u.agent_created_report(): counts["checked"] += 1 @@ -272,6 +292,13 @@ def apply_automatic_transitions(now: Optional[datetime] = None) -> Dict[str, int if row.get("pinned"): continue + # First sight of a curation-eligible skill with no persisted record + # (e.g. a newly-eligible built-in): anchor its clock to now and defer. + if not row.get("_persisted", True): + _u.seed_record_if_missing(name) + counts["seeded"] += 1 + continue + last_activity = _parse_iso(row.get("last_activity_at")) # If never active, treat created_at as the anchor so new skills don't # immediately archive themselves. @@ -1484,14 +1511,30 @@ def run_curator_review( "error": None, } else: + # When pruning built-ins is enabled, the candidate list now + # includes bundled skills. Override the default "don't touch + # bundled" rule for them — but only archiving is permitted, and + # hub-installed skills remain strictly off-limits. + builtins_note = "" + if get_prune_builtins(): + builtins_note = ( + "\n\nPRUNE-BUILTINS MODE IS ON: bundled built-in skills " + "ARE included in the candidate list below and MAY be " + "archived for staleness/irrelevance, overriding hard " + "rule #1 for bundled skills ONLY. Hub-installed skills " + "remain strictly off-limits. Treat a stale built-in the " + "same as a stale agent-created skill: archive it (never " + "delete). It will be restored on `hermes update` only if " + "the user explicitly restores it." + ) if dry_run: prompt = ( f"{CURATOR_DRY_RUN_BANNER}\n\n" - f"{CURATOR_REVIEW_PROMPT}\n\n" + f"{CURATOR_REVIEW_PROMPT}{builtins_note}\n\n" f"{candidate_list}" ) else: - prompt = f"{CURATOR_REVIEW_PROMPT}\n\n{candidate_list}" + prompt = f"{CURATOR_REVIEW_PROMPT}{builtins_note}\n\n{candidate_list}" llm_meta = _run_llm_review(prompt) final_summary = ( f"{prefix}{auto_summary}; llm: {llm_meta.get('summary', 'no change')}" diff --git a/agent/curator_backup.py b/agent/curator_backup.py index 1961b99de..7725f1c71 100644 --- a/agent/curator_backup.py +++ b/agent/curator_backup.py @@ -21,6 +21,8 @@ It DOES include: pointer — otherwise the curator would immediately re-fire on the next tick) - ``.bundled_manifest`` (so protection markers stay consistent) + - ``.curator_suppressed`` (so rollback restores the set of pruned built-ins + the re-seeder must leave archived) Alongside the skills tarball, each snapshot also captures a copy of ``~/.hermes/cron/jobs.json`` as ``cron-jobs.json`` when it exists. Cron diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 46d933722..223d4239f 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -1648,6 +1648,17 @@ DEFAULT_CONFIG = { # Archive a skill (move to skills/.archive/) after this many days # without use. Archived skills are recoverable — no auto-deletion. "archive_after_days": 90, + # Also prune (archive) bundled built-in skills after the inactivity + # period, not just agent-created ones. ON by default. Built-ins are + # normally restored on every `hermes update`, so pruning them only + # sticks because a suppression list tells the re-seeder to leave them + # archived. Hub-installed skills are NEVER pruned here — they have an + # external upstream owner. Built-ins accrue usage telemetry and their + # inactivity clock starts the first time the curator sees them, so a + # long-unused built-in is archived only after archive_after_days of + # genuine non-use (never a mass-prune on the first run). Set to false + # to keep all bundled built-ins permanently. + "prune_builtins": True, # Pre-run backup: before every real curator pass (dry-run is # skipped), snapshot ~/.hermes/skills/ into # ~/.hermes/skills/.curator_backups//skills.tar.gz so the diff --git a/tests/agent/test_curator.py b/tests/agent/test_curator.py index 764f71489..cf9a00288 100644 --- a/tests/agent/test_curator.py +++ b/tests/agent/test_curator.py @@ -31,6 +31,11 @@ def curator_env(tmp_path, monkeypatch): # Default: no config file → curator defaults. Tests can override. monkeypatch.setattr(curator, "_load_config", lambda: {}) + # Pin prune_builtins OFF by default so transition tests don't pick up + # built-ins unless they explicitly enable it. Both config-reading paths + # are pinned (curator reads via _load_config; skill_usage reads config + # directly). Tests opt in with _enable_prune_builtins(...). + monkeypatch.setattr(usage, "_prune_builtins_enabled", lambda: False) return {"home": home, "curator": curator, "usage": usage} @@ -285,6 +290,126 @@ def test_bundled_skill_not_touched_by_transitions(curator_env): assert (skills_dir / "bundled").exists() # never moved +# --------------------------------------------------------------------------- +# prune_builtins: curator may archive bundled built-ins after inactivity +# --------------------------------------------------------------------------- + +def _enable_prune_builtins(curator_env, monkeypatch): + """Flip curator.prune_builtins on for both config-reading paths.""" + c = curator_env["curator"] + u = curator_env["usage"] + monkeypatch.setattr(c, "_load_config", lambda: {"prune_builtins": True}) + monkeypatch.setattr(u, "_prune_builtins_enabled", lambda: True) + + +def _disable_prune_builtins(curator_env, monkeypatch): + """Flip curator.prune_builtins off for both config-reading paths.""" + c = curator_env["curator"] + u = curator_env["usage"] + monkeypatch.setattr(c, "_load_config", lambda: {"prune_builtins": False}) + monkeypatch.setattr(u, "_prune_builtins_enabled", lambda: False) + + +def test_prune_builtins_default_on(curator_env): + # Shipped default is ON: with no explicit config, built-ins are eligible. + c = curator_env["curator"] + # _load_config returns {} (fixture) → default True surfaces. + assert c.get_prune_builtins() is True + + +def test_prune_builtins_off_excludes_bundled(curator_env, monkeypatch): + c = curator_env["curator"] + skills_dir = curator_env["home"] / "skills" + _write_skill(skills_dir, "bundled") + (skills_dir / ".bundled_manifest").write_text("bundled:abc\n", encoding="utf-8") + + # Explicitly off → bundled is not a candidate (the opt-out path). + _disable_prune_builtins(curator_env, monkeypatch) + assert c.get_prune_builtins() is False + counts = c.apply_automatic_transitions() + assert counts["checked"] == 0 + assert (skills_dir / "bundled").exists() + + +def test_prune_builtins_seeds_clock_on_first_sight(curator_env, monkeypatch): + c = curator_env["curator"] + u = curator_env["usage"] + skills_dir = curator_env["home"] / "skills" + _write_skill(skills_dir, "bundled") + (skills_dir / ".bundled_manifest").write_text("bundled:abc\n", encoding="utf-8") + _enable_prune_builtins(curator_env, monkeypatch) + + # First pass: built-in has no record yet → it's seeded, NOT archived, + # even though it's "old" on disk. The inactivity clock starts now. + counts = c.apply_automatic_transitions() + assert counts["checked"] == 1 + assert counts["seeded"] == 1 + assert counts["archived"] == 0 + assert (skills_dir / "bundled").exists() + # A record now exists with created_at ~ now. + assert isinstance(u.load_usage().get("bundled"), dict) + + +def test_prune_builtins_archives_stale_bundled_and_suppresses(curator_env, monkeypatch): + c = curator_env["curator"] + u = curator_env["usage"] + skills_dir = curator_env["home"] / "skills" + _write_skill(skills_dir, "bundled") + (skills_dir / ".bundled_manifest").write_text("bundled:abc\n", encoding="utf-8") + _enable_prune_builtins(curator_env, monkeypatch) + + # Seed a record whose last activity is far past the archive cutoff. + super_old = (datetime.now(timezone.utc) - timedelta(days=500)).isoformat() + data = u.load_usage() + data["bundled"] = u._empty_record() + data["bundled"]["last_used_at"] = super_old + u.save_usage(data) + + counts = c.apply_automatic_transitions() + assert counts["archived"] == 1 + # Directory moved into .archive/, suppression recorded so update won't restore. + assert not (skills_dir / "bundled").exists() + assert (skills_dir / ".archive" / "bundled").exists() + assert "bundled" in u.read_suppressed_names() + + +def test_prune_builtins_restore_clears_suppression(curator_env, monkeypatch): + u = curator_env["usage"] + skills_dir = curator_env["home"] / "skills" + _write_skill(skills_dir, "bundled") + (skills_dir / ".bundled_manifest").write_text("bundled:abc\n", encoding="utf-8") + _enable_prune_builtins(curator_env, monkeypatch) + + ok, _ = u.archive_skill("bundled") + assert ok + assert "bundled" in u.read_suppressed_names() + + ok, _ = u.restore_skill("bundled") + assert ok + assert (skills_dir / "bundled").exists() + assert "bundled" not in u.read_suppressed_names() + + +def test_prune_builtins_never_touches_hub_skills(curator_env, monkeypatch): + u = curator_env["usage"] + skills_dir = curator_env["home"] / "skills" + _write_skill(skills_dir, "hubskill") + hub_dir = skills_dir / ".hub" + hub_dir.mkdir(parents=True, exist_ok=True) + (hub_dir / "lock.json").write_text( + '{"version": 1, "installed": {"hubskill": {"install_path": "hubskill"}}}', + encoding="utf-8", + ) + _enable_prune_builtins(curator_env, monkeypatch) + + # Even with prune_builtins on, hub-installed skills stay off-limits. + assert u.is_curation_eligible("hubskill") is False + ok, msg = u.archive_skill("hubskill") + assert ok is False + assert "hub-installed" in msg + assert (skills_dir / "hubskill").exists() + + # --------------------------------------------------------------------------- # run_curator_review orchestration # --------------------------------------------------------------------------- diff --git a/tests/tools/test_skill_usage.py b/tests/tools/test_skill_usage.py index ad306b9c5..066b7c019 100644 --- a/tests/tools/test_skill_usage.py +++ b/tests/tools/test_skill_usage.py @@ -18,7 +18,13 @@ def _bump_view_many(hermes_home: str, skill_name: str, iterations: int) -> None: @pytest.fixture def skills_home(tmp_path, monkeypatch): - """Isolated HERMES_HOME with a clean skills/ dir for each test.""" + """Isolated HERMES_HOME with a clean skills/ dir for each test. + + Pins ``curator.prune_builtins`` OFF so the bundled/hub-protection tests in + this module exercise the off-path semantics regardless of the shipped + default. Tests that want built-ins to be curation-eligible flip it back on + explicitly via ``monkeypatch.setattr(mod, "_prune_builtins_enabled", ...)``. + """ home = tmp_path / ".hermes" home.mkdir() (home / "skills").mkdir() @@ -28,6 +34,7 @@ def skills_home(tmp_path, monkeypatch): import importlib import tools.skill_usage as mod importlib.reload(mod) + monkeypatch.setattr(mod, "_prune_builtins_enabled", lambda: False) return home @@ -520,27 +527,35 @@ def test_agent_created_report_derives_activity_from_view_and_patch(skills_home, # --------------------------------------------------------------------------- -# Provenance guard — telemetry must not leak records for bundled/hub skills +# Telemetry vs curation — usage is tracked for ALL skills; curation is not # --------------------------------------------------------------------------- -def test_bump_view_no_op_for_bundled_skill(skills_home): - """Telemetry bumps on bundled skills are dropped — the sidecar must stay - focused on agent-created skills only.""" - from tools.skill_usage import bump_view, load_usage +def test_bump_view_tracks_bundled_skill(skills_home): + """Telemetry IS recorded for bundled skills (observability), but the record + must NOT make the skill a curation candidate by itself.""" + from tools.skill_usage import ( + bump_view, load_usage, list_agent_created_skill_names, + ) skills_dir = skills_home / "skills" + _write_skill(skills_dir, "ship-bundled") (skills_dir / ".bundled_manifest").write_text( "ship-bundled:abc\n", encoding="utf-8", ) bump_view("ship-bundled") - assert "ship-bundled" not in load_usage(), ( - "bundled skill leaked into .usage.json" + rec = load_usage().get("ship-bundled") + assert isinstance(rec, dict), "bundled skill telemetry should be recorded" + assert rec["view_count"] == 1 + # Pruning is off by default in this fixture → not a curation candidate. + assert "ship-bundled" not in list_agent_created_skill_names() + + +def test_bump_patch_tracks_hub_skill(skills_home): + from tools.skill_usage import ( + bump_patch, load_usage, list_agent_created_skill_names, ) - - -def test_bump_patch_no_op_for_hub_skill(skills_home): - from tools.skill_usage import bump_patch, load_usage skills_dir = skills_home / "skills" + _write_skill(skills_dir, "from-hub") hub = skills_dir / ".hub" hub.mkdir() (hub / "lock.json").write_text( @@ -548,12 +563,17 @@ def test_bump_patch_no_op_for_hub_skill(skills_home): ) bump_patch("from-hub") - assert "from-hub" not in load_usage() + rec = load_usage().get("from-hub") + assert isinstance(rec, dict), "hub skill telemetry should be recorded" + assert rec["patch_count"] == 1 + # Hub skills are NEVER curation candidates regardless of any flag. + assert "from-hub" not in list_agent_created_skill_names() -def test_bump_use_no_op_for_hub_skill(skills_home): +def test_bump_use_tracks_hub_skill(skills_home): from tools.skill_usage import bump_use, load_usage skills_dir = skills_home / "skills" + _write_skill(skills_dir, "from-hub") hub = skills_dir / ".hub" hub.mkdir() (hub / "lock.json").write_text( @@ -561,7 +581,9 @@ def test_bump_use_no_op_for_hub_skill(skills_home): ) bump_use("from-hub") - assert "from-hub" not in load_usage() + rec = load_usage().get("from-hub") + assert isinstance(rec, dict) + assert rec["use_count"] == 1 def test_set_state_no_op_for_bundled_skill(skills_home): @@ -593,12 +615,17 @@ def test_restore_refuses_to_shadow_bundled_skill(skills_home): assert "bundled" in msg.lower() or "shadow" in msg.lower() -def test_end_to_end_no_code_path_mutates_bundled_skill(skills_home): - """The combined guarantee: no curator code path can archive, mark stale, - set-state, or persist telemetry for a bundled or hub-installed skill.""" +def test_end_to_end_telemetry_tracked_but_lifecycle_refused(skills_home): + """The combined guarantee under decoupled telemetry/curation: + + - Usage telemetry (view/use/patch) IS recorded for bundled & hub skills. + - Lifecycle mutations (set_state, set_pinned, archive) are REFUSED for them + (with pruning off, the fixture default), so no state/pinned/archived flag + lands and the directories stay on disk. + """ from tools.skill_usage import ( bump_view, bump_use, bump_patch, set_state, set_pinned, - archive_skill, load_usage, STATE_STALE, STATE_ARCHIVED, + archive_skill, load_usage, STATE_ACTIVE, STATE_STALE, STATE_ARCHIVED, ) skills_dir = skills_home / "skills" _write_skill(skills_dir, "bundled-one") @@ -614,7 +641,6 @@ def test_end_to_end_no_code_path_mutates_bundled_skill(skills_home): json.dumps({"installed": {"hub-one": {}}}), encoding="utf-8", ) - # Hammer every mutator at the bundled/hub names for name in ("bundled-one", "hub-one"): bump_view(name) bump_use(name) @@ -625,15 +651,55 @@ def test_end_to_end_no_code_path_mutates_bundled_skill(skills_home): ok, _msg = archive_skill(name) assert not ok, f"archive_skill(\"{name}\") should refuse" - # Sidecar must be clean of all three data = load_usage() - assert "bundled-one" not in data - assert "hub-one" not in data + # Telemetry landed for both. + for name in ("bundled-one", "hub-one"): + assert name in data, f"{name} telemetry should be recorded" + assert data[name]["view_count"] == 1 + assert data[name]["use_count"] == 1 + assert data[name]["patch_count"] == 1 + # But lifecycle mutators were refused — state stays the default, never + # archived/stale/pinned, and created_by is never agent. + assert data[name]["state"] == STATE_ACTIVE + assert data[name]["archived_at"] is None + assert data[name]["pinned"] is False + assert data[name].get("created_by") != "agent" - # Directories must still be in place on disk + # Directories must still be in place on disk. assert (skills_dir / "bundled-one" / "SKILL.md").exists() assert (skills_dir / "hub-one" / "SKILL.md").exists() - # The agent-created skill can still be mutated normally + # The agent-created skill can still be mutated normally. bump_view("mine") assert load_usage()["mine"]["view_count"] == 1 + + +def test_usage_report_covers_all_provenance(skills_home): + """usage_report() surfaces every skill with provenance, unlike the + curator-scoped agent_created_report().""" + from tools.skill_usage import ( + bump_use, usage_report, mark_agent_created, + ) + skills_dir = skills_home / "skills" + _write_skill(skills_dir, "bundled-one") + _write_skill(skills_dir, "hub-one") + _write_skill(skills_dir, "mine") + (skills_dir / ".bundled_manifest").write_text("bundled-one:abc\n", encoding="utf-8") + hub = skills_dir / ".hub" + hub.mkdir() + (hub / "lock.json").write_text( + json.dumps({"installed": {"hub-one": {}}}), encoding="utf-8", + ) + mark_agent_created("mine") + for n in ("bundled-one", "hub-one", "mine"): + bump_use(n) + + rows = {r["name"]: r for r in usage_report()} + assert set(rows) == {"bundled-one", "hub-one", "mine"} + assert rows["bundled-one"]["provenance"] == "bundled" + assert rows["hub-one"]["provenance"] == "hub" + assert rows["mine"]["provenance"] == "agent" + # All carry real usage now. + for n in rows: + assert rows[n]["use_count"] == 1 + assert rows[n]["_persisted"] is True diff --git a/tests/tools/test_skills_sync.py b/tests/tools/test_skills_sync.py index c13ed1872..c39e2fcd3 100644 --- a/tests/tools/test_skills_sync.py +++ b/tests/tools/test_skills_sync.py @@ -201,6 +201,26 @@ class TestSyncSkills: stack.enter_context(patch("tools.skills_sync.MANIFEST_FILE", manifest_file)) return stack + def test_suppressed_builtin_not_reseeded(self, tmp_path): + """A curator-pruned built-in in the suppression list must NOT be + re-copied on sync — that's what makes the prune durable across updates. + """ + bundled = self._setup_bundled(tmp_path) + skills_dir = tmp_path / "user_skills" + manifest_file = skills_dir / ".bundled_manifest" + + with self._patches(bundled, skills_dir, manifest_file), \ + patch("tools.skills_sync._read_suppressed_names", return_value={"old-skill"}): + result = sync_skills(quiet=True) + + # old-skill is suppressed → skipped, not copied. + assert "old-skill" in result["suppressed"] + assert "old-skill" not in result["copied"] + assert not (skills_dir / "old-skill").exists() + # The non-suppressed bundled skill is still copied normally. + assert "new-skill" in result["copied"] + assert (skills_dir / "category" / "new-skill" / "SKILL.md").exists() + def test_fresh_install_copies_all(self, tmp_path): bundled = self._setup_bundled(tmp_path) skills_dir = tmp_path / "user_skills" @@ -598,7 +618,7 @@ class TestSyncSkills: result = sync_skills(quiet=True) assert result == { "copied": [], "updated": [], "skipped": 0, - "user_modified": [], "cleaned": [], "total_bundled": 0, + "user_modified": [], "cleaned": [], "suppressed": [], "total_bundled": 0, "optional_provenance_backfilled": [], } diff --git a/tools/skill_usage.py b/tools/skill_usage.py index 745b68ead..1e1cc5c7c 100644 --- a/tools/skill_usage.py +++ b/tools/skill_usage.py @@ -217,21 +217,111 @@ def _read_hub_installed_names() -> Set[str]: return set() -def list_agent_created_skill_names() -> List[str]: - """Enumerate skills explicitly authored by the agent. +def _prune_builtins_enabled() -> bool: + """Whether bundled built-in skills are eligible for curator pruning. - The curator operates exclusively on this set. Skills are only eligible - after ``skill_manage(action="create")`` marks them in ``.usage.json``; - manually authored skills must not be inferred from filesystem location. - Bundled / hub skills are maintained by their upstream sources and must - never be pruned here. + Reads ``curator.prune_builtins`` from config (default True). Lazy import + keeps this module importable without the CLI config layer (e.g. in the + update/sync context); on any failure we fall back to the default. The real + safety against a mass-prune is the curator's seed-on-first-sight, not this + flag — built-ins only archive after a fresh inactivity window. + """ + try: + from hermes_cli.config import load_config + + cfg = load_config() + cur = cfg.get("curator") if isinstance(cfg, dict) else None + if isinstance(cur, dict): + return bool(cur.get("prune_builtins", True)) + except Exception as e: # pragma: no cover — best-effort config read + logger.debug("Failed to read curator.prune_builtins: %s", e) + return True + + +def _suppressed_file() -> Path: + return _skills_dir() / ".curator_suppressed" + + +def read_suppressed_names() -> Set[str]: + """Built-in skills the curator pruned — the re-seeder must leave archived. + + One skill name per line in ``~/.hermes/skills/.curator_suppressed``. This is + what makes pruning a built-in durable: without it, ``hermes update`` would + re-copy the bundled skill on the next sync. + """ + path = _suppressed_file() + if not path.exists(): + return set() + names: Set[str] = set() + try: + for line in path.read_text(encoding="utf-8").splitlines(): + line = line.strip() + if line and not line.startswith("#"): + names.add(line) + except OSError as e: + logger.debug("Failed to read curator suppression list: %s", e) + return names + + +def _write_suppressed_names(names: Set[str]) -> None: + path = _suppressed_file() + try: + path.parent.mkdir(parents=True, exist_ok=True) + data = "\n".join(sorted(names)) + ("\n" if names else "") + fd, tmp = tempfile.mkstemp(dir=str(path.parent), prefix=".curator_suppressed_", suffix=".tmp") + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(data) + f.flush() + os.fsync(f.fileno()) + os.replace(tmp, path) + except BaseException: + try: + os.unlink(tmp) + except OSError: + pass + raise + except Exception as e: + logger.debug("Failed to write curator suppression list: %s", e, exc_info=True) + + +def add_suppressed_name(skill_name: str) -> None: + """Record that a built-in skill was pruned, so sync won't restore it.""" + if not skill_name: + return + names = read_suppressed_names() + if skill_name not in names: + names.add(skill_name) + _write_suppressed_names(names) + + +def remove_suppressed_name(skill_name: str) -> None: + """Clear a built-in's suppression entry (e.g. on restore).""" + if not skill_name: + return + names = read_suppressed_names() + if skill_name in names: + names.discard(skill_name) + _write_suppressed_names(names) + + +def list_agent_created_skill_names() -> List[str]: + """Enumerate skills the curator may manage. + + Always includes agent-authored skills (those marked in ``.usage.json`` via + ``skill_manage(action="create")``). When ``curator.prune_builtins`` is + enabled, bundled built-in skills are ALSO included even though they have no + agent-created usage record — their inactivity clock is anchored on first + sight (see ``apply_automatic_transitions``). Hub-installed skills are never + included; manually authored skills are not inferred from filesystem + location. """ base = _skills_dir() if not base.exists(): return [] - bundled = _read_bundled_manifest_names() hub = _read_hub_installed_names() - off_limits = bundled | hub + bundled = _read_bundled_manifest_names() + prune_builtins = _prune_builtins_enabled() usage = load_usage() names: List[str] = [] @@ -241,12 +331,21 @@ def list_agent_created_skill_names() -> List[str]: if is_excluded_skill_path(skill_md): continue try: - rel = skill_md.relative_to(base) + skill_md.relative_to(base) except ValueError: continue name = _read_skill_name(skill_md, fallback=skill_md.parent.name) - if name in off_limits: + # Hub-installed skills are always off-limits. + if name in hub: 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. + if not prune_builtins: + continue + names.append(name) + continue + # Agent-authored (or local-manual) skills must opt in via their record. if not _is_curator_managed_record(usage.get(name)): continue names.append(name) @@ -293,6 +392,30 @@ def is_agent_created(skill_name: str) -> bool: return skill_name not in off_limits +def is_hub_installed(skill_name: str) -> bool: + """Whether *skill_name* was installed via the Skills Hub.""" + return skill_name in _read_hub_installed_names() + + +def is_bundled(skill_name: str) -> bool: + """Whether *skill_name* was seeded from the bundled repo skills.""" + return skill_name in _read_bundled_manifest_names() + + +def is_curation_eligible(skill_name: str) -> bool: + """Whether the curator may track/archive *skill_name*. + + 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. + """ + if is_hub_installed(skill_name): + return False + if is_bundled(skill_name): + return _prune_builtins_enabled() + return True + + def _is_curator_managed_record(record: Any) -> bool: """Return True when a usage record opts a skill into curator management.""" if not isinstance(record, dict): @@ -377,17 +500,43 @@ def get_record(skill_name: str) -> Dict[str, Any]: return rec -def _mutate(skill_name: str, mutator) -> None: +def seed_record_if_missing(skill_name: str) -> None: + """Persist a baseline usage record for a curation-eligible skill. + + Built-ins carry no usage record until something touches them, which leaves + their inactivity clock with no anchor. Seeding a record here fixes + ``created_at`` to the moment the curator first sees the skill, so the + archive/stale clock measures non-use FROM THEN — not from epoch. No-op when + a record already exists or the skill isn't curation-eligible. + """ + if not skill_name or not is_curation_eligible(skill_name): + return + try: + with _usage_file_lock(): + data = load_usage() + if isinstance(data.get(skill_name), dict): + return + data[skill_name] = _empty_record() + save_usage(data) + except Exception as e: + logger.debug("skill_usage.seed_record_if_missing(%s) failed: %s", skill_name, e, exc_info=True) + + +def _mutate(skill_name: str, mutator, *, require_curation_eligible: bool = False) -> None: """Load, apply *mutator(record)* in place, save. Best-effort. - Bundled and hub-installed skills are NEVER recorded in the sidecar. - Local manual skills may still accrue usage telemetry, but they only - become curator-managed when ``created_by`` is explicitly marked. + By default this records telemetry for ANY skill — bundled, hub-installed, + or agent-created — because usage tracking is pure observability and is + orthogonal to whether a skill is ever curated. Lifecycle mutators + (``set_state``, ``set_pinned``, ``mark_agent_created``) pass + ``require_curation_eligible=True`` so they never write meaningless state + onto a skill the curator can't manage (e.g. an ``archived`` flag on a + hub-installed skill). """ if not skill_name: return try: - if not is_agent_created(skill_name): + if require_curation_eligible and not is_curation_eligible(skill_name): return with _usage_file_lock(): data = load_usage() @@ -402,11 +551,15 @@ def _mutate(skill_name: str, mutator) -> None: # --------------------------------------------------------------------------- -# Public counter-bump helpers +# Public counter-bump helpers — telemetry for ALL skills (observability only) # --------------------------------------------------------------------------- def bump_view(skill_name: str) -> None: - """Bump view_count and last_viewed_at. Called from skill_view().""" + """Bump view_count and last_viewed_at. Called from skill_view(). + + Tracks every skill regardless of provenance — built-ins and hub skills + included. Usage telemetry is observability, not a curation signal. + """ def _apply(rec: Dict[str, Any]) -> None: rec["view_count"] = int(rec.get("view_count") or 0) + 1 rec["last_viewed_at"] = _now_iso() @@ -415,7 +568,10 @@ def bump_view(skill_name: str) -> None: def bump_use(skill_name: str) -> None: """Bump use_count and last_used_at. Called when a skill is actively used - (e.g. loaded into the prompt path or referenced from an assistant turn).""" + (e.g. loaded into the prompt path or referenced from an assistant turn). + + Tracks every skill regardless of provenance. + """ def _apply(rec: Dict[str, Any]) -> None: rec["use_count"] = int(rec.get("use_count") or 0) + 1 rec["last_used_at"] = _now_iso() @@ -423,7 +579,10 @@ def bump_use(skill_name: str) -> None: def bump_patch(skill_name: str) -> None: - """Bump patch_count and last_patched_at. Called from skill_manage (patch/edit).""" + """Bump patch_count and last_patched_at. Called from skill_manage (patch/edit). + + Tracks every skill regardless of provenance. + """ def _apply(rec: Dict[str, Any]) -> None: rec["patch_count"] = int(rec.get("patch_count") or 0) + 1 rec["last_patched_at"] = _now_iso() @@ -438,11 +597,12 @@ def mark_agent_created(skill_name: str) -> None: """ def _apply(rec: Dict[str, Any]) -> None: rec["created_by"] = "agent" - _mutate(skill_name, _apply) + _mutate(skill_name, _apply, require_curation_eligible=True) def set_state(skill_name: str, state: str) -> None: - """Set lifecycle state. No-op if *state* is invalid.""" + """Set lifecycle state. No-op if *state* is invalid or the skill isn't + curator-manageable (hub skills, or built-ins with pruning disabled).""" if state not in _VALID_STATES: logger.debug("set_state: invalid state %r for %s", state, skill_name) return @@ -452,13 +612,13 @@ def set_state(skill_name: str, state: str) -> None: rec["archived_at"] = _now_iso() elif state == STATE_ACTIVE: rec["archived_at"] = None - _mutate(skill_name, _apply) + _mutate(skill_name, _apply, require_curation_eligible=True) def set_pinned(skill_name: str, pinned: bool) -> None: def _apply(rec: Dict[str, Any]) -> None: rec["pinned"] = bool(pinned) - _mutate(skill_name, _apply) + _mutate(skill_name, _apply, require_curation_eligible=True) def forget(skill_name: str) -> None: @@ -480,13 +640,20 @@ def forget(skill_name: str) -> None: # --------------------------------------------------------------------------- def archive_skill(skill_name: str) -> Tuple[bool, str]: - """Move an agent-created skill directory to ~/.hermes/skills/.archive/. + """Move a curator-eligible skill directory to ~/.hermes/skills/.archive/. - Returns (ok, message). Never archives bundled or hub skills — callers are - responsible for checking provenance, but we double-check here as a safety net. + Returns (ok, message). Never archives hub-installed skills. Bundled + built-ins are only archivable when ``curator.prune_builtins`` is enabled; + when one is archived, its name is added to the suppression list so the + update-time re-seeder leaves it archived instead of restoring it. """ - if not is_agent_created(skill_name): - return False, f"skill '{skill_name}' is bundled or hub-installed; never archive" + if not is_curation_eligible(skill_name): + if is_hub_installed(skill_name): + return False, f"skill '{skill_name}' is hub-installed; never archive" + return False, ( + f"skill '{skill_name}' is a bundled built-in; enable " + "curator.prune_builtins to allow pruning it" + ) skill_dir = _find_skill_dir(skill_name) if skill_dir is None: @@ -514,6 +681,10 @@ def archive_skill(skill_name: str) -> Tuple[bool, str]: except Exception as e2: return False, f"failed to archive: {e2}" + # Pruning a built-in only sticks if the re-seeder is told to leave it alone. + if is_bundled(skill_name): + add_suppressed_name(skill_name) + set_state(skill_name, STATE_ARCHIVED) return True, f"archived to {dest}" @@ -522,14 +693,24 @@ def restore_skill(skill_name: str) -> Tuple[bool, str]: """Move an archived skill back to ~/.hermes/skills/. Restores to the flat top-level layout; original category nesting is NOT reconstructed. - Refuses to restore under a name that now collides with a bundled or - hub-installed skill — that would shadow the upstream version. + Refuses to restore under a name that now collides with a hub-installed + skill — that would shadow the upstream version. Also refuses to restore + over a bundled built-in UNLESS ``curator.prune_builtins`` is enabled (in + which case built-ins are curator-managed and restoring is the documented + way to lift a prune). Restoring clears any suppression entry so future + updates may re-seed the built-in again. """ - # If a bundled or hub skill has since been installed under the same - # name, refuse to restore rather than shadow it. - if not is_agent_created(skill_name): + # Hub skills always have an external upstream owner — never shadow them. + if is_hub_installed(skill_name): return False, ( - f"skill '{skill_name}' is now bundled or hub-installed; " + f"skill '{skill_name}' is now hub-installed; " + "restore would shadow the upstream version" + ) + # A bundled built-in is upstream-owned UNLESS prune_builtins is on. With the + # flag off, restoring over it would shadow the bundled version. + if is_bundled(skill_name) and not _prune_builtins_enabled(): + return False, ( + f"skill '{skill_name}' is now bundled; " "restore would shadow the upstream version" ) archive_root = _archive_dir() @@ -563,6 +744,9 @@ def restore_skill(skill_name: str) -> Tuple[bool, str]: except Exception as e: return False, f"failed to restore: {e}" + # Restoring a pruned built-in lifts its suppression so updates can manage it. + remove_suppressed_name(skill_name) + set_state(skill_name, STATE_ACTIVE) return True, f"restored to {dest}" @@ -590,19 +774,79 @@ def _find_skill_dir(skill_name: str) -> Optional[Path]: def agent_created_report() -> List[Dict[str, Any]]: """Return a list of {name, state, pinned, last_activity_at, ...} - records for every agent-created skill. Missing usage records are backfilled - with defaults so callers can always index fields.""" + records for every curator-managed skill. Missing usage records are + backfilled with defaults so callers can always index fields. + + Each row carries ``_persisted``: True when a real record exists in + ``.usage.json``, False when the row is a fresh backfill (e.g. a built-in + seen for the first time). The curator uses this to seed the inactivity + clock instead of treating an unrecorded skill as ancient. + """ data = load_usage() rows: List[Dict[str, Any]] = [] for name in list_agent_created_skill_names(): - rec = data.get(name) - if not isinstance(rec, dict): - rec = _empty_record() + raw = data.get(name) + persisted = isinstance(raw, dict) + rec: Dict[str, Any] = raw if isinstance(raw, dict) else _empty_record() base = _empty_record() for k, v in base.items(): rec.setdefault(k, v) - row = {"name": name, **rec} + row = {"name": name, **rec, "_persisted": persisted} row["last_activity_at"] = latest_activity_at(row) row["activity_count"] = activity_count(row) rows.append(row) return rows + + +def provenance(skill_name: str) -> str: + """Classify a skill's origin: 'hub', 'bundled', or 'agent'. + + 'agent' covers both agent-authored and local manually-authored skills — + anything not seeded from the bundled repo or installed via the hub. + """ + if is_hub_installed(skill_name): + return "hub" + if is_bundled(skill_name): + return "bundled" + return "agent" + + +def usage_report() -> List[Dict[str, Any]]: + """Return usage telemetry for EVERY skill on disk, with provenance. + + Unlike ``agent_created_report()`` (which is scoped to curator-managed + candidates), this surfaces all skills — bundled built-ins and + hub-installed included — so callers can answer "how often is this skill + used" independent of whether it's ever curated. Rows carry a + ``provenance`` field ('agent' | 'bundled' | 'hub') and ``_persisted`` + (whether a real ``.usage.json`` record backs the row). + """ + base = _skills_dir() + if not base.exists(): + return [] + data = load_usage() + rows: List[Dict[str, Any]] = [] + seen: set = set() + for skill_md in base.rglob("SKILL.md"): + if is_excluded_skill_path(skill_md): + continue + name = _read_skill_name(skill_md, fallback=skill_md.parent.name) + if name in seen: + continue + seen.add(name) + raw = data.get(name) + persisted = isinstance(raw, dict) + rec: Dict[str, Any] = raw if isinstance(raw, dict) else _empty_record() + base_rec = _empty_record() + for k, v in base_rec.items(): + rec.setdefault(k, v) + row = { + "name": name, + **rec, + "provenance": provenance(name), + "_persisted": persisted, + } + row["last_activity_at"] = latest_activity_at(row) + row["activity_count"] = activity_count(row) + rows.append(row) + return sorted(rows, key=lambda r: r["name"]) diff --git a/tools/skills_sync.py b/tools/skills_sync.py index 11d031cde..a7fc1e4a5 100644 --- a/tools/skills_sync.py +++ b/tools/skills_sync.py @@ -83,6 +83,32 @@ def _read_manifest() -> Dict[str, str]: return {} +def _read_suppressed_names() -> set: + """Built-in skills the curator pruned — must NOT be re-seeded on sync. + + Delegates to ``tools.skill_usage`` (single source of truth) and falls back + to reading ``~/.hermes/skills/.curator_suppressed`` directly if that import + is unavailable in a packaged/update context. + """ + try: + from tools.skill_usage import read_suppressed_names + + return read_suppressed_names() + except Exception: + path = SKILLS_DIR / ".curator_suppressed" + if not path.exists(): + return set() + names = set() + try: + for line in path.read_text(encoding="utf-8").splitlines(): + line = line.strip() + if line and not line.startswith("#"): + names.add(line) + except OSError: + pass + return names + + def _write_manifest(entries: Dict[str, str]): """Write the manifest file atomically in v2 format (name:hash). @@ -428,7 +454,7 @@ def sync_skills(quiet: bool = False) -> dict: if not bundled_dir.exists(): return { "copied": [], "updated": [], "skipped": 0, - "user_modified": [], "cleaned": [], "total_bundled": 0, + "user_modified": [], "cleaned": [], "suppressed": [], "total_bundled": 0, "optional_provenance_backfilled": [], } @@ -436,13 +462,24 @@ def sync_skills(quiet: bool = False) -> dict: manifest = _read_manifest() bundled_skills = _discover_bundled_skills(bundled_dir) bundled_names = {name for name, _ in bundled_skills} + suppressed = _read_suppressed_names() copied = [] updated = [] user_modified = [] + suppressed_skipped: List[str] = [] skipped = 0 for skill_name, skill_src in bundled_skills: + # Curator-pruned built-ins: do not re-seed. The suppression list + # (~/.hermes/skills/.curator_suppressed) is written when the curator + # archives a bundled skill with curator.prune_builtins enabled. Without + # this skip, every `hermes update` would resurrect a skill the user + # deliberately pruned. Restoring the skill clears its suppression entry. + if skill_name in suppressed: + suppressed_skipped.append(skill_name) + continue + dest = _compute_relative_dest(skill_src, bundled_dir) bundled_hash = _dir_hash(skill_src) @@ -561,6 +598,7 @@ def sync_skills(quiet: bool = False) -> dict: "skipped": skipped, "user_modified": user_modified, "cleaned": cleaned, + "suppressed": suppressed_skipped, "total_bundled": len(bundled_skills), "optional_provenance_backfilled": optional_provenance_backfilled, }