Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 78 additions & 1 deletion agent/prompt_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -1369,6 +1369,77 @@ def _build_snapshot_entry(
# Skills index
# =========================================================================

# Character budget for pinned-skill descriptions in the <available_skills>
# index. Keeps the prompt bounded regardless of how many skills get pinned.
_PINNED_SKILLS_CHAR_BUDGET = 1200


def _skill_usage_epoch() -> int:
"""Return sidecar mtime_ns so cache keys reflect pin-state changes.

Returns 0 when the sidecar does not yet exist.
"""
try:
sidecar = get_hermes_home() / "skills" / ".usage.json"
return sidecar.stat().st_mtime_ns
except FileNotFoundError:
return 0


def _get_pinned_candidates(
skills_by_category: "dict[str, list[tuple[str, str]]]",
) -> "list[tuple[str, str]]":
"""Return skills that should have their descriptions shown in the index.

Source of truth is the skill_usage sidecar (``~/.hermes/skills/.usage.json``),
reached via ``tools/skill_usage.agent_created_report()`` which merges the
on-disk skill list with sidecar fields (``pinned``, ``view_count``,
``use_count``, ``state``, ...). Honours explicit user/agent pin choices.

Only skills with ``pinned=True`` and ``state != archived`` are returned,
ordered by ``activity_count`` descending and trimmed to
``_PINNED_SKILLS_CHAR_BUDGET`` so the description budget stays bounded.

When nothing qualifies, returns ``[]``: all skills appear as names-only.
Non-pinned skills are always visible by name — only their descriptions
are omitted to save tokens.
"""
all_skills: dict[str, tuple[str, str]] = {}
for entries in skills_by_category.values():
for name, desc in entries:
all_skills.setdefault(name, (name, desc))
if not all_skills:
return []

try:
from tools.skill_usage import agent_created_report
rows = agent_created_report()
except Exception:
logger.debug("skill_usage unavailable; no pinned skills will be surfaced", exc_info=True)
return []

pinned_rows = [
r for r in rows
if r.get("pinned") and r.get("state") != "archived"
]
if not pinned_rows:
return []

pinned_rows.sort(key=lambda r: -int(r.get("activity_count") or 0))
result: list[tuple[str, str]] = []
chars = 0
for row in pinned_rows:
entry = all_skills.get(row.get("name") or "")
if entry is None:
continue
cost = len(entry[0]) + len(entry[1]) + 6 # " - name: desc\n"
if chars + cost > _PINNED_SKILLS_CHAR_BUDGET:
break
result.append(entry)
chars += cost
return result


def _parse_skill_file(skill_file: Path) -> tuple[bool, dict, str]:
"""Read a SKILL.md once and return platform compatibility, frontmatter, and description.

Expand Down Expand Up @@ -1486,6 +1557,7 @@ def build_skills_system_prompt(
_platform_hint,
tuple(sorted(disabled)),
tuple(sorted(compact_categories or ())),
_skill_usage_epoch(),
)
with _SKILLS_PROMPT_CACHE_LOCK:
cached = _SKILLS_PROMPT_CACHE.get(cache_key)
Expand Down Expand Up @@ -1644,6 +1716,11 @@ def build_skills_system_prompt(
if not skills_by_category:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this global replacement removes unpinned skill names from the default prompt. Current main intentionally preserves every name after a real capability-loss regression (ee1a744ac; see current agent/prompt_builder.py:1622-1667), and even names-only demotion is opt-in (4d6a133a9). Please re-scope onto that demote-never-hide contract.

result = ""
else:
# Pinned skills show their descriptions; everything else is names-only
# to save tokens while keeping all names visible (demote-never-hide).
pinned = _get_pinned_candidates(skills_by_category)
pinned_names: set[str] = {name for name, _ in pinned}

index_lines = []
for category in sorted(skills_by_category.keys()):
# Deduplicate and sort skills within each category
Expand All @@ -1661,7 +1738,7 @@ def build_skills_system_prompt(
if name in seen:
continue
seen.add(name)
if desc:
if name in pinned_names and desc:
index_lines.append(f" - {name}: {desc}")
else:
index_lines.append(f" - {name}")
Expand Down
124 changes: 118 additions & 6 deletions tests/agent/test_prompt_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,8 +412,9 @@ def test_builds_index_with_skills(self, monkeypatch, tmp_path):
)
result = build_skills_system_prompt()
assert "python-debug" in result
assert "Debug Python scripts" in result
assert "available_skills" in result
# Descriptions are only shown for pinned skills; non-pinned are names-only.
assert "Debug Python scripts" not in result

def test_deduplicates_skills(self, monkeypatch, tmp_path):
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
Expand Down Expand Up @@ -445,8 +446,9 @@ def test_compact_categories_demoted_to_names_only(self, monkeypatch, tmp_path):
result = build_skills_system_prompt(
compact_categories=frozenset({"social-media"})
)
# Coding-adjacent category keeps its full entry.
assert "pr-review" in result and "Does pr-review things" in result
# Coding-adjacent category keeps its name visible (non-pinned = names-only).
assert "pr-review" in result
assert "Does pr-review things" not in result
# Demoted category: name stays visible, description is dropped.
assert "tweet-stuff" in result
assert "Does tweet-stuff things" not in result
Expand All @@ -470,9 +472,11 @@ def test_compact_categories_demote_nested_and_miss_cache_separately(
)
assert "thread-writer" in compact
assert "Write threads" not in compact
# Unfiltered call must not be served from the compacted cache entry.
# Unfiltered call must not be served from the compacted cache entry;
# name visible but description absent (non-pinned).
full = build_skills_system_prompt()
assert "Write threads" in full
assert "thread-writer" in full
assert "Write threads" not in full

def test_excludes_incompatible_platform_skills(self, monkeypatch, tmp_path):
"""Skills with platforms: [macos] should not appear on Linux."""
Expand Down Expand Up @@ -519,8 +523,9 @@ def test_includes_matching_platform_skills(self, monkeypatch, tmp_path):
mock_sys.platform = "darwin"
result = build_skills_system_prompt()

# Skill name visible; description absent (non-pinned = names-only).
assert "imessage" in result
assert "Send iMessages" in result
assert "Send iMessages" not in result

def test_excludes_disabled_skills(self, monkeypatch, tmp_path):
"""Skills in the user's disabled list should not appear in the system prompt."""
Expand Down Expand Up @@ -626,6 +631,113 @@ def test_non_local_backend_keeps_skill_visible_without_probe(
assert "backend-skill" in result


class TestPinnedSkillsInIndex:
"""Pinned skills show descriptions inline; non-pinned are names-only."""

@pytest.fixture(autouse=True)
def _clear_skills_cache(self):
from agent.prompt_builder import clear_skills_system_prompt_cache
clear_skills_system_prompt_cache(clear_snapshot=True)
yield
clear_skills_system_prompt_cache(clear_snapshot=True)

def test_unpinned_skills_show_names_only(self, monkeypatch, tmp_path):
"""Non-pinned skill names appear in the index but descriptions are omitted."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
d = tmp_path / "skills" / "tools" / "plain-skill"
d.mkdir(parents=True)
(d / "SKILL.md").write_text(
"---\nname: plain-skill\ndescription: Should not appear\n---\n"
)

from unittest.mock import patch

with patch("tools.skill_usage.agent_created_report", return_value=[]):
result = build_skills_system_prompt()

assert "plain-skill" in result
assert "Should not appear" not in result

def test_pinned_skill_shows_description_inline(self, monkeypatch, tmp_path):
"""A pinned skill's description appears inline in <available_skills>."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
pinned_dir = tmp_path / "skills" / "tools" / "my-pinned"
pinned_dir.mkdir(parents=True)
(pinned_dir / "SKILL.md").write_text(
"---\nname: my-pinned\ndescription: Pinned skill desc\n---\n"
)
other_dir = tmp_path / "skills" / "tools" / "other-skill"
other_dir.mkdir(parents=True)
(other_dir / "SKILL.md").write_text(
"---\nname: other-skill\ndescription: Not pinned desc\n---\n"
)

from unittest.mock import patch

fake_report = [
{"name": "my-pinned", "pinned": True, "state": "active", "activity_count": 10},
{"name": "other-skill", "pinned": False, "state": "active", "activity_count": 99},
]
with patch("tools.skill_usage.agent_created_report", return_value=fake_report):
result = build_skills_system_prompt()

# Pinned skill: description shown.
assert "my-pinned: Pinned skill desc" in result
# Non-pinned skill: name only, description absent.
assert "other-skill" in result
assert "Not pinned desc" not in result

def test_pin_change_invalidates_prompt_cache(self, monkeypatch, tmp_path):
"""Toggling pinned state in the sidecar surfaces on the next build."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
d = tmp_path / "skills" / "tools" / "togglable"
d.mkdir(parents=True)
(d / "SKILL.md").write_text(
"---\nname: togglable\ndescription: A skill desc\n---\n"
)

from unittest.mock import patch

# First build: no pins → description absent.
with patch("tools.skill_usage.agent_created_report", return_value=[]):
first = build_skills_system_prompt()
assert "togglable" in first
assert "A skill desc" not in first

# Sidecar rewritten (bumps mtime → cache key differs).
sidecar = tmp_path / "skills" / ".usage.json"
sidecar.parent.mkdir(parents=True, exist_ok=True)
sidecar.write_text("{}")
pinned_report = [
{"name": "togglable", "pinned": True, "state": "active", "activity_count": 1}
]
with patch("tools.skill_usage.agent_created_report", return_value=pinned_report):
second = build_skills_system_prompt()

assert "togglable: A skill desc" in second

def test_archived_pinned_skill_description_hidden(self, monkeypatch, tmp_path):
"""Archived skills are excluded from pinned candidates."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
d = tmp_path / "skills" / "tools" / "archived-skill"
d.mkdir(parents=True)
(d / "SKILL.md").write_text(
"---\nname: archived-skill\ndescription: Old desc\n---\n"
)

from unittest.mock import patch

archived_report = [
{"name": "archived-skill", "pinned": True, "state": "archived", "activity_count": 5}
]
with patch("tools.skill_usage.agent_created_report", return_value=archived_report):
result = build_skills_system_prompt()

# Archived skills are excluded from pinned candidates → names-only.
assert "archived-skill" in result
assert "Old desc" not in result


class TestBuildNousSubscriptionPrompt:
def test_includes_active_subscription_features(self, monkeypatch):
monkeypatch.setattr("tools.tool_backend_helpers.managed_nous_tools_enabled", lambda: True)
Expand Down