mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-26 17:38:36 +00:00
feat(curator): surface unmanaged skills and add curator adopt
`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.
This commit is contained in:
Teknium
parent
b9fedab47a
commit
72de75c0ab
7 changed files with 733 additions and 1 deletions
|
|
@ -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 <name>` 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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -103,6 +103,8 @@ hermes curator pause # stop runs until resumed
|
|||
hermes curator resume
|
||||
hermes curator pin <skill> # never auto-transition this skill
|
||||
hermes curator unpin <skill>
|
||||
hermes curator adopt <skill> # hand an unmanaged skill to the curator
|
||||
hermes curator adopt --all-unmanaged # hand over every unmanaged skill
|
||||
hermes curator restore <skill> # move an archived skill back to active
|
||||
hermes curator list-archived # list skills currently in ~/.hermes/skills/.archive/
|
||||
hermes curator archive <skill> # 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 <name>` 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 <name> [<name> ...] # 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`
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue