mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-30 11:52:04 +00:00
fix(curator): never archive cron-referenced skills + floor use=0 pruning (#54443)
The curator's inactivity prune archived any non-pinned agent-created skill whose activity was older than archive_after_days (90d). A skill loaded only by a cron job had its usage bumped solely when the job fired, so paused jobs, infrequent (quarterly/annual) schedules, and far-future one-shots aged their skills out from under them — the next run then failed to load the now-archived skill. - cron/jobs.py: add referenced_skill_names() returning skills used by ANY job (incl. paused/disabled). - curator.apply_automatic_transitions(): skip cron-referenced skills like pinned; add a use=0 grace floor so a never-used skill is not marked stale/archived until it is at least stale_after_days old. - LLM review pass: candidate list marks cron=yes; prompt forbids pruning cron-referenced skills and never-used skills under 30 days. Tested E2E against a real cron job + real usage records and with 4 new unit tests.
This commit is contained in:
parent
28097d9cd9
commit
4c2961c511
3 changed files with 168 additions and 2 deletions
|
|
@ -273,6 +273,21 @@ def should_run_now(now: Optional[datetime] = None) -> bool:
|
|||
# Automatic state transitions (pure function, no LLM)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _cron_referenced_skills() -> Set[str]:
|
||||
"""Skill names referenced by any cron job (incl. paused/disabled).
|
||||
|
||||
Best-effort: a cron-module import error or corrupt jobs store must never
|
||||
break the curator, so any failure yields an empty set (no protection,
|
||||
but no crash).
|
||||
"""
|
||||
try:
|
||||
from cron.jobs import referenced_skill_names as _refs
|
||||
return _refs()
|
||||
except Exception as e:
|
||||
logger.debug("Curator could not read cron skill references: %s", e, exc_info=True)
|
||||
return set()
|
||||
|
||||
|
||||
def apply_automatic_transitions(now: Optional[datetime] = None) -> Dict[str, int]:
|
||||
"""Walk every curator-managed skill and move active/stale/archived based on
|
||||
the latest real activity timestamp. Pinned skills are never touched.
|
||||
|
|
@ -292,6 +307,8 @@ def apply_automatic_transitions(now: Optional[datetime] = None) -> Dict[str, int
|
|||
stale_cutoff = now - timedelta(days=get_stale_after_days())
|
||||
archive_cutoff = now - timedelta(days=get_archive_after_days())
|
||||
|
||||
cron_referenced = _cron_referenced_skills()
|
||||
|
||||
counts = {"marked_stale": 0, "archived": 0, "reactivated": 0, "checked": 0, "seeded": 0}
|
||||
|
||||
for row in _u.agent_created_report():
|
||||
|
|
@ -300,6 +317,15 @@ def apply_automatic_transitions(now: Optional[datetime] = None) -> Dict[str, int
|
|||
if row.get("pinned"):
|
||||
continue
|
||||
|
||||
# A skill referenced by any cron job (incl. paused/disabled) is in
|
||||
# use by definition — resuming or the next fire must find it. The
|
||||
# scheduler only bumps usage when a job actually fires, so jobs that
|
||||
# fire less often than archive_after_days, paused jobs, and far-future
|
||||
# one-shots would otherwise have their skills aged out from under
|
||||
# them. Treat referenced skills like pinned: never auto-transition.
|
||||
if name in cron_referenced:
|
||||
continue
|
||||
|
||||
# First sight of a curation-eligible skill with no persisted record
|
||||
# (e.g. a newly-eligible built-in): anchor its clock to now and defer.
|
||||
if not row.get("_persisted", True):
|
||||
|
|
@ -316,6 +342,18 @@ def apply_automatic_transitions(now: Optional[datetime] = None) -> Dict[str, int
|
|||
|
||||
current = row.get("state", _u.STATE_ACTIVE)
|
||||
|
||||
# Never-used skills (use_count == 0) get a grace floor: don't archive
|
||||
# one until it is at least stale_after_days old. A use=0 skill is
|
||||
# absence of evidence, not evidence of staleness — a skill created
|
||||
# recently may simply not have had its trigger come up yet.
|
||||
never_used = int(row.get("use_count", 0) or 0) == 0
|
||||
if never_used and anchor > stale_cutoff:
|
||||
# Younger than the stale window — leave it alone entirely.
|
||||
if current == _u.STATE_STALE:
|
||||
_u.set_state(name, _u.STATE_ACTIVE)
|
||||
counts["reactivated"] += 1
|
||||
continue
|
||||
|
||||
if anchor <= archive_cutoff and current != _u.STATE_ARCHIVED:
|
||||
ok, _msg = _u.archive_skill(name)
|
||||
if ok:
|
||||
|
|
@ -390,10 +428,19 @@ CURATOR_REVIEW_PROMPT = (
|
|||
"back load-bearing UX (slash-command entry points referenced in docs and "
|
||||
"tips) and are filtered out of the candidate list below — never resurrect "
|
||||
"one as an archive or absorb target.\n"
|
||||
"3c. DO NOT archive or prune any skill marked `cron=yes` in the candidate "
|
||||
"list. A cron job depends on it and will fail to load it on its next "
|
||||
"run. You MAY still consolidate it into an umbrella — but only because "
|
||||
"the curator rewrites cron job skill references to follow consolidations; "
|
||||
"never simply prune it.\n"
|
||||
"4. DO NOT use usage counters as a reason to skip consolidation. The "
|
||||
"counters are new and often mostly zero. Judge overlap on CONTENT, "
|
||||
"not on use_count. 'use=0' is not evidence a skill is valuable; it's "
|
||||
"absence of evidence either way.\n"
|
||||
"absence of evidence either way. Corollary: 'use=0' is ALSO not a "
|
||||
"reason to PRUNE a skill. Never archive a never-used skill (use=0) "
|
||||
"unless it is at least 30 days old (check last_activity / created date) "
|
||||
"AND its content is genuinely obsolete or fully absorbed elsewhere — a "
|
||||
"recently-created skill simply may not have had its trigger come up yet.\n"
|
||||
"5. DO NOT reject consolidation on the grounds that 'each skill has "
|
||||
"a distinct trigger'. Pairwise distinctness is the wrong bar. The "
|
||||
"right bar is: 'would a human maintainer write this as N separate "
|
||||
|
|
@ -1413,12 +1460,14 @@ def _render_candidate_list() -> str:
|
|||
rows = skill_usage.agent_created_report()
|
||||
if not rows:
|
||||
return "No agent-created skills to review."
|
||||
cron_referenced = _cron_referenced_skills()
|
||||
lines = [f"Agent-created skills ({len(rows)}):\n"]
|
||||
for r in rows:
|
||||
lines.append(
|
||||
f"- {r['name']} "
|
||||
f"state={r['state']} "
|
||||
f"pinned={'yes' if r.get('pinned') else 'no'} "
|
||||
f"cron={'yes' if r['name'] in cron_referenced else 'no'} "
|
||||
f"activity={r.get('activity_count', 0)} "
|
||||
f"use={r.get('use_count', 0)} "
|
||||
f"view={r.get('view_count', 0)} "
|
||||
|
|
|
|||
31
cron/jobs.py
31
cron/jobs.py
|
|
@ -32,7 +32,7 @@ except ImportError: # pragma: no cover - non-Windows
|
|||
from datetime import datetime, timedelta
|
||||
from pathlib import Path
|
||||
from hermes_constants import get_hermes_home
|
||||
from typing import Optional, Dict, List, Any, Tuple, Union
|
||||
from typing import Optional, Dict, List, Any, Set, Tuple, Union
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
@ -1634,6 +1634,35 @@ def save_job_output(job_id: str, output: str):
|
|||
# Skill reference rewriting (curator integration)
|
||||
# =============================================================================
|
||||
|
||||
def referenced_skill_names() -> Set[str]:
|
||||
"""Return the set of skill names referenced by ANY cron job.
|
||||
|
||||
Includes paused and disabled jobs deliberately: a paused job never
|
||||
fires, so its skills never get a ``bump_use`` from the scheduler, yet
|
||||
resuming it must still find its skills present. The curator uses this
|
||||
set to protect referenced skills from inactivity archival — a skill a
|
||||
live job depends on is "in use" regardless of when it was last loaded.
|
||||
|
||||
Best-effort: a corrupt/unreadable jobs store returns an empty set
|
||||
rather than raising, so a cron issue can never break the curator.
|
||||
"""
|
||||
try:
|
||||
jobs = load_jobs()
|
||||
except Exception:
|
||||
logger.debug("referenced_skill_names: failed to load cron jobs", exc_info=True)
|
||||
return set()
|
||||
|
||||
names: Set[str] = set()
|
||||
for job in jobs:
|
||||
if not isinstance(job, dict):
|
||||
continue
|
||||
for name in _normalize_skill_list(job.get("skill"), job.get("skills")):
|
||||
cleaned = str(name).strip().lstrip("/")
|
||||
if cleaned:
|
||||
names.add(cleaned)
|
||||
return names
|
||||
|
||||
|
||||
def rewrite_skill_refs(
|
||||
consolidated: Optional[Dict[str, str]] = None,
|
||||
pruned: Optional[List[str]] = None,
|
||||
|
|
|
|||
|
|
@ -263,6 +263,94 @@ def test_new_skill_without_last_used_not_immediately_archived(curator_env):
|
|||
assert (skills_dir / "fresh").exists()
|
||||
|
||||
|
||||
def _backdate(u, name: str, days: int, *, use_count: int = 1):
|
||||
"""Write an agent-created usage record whose activity is *days* old."""
|
||||
ts = (datetime.now(timezone.utc) - timedelta(days=days)).isoformat()
|
||||
data = u.load_usage()
|
||||
data[name] = u._empty_record()
|
||||
data[name]["created_by"] = "agent"
|
||||
data[name]["created_at"] = ts
|
||||
data[name]["last_used_at"] = ts if use_count else None
|
||||
data[name]["last_activity_at"] = ts if use_count else None
|
||||
data[name]["use_count"] = use_count
|
||||
u.save_usage(data)
|
||||
|
||||
|
||||
def test_cron_referenced_skill_is_not_archived(curator_env, monkeypatch):
|
||||
"""A skill referenced by a cron job must survive inactivity archival even
|
||||
when its activity is well past archive_after_days. The scheduler only
|
||||
bumps usage when a job fires, so paused / infrequent / far-future jobs
|
||||
would otherwise have their skills aged out from under them."""
|
||||
c = curator_env["curator"]
|
||||
u = curator_env["usage"]
|
||||
skills_dir = curator_env["home"] / "skills"
|
||||
_write_skill(skills_dir, "cron-dep")
|
||||
_write_skill(skills_dir, "orphan")
|
||||
_backdate(u, "cron-dep", 200)
|
||||
_backdate(u, "orphan", 200)
|
||||
|
||||
# Pretend a (paused/infrequent) cron job references "cron-dep" only.
|
||||
monkeypatch.setattr(c, "_cron_referenced_skills", lambda: {"cron-dep"})
|
||||
|
||||
counts = c.apply_automatic_transitions()
|
||||
|
||||
assert u.get_record("cron-dep")["state"] == "active" # protected
|
||||
assert (skills_dir / "cron-dep").exists()
|
||||
assert u.get_record("orphan")["state"] == "archived" # control
|
||||
assert counts["archived"] == 1
|
||||
|
||||
|
||||
def test_unused_skill_not_archived_before_stale_floor(curator_env):
|
||||
"""A never-used skill (use_count == 0) younger than stale_after_days must
|
||||
not be marked stale or archived — absence of use is not evidence of
|
||||
staleness when the skill simply hasn't had its trigger come up yet."""
|
||||
c = curator_env["curator"]
|
||||
u = curator_env["usage"]
|
||||
skills_dir = curator_env["home"] / "skills"
|
||||
_write_skill(skills_dir, "young-unused")
|
||||
_backdate(u, "young-unused", 10, use_count=0) # < 30d stale floor
|
||||
|
||||
counts = c.apply_automatic_transitions()
|
||||
|
||||
assert u.get_record("young-unused")["state"] == "active"
|
||||
assert counts["archived"] == 0
|
||||
assert counts["marked_stale"] == 0
|
||||
|
||||
|
||||
def test_unused_skill_archived_past_archive_window(curator_env):
|
||||
"""The use=0 floor only protects YOUNG skills — a never-used skill older
|
||||
than archive_after_days still archives (no perpetual reprieve)."""
|
||||
c = curator_env["curator"]
|
||||
u = curator_env["usage"]
|
||||
skills_dir = curator_env["home"] / "skills"
|
||||
_write_skill(skills_dir, "old-unused")
|
||||
_backdate(u, "old-unused", 200, use_count=0)
|
||||
|
||||
counts = c.apply_automatic_transitions()
|
||||
|
||||
assert u.get_record("old-unused")["state"] == "archived"
|
||||
assert counts["archived"] == 1
|
||||
|
||||
|
||||
def test_candidate_list_marks_cron_referenced_skills(curator_env, monkeypatch):
|
||||
"""The LLM review candidate list flags cron-referenced skills so the
|
||||
review pass knows not to prune them."""
|
||||
c = curator_env["curator"]
|
||||
u = curator_env["usage"]
|
||||
skills_dir = curator_env["home"] / "skills"
|
||||
_write_skill(skills_dir, "cron-dep")
|
||||
_write_skill(skills_dir, "plain")
|
||||
_backdate(u, "cron-dep", 1)
|
||||
_backdate(u, "plain", 1)
|
||||
monkeypatch.setattr(c, "_cron_referenced_skills", lambda: {"cron-dep"})
|
||||
|
||||
listing = c._render_candidate_list()
|
||||
cron_line = next(l for l in listing.splitlines() if l.startswith("- cron-dep"))
|
||||
plain_line = next(l for l in listing.splitlines() if l.startswith("- plain"))
|
||||
assert "cron=yes" in cron_line
|
||||
assert "cron=no" in plain_line
|
||||
|
||||
|
||||
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."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue