fix(skills): keep manual skills out of curator

This commit is contained in:
LeonSGP43 2026-05-03 21:44:04 +08:00 committed by Teknium
parent cac4f2c0e6
commit abcaf05229
5 changed files with 102 additions and 18 deletions

View file

@ -154,6 +154,7 @@ def test_unused_skill_transitions_to_stale(curator_env):
long_ago = (datetime.now(timezone.utc) - timedelta(days=45)).isoformat() long_ago = (datetime.now(timezone.utc) - timedelta(days=45)).isoformat()
data = u.load_usage() data = u.load_usage()
data["old-skill"] = u._empty_record() data["old-skill"] = u._empty_record()
data["old-skill"]["created_by"] = "agent"
data["old-skill"]["last_used_at"] = long_ago data["old-skill"]["last_used_at"] = long_ago
data["old-skill"]["created_at"] = long_ago data["old-skill"]["created_at"] = long_ago
u.save_usage(data) u.save_usage(data)
@ -172,6 +173,7 @@ def test_very_old_skill_gets_archived(curator_env):
super_old = (datetime.now(timezone.utc) - timedelta(days=120)).isoformat() super_old = (datetime.now(timezone.utc) - timedelta(days=120)).isoformat()
data = u.load_usage() data = u.load_usage()
data["ancient"] = u._empty_record() data["ancient"] = u._empty_record()
data["ancient"]["created_by"] = "agent"
data["ancient"]["last_used_at"] = super_old data["ancient"]["last_used_at"] = super_old
data["ancient"]["created_at"] = super_old data["ancient"]["created_at"] = super_old
u.save_usage(data) u.save_usage(data)
@ -192,6 +194,7 @@ def test_pinned_skill_is_never_touched(curator_env):
super_old = (datetime.now(timezone.utc) - timedelta(days=365)).isoformat() super_old = (datetime.now(timezone.utc) - timedelta(days=365)).isoformat()
data = u.load_usage() data = u.load_usage()
data["precious"] = u._empty_record() data["precious"] = u._empty_record()
data["precious"]["created_by"] = "agent"
data["precious"]["last_used_at"] = super_old data["precious"]["last_used_at"] = super_old
data["precious"]["created_at"] = super_old data["precious"]["created_at"] = super_old
data["precious"]["pinned"] = True data["precious"]["pinned"] = True
@ -214,6 +217,7 @@ def test_stale_skill_reactivates_on_recent_use(curator_env):
recent = datetime.now(timezone.utc).isoformat() recent = datetime.now(timezone.utc).isoformat()
data = u.load_usage() data = u.load_usage()
data["revived"] = u._empty_record() data["revived"] = u._empty_record()
data["revived"]["created_by"] = "agent"
data["revived"]["state"] = "stale" data["revived"]["state"] = "stale"
data["revived"]["last_used_at"] = recent data["revived"]["last_used_at"] = recent
data["revived"]["created_at"] = recent data["revived"]["created_at"] = recent
@ -240,6 +244,27 @@ def test_new_skill_without_last_used_not_immediately_archived(curator_env):
assert (skills_dir / "fresh").exists() assert (skills_dir / "fresh").exists()
def test_manual_skill_is_not_auto_archived(curator_env):
"""Manual skills can have usage records, but without the agent-created
marker they must stay out of curator transitions."""
c = curator_env["curator"]
u = curator_env["usage"]
skills_dir = curator_env["home"] / "skills"
skill_dir = _write_skill(skills_dir, "manual")
super_old = (datetime.now(timezone.utc) - timedelta(days=365)).isoformat()
data = u.load_usage()
data["manual"] = u._empty_record()
data["manual"]["last_used_at"] = super_old
data["manual"]["created_at"] = super_old
u.save_usage(data)
counts = c.apply_automatic_transitions()
assert counts["checked"] == 0
assert counts["archived"] == 0
assert skill_dir.exists()
def test_bundled_skill_not_touched_by_transitions(curator_env): def test_bundled_skill_not_touched_by_transitions(curator_env):
c = curator_env["curator"] c = curator_env["curator"]
u = curator_env["usage"] u = curator_env["usage"]
@ -267,8 +292,10 @@ def test_bundled_skill_not_touched_by_transitions(curator_env):
def test_run_review_records_state(curator_env): def test_run_review_records_state(curator_env):
c = curator_env["curator"] c = curator_env["curator"]
u = curator_env["usage"]
skills_dir = curator_env["home"] / "skills" skills_dir = curator_env["home"] / "skills"
_write_skill(skills_dir, "a") _write_skill(skills_dir, "a")
u.mark_agent_created("a")
result = c.run_curator_review(synchronous=True) result = c.run_curator_review(synchronous=True)
assert "started_at" in result assert "started_at" in result
@ -284,8 +311,10 @@ def test_dry_run_does_not_advance_state(curator_env, monkeypatch):
`hermes curator status`. Fixes #18373. `hermes curator status`. Fixes #18373.
""" """
c = curator_env["curator"] c = curator_env["curator"]
u = curator_env["usage"]
skills_dir = curator_env["home"] / "skills" skills_dir = curator_env["home"] / "skills"
_write_skill(skills_dir, "a") _write_skill(skills_dir, "a")
u.mark_agent_created("a")
# Stub the LLM so the test doesn't need a provider. # Stub the LLM so the test doesn't need a provider.
monkeypatch.setattr( monkeypatch.setattr(
@ -311,8 +340,10 @@ def test_dry_run_injects_report_only_banner(curator_env, monkeypatch):
skips automatic transitions but the LLM prompt is the only guard skips automatic transitions but the LLM prompt is the only guard
against the model calling skill_manage directly.""" against the model calling skill_manage directly."""
c = curator_env["curator"] c = curator_env["curator"]
u = curator_env["usage"]
skills_dir = curator_env["home"] / "skills" skills_dir = curator_env["home"] / "skills"
_write_skill(skills_dir, "a") _write_skill(skills_dir, "a")
u.mark_agent_created("a")
captured = {} captured = {}
def _stub(prompt): def _stub(prompt):
@ -331,8 +362,10 @@ def test_dry_run_skips_automatic_transitions(curator_env, monkeypatch):
archives skills deterministically, and a preview must not touch the archives skills deterministically, and a preview must not touch the
filesystem.""" filesystem."""
c = curator_env["curator"] c = curator_env["curator"]
u = curator_env["usage"]
skills_dir = curator_env["home"] / "skills" skills_dir = curator_env["home"] / "skills"
_write_skill(skills_dir, "a") _write_skill(skills_dir, "a")
u.mark_agent_created("a")
called = {"n": 0} called = {"n": 0}
def _explode(*_a, **_kw): def _explode(*_a, **_kw):
@ -351,8 +384,10 @@ def test_dry_run_skips_automatic_transitions(curator_env, monkeypatch):
def test_run_review_synchronous_invokes_llm_stub(curator_env, monkeypatch): def test_run_review_synchronous_invokes_llm_stub(curator_env, monkeypatch):
c = curator_env["curator"] c = curator_env["curator"]
u = curator_env["usage"]
skills_dir = curator_env["home"] / "skills" skills_dir = curator_env["home"] / "skills"
_write_skill(skills_dir, "a") _write_skill(skills_dir, "a")
u.mark_agent_created("a")
calls = [] calls = []
def _stub(prompt): def _stub(prompt):
@ -409,8 +444,10 @@ def test_maybe_run_curator_enforces_idle_gate(curator_env, monkeypatch):
def test_maybe_run_curator_runs_when_eligible(curator_env, monkeypatch): def test_maybe_run_curator_runs_when_eligible(curator_env, monkeypatch):
c = curator_env["curator"] c = curator_env["curator"]
u = curator_env["usage"]
skills_dir = curator_env["home"] / "skills" skills_dir = curator_env["home"] / "skills"
_write_skill(skills_dir, "a") _write_skill(skills_dir, "a")
u.mark_agent_created("a")
# Seed last_run_at far in the past so the interval gate opens — the # Seed last_run_at far in the past so the interval gate opens — the
# "no state" path intentionally defers the first run now (#18373). # "no state" path intentionally defers the first run now (#18373).
long_ago = datetime.now(timezone.utc) - timedelta(hours=c.get_interval_hours() * 2) long_ago = datetime.now(timezone.utc) - timedelta(hours=c.get_interval_hours() * 2)

View file

@ -533,8 +533,11 @@ class TestSkillManageDispatcher:
def test_full_create_via_dispatcher(self, tmp_path): def test_full_create_via_dispatcher(self, tmp_path):
with _skill_dir(tmp_path): with _skill_dir(tmp_path):
raw = skill_manage(action="create", name="test-skill", content=VALID_SKILL_CONTENT) raw = skill_manage(action="create", name="test-skill", content=VALID_SKILL_CONTENT)
from tools.skill_usage import load_usage
usage = load_usage()
result = json.loads(raw) result = json.loads(raw)
assert result["success"] is True assert result["success"] is True
assert usage["test-skill"]["created_by"] == "agent"
def test_delete_via_dispatcher_threads_absorbed_into(self, tmp_path): def test_delete_via_dispatcher_threads_absorbed_into(self, tmp_path):
# Dispatcher must plumb absorbed_into through to _delete_skill so the # Dispatcher must plumb absorbed_into through to _delete_skill so the

View file

@ -194,10 +194,11 @@ def test_forget_removes_record(skills_home):
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
def test_agent_created_excludes_bundled(skills_home): def test_agent_created_excludes_bundled(skills_home):
from tools.skill_usage import list_agent_created_skill_names from tools.skill_usage import list_agent_created_skill_names, mark_agent_created
skills_dir = skills_home / "skills" skills_dir = skills_home / "skills"
_write_skill(skills_dir, "bundled-skill", category="github") _write_skill(skills_dir, "bundled-skill", category="github")
_write_skill(skills_dir, "my-skill") _write_skill(skills_dir, "my-skill")
mark_agent_created("my-skill")
# Seed a bundled manifest marking bundled-skill as upstream # Seed a bundled manifest marking bundled-skill as upstream
(skills_dir / ".bundled_manifest").write_text( (skills_dir / ".bundled_manifest").write_text(
"bundled-skill:abc123\n", encoding="utf-8", "bundled-skill:abc123\n", encoding="utf-8",
@ -208,10 +209,11 @@ def test_agent_created_excludes_bundled(skills_home):
def test_agent_created_excludes_hub_installed(skills_home): def test_agent_created_excludes_hub_installed(skills_home):
from tools.skill_usage import list_agent_created_skill_names from tools.skill_usage import list_agent_created_skill_names, mark_agent_created
skills_dir = skills_home / "skills" skills_dir = skills_home / "skills"
_write_skill(skills_dir, "hub-skill") _write_skill(skills_dir, "hub-skill")
_write_skill(skills_dir, "my-skill") _write_skill(skills_dir, "my-skill")
mark_agent_created("my-skill")
hub_dir = skills_dir / ".hub" hub_dir = skills_dir / ".hub"
hub_dir.mkdir() hub_dir.mkdir()
(hub_dir / "lock.json").write_text( (hub_dir / "lock.json").write_text(
@ -238,9 +240,10 @@ def test_is_agent_created(skills_home):
def test_agent_created_skips_archive_and_hub_dirs(skills_home): def test_agent_created_skips_archive_and_hub_dirs(skills_home):
from tools.skill_usage import list_agent_created_skill_names from tools.skill_usage import list_agent_created_skill_names, mark_agent_created
skills_dir = skills_home / "skills" skills_dir = skills_home / "skills"
_write_skill(skills_dir, "real-skill") _write_skill(skills_dir, "real-skill")
mark_agent_created("real-skill")
# Dot-prefixed dirs must be ignored even if they contain SKILL.md # Dot-prefixed dirs must be ignored even if they contain SKILL.md
archive = skills_dir / ".archive" / "old-skill" archive = skills_dir / ".archive" / "old-skill"
archive.mkdir(parents=True) archive.mkdir(parents=True)
@ -368,27 +371,41 @@ def test_archive_collision_gets_suffix(skills_home):
# Reporting # Reporting
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
def test_agent_created_report_includes_defaults(skills_home): def test_agent_created_report_includes_marked_skills_with_defaults(skills_home):
from tools.skill_usage import agent_created_report, bump_view from tools.skill_usage import agent_created_report, bump_view, mark_agent_created
skills_dir = skills_home / "skills" skills_dir = skills_home / "skills"
_write_skill(skills_dir, "a") _write_skill(skills_dir, "a")
_write_skill(skills_dir, "b") _write_skill(skills_dir, "b")
mark_agent_created("a")
mark_agent_created("b")
bump_view("a") bump_view("a")
rows = agent_created_report() rows = agent_created_report()
by_name = {r["name"]: r for r in rows} by_name = {r["name"]: r for r in rows}
assert "a" in by_name and "b" in by_name assert "a" in by_name and "b" in by_name
assert by_name["a"]["view_count"] == 1 assert by_name["a"]["view_count"] == 1
# b has no usage record yet — must still appear with defaults # b has only the provenance marker — activity fields still default.
assert by_name["b"]["view_count"] == 0 assert by_name["b"]["view_count"] == 0
assert by_name["b"]["state"] == "active" assert by_name["b"]["state"] == "active"
def test_manual_skill_with_usage_is_not_curator_managed(skills_home):
from tools.skill_usage import agent_created_report, bump_view, list_agent_created_skill_names
skills_dir = skills_home / "skills"
_write_skill(skills_dir, "manual-skill")
bump_view("manual-skill")
assert "manual-skill" not in list_agent_created_skill_names()
assert "manual-skill" not in {r["name"] for r in agent_created_report()}
def test_agent_created_report_excludes_bundled_and_hub(skills_home): def test_agent_created_report_excludes_bundled_and_hub(skills_home):
from tools.skill_usage import agent_created_report from tools.skill_usage import agent_created_report, mark_agent_created
skills_dir = skills_home / "skills" skills_dir = skills_home / "skills"
_write_skill(skills_dir, "mine") _write_skill(skills_dir, "mine")
_write_skill(skills_dir, "bundled") _write_skill(skills_dir, "bundled")
_write_skill(skills_dir, "hubbed") _write_skill(skills_dir, "hubbed")
mark_agent_created("mine")
(skills_dir / ".bundled_manifest").write_text("bundled:abc\n", encoding="utf-8") (skills_dir / ".bundled_manifest").write_text("bundled:abc\n", encoding="utf-8")
hub = skills_dir / ".hub" hub = skills_dir / ".hub"
hub.mkdir() hub.mkdir()
@ -414,6 +431,7 @@ def test_agent_created_report_derives_activity_from_view_and_patch(skills_home,
]) ])
monkeypatch.setattr(skill_usage, "_now_iso", lambda: next(timestamps)) monkeypatch.setattr(skill_usage, "_now_iso", lambda: next(timestamps))
skill_usage.mark_agent_created("mine")
skill_usage.bump_view("mine") skill_usage.bump_view("mine")
skill_usage.bump_patch("mine") skill_usage.bump_patch("mine")

View file

@ -786,8 +786,10 @@ def skill_manage(
# that mutate an existing skill's guidance), drop the record on delete. # that mutate an existing skill's guidance), drop the record on delete.
# Best-effort; telemetry failures never break the tool. # Best-effort; telemetry failures never break the tool.
try: try:
from tools.skill_usage import bump_patch, forget from tools.skill_usage import bump_patch, forget, mark_agent_created
if action in ("patch", "edit", "write_file", "remove_file"): if action == "create":
mark_agent_created(name)
elif action in ("patch", "edit", "write_file", "remove_file"):
bump_patch(name) bump_patch(name)
elif action == "delete": elif action == "delete":
forget(name) forget(name)

View file

@ -11,8 +11,9 @@ Design notes:
- Atomic writes via tempfile + os.replace (same pattern as .bundled_manifest). - Atomic writes via tempfile + os.replace (same pattern as .bundled_manifest).
- All counter bumps are best-effort: failures log at DEBUG and return silently. - All counter bumps are best-effort: failures log at DEBUG and return silently.
A broken sidecar never breaks the underlying tool call. A broken sidecar never breaks the underlying tool call.
- Provenance filter: "agent-created" == not in .bundled_manifest AND not in - Provenance filter: curator-managed skills are explicitly marked when
.hub/lock.json. The curator only ever mutates agent-created skills. created through skill_manage. Bundled / hub-installed skills stay
off-limits, and manually authored skills are not inferred from location.
Lifecycle states: Lifecycle states:
active -> default active -> default
@ -149,11 +150,13 @@ def _read_hub_installed_names() -> Set[str]:
def list_agent_created_skill_names() -> List[str]: def list_agent_created_skill_names() -> List[str]:
"""Enumerate skills that were authored by the agent (or user), NOT by a """Enumerate skills explicitly authored by the agent.
bundled or hub-installed source.
The curator operates exclusively on this set. Bundled / hub skills are The curator operates exclusively on this set. Skills are only eligible
maintained by their upstream sources and must never be pruned here. 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.
""" """
base = _skills_dir() base = _skills_dir()
if not base.exists(): if not base.exists():
@ -161,6 +164,7 @@ def list_agent_created_skill_names() -> List[str]:
bundled = _read_bundled_manifest_names() bundled = _read_bundled_manifest_names()
hub = _read_hub_installed_names() hub = _read_hub_installed_names()
off_limits = bundled | hub off_limits = bundled | hub
usage = load_usage()
names: List[str] = [] names: List[str] = []
# Top-level SKILL.md files (flat layout) AND nested category/skill/SKILL.md # Top-level SKILL.md files (flat layout) AND nested category/skill/SKILL.md
@ -176,6 +180,8 @@ def list_agent_created_skill_names() -> List[str]:
name = _read_skill_name(skill_md, fallback=skill_md.parent.name) name = _read_skill_name(skill_md, fallback=skill_md.parent.name)
if name in off_limits: if name in off_limits:
continue continue
if not _is_curator_managed_record(usage.get(name)):
continue
names.append(name) names.append(name)
return sorted(set(names)) return sorted(set(names))
@ -207,12 +213,20 @@ def is_agent_created(skill_name: str) -> bool:
return skill_name not in off_limits return skill_name not in off_limits
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):
return False
return record.get("created_by") == "agent" or record.get("agent_created") is True
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Sidecar I/O # Sidecar I/O
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
def _empty_record() -> Dict[str, Any]: def _empty_record() -> Dict[str, Any]:
return { return {
"created_by": None,
"use_count": 0, "use_count": 0,
"view_count": 0, "view_count": 0,
"last_used_at": None, "last_used_at": None,
@ -287,9 +301,8 @@ 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. Bundled and hub-installed skills are NEVER recorded in the sidecar.
This keeps .usage.json focused on agent-created skills (the only ones Local manual skills may still accrue usage telemetry, but they only
the curator considers) and prevents stale counters from hanging around become curator-managed when ``created_by`` is explicitly marked.
for upstream-managed skills.
""" """
if not skill_name: if not skill_name:
return return
@ -336,6 +349,17 @@ def bump_patch(skill_name: str) -> None:
_mutate(skill_name, _apply) _mutate(skill_name, _apply)
def mark_agent_created(skill_name: str) -> None:
"""Opt a skill created by skill_manage into curator management.
Viewing or invoking a manually authored skill may still create telemetry,
but only this explicit marker makes it eligible for automatic curation.
"""
def _apply(rec: Dict[str, Any]) -> None:
rec["created_by"] = "agent"
_mutate(skill_name, _apply)
def set_state(skill_name: str, state: str) -> None: 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."""
if state not in _VALID_STATES: if state not in _VALID_STATES: