mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-24 16:54:43 +00:00
fix(skills): scope 60-char description enforcement to the create path
The blanket MAX_DESCRIPTION_LENGTH=1024->60 change is narrowed: create-time validation now rejects new skills whose description exceeds SKILL_PROMPT_DESC_LIMIT (60) with actionable guidance, while edit/patch paths stay permissive (warning via system_prompt_preview) so existing over-limit skills remain maintainable. Runtime display truncation in skills_tool is left at 1024 (display behavior is a separate concern from authoring validation). Boundary tests: 60 accepted, 61 rejected at create; edit/patch on over-budget skills still succeed.
This commit is contained in:
parent
0a262b7dbf
commit
8611b69dad
3 changed files with 37 additions and 13 deletions
|
|
@ -275,14 +275,11 @@ class TestCreateSkill:
|
|||
assert f"Invalid category '{outside}'" in result["error"]
|
||||
assert not (outside / "my-skill" / "SKILL.md").exists()
|
||||
|
||||
def test_create_long_desc_includes_prompt_preview(self, tmp_path):
|
||||
def test_create_long_desc_rejected(self, tmp_path):
|
||||
with _skill_dir(tmp_path):
|
||||
result = _create_skill("long-desc", LONG_DESC_CONTENT)
|
||||
assert result["success"] is True
|
||||
assert "system_prompt_preview" in result
|
||||
assert "System prompt will show" in result["system_prompt_preview"]
|
||||
fm, _ = parse_frontmatter(LONG_DESC_CONTENT)
|
||||
assert extract_skill_description(fm) in result["system_prompt_preview"]
|
||||
assert result["success"] is False
|
||||
assert "system-prompt budget" in result["error"]
|
||||
|
||||
def test_create_short_desc_no_prompt_preview(self, tmp_path):
|
||||
with _skill_dir(tmp_path):
|
||||
|
|
@ -290,7 +287,7 @@ class TestCreateSkill:
|
|||
assert result["success"] is True
|
||||
assert "system_prompt_preview" not in result
|
||||
|
||||
def test_create_boundary_at_limit_no_preview(self, tmp_path):
|
||||
def test_create_boundary_at_limit_accepted_no_preview(self, tmp_path):
|
||||
desc = "U" * SKILL_PROMPT_DESC_LIMIT
|
||||
content = f"---\nname: boundary-at\ndescription: {desc}\n---\n\n# Boundary\n\nStep 1.\n"
|
||||
with _skill_dir(tmp_path):
|
||||
|
|
@ -298,13 +295,25 @@ class TestCreateSkill:
|
|||
assert result["success"] is True
|
||||
assert "system_prompt_preview" not in result
|
||||
|
||||
def test_create_boundary_over_limit_has_preview(self, tmp_path):
|
||||
def test_create_boundary_over_limit_rejected(self, tmp_path):
|
||||
desc = "U" * (SKILL_PROMPT_DESC_LIMIT + 1)
|
||||
content = f"---\nname: boundary-over\ndescription: {desc}\n---\n\n# Boundary\n\nStep 1.\n"
|
||||
with _skill_dir(tmp_path):
|
||||
result = _create_skill("boundary-over", content)
|
||||
assert result["success"] is False
|
||||
assert "system-prompt budget" in result["error"]
|
||||
|
||||
def test_edit_long_desc_still_allowed_with_preview(self, tmp_path):
|
||||
"""Edit/patch paths stay permissive so existing over-limit skills
|
||||
remain maintainable — they warn via system_prompt_preview instead."""
|
||||
with _skill_dir(tmp_path):
|
||||
_create_skill("my-skill", VALID_SKILL_CONTENT)
|
||||
result = _edit_skill("my-skill", LONG_DESC_CONTENT)
|
||||
assert result["success"] is True
|
||||
assert "system_prompt_preview" in result
|
||||
assert "System prompt will show" in result["system_prompt_preview"]
|
||||
fm, _ = parse_frontmatter(LONG_DESC_CONTENT)
|
||||
assert extract_skill_description(fm) in result["system_prompt_preview"]
|
||||
|
||||
|
||||
class TestEditSkill:
|
||||
|
|
|
|||
|
|
@ -174,7 +174,7 @@ def _skills_dir() -> Path:
|
|||
return get_hermes_home() / "skills"
|
||||
|
||||
MAX_NAME_LENGTH = 64
|
||||
MAX_DESCRIPTION_LENGTH = 60
|
||||
MAX_DESCRIPTION_LENGTH = 1024
|
||||
|
||||
|
||||
def _containing_skills_root(skill_path: Path) -> Path:
|
||||
|
|
@ -544,10 +544,16 @@ def _validate_category(category: Optional[str]) -> Optional[str]:
|
|||
return None
|
||||
|
||||
|
||||
def _validate_frontmatter(content: str) -> Optional[str]:
|
||||
def _validate_frontmatter(content: str, *, new_skill: bool = False) -> Optional[str]:
|
||||
"""
|
||||
Validate that SKILL.md content has proper frontmatter with required fields.
|
||||
Returns error message or None if valid.
|
||||
|
||||
When ``new_skill`` is True (create path only), the description must also
|
||||
fit the 60-char system-prompt budget (SKILL_PROMPT_DESC_LIMIT) so newly
|
||||
authored skills never lose routing signal to index truncation. Edit and
|
||||
patch paths deliberately skip this so existing over-limit skills remain
|
||||
maintainable while their descriptions are cleaned up.
|
||||
"""
|
||||
if not content.strip():
|
||||
return "Content cannot be empty."
|
||||
|
|
@ -576,8 +582,17 @@ def _validate_frontmatter(content: str) -> Optional[str]:
|
|||
return "Frontmatter must include 'name' field."
|
||||
if "description" not in parsed:
|
||||
return "Frontmatter must include 'description' field."
|
||||
if len(str(parsed["description"])) > MAX_DESCRIPTION_LENGTH:
|
||||
desc = str(parsed["description"])
|
||||
if len(desc) > MAX_DESCRIPTION_LENGTH:
|
||||
return f"Description exceeds {MAX_DESCRIPTION_LENGTH} characters."
|
||||
if new_skill and len(desc.strip().strip("'\"")) > SKILL_PROMPT_DESC_LIMIT:
|
||||
return (
|
||||
f"Description is {len(desc.strip())} chars — new skills must fit the "
|
||||
f"{SKILL_PROMPT_DESC_LIMIT}-char system-prompt budget (one sentence, "
|
||||
f"trigger first, ends with a period). The skill index truncates "
|
||||
f"longer descriptions to {SKILL_PROMPT_DESC_LIMIT - 3} chars + '...', "
|
||||
f"destroying the routing signal. Move detail into the skill body."
|
||||
)
|
||||
|
||||
body = content[end_match.end() + 3:].strip()
|
||||
if not body:
|
||||
|
|
@ -840,7 +855,7 @@ def _create_skill(name: str, content: str, category: str = None) -> Dict[str, An
|
|||
return {"success": False, "error": err}
|
||||
|
||||
# Validate content
|
||||
err = _validate_frontmatter(content)
|
||||
err = _validate_frontmatter(content, new_skill=True)
|
||||
if err:
|
||||
return {"success": False, "error": err}
|
||||
|
||||
|
|
|
|||
|
|
@ -160,7 +160,7 @@ def _skills_dir() -> Path:
|
|||
|
||||
# Anthropic-recommended limits for progressive disclosure efficiency
|
||||
MAX_NAME_LENGTH = 64
|
||||
MAX_DESCRIPTION_LENGTH = 60
|
||||
MAX_DESCRIPTION_LENGTH = 1024
|
||||
|
||||
# Platform identifiers for the 'platforms' frontmatter field.
|
||||
# Maps user-friendly names to sys.platform prefixes.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue