From 72de75c0ab367e231c207075ee972f7d2fcc0744 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sat, 25 Jul 2026 17:01:22 -0700 Subject: [PATCH] feat(curator): surface unmanaged skills and add `curator adopt` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hermes curator status` reported only the skills it manages, staying silent about curation-eligible skills it can never touch. On a 237-skill library that meant 112 skills were invisible to every automatic transition with no signal anywhere — the curator looked broken when it was working as designed. A skill becomes curator-managed only when `created_by: agent` lands on its usage record, and only the background review fork writes that marker. Two populations therefore never qualify: records written before the marker existed (no key at all, authorship unknowable) and every foreground `skill_manage(create)` (unset by design — those skills belong to the user). - skill_usage: `list_unmanaged_skill_names()` / `unmanaged_report()` enumerate the blind spot, tagging each row with `has_provenance_key` so the two causes are distinguishable. `adopt_skill()` writes the marker on user declaration and refuses bundled, hub-installed, external, and protected built-ins. Adoption never resets the inactivity clock. - curator CLI: status prints an `unmanaged (no provenance marker)` block on BOTH the managed and no-managed-skills paths; new `adopt` verb takes names or `--all-unmanaged`, with `--dry-run` and a confirmation prompt on bulk. - skills_sync: `_backfill_optional_provenance()` matched candidates by repo-derived path only, so a skill installed at `mlops/chroma` that upstream later moved to `mlops/vector-databases/chroma` was skipped forever and `hermes skills repair-optional` could not fix it. Falls back to an unambiguous name match, still gated on identical content, and records the ACTUAL install path. Provenance stays a declaration, never an inference: a high patch count proves the agent MAINTAINS a skill, not that it authored one, since Hermes edits user-written skills on the user's behalf routinely. An "looks agent-made" heuristic would eventually archive hand-written work. Validation: 347 targeted tests pass. Each new test verified via sabotage run (revert the fix, confirm the test goes red) — one initially passed for the wrong reason because the fixture pinned `prune_builtins` off, masking the guard under test; fixed to force the shipped default on. --- hermes_cli/curator.py | 111 ++++++++++++ tests/hermes_cli/test_curator_status.py | 142 ++++++++++++++++ tests/tools/test_skill_usage.py | 178 ++++++++++++++++++++ tests/tools/test_skills_sync.py | 81 +++++++++ tools/skill_usage.py | 122 ++++++++++++++ tools/skills_sync.py | 47 +++++- website/docs/user-guide/features/curator.md | 53 ++++++ 7 files changed, 733 insertions(+), 1 deletion(-) diff --git a/hermes_cli/curator.py b/hermes_cli/curator.py index 4d5bc70f39ec..9fe75a62470b 100644 --- a/hermes_cli/curator.py +++ b/hermes_cli/curator.py @@ -36,6 +36,35 @@ def _fmt_ts(ts: Optional[str]) -> str: return f"{secs // 86400}d ago" +def _print_unmanaged_summary() -> None: + """Report curation-eligible skills that carry no provenance marker. + + A skill only becomes curator-managed once ``created_by: agent`` lands on + its usage record, which happens ONLY for background-review creations. + Skills predating that marker, plus every foreground + ``skill_manage(create)``, are eligible but unmanaged — no automatic + transition ever considers them. Printing just the managed count made a + large library look fully curated while a big slice was untouchable. + """ + from tools import skill_usage + + try: + unmanaged = skill_usage.unmanaged_report() + except Exception: + return + if not unmanaged: + return + legacy = sum(1 for r in unmanaged if not r.get("has_provenance_key")) + foreground = len(unmanaged) - legacy + print(f"\nunmanaged (no provenance marker): {len(unmanaged)} total") + print(f" pre-dates marker {legacy}") + print(f" foreground-created {foreground}") + print( + " never auto-staled or archived — " + "`hermes curator adopt ` hands one over" + ) + + def _cmd_status(args) -> int: from agent import curator from tools import skill_usage @@ -85,6 +114,7 @@ def _cmd_status(args) -> int: rows = skill_usage.curated_report() if not rows: print("\nno curator-managed skills") + _print_unmanaged_summary() return 0 by_state = {"active": [], "stale": [], "archived": []} @@ -111,6 +141,9 @@ def _cmd_status(args) -> int: if pinned: print(f"\npinned ({len(pinned)}): {', '.join(pinned)}") + # Surface the curation blind spot on the managed path too. + _print_unmanaged_summary() + # Show top 5 least-recently-active skills. Views and edits are activity too: # curator should not report a skill as "never used" right after skill_view() # or skill_manage() touched it. @@ -280,6 +313,62 @@ def _cmd_unpin(args) -> int: return 0 +def _cmd_adopt(args) -> int: + """Hand unmanaged skills to the curator by explicit user declaration. + + Provenance cannot be inferred from telemetry: a high patch count proves + the agent MAINTAINS a skill, not that it AUTHORED it (the agent edits + user-written skills on the user's behalf constantly). So adoption is never + automatic — the user names what they're handing over, or passes + ``--all-unmanaged`` to hand over every eligible skill at once. + """ + from tools import skill_usage + + names = list(getattr(args, "skill", None) or []) + adopt_all = bool(getattr(args, "all_unmanaged", False)) + if adopt_all: + if names: + print("curator: pass either skill names or --all-unmanaged, not both") + return 1 + names = skill_usage.list_unmanaged_skill_names() + if not names: + print("curator: no unmanaged skills to adopt") + return 0 + if not names: + print("curator: name a skill to adopt, or pass --all-unmanaged") + return 1 + + dry_run = bool(getattr(args, "dry_run", False)) + if dry_run: + print(f"curator: would adopt {len(names)} skill(s) (dry run):") + for n in names: + print(f" + {n}") + return 0 + + # Bulk adoption is a real lifecycle change (adopted skills become + # archivable), so confirm unless the caller opted out. + if adopt_all and not bool(getattr(args, "yes", False)): + print(f"curator: adopt {len(names)} unmanaged skill(s) into curator management?") + print(" they become eligible for automatic staleness + archival") + try: + reply = input(" proceed? [y/N] ").strip().lower() + except (EOFError, KeyboardInterrupt): + reply = "" + if reply not in {"y", "yes"}: + print("curator: aborted") + return 1 + + failed = 0 + for n in names: + ok, msg = skill_usage.adopt_skill(n) + print(f"curator: {msg}") + if not ok: + failed += 1 + if len(names) > 1: + print(f"curator: adopted {len(names) - failed}/{len(names)}") + return 1 if failed else 0 + + def _cmd_restore(args) -> int: from tools import skill_usage ok, msg = skill_usage.restore_skill(args.skill) @@ -627,6 +716,28 @@ def register_cli(parent: argparse.ArgumentParser) -> None: p_unpin.add_argument("skill", help="Skill name") p_unpin.set_defaults(func=_cmd_unpin) + p_adopt = subs.add_parser( + "adopt", + help="Hand unmanaged skills to the curator (provenance is a user declaration)", + ) + p_adopt.add_argument( + "skill", nargs="*", + help="Skill name(s) to adopt. Omit when using --all-unmanaged.", + ) + p_adopt.add_argument( + "--all-unmanaged", action="store_true", + help="Adopt every curation-eligible skill that has no provenance marker", + ) + p_adopt.add_argument( + "--dry-run", action="store_true", + help="List what would be adopted without writing anything", + ) + p_adopt.add_argument( + "--yes", action="store_true", + help="Skip the confirmation prompt for --all-unmanaged", + ) + p_adopt.set_defaults(func=_cmd_adopt) + p_restore = subs.add_parser("restore", help="Restore an archived skill") p_restore.add_argument("skill", help="Skill name") p_restore.set_defaults(func=_cmd_restore) diff --git a/tests/hermes_cli/test_curator_status.py b/tests/hermes_cli/test_curator_status.py index 46d3b9fb8d93..9d28c2da825e 100644 --- a/tests/hermes_cli/test_curator_status.py +++ b/tests/hermes_cli/test_curator_status.py @@ -200,3 +200,145 @@ def test_status_marks_missing_last_report_path(monkeypatch, capsys, tmp_path): out = capsys.readouterr().out assert f"last report: {missing_report} (missing)" in out + + +# --------------------------------------------------------------------------- +# Unmanaged blind spot + adopt verb +# --------------------------------------------------------------------------- + +def test_status_surfaces_unmanaged_skills(curator_status_env): + """A skill with no provenance marker is invisible to every automatic + transition, so status must SAY so rather than reporting only the managed + count — otherwise a large library looks fully curated while much of it is + untouchable.""" + env = curator_status_env + env["make_skill"]("managed-one") + env["make_skill"]("unmanaged-one") + env["skill_usage"].mark_agent_created("managed-one") + + out = _capture_status(env["curator_cli"]) + + assert "unmanaged (no provenance marker): 1 total" in out + assert "curator adopt" in out + + +def test_status_reports_unmanaged_even_with_no_managed_skills(curator_status_env): + """The early 'no curator-managed skills' return must not swallow the + unmanaged summary — that combination is exactly the confusing case.""" + env = curator_status_env + env["make_skill"]("unmanaged-one") + + out = _capture_status(env["curator_cli"]) + + assert "no curator-managed skills" in out + assert "unmanaged (no provenance marker): 1 total" in out + + +def test_status_omits_unmanaged_section_when_none(curator_status_env): + env = curator_status_env + env["make_skill"]("managed-one") + env["skill_usage"].mark_agent_created("managed-one") + + out = _capture_status(env["curator_cli"]) + + assert "unmanaged (no provenance marker)" not in out + + +def test_adopt_names_a_skill(curator_status_env): + env = curator_status_env + env["make_skill"]("legacy-one") + cli = env["curator_cli"] + + rc = cli._cmd_adopt(SimpleNamespace( + skill=["legacy-one"], all_unmanaged=False, dry_run=False, yes=False, + )) + + assert rc == 0 + assert env["skill_usage"].get_record("legacy-one").get("created_by") == "agent" + + +def test_adopt_dry_run_writes_nothing(curator_status_env): + env = curator_status_env + env["make_skill"]("legacy-one") + cli = env["curator_cli"] + + rc = cli._cmd_adopt(SimpleNamespace( + skill=[], all_unmanaged=True, dry_run=True, yes=False, + )) + + assert rc == 0 + assert env["skill_usage"].get_record("legacy-one").get("created_by") != "agent" + + +def test_adopt_all_unmanaged_requires_confirmation(curator_status_env, monkeypatch): + """Bulk adoption makes skills archivable, so it must confirm by default.""" + env = curator_status_env + env["make_skill"]("legacy-one") + cli = env["curator_cli"] + monkeypatch.setattr("builtins.input", lambda *_a: "n") + + rc = cli._cmd_adopt(SimpleNamespace( + skill=[], all_unmanaged=True, dry_run=False, yes=False, + )) + + assert rc == 1 + assert env["skill_usage"].get_record("legacy-one").get("created_by") != "agent" + + +def test_adopt_all_unmanaged_with_yes_skips_prompt(curator_status_env): + env = curator_status_env + env["make_skill"]("legacy-one") + env["make_skill"]("legacy-two") + cli = env["curator_cli"] + + rc = cli._cmd_adopt(SimpleNamespace( + skill=[], all_unmanaged=True, dry_run=False, yes=True, + )) + + assert rc == 0 + for n in ("legacy-one", "legacy-two"): + assert env["skill_usage"].get_record(n).get("created_by") == "agent" + + +def test_adopt_rejects_names_combined_with_all_unmanaged(curator_status_env): + env = curator_status_env + env["make_skill"]("legacy-one") + cli = env["curator_cli"] + + rc = cli._cmd_adopt(SimpleNamespace( + skill=["legacy-one"], all_unmanaged=True, dry_run=False, yes=True, + )) + + assert rc == 1 + + +def test_adopt_with_no_target_is_an_error(curator_status_env): + cli = curator_status_env["curator_cli"] + + rc = cli._cmd_adopt(SimpleNamespace( + skill=[], all_unmanaged=False, dry_run=False, yes=False, + )) + + assert rc == 1 + + +def test_adopt_subcommand_is_registered(): + """The verb must be reachable through the real argparse tree, not just as a + callable — a handler nobody can dispatch to is dead code.""" + import argparse + + import hermes_cli.curator as curator_cli + + parser = argparse.ArgumentParser() + curator_cli.register_cli(parser) + + args = parser.parse_args(["adopt", "--all-unmanaged", "--dry-run"]) + assert args.func is curator_cli._cmd_adopt + assert args.all_unmanaged is True + assert args.dry_run is True + assert args.skill == [] + + named = parser.parse_args(["adopt", "alpha", "beta"]) + assert named.skill == ["alpha", "beta"] + assert named.all_unmanaged is False + diff --git a/tests/tools/test_skill_usage.py b/tests/tools/test_skill_usage.py index fa5b9b4e89f1..99f151126bcc 100644 --- a/tests/tools/test_skill_usage.py +++ b/tests/tools/test_skill_usage.py @@ -813,3 +813,181 @@ def test_usage_report_covers_all_provenance(skills_home): for n in rows: assert rows[n]["use_count"] == 1 assert rows[n]["_persisted"] is True + + +# --------------------------------------------------------------------------- +# Unmanaged enumeration + adoption +# +# A skill only becomes curator-managed when ``created_by: agent`` lands on its +# usage record, and that only happens for background-review creations. Records +# written before the marker existed carry no key at all, and every foreground +# `skill_manage(create)` leaves it unset — both are curation-eligible yet +# invisible to every automatic transition. These tests pin the contract that +# the blind spot is enumerable and that adoption is an explicit declaration: +# never inferred from telemetry, never silently reached by the curator. +# --------------------------------------------------------------------------- + +def _seed_usage(skills_dir: Path, records: dict) -> None: + (skills_dir / ".usage.json").write_text( + json.dumps(records, indent=1), encoding="utf-8" + ) + + +def test_unmanaged_lists_eligible_skills_without_provenance(skills_home): + from tools.skill_usage import list_unmanaged_skill_names + + skills_dir = skills_home / "skills" + _write_skill(skills_dir, "legacy") # record with NO created_by key + _write_skill(skills_dir, "foreground") # created_by present but unset + _write_skill(skills_dir, "managed") # real provenance + _seed_usage(skills_dir, { + "legacy": {"use_count": 3, "patch_count": 40}, + "foreground": {"created_by": None, "use_count": 1}, + "managed": {"created_by": "agent"}, + }) + + names = list_unmanaged_skill_names() + assert "legacy" in names + assert "foreground" in names + assert "managed" not in names + + +def test_unmanaged_excludes_externally_owned_skills(skills_home): + from tools.skill_usage import list_unmanaged_skill_names + + 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", + ) + + names = list_unmanaged_skill_names() + # Bundled and hub skills have an owner other than the user; adoption is not + # the mechanism that governs them. + assert "bundled-one" not in names + assert "hub-one" not in names + assert "mine" in names + + +def test_unmanaged_report_distinguishes_legacy_from_foreground(skills_home): + from tools.skill_usage import unmanaged_report + + skills_dir = skills_home / "skills" + _write_skill(skills_dir, "legacy") + _write_skill(skills_dir, "foreground") + _seed_usage(skills_dir, { + "legacy": {"use_count": 1}, + "foreground": {"created_by": None}, + }) + + rows = {r["name"]: r for r in unmanaged_report()} + # No created_by key at all => predates the mechanism, authorship unknowable. + assert rows["legacy"]["has_provenance_key"] is False + # Key present but unset => a foreground create under the current policy. + assert rows["foreground"]["has_provenance_key"] is True + + +def test_adopt_marks_skill_curator_managed(skills_home): + from tools.skill_usage import adopt_skill, curated_report, list_unmanaged_skill_names + + skills_dir = skills_home / "skills" + _write_skill(skills_dir, "legacy") + _seed_usage(skills_dir, {"legacy": {"use_count": 2, "patch_count": 9}}) + + assert "legacy" in list_unmanaged_skill_names() + ok, _msg = adopt_skill("legacy") + assert ok is True + assert "legacy" in {r["name"] for r in curated_report()} + assert "legacy" not in list_unmanaged_skill_names() + + +def test_adopt_preserves_the_inactivity_clock(skills_home): + """Adoption must not reset staleness — it hands over an EXISTING history. + + If adopting re-anchored the clock to now, every legacy skill would buy a + fresh archive_after_days window, which is the opposite of what the user + wants when they hand over a library they already stopped using. + """ + from tools.skill_usage import adopt_skill, get_record, latest_activity_at + + skills_dir = skills_home / "skills" + _write_skill(skills_dir, "legacy") + _seed_usage(skills_dir, { + "legacy": { + "use_count": 5, + "patch_count": 7, + "last_used_at": "2026-04-29T00:00:00+00:00", + "created_at": "2026-04-28T00:00:00+00:00", + } + }) + before = latest_activity_at(get_record("legacy")) + + ok, _msg = adopt_skill("legacy") + assert ok is True + rec = get_record("legacy") + assert latest_activity_at(rec) == before + assert rec["use_count"] == 5 + assert rec["patch_count"] == 7 + + +def test_adopt_is_idempotent(skills_home): + from tools.skill_usage import adopt_skill + + skills_dir = skills_home / "skills" + _write_skill(skills_dir, "mine") + assert adopt_skill("mine")[0] is True + ok, msg = adopt_skill("mine") + assert ok is True + assert "already" in msg + + +@pytest.mark.parametrize("kind", ["bundled", "hub", "protected", "missing"]) +def test_adopt_refuses_skills_the_user_does_not_own(skills_home, monkeypatch, kind): + """Adoption writes a provenance claim, so it must refuse anything with an + external owner rather than stamping a lie onto the record. + + ``prune_builtins`` is forced ON here — the shipped default — because that + is the configuration in which a bundled skill is otherwise curation- + eligible. With it off, ``mark_agent_created``'s own eligibility gate would + block the write and this test would pass without exercising adopt's guard + at all. + """ + from tools import skill_usage + from tools.skill_usage import adopt_skill, load_usage + + monkeypatch.setattr(skill_usage, "_prune_builtins_enabled", lambda: True) + + skills_dir = skills_home / "skills" + if kind == "bundled": + name = "bundled-one" + _write_skill(skills_dir, name) + (skills_dir / ".bundled_manifest").write_text(f"{name}:abc\n", encoding="utf-8") + elif kind == "hub": + name = "hub-one" + _write_skill(skills_dir, name) + hub = skills_dir / ".hub" + hub.mkdir() + (hub / "lock.json").write_text( + json.dumps({"installed": {name: {}}}), encoding="utf-8", + ) + elif kind == "protected": + name = sorted(skill_usage.PROTECTED_BUILTIN_SKILLS)[0] + _write_skill(skills_dir, name) + else: + name = "no-such-skill" + + ok, _msg = adopt_skill(name) + assert ok is False + assert load_usage().get(name, {}).get("created_by") != "agent" + + +def test_adopt_rejects_empty_name(skills_home): + from tools.skill_usage import adopt_skill + + assert adopt_skill("")[0] is False + diff --git a/tests/tools/test_skills_sync.py b/tests/tools/test_skills_sync.py index 9034e964a74a..47f957fda40c 100644 --- a/tests/tools/test_skills_sync.py +++ b/tests/tools/test_skills_sync.py @@ -906,6 +906,87 @@ class TestSyncSkills: assert result["optional_provenance_backfilled"] == [] assert not (skills_dir / ".hub" / "lock.json").exists() + def test_backfills_optional_provenance_for_relocated_skill(self, tmp_path): + """Upstream recategorization must not blind provenance repair. + + When a skill was installed at ``mlops/chroma`` and upstream later moved + it to ``mlops/vector-databases/chroma``, the repo-derived install path + no longer exists in the active tree. A path-only lookup skips it + forever, so `hermes skills repair-optional` can never fix it. The + recorded install_path must be the ACTUAL location, not the repo's. + """ + bundled = self._setup_bundled(tmp_path) + optional = tmp_path / "optional-skills" + optional_skill = optional / "mlops" / "vector-databases" / "chroma" + optional_skill.mkdir(parents=True) + (optional_skill / "SKILL.md").write_text("---\nname: chroma\n---\n# Chroma\n") + + skills_dir = tmp_path / "user_skills" + manifest_file = skills_dir / ".bundled_manifest" + # Installed under the OLD category path. + active = skills_dir / "mlops" / "chroma" + active.mkdir(parents=True) + (active / "SKILL.md").write_text("---\nname: chroma\n---\n# Chroma\n") + + with self._patches(bundled, skills_dir, manifest_file): + with patch("tools.skills_sync._get_optional_dir", return_value=optional): + result = sync_skills(quiet=True) + + assert result["optional_provenance_backfilled"] == ["chroma"] + data = json.loads((skills_dir / ".hub" / "lock.json").read_text()) + entry = data["installed"]["chroma"] + assert entry["source"] == "official" + assert entry["install_path"] == "mlops/chroma" + + def test_relocated_backfill_still_requires_identical_content(self, tmp_path): + """The name fallback must not weaken the content check. + + Finding the skill by name is only a locator; a locally-edited copy is + still the user's and must not be claimed as official. + """ + bundled = self._setup_bundled(tmp_path) + optional = tmp_path / "optional-skills" + optional_skill = optional / "mlops" / "vector-databases" / "chroma" + optional_skill.mkdir(parents=True) + (optional_skill / "SKILL.md").write_text("---\nname: chroma\n---\n# upstream\n") + + skills_dir = tmp_path / "user_skills" + manifest_file = skills_dir / ".bundled_manifest" + active = skills_dir / "mlops" / "chroma" + active.mkdir(parents=True) + (active / "SKILL.md").write_text("---\nname: chroma\n---\n# LOCALLY EDITED\n") + + with self._patches(bundled, skills_dir, manifest_file): + with patch("tools.skills_sync._get_optional_dir", return_value=optional): + result = sync_skills(quiet=True) + + assert result["optional_provenance_backfilled"] == [] + + def test_relocated_backfill_refuses_ambiguous_names(self, tmp_path): + """Two installed dirs sharing a name give no basis to pick one. + + Guessing would write official provenance onto the wrong skill, so the + fallback must decline rather than choose. + """ + bundled = self._setup_bundled(tmp_path) + optional = tmp_path / "optional-skills" + optional_skill = optional / "cat" / "dupe" + optional_skill.mkdir(parents=True) + (optional_skill / "SKILL.md").write_text("---\nname: dupe\n---\n# D\n") + + skills_dir = tmp_path / "user_skills" + manifest_file = skills_dir / ".bundled_manifest" + for parent in ("x", "y"): + d = skills_dir / parent / "dupe" + d.mkdir(parents=True) + (d / "SKILL.md").write_text("---\nname: dupe\n---\n# D\n") + + with self._patches(bundled, skills_dir, manifest_file): + with patch("tools.skills_sync._get_optional_dir", return_value=optional): + result = sync_skills(quiet=True) + + assert result["optional_provenance_backfilled"] == [] + def test_repair_official_optional_restores_reorganized_skill_with_backup(self, tmp_path): bundled = self._setup_bundled(tmp_path) optional = tmp_path / "optional-skills" diff --git a/tools/skill_usage.py b/tools/skill_usage.py index e62fbefbffe3..f5d5c865dd60 100644 --- a/tools/skill_usage.py +++ b/tools/skill_usage.py @@ -485,6 +485,128 @@ def _is_curator_managed_record(record: Any) -> bool: return record.get("created_by") == "agent" or record.get("agent_created") is True +def list_unmanaged_skill_names() -> List[str]: + """Enumerate curation-ELIGIBLE skills that carry no provenance marker. + + These are skills the curator *could* manage (they are not hub-installed, + not external, not protected built-ins) but never will, because nothing + ever wrote ``created_by: agent`` onto their usage record. Two ways a skill + lands here: + + * It predates the provenance mechanism entirely — records written before + ``created_by`` existed carry no key at all, so their authorship is + unknowable from the record alone. + * It was created by a FOREGROUND ``skill_manage(action="create")`` call, + which deliberately does not mark provenance (skills a user asks for + belong to the user). + + Either way the skill is invisible to ``curated_report()`` and therefore to + every automatic transition. ``hermes curator status`` surfaces this count + so the blind spot is legible instead of silent, and ``hermes curator + adopt`` lets the user hand specific skills over explicitly. + + Provenance is a DECLARATION, never an inference: this function only + reports, and callers must not auto-adopt what it returns. Heavy patch or + use counts are evidence of maintenance, not of authorship — the agent + edits user-authored skills on the user's behalf routinely. + """ + base = _skills_dir() + if not base.exists(): + return [] + hub = _read_hub_installed_names() + bundled = _read_bundled_manifest_names() + usage = load_usage() + + names: List[str] = [] + for skill_md in base.rglob("SKILL.md"): + if is_excluded_skill_path(skill_md) or is_external_skill_path(skill_md): + continue + try: + skill_md.relative_to(base) + except ValueError: + continue + name = _read_skill_name(skill_md, fallback=skill_md.parent.name) + # Anything with an external owner or a bundled/protected identity is + # outside the adoption question entirely. + if name in hub or name in bundled or is_protected_builtin(name): + continue + if _is_curator_managed_record(usage.get(name)): + continue + if not is_curation_eligible(name, skill_md): + continue + names.append(name) + return sorted(set(names)) + + +def unmanaged_report() -> List[Dict[str, Any]]: + """Rows for every skill :func:`list_unmanaged_skill_names` returns. + + Each row carries the usual activity fields plus ``has_provenance_key``: + False when the record has no ``created_by`` key at all (pre-dates the + mechanism), True when the key is present but unset (a foreground create + under the current policy). The distinction matters for explaining WHY a + skill is unmanaged; it is not a signal to adopt on. + """ + usage = load_usage() + rows: List[Dict[str, Any]] = [] + for name in list_unmanaged_skill_names(): + raw = usage.get(name) + rec: Dict[str, Any] = dict(raw) if isinstance(raw, dict) else _empty_record() + for k, v in _empty_record().items(): + rec.setdefault(k, v) + row = {"name": name, **rec} + row["has_provenance_key"] = isinstance(raw, dict) and "created_by" in raw + row["has_record"] = isinstance(raw, dict) + row["last_activity_at"] = latest_activity_at(row) + row["activity_count"] = activity_count(row) + rows.append(row) + return rows + + +def adopt_skill(skill_name: str) -> Tuple[bool, str]: + """Hand *skill_name* to the curator by user declaration. + + Writes the same ``created_by: agent`` marker the background review fork + writes, so the skill joins ``curated_report()`` and the automatic + transition walk. The inactivity clock is NOT reset: the skill's existing + ``last_activity_at`` still governs staleness, so adopting something idle + for months does not buy it a fresh window (nor does it archive it on the + spot — the state machine decides on the next pass). + + Returns (ok, message). Refuses hub-installed, external, and protected + built-in skills, which have an owner other than the user. + """ + if not skill_name: + return False, "no skill name given" + if is_protected_builtin(skill_name): + return False, f"'{skill_name}' is a protected built-in; the curator never manages it" + if is_hub_installed(skill_name): + return False, f"'{skill_name}' is hub-installed; its upstream owns it" + if is_bundled(skill_name): + # Bundled skills already fall under the curator via + # ``curator.prune_builtins``; stamping created_by=agent on one would + # claim Hermes' own shipped skill was agent-authored and change nothing + # about its eligibility. + return False, ( + f"'{skill_name}' is a bundled built-in — it is governed by " + "curator.prune_builtins, not by adoption" + ) + skill_dir = _find_skill_dir(skill_name) + if skill_dir is None: + if _find_external_skill_dir(skill_name) is not None: + return False, f"'{skill_name}' lives in skills.external_dirs and is read-only to the curator" + return False, f"skill '{skill_name}' not found" + if is_external_skill_path(skill_dir): + return False, _external_read_only_message(skill_name) + usage = load_usage() + if _is_curator_managed_record(usage.get(skill_name)): + return True, f"'{skill_name}' is already curator-managed" + mark_agent_created(skill_name) + if not _is_curator_managed_record(load_usage().get(skill_name)): + return False, f"could not mark '{skill_name}' as curator-managed" + return True, f"adopted '{skill_name}' into curator management" + + # --------------------------------------------------------------------------- # Sidecar I/O # --------------------------------------------------------------------------- diff --git a/tools/skills_sync.py b/tools/skills_sync.py index b76449593ff3..de0545e308cb 100644 --- a/tools/skills_sync.py +++ b/tools/skills_sync.py @@ -402,6 +402,36 @@ def restore_official_optional_skill(name: str, *, restore: bool = False) -> dict } +def _find_installed_skill_dir_by_name(skill_dir_name: str) -> Optional[Path]: + """Locate an installed skill directory by its directory name. + + Used only as a fallback when the repo-derived install path doesn't exist in + the active tree (upstream recategorized the skill after it was installed). + Returns None when there is no match, or when the name is AMBIGUOUS — two + skills sharing a directory name give us no basis to pick one, and guessing + would write provenance onto the wrong skill. The caller still verifies a + byte-identical content hash before recording anything. + """ + if not skill_dir_name or not SKILLS_DIR.exists(): + return None + matches: List[Path] = [] + for skill_md in SKILLS_DIR.rglob("SKILL.md"): + if is_excluded_skill_path(skill_md): + continue + candidate = skill_md.parent + if candidate.name != skill_dir_name: + continue + # Never reach outside the skills tree (symlinked/external dirs). + try: + candidate.resolve().relative_to(SKILLS_DIR.resolve()) + except (OSError, ValueError): + continue + matches.append(candidate) + if len(matches) != 1: + return None + return matches[0] + + def _backfill_optional_provenance(quiet: bool = False) -> List[str]: """Mark already-present official optional skills as hub-installed. @@ -440,7 +470,22 @@ def _backfill_optional_provenance(quiet: bool = False) -> List[str]: continue dest = SKILLS_DIR / Path(*install_path.split("/")) if not dest.exists() or not dest.is_dir(): - continue + # The active tree may hold the same skill under a DIFFERENT + # category path than the repo uses — categories get reorganized + # upstream (e.g. mlops/chroma → mlops/vector-databases/chroma) + # while the already-installed copy keeps its old location. A + # path-only lookup misses every one of those, so provenance repair + # silently skips them forever. Fall back to a unique + # same-directory-name match anywhere in the tree, then still + # require a byte-identical hash below before claiming provenance. + dest = _find_installed_skill_dir_by_name(src.name) + if dest is None: + continue + try: + install_path = _safe_rel_install_path(dest, SKILLS_DIR) + except ValueError as e: + logger.debug("Skipping relocated optional skill %s: %s", dest, e) + continue if _dir_hash(dest) != _dir_hash(src): continue diff --git a/website/docs/user-guide/features/curator.md b/website/docs/user-guide/features/curator.md index 0601e65fb853..6e7e8ac9f42c 100644 --- a/website/docs/user-guide/features/curator.md +++ b/website/docs/user-guide/features/curator.md @@ -103,6 +103,8 @@ hermes curator pause # stop runs until resumed hermes curator resume hermes curator pin # never auto-transition this skill hermes curator unpin +hermes curator adopt # hand an unmanaged skill to the curator +hermes curator adopt --all-unmanaged # hand over every unmanaged skill hermes curator restore # move an archived skill back to active hermes curator list-archived # list skills currently in ~/.hermes/skills/.archive/ hermes curator archive # manually archive a single skill now @@ -170,6 +172,57 @@ jurisdiction — the LLM review pass is skipped and the report will show `Model: (not resolved) via (not resolved)` with `Duration: 0s`. ::: +### Adopting unmanaged skills + +`hermes curator status` reports an **unmanaged** count alongside the managed +one: + +``` +curator-managed skills: 43 total (agent-created=43 bundled=0) + active 41 + stale 2 + archived 0 + +unmanaged (no provenance marker): 112 total + pre-dates marker 34 + foreground-created 78 + never auto-staled or archived — `hermes curator adopt ` hands one over +``` + +Those 112 are curation-*eligible* but permanently invisible to the lifecycle, +for one of two reasons: + +- **pre-dates marker** — the record was written before `created_by` existed, so + it carries no provenance signal at all. Authorship is genuinely unknowable + from the record. +- **foreground-created** — a foreground `skill_manage(create)` left the marker + unset by design, since skills you ask for belong to you. + +A large library can therefore look fully curated while most of it is +untouchable. `adopt` closes that gap by **declaration**: + +```bash +hermes curator adopt [ ...] # hand specific skills over +hermes curator adopt --all-unmanaged --dry-run # preview the full list +hermes curator adopt --all-unmanaged # hand over everything (prompts) +hermes curator adopt --all-unmanaged --yes # skip the prompt +``` + +Adoption writes the same `created_by: agent` marker the background review fork +writes. It does **not** reset the inactivity clock — an adopted skill keeps its +existing `last_activity_at`, so handing over a library you already stopped +using does not buy it a fresh 90-day window. Expect adopted long-idle skills to +go `stale` (or `archived`) on the next pass; that is the point. + +:::note Provenance is declared, never inferred +Adoption is deliberately manual. Telemetry cannot establish authorship: a skill +with thousands of patches proves the agent **maintains** it, not that the agent +**wrote** it — Hermes edits user-authored skills on your behalf constantly. An +automatic "looks agent-made, adopt it" heuristic would eventually archive +something you hand-wrote. `adopt` refuses bundled, hub-installed, external, and +protected built-in skills, which have an owner other than you. +::: + Skills that ARE agent-created follow the full lifecycle: - `active` → (30d unused) `stale` → (90d unused) `archived`