diff --git a/tests/tools/test_skill_manager_tool.py b/tests/tools/test_skill_manager_tool.py index fad2f682c7b2..397133ed93cf 100644 --- a/tests/tools/test_skill_manager_tool.py +++ b/tests/tools/test_skill_manager_tool.py @@ -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: diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index d1299783e724..88d7e47e17ae 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -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} diff --git a/tools/skills_tool.py b/tools/skills_tool.py index fc4aa3763b6b..a5613f62c4c8 100644 --- a/tools/skills_tool.py +++ b/tools/skills_tool.py @@ -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.