Skill usage counter never called, curator prunes active skills

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #17932 fix(skills): wire bump_use() into skill invocation, preload, and skill_view tool (#17782 salvage)

  merged to main

  files: agent/skill_commands.py, tools/skills_tool.py

• #19433 fix(cron): bump skill usage when cron jobs load skills

  merged to main

  files: cron/scheduler.py, tests/cron/test_scheduler.py

  Show the diff

  • cron/scheduler.py

    @@ -782,6 +782,7 @@ def _build_job_prompt(job: dict, prerun_script: Optional[tuple] = None) -> str:
             return prompt
     
         from tools.skills_tool import skill_view
    +    from tools.skill_usage import bump_use
     
         parts = []
         skipped: list[str] = []
    @@ -793,6 +794,12 @@ def _build_job_prompt(job: dict, prerun_script: Optional[tuple] = None) -> str:
                 skipped.append(skill_name)
                 continue
     
    +        # Bump usage so the curator sees this skill as actively used.
    +        try:
    +            bump_use(skill_name)
    +        except Exception:
    +            logger.debug("Cron job: failed to bump skill usage for '%s'", skill_name, exc_info=True)
    +
             content = str(loaded.get("content") or "").strip()
             if parts:
                 parts.append("")

  • tests/cron/test_scheduler.py

    @@ -1857,6 +1857,54 @@ def _mixed_skill_view(name: str) -> str:
             assert "go" in result
     
     
    +class TestBuildJobPromptBumpUse:
    +    """Verify that cron jobs bump skill usage counters so the curator sees them as active."""
    +
    +    def test_bump_use_called_for_loaded_skill(self):
    +        """bump_use is called for each successfully loaded skill."""
    +
    +        def _skill_view(name: str) -> str:
    +            return json.dumps({"success": True, "content": f"Content for {name}."})
    +
    +        with patch("tools.skills_tool.skill_view", side_effect=_skill_view), \
    +             patch("tools.skill_usage.bump_use") as mock_bump:
    +            _build_job_prompt({"skills": ["alpha", "beta"], "prompt": "go"})
    +
    +        assert mock_bump.call_count == 2
    +        calls = [c[0][0] for c in mock_bump.call_args_list]
    +        assert "alpha" in calls
    +        assert "beta" in calls
    +
    +    def test_bump_use_not_called_for_missing_skill(self):
    +        """bump_use is NOT called when a skill fails to load."""
    +
    +        def _missing_view(name: str) -> str:
    +            return json.dumps({"success": False, "error": "not found"})
    +
    +        with patch("tools.skills_tool.skill_view", side_effect=_missing_view), \
    +             patch("tools.skill_usage.bump_use") as mock_bump:
    +            _build_job_prompt({"skills": ["ghost"], "prompt": "go"})
    +
    +        assert mock_bump.call_count == 0
    +
    +    def test_bump_failure_does_not_break_prompt(self, caplog):
    +        """If bump_use raises, the prompt still builds — error is logged at DEBUG."""
    +
    +        def _skill_view(name: str) -> str:
    +            return json.dumps({"success": True, "content": "Works."})
    +
    +        with patch("tools.skills_tool.skill_view", side_effect=_skill_view), \
    +             patch("tools.skill_usage.bump_use", side_effect=RuntimeError("boom")), \
    +             caplog.at_level(logging.DEBUG, logger="cron.scheduler"):
    +            result = _build_job_prompt({"skills": ["good-skill"], "prompt": "go"})
    +
    +        # Prompt should still contain the skill content and original instruction
    +        assert "Works." in result
    +        assert "go" in result
    +        # The error should be logged at DEBUG level, not crash
    +        assert any("failed to bump" in r.message for r in caplog.records)
    +
    +
     class TestSendMediaViaAdapter:
         """Unit tests for _send_media_via_adapter — routes files to typed adapter methods."""
     

• #54443 fix(curator): never archive cron-referenced skills + floor use=0 pruning

  merged to main

  files: agent/curator.py, cron/jobs.py, tests/agent/test_curator.py

  Show the diff

  • agent/curator.py

    @@ -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 @@ def apply_automatic_transitions(now: Optional[datetime] = None) -> Dict[str, int
         "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)}  "

  • cron/jobs.py

    @@ -32,7 +32,7 @@
     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,

  • tests/agent/test_curator.py

    @@ -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."""