common/hermes
2
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

refactor(reload-skills): queue note for next turn, drop cache invalidation + agent tool

Browse Source 
Salvage-follow-up to @shannonsands's /reload-skills PR. Trims the feature to
match the design: user-initiated rescan, no prompt-cache reset, no new
schema surface, no phantom user turn, and the next-turn note carries each
added/removed skill's 60-char description (not just its name).

Changes vs the original PR:

* Drop the in-process skills prompt-cache clear in reload_skills(). Skills
  are invoked at runtime via /skill-name, skills_list, or skill_view —
  they don't need to live in the system prompt for the model to use them.
  Keeping the cache intact preserves prefix caching across the reload so
  /reload-skills pays no cache-reset cost. (MCP has to break the cache
  because tool schemas must be known at conversation start; skills do not.)

* Drop the skills_reload agent tool and SKILLS_RELOAD_SCHEMA from
  tools/skills_tool.py, plus the four skills_reload enumerations in
  toolsets.py. No new schema surface — agents can already see a freshly-
  installed skill via skill_view / skills_list the moment it's on disk.

* Replace the phantom 'role: user' turn injection with a one-shot queued
  note. CLI uses self._pending_skills_reload_note (same pattern as
  _pending_model_switch_note, prepended to the next API call and cleared).
  Gateway uses self._pending_skills_reload_notes[session_key]. The note
  is prepended to the NEXT real user message in this session, so message
  alternation stays intact and nothing out-of-band is persisted to the
  transcript.

* reload_skills() now returns added/removed as
  [{'name': str, 'description': str}, ...] (description truncated to 60
  chars — matches the curator / gateway adapter budget). The injected
  next-turn note formats each entry as 'name — description' so the model
  can actually reason about which new skills to call without running
  skills_list first.

* Only emit the note when the diff is non-empty. On empty diff, print
  'No new skills detected' and do nothing else.

* Tests rewritten to cover the queue semantics, the description payload,
  and a regression guard that the prompt-cache snapshot is preserved.
This commit is contained in:
teknium1
2026-04-29 20:39:15 -07:00
committed by Teknium
parent 7966560fb5
commit dd2d1ba5e6
8 changed files with 304 additions and 277 deletions
Download Patch File Download Diff File Expand all files Collapse all files

114
tests/agent/test_skill_commands_reload.py
View File

