diff --git a/hermes_cli/curator.py b/hermes_cli/curator.py index 1dbf5420fa..0325cf5cf6 100644 --- a/hermes_cli/curator.py +++ b/hermes_cli/curator.py @@ -151,6 +151,12 @@ def _cmd_pin(args) -> int: def _cmd_unpin(args) -> int: from tools import skill_usage + if not skill_usage.is_agent_created(args.skill): + print( + f"curator: '{args.skill}' is bundled or hub-installed — " + "there's nothing to unpin (curator only tracks agent-created skills)" + ) + return 1 skill_usage.set_pinned(args.skill, False) print(f"curator: unpinned '{args.skill}'") return 0 diff --git a/tests/agent/test_curator.py b/tests/agent/test_curator.py index 766a49e6d1..a9b0929b97 100644 --- a/tests/agent/test_curator.py +++ b/tests/agent/test_curator.py @@ -393,3 +393,39 @@ def test_curator_does_not_instruct_model_to_pin(): assert not any(l.strip().startswith("pin ") for l in lines), ( f"Found a pin action line in:\n{decision_block}" ) + + + +def test_cli_unpin_refuses_bundled_skill(curator_env, capsys): + """hermes curator unpin must refuse bundled/hub skills too (matches pin).""" + from hermes_cli import curator as cli + skills_dir = curator_env["home"] / "skills" + _write_skill(skills_dir, "ship-skill") + (skills_dir / ".bundled_manifest").write_text( + "ship-skill:abc\n", encoding="utf-8", + ) + + class _A: + skill = "ship-skill" + + rc = cli._cmd_unpin(_A()) + captured = capsys.readouterr() + assert rc == 1 + assert "bundled" in captured.out.lower() or "hub" in captured.out.lower() + + +def test_cli_pin_refuses_bundled_skill(curator_env, capsys): + from hermes_cli import curator as cli + skills_dir = curator_env["home"] / "skills" + _write_skill(skills_dir, "ship-skill") + (skills_dir / ".bundled_manifest").write_text( + "ship-skill:abc\n", encoding="utf-8", + ) + + class _A: + skill = "ship-skill" + + rc = cli._cmd_pin(_A()) + captured = capsys.readouterr() + assert rc == 1 + assert "bundled" in captured.out.lower() or "hub" in captured.out.lower() diff --git a/tests/tools/test_skill_usage.py b/tests/tools/test_skill_usage.py index ec23f18071..1e7b554bc3 100644 --- a/tests/tools/test_skill_usage.py +++ b/tests/tools/test_skill_usage.py @@ -364,3 +364,124 @@ def test_agent_created_report_excludes_bundled_and_hub(skills_home): assert "mine" in names assert "bundled" not in names assert "hubbed" not in names + + + +# --------------------------------------------------------------------------- +# Provenance guard — telemetry must not leak records for bundled/hub skills +# --------------------------------------------------------------------------- + +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 + skills_dir = skills_home / "skills" + (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" + ) + + +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" + hub = skills_dir / ".hub" + hub.mkdir() + (hub / "lock.json").write_text( + json.dumps({"installed": {"from-hub": {}}}), encoding="utf-8", + ) + + bump_patch("from-hub") + assert "from-hub" not in load_usage() + + +def test_bump_use_no_op_for_hub_skill(skills_home): + from tools.skill_usage import bump_use, load_usage + skills_dir = skills_home / "skills" + hub = skills_dir / ".hub" + hub.mkdir() + (hub / "lock.json").write_text( + json.dumps({"installed": {"from-hub": {}}}), encoding="utf-8", + ) + + bump_use("from-hub") + assert "from-hub" not in load_usage() + + +def test_set_state_no_op_for_bundled_skill(skills_home): + """State transitions on bundled skills must not land in the sidecar.""" + from tools.skill_usage import set_state, load_usage, STATE_ARCHIVED + skills_dir = skills_home / "skills" + (skills_dir / ".bundled_manifest").write_text( + "locked:abc\n", encoding="utf-8", + ) + set_state("locked", STATE_ARCHIVED) + assert "locked" not in load_usage() + + +def test_restore_refuses_to_shadow_bundled_skill(skills_home): + """If a bundled skill now occupies the name, refuse to restore.""" + from tools.skill_usage import archive_skill, restore_skill + skills_dir = skills_home / "skills" + _write_skill(skills_dir, "shared-name") + archive_skill("shared-name") + + # Now a bundled skill appears with the same name + (skills_dir / ".bundled_manifest").write_text( + "shared-name:abc\n", encoding="utf-8", + ) + _write_skill(skills_dir, "shared-name") # bundled install landed + + ok, msg = restore_skill("shared-name") + assert not ok + 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.""" + from tools.skill_usage import ( + bump_view, bump_use, bump_patch, set_state, set_pinned, + archive_skill, load_usage, STATE_STALE, STATE_ARCHIVED, + ) + 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", + ) + + # Hammer every mutator at the bundled/hub names + for name in ("bundled-one", "hub-one"): + bump_view(name) + bump_use(name) + bump_patch(name) + set_state(name, STATE_STALE) + set_state(name, STATE_ARCHIVED) + set_pinned(name, True) + 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 + + # 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 + bump_view("mine") + assert load_usage()["mine"]["view_count"] == 1 diff --git a/tools/skill_usage.py b/tools/skill_usage.py index 1b07a3b697..8bf73b3e13 100644 --- a/tools/skill_usage.py +++ b/tools/skill_usage.py @@ -239,10 +239,18 @@ def get_record(skill_name: str) -> Dict[str, Any]: def _mutate(skill_name: str, mutator) -> None: - """Load, apply *mutator(record)* in place, save. Best-effort.""" + """Load, apply *mutator(record)* in place, save. Best-effort. + + Bundled and hub-installed skills are NEVER recorded in the sidecar. + This keeps .usage.json focused on agent-created skills (the only ones + the curator considers) and prevents stale counters from hanging around + for upstream-managed skills. + """ if not skill_name: return try: + if not is_agent_created(skill_name): + return data = load_usage() rec = data.get(skill_name) if not isinstance(rec, dict): @@ -361,7 +369,18 @@ def archive_skill(skill_name: str) -> Tuple[bool, str]: 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.""" + 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. + """ + # 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): + return False, ( + f"skill '{skill_name}' is now bundled or hub-installed; " + "restore would shadow the upstream version" + ) archive_root = _archive_dir() if not archive_root.exists(): return False, "no archive directory"