@@ -1,11 +1,15 @@
"""Tests for ``agent.skill_commands.reload_skills`` and the ``skills_reload`` tool.
"""Tests for ``agent.skill_commands.reload_skills``.
Covers the helper that powers ``/reload-skills`` (CLI + gateway slash command)
and the ``skills_reload`` agent tool — both clear in-process skill caches and
return a diff of newly-visible / removed skill names.
Covers the helper that powers ``/reload-skills`` (CLI + gateway slash command).
The helper rescans the skills directory and returns a diff of what changed.
It does NOT invalidate the skills system-prompt cache — skills are invoked
at runtime via ``/skill-name``, ``skills_list``, or ``skill_view`` and don't
need to live in the system prompt.
``added`` and ``removed`` are lists of ``{"name": str, "description": str}``
dicts. Descriptions are truncated to 60 chars.
"""
import json
import shutil
import tempfile
import textwrap
@@ -72,49 +76,53 @@ class TestReloadSkillsHelper:
assert result["added"] == []
assert result["removed"] == []
def test_detects_newly_added_skill(self, hermes_home):
def test_detects_newly_added_skill_with_description(self, hermes_home):
from agent.skill_commands import reload_skills, get_skill_commands
# Prime the cache so subsequent diff is meaningful
get_skill_commands()
_write_skill(hermes_home / "skills", "demo")
_write_skill(hermes_home / "skills", "demo", "a demo skill")
result = reload_skills()
assert result["added"] == ["demo"]
assert result["added"] == [{"name": "demo", "description": "a demo skill"}]
assert result["removed"] == []
assert result["total"] == 1
assert result["commands"] == 1
def test_detects_removed_skill(self, hermes_home):
def test_detects_removed_skill_carries_description(self, hermes_home):
from agent.skill_commands import reload_skills
skill_dir = _write_skill(hermes_home / "skills", "demo")
skill_dir = _write_skill(hermes_home / "skills", "demo", "soon to be gone")
# First reload: demo present
first = reload_skills()
assert first["total"] == 1
assert first["added"] == [{"name": "demo", "description": "soon to be gone"}]
# Remove and reload
# Remove and reload — the description must survive the removal diff
# (we cached it from the pre-rescan snapshot).
shutil.rmtree(skill_dir)
second = reload_skills()
assert second["removed"] == ["demo"]
assert second["removed"] == [{"name": "demo", "description": "soon to be gone"}]
assert second["added"] == []
assert second["total"] == 0
def test_clears_prompt_cache_snapshot(self, hermes_home):
"""The disk snapshot at ``.skills_prompt_snapshot.json`` must be removed."""
from agent.prompt_builder import _skills_prompt_snapshot_path
from agent.skill_commands import reload_skills
def test_description_passes_through_verbatim(self, hermes_home):
"""``description`` must be the full SKILL.md frontmatter string — no
truncation. The system prompt renders skills as
`` - name: description`` without a length cap, and the reload
note mirrors that format, so truncating here would make the diff
render differently from the original catalog."""
from agent.skill_commands import reload_skills, get_skill_commands
snapshot = _skills_prompt_snapshot_path()
snapshot.parent.mkdir(parents=True, exist_ok=True)
snapshot.write_text("{}")
assert snapshot.exists()
get_skill_commands() # prime
long_desc = "x" * 200
_write_skill(hermes_home / "skills", "longdesc", long_desc)
reload_skills()
assert not snapshot.exists(), "prompt cache snapshot should be removed"
result = reload_skills()
assert len(result["added"]) == 1
assert result["added"][0]["description"] == long_desc
def test_unchanged_skills_appear_in_unchanged_list(self, hermes_home):
from agent.skill_commands import reload_skills, get_skill_commands
@@ -129,50 +137,24 @@ class TestReloadSkillsHelper:
assert result["added"] == []
assert result["removed"] == []
def test_does_not_invalidate_prompt_cache_snapshot(self, hermes_home):
"""reload_skills must NOT delete the skills prompt-cache snapshot.
class TestSkillsReloadTool:
"""``tools.skills_tool.skills_reload`` — the agent-facing tool."""
Skills are called at runtime — the system prompt doesn't need to
mention them for the model to use them — so reloading them should
preserve prefix caching.
"""
from agent.prompt_builder import _skills_prompt_snapshot_path
from agent.skill_commands import reload_skills
def test_tool_returns_json(self, hermes_home):
from tools.skills_tool import skills_reload
snapshot = _skills_prompt_snapshot_path()
snapshot.parent.mkdir(parents=True, exist_ok=True)
snapshot.write_text("{}")
assert snapshot.exists()
out = skills_reload()
result = json.loads(out)
assert result["success"] is True
assert set(result) == {
"success",
"added",
"removed",
"unchanged_count",
"total",
"commands",
}
reload_skills()
def test_tool_reports_added_skill(self, hermes_home):
from agent.skill_commands import get_skill_commands
from tools.skills_tool import skills_reload
get_skill_commands() # prime cache
_write_skill(hermes_home / "skills", "freshly-added", "fresh skill")
result = json.loads(skills_reload())
assert result["success"] is True
assert result["added"] == ["freshly-added"]
assert result["total"] == 1
def test_tool_is_registered_in_skills_toolset(self, hermes_home):
# Importing the module triggers registry.register
import tools.skills_tool # noqa: F401
from tools.registry import registry
assert "skills_reload" in registry.get_tool_names_for_toolset("skills")
assert registry.get_toolset_for_tool("skills_reload") == "skills"
def test_tool_schema_has_no_required_args(self, hermes_home):
import tools.skills_tool # noqa: F401
from tools.registry import registry
schema = registry.get_schema("skills_reload")
assert schema["name"] == "skills_reload"
# Caller invokes with no arguments; tool returns the diff verbatim.
assert schema["parameters"].get("required", []) == []
assert snapshot.exists(), (
"prompt cache snapshot should be preserved — skills don't live "
"in the system prompt so there's no reason to invalidate it"
)

60
tests/cli/test_cli_reload_skills.py
View File

@@ -1,6 +1,14 @@
"""Tests for the ``/reload-skills`` CLI slash command (``HermesCLI._reload_skills``)."""
"""Tests for the ``/reload-skills`` CLI slash command (``HermesCLI._reload_skills``).
from unittest.mock import MagicMock, patch
The CLI handler prints the diff (name + description) for the user and —
when any skills were added or removed — queues a one-shot note on
``self._pending_skills_reload_note``. The note is prepended to the NEXT
user message (see cli.py ~L8770, same pattern as
``_pending_model_switch_note``) and cleared after use, so no phantom user
turn is persisted to ``conversation_history``.
"""
from unittest.mock import patch
def _make_cli():
@@ -15,13 +23,18 @@ def _make_cli():
class TestReloadSkillsCLI:
def test_reports_added_and_removed(self, capsys):
def test_reports_added_and_removed_and_queues_note(self, capsys):
cli = _make_cli()
with patch(
"agent.skill_commands.reload_skills",
return_value={
"added": ["alpha", "beta"],
"removed": ["gamma"],
"added": [
{"name": "alpha", "description": "Run alpha to do xyz"},
{"name": "beta", "description": "Run beta to do abc"},
],
"removed": [
{"name": "gamma", "description": "Old removed skill"},
],
"unchanged": ["delta"],
"total": 3,
"commands": 3,
@@ -30,19 +43,28 @@ class TestReloadSkillsCLI:
cli._reload_skills()
out = capsys.readouterr().out
assert "Added: alpha, beta" in out
assert "Removed: gamma" in out
assert "Added Skills:" in out
assert "- alpha: Run alpha to do xyz" in out
assert "- beta: Run beta to do abc" in out
assert "Removed Skills:" in out
assert "- gamma: Old removed skill" in out
assert "3 skill(s) available" in out
# An informational message should be appended to the conversation
# so the model picks up the diff on the next turn.
assert len(cli.conversation_history) == 1
msg = cli.conversation_history[0]
assert msg["role"] == "user"
assert "Skills have been reloaded" in msg["content"]
assert "alpha" in msg["content"]
assert "gamma" in msg["content"]
def test_reports_no_changes(self, capsys):
# Must NOT pollute conversation_history — alternation-safe.
assert cli.conversation_history == []
# One-shot note queued with system-prompt-style formatting.
note = getattr(cli, "_pending_skills_reload_note", None)
assert note is not None
assert note.startswith("[USER INITIATED SKILLS RELOAD:")
assert note.endswith("Use skills_list to see the updated catalog.]")
assert "Added Skills:" in note
assert " - alpha: Run alpha to do xyz" in note
assert " - beta: Run beta to do abc" in note
assert "Removed Skills:" in note
assert " - gamma: Old removed skill" in note
def test_reports_no_changes_and_queues_nothing(self, capsys):
cli = _make_cli()
with patch(
"agent.skill_commands.reload_skills",
@@ -57,10 +79,10 @@ class TestReloadSkillsCLI:
cli._reload_skills()
out = capsys.readouterr().out
assert "No changes detected" in out
assert "No new skills detected" in out
assert "1 skill(s) available" in out
# Nothing changed — don't pollute history.
assert cli.conversation_history == []
assert getattr(cli, "_pending_skills_reload_note", None) is None
def test_handles_reload_failure_gracefully(self, capsys):
cli = _make_cli()
@@ -73,5 +95,5 @@ class TestReloadSkillsCLI:
out = capsys.readouterr().out
assert "Skills reload failed" in out
assert "boom" in out
# Failure must not append a misleading "skills reloaded" note.
assert cli.conversation_history == []
assert getattr(cli, "_pending_skills_reload_note", None) is None

72
tests/gateway/test_reload_skills_command.py
View File

@@ -1,10 +1,16 @@
"""Tests for the ``/reload-skills`` gateway slash command handler.
Verifies the gateway path that mirrors ``/reload-mcp``:
Verifies:
* dispatcher routes ``/reload-skills`` to ``_handle_reload_skills_command``
* the underscored alias ``/reload_skills`` is not flagged as unknown
* the handler invokes ``agent.skill_commands.reload_skills`` and renders a
human-readable diff
* when any skills changed, a one-shot note is queued on
``runner._pending_skills_reload_notes[session_key]`` (the agent loop
consumes and clears it on the next user turn — see ``gateway/run.py``
near the ``_has_fresh_tool_tail`` block)
* the handler does NOT append to the session transcript out-of-band —
message alternation must not be broken by a phantom user turn
"""
from datetime import datetime
@@ -75,48 +81,66 @@ def _make_runner():
runner._is_user_authorized = lambda _source: True
runner._set_session_env = lambda _context: None
runner._should_send_voice_reply = lambda *_args, **_kwargs: False
# Use the real _session_key_for_source binding so the key matches what
# the agent-loop consumer will look up later.
from gateway.run import GatewayRunner as _GR
runner._session_key_for_source = _GR._session_key_for_source.__get__(runner, _GR)
return runner
@pytest.mark.asyncio
async def test_reload_skills_handler_renders_added_and_removed(monkeypatch):
"""The handler should call ``reload_skills`` and surface the diff."""
import gateway.run as gateway_run
async def test_reload_skills_handler_queues_note_on_diff(monkeypatch):
"""Diff non-empty → handler queues a one-shot note and does NOT touch transcript."""
fake_result = {
"added": ["alpha", "beta"],
"removed": ["gamma"],
"added": [
{"name": "alpha", "description": "Run alpha to do xyz"},
{"name": "beta", "description": "Run beta to do abc"},
],
"removed": [
{"name": "gamma", "description": "Old removed skill"},
],
"unchanged": ["delta"],
"total": 3,
"commands": 3,
}
def _fake_reload_skills():
return fake_result
# Patch the symbol where ``_handle_reload_skills_command`` imports it from.
import agent.skill_commands as skill_commands_mod
monkeypatch.setattr(skill_commands_mod, "reload_skills", _fake_reload_skills)
monkeypatch.setattr(skill_commands_mod, "reload_skills", lambda: fake_result)
runner = _make_runner()
out = await runner._handle_reload_skills_command(_make_event("/reload-skills"))
event = _make_event("/reload-skills")
out = await runner._handle_reload_skills_command(event)
assert out is not None
assert "Skills Reloaded" in out
assert "alpha" in out and "beta" in out
assert "gamma" in out
assert "Added Skills:" in out
assert "- alpha: Run alpha to do xyz" in out
assert "- beta: Run beta to do abc" in out
assert "Removed Skills:" in out
assert "- gamma: Old removed skill" in out
assert "3 skill(s) available" in out
# A history note should be appended so the model sees the diff next turn.
runner.session_store.append_to_transcript.assert_called_once()
appended = runner.session_store.append_to_transcript.call_args[0][1]
assert appended["role"] == "user"
assert "Skills have been reloaded" in appended["content"]
# MUST NOT write to the session transcript — that would break alternation.
runner.session_store.append_to_transcript.assert_not_called()
# MUST have queued a one-shot note keyed on the session.
pending = getattr(runner, "_pending_skills_reload_notes", None)
assert pending is not None
session_key = runner._session_key_for_source(event.source)
assert session_key in pending
note = pending[session_key]
assert note.startswith("[USER INITIATED SKILLS RELOAD:")
assert note.endswith("Use skills_list to see the updated catalog.]")
assert "Added Skills:" in note
assert " - alpha: Run alpha to do xyz" in note
assert " - beta: Run beta to do abc" in note
assert "Removed Skills:" in note
assert " - gamma: Old removed skill" in note
@pytest.mark.asyncio
async def test_reload_skills_handler_reports_no_changes(monkeypatch):
"""When nothing changed, the handler should say so without injecting a note."""
"""No diff → no queued note, no transcript write."""
import agent.skill_commands as skill_commands_mod
monkeypatch.setattr(
@@ -134,10 +158,12 @@ async def test_reload_skills_handler_reports_no_changes(monkeypatch):
runner = _make_runner()
out = await runner._handle_reload_skills_command(_make_event("/reload-skills"))
assert "No changes detected" in out
assert "No new skills detected" in out
assert "1 skill(s) available" in out
# No history note when nothing changed — preserves prompt cache.
runner.session_store.append_to_transcript.assert_not_called()
# No queued note when nothing changed.
pending = getattr(runner, "_pending_skills_reload_notes", None)
assert not pending # None or empty dict
@pytest.mark.asyncio