From e0a5ab2821731338e427d2cc2940f834ad6e462d Mon Sep 17 00:00:00 2001 From: Teknium Date: Tue, 31 Mar 2026 19:33:16 -0700 Subject: [PATCH] feat(skills): add content size limits for agent-created skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent writes via skill_manage (create/edit/patch/write_file) are now constrained to prevent unbounded growth: - SKILL.md and supporting files: 100,000 character limit - Supporting files: additional 1 MiB byte limit - Patches on oversized hand-placed skills that reduce the size are allowed (shrink path), but patches that grow beyond the limit are rejected Hand-placed skills and hub-installed skills have NO hard limit — they load and function normally regardless of size. Hub installs get a warning in the log if SKILL.md exceeds 100k chars. This mirrors the memory system's char_limit pattern. Without this, the agent auto-grows skills indefinitely through iterative patches (hermes-agent-dev reached 197k chars / 72k tokens — 40x larger than the largest skill in the entire skills.sh ecosystem). Constants: MAX_SKILL_CONTENT_CHARS (100k), MAX_SKILL_FILE_BYTES (1MiB) Tests: 14 new tests covering all write paths and edge cases --- tests/tools/test_skill_size_limits.py | 215 ++++++++++++++++++++++++++ tools/skill_manager_tool.py | 46 ++++++ tools/skills_hub.py | 16 ++ 3 files changed, 277 insertions(+) create mode 100644 tests/tools/test_skill_size_limits.py diff --git a/tests/tools/test_skill_size_limits.py b/tests/tools/test_skill_size_limits.py new file mode 100644 index 000000000000..c94ba02e81da --- /dev/null +++ b/tests/tools/test_skill_size_limits.py @@ -0,0 +1,215 @@ +"""Tests for skill content size limits. + +Agent writes (create/edit/patch/write_file) are constrained to +MAX_SKILL_CONTENT_CHARS (100k) and MAX_SKILL_FILE_BYTES (1 MiB). +Hand-placed and hub-installed skills have no hard limit. +""" + +import json +import os +from pathlib import Path +from unittest.mock import patch + +import pytest + +from tools.skill_manager_tool import ( + MAX_SKILL_CONTENT_CHARS, + MAX_SKILL_FILE_BYTES, + _validate_content_size, + skill_manage, +) + + +@pytest.fixture(autouse=True) +def isolate_skills(tmp_path, monkeypatch): + """Redirect SKILLS_DIR to a temp directory.""" + skills_dir = tmp_path / "skills" + skills_dir.mkdir() + monkeypatch.setattr("tools.skill_manager_tool.SKILLS_DIR", skills_dir) + monkeypatch.setattr("tools.skills_tool.SKILLS_DIR", skills_dir) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + return skills_dir + + +def _make_skill_content(body_chars: int) -> str: + """Generate valid SKILL.md content with a body of the given character count.""" + frontmatter = ( + "---\n" + "name: test-skill\n" + "description: A test skill\n" + "---\n" + ) + body = "# Test Skill\n\n" + ("x" * max(0, body_chars - 15)) + return frontmatter + body + + +class TestValidateContentSize: + """Unit tests for _validate_content_size.""" + + def test_within_limit(self): + assert _validate_content_size("a" * 1000) is None + + def test_at_limit(self): + assert _validate_content_size("a" * MAX_SKILL_CONTENT_CHARS) is None + + def test_over_limit(self): + err = _validate_content_size("a" * (MAX_SKILL_CONTENT_CHARS + 1)) + assert err is not None + assert "100,001" in err + assert "100,000" in err + + def test_custom_label(self): + err = _validate_content_size("a" * (MAX_SKILL_CONTENT_CHARS + 1), label="references/api.md") + assert "references/api.md" in err + + +class TestCreateSkillSizeLimit: + """create action rejects oversized content.""" + + def test_create_within_limit(self, isolate_skills): + content = _make_skill_content(5000) + result = json.loads(skill_manage(action="create", name="small-skill", content=content)) + assert result["success"] is True + + def test_create_over_limit(self, isolate_skills): + content = _make_skill_content(MAX_SKILL_CONTENT_CHARS + 100) + result = json.loads(skill_manage(action="create", name="huge-skill", content=content)) + assert result["success"] is False + assert "100,000" in result["error"] + + def test_create_at_limit(self, isolate_skills): + # Content at exactly the limit should succeed + frontmatter = "---\nname: edge-skill\ndescription: Edge case\n---\n# Edge\n\n" + body_budget = MAX_SKILL_CONTENT_CHARS - len(frontmatter) + content = frontmatter + ("x" * body_budget) + assert len(content) == MAX_SKILL_CONTENT_CHARS + result = json.loads(skill_manage(action="create", name="edge-skill", content=content)) + assert result["success"] is True + + +class TestEditSkillSizeLimit: + """edit action rejects oversized content.""" + + def test_edit_over_limit(self, isolate_skills): + # Create a small skill first + small = _make_skill_content(1000) + json.loads(skill_manage(action="create", name="grow-me", content=small)) + + # Try to edit it to be oversized + big = _make_skill_content(MAX_SKILL_CONTENT_CHARS + 100) + # Fix the name in frontmatter + big = big.replace("name: test-skill", "name: grow-me") + result = json.loads(skill_manage(action="edit", name="grow-me", content=big)) + assert result["success"] is False + assert "100,000" in result["error"] + + +class TestPatchSkillSizeLimit: + """patch action checks resulting size, not just the new_string.""" + + def test_patch_that_would_exceed_limit(self, isolate_skills): + # Create a skill near the limit + near_limit = _make_skill_content(MAX_SKILL_CONTENT_CHARS - 50) + json.loads(skill_manage(action="create", name="near-limit", content=near_limit)) + + # Patch that adds enough to go over + result = json.loads(skill_manage( + action="patch", + name="near-limit", + old_string="# Test Skill", + new_string="# Test Skill\n" + ("y" * 200), + )) + assert result["success"] is False + assert "100,000" in result["error"] + + def test_patch_that_reduces_size_on_oversized_skill(self, isolate_skills, tmp_path): + """Patches that shrink an already-oversized skill should succeed.""" + # Manually create an oversized skill (simulating hand-placed) + skill_dir = tmp_path / "skills" / "bloated" + skill_dir.mkdir(parents=True) + oversized = _make_skill_content(MAX_SKILL_CONTENT_CHARS + 5000) + oversized = oversized.replace("name: test-skill", "name: bloated") + (skill_dir / "SKILL.md").write_text(oversized, encoding="utf-8") + assert len(oversized) > MAX_SKILL_CONTENT_CHARS + + # Patch that removes content to bring it under the limit. + # Use replace_all to replace the repeated x's with a shorter string. + result = json.loads(skill_manage( + action="patch", + name="bloated", + old_string="x" * 100, + new_string="y", + replace_all=True, + )) + # Should succeed because the result is well within limits + assert result["success"] is True + + def test_patch_supporting_file_size_limit(self, isolate_skills): + """Patch on a supporting file also checks size.""" + small = _make_skill_content(1000) + json.loads(skill_manage(action="create", name="with-ref", content=small)) + # Create a supporting file + json.loads(skill_manage( + action="write_file", + name="with-ref", + file_path="references/data.md", + file_content="# Data\n\nSmall content.", + )) + # Try to patch it to be oversized + result = json.loads(skill_manage( + action="patch", + name="with-ref", + old_string="Small content.", + new_string="x" * (MAX_SKILL_CONTENT_CHARS + 100), + file_path="references/data.md", + )) + assert result["success"] is False + assert "references/data.md" in result["error"] + + +class TestWriteFileSizeLimit: + """write_file action enforces both char and byte limits.""" + + def test_write_file_over_char_limit(self, isolate_skills): + small = _make_skill_content(1000) + json.loads(skill_manage(action="create", name="file-test", content=small)) + + result = json.loads(skill_manage( + action="write_file", + name="file-test", + file_path="references/huge.md", + file_content="x" * (MAX_SKILL_CONTENT_CHARS + 1), + )) + assert result["success"] is False + assert "100,000" in result["error"] + + def test_write_file_within_limit(self, isolate_skills): + small = _make_skill_content(1000) + json.loads(skill_manage(action="create", name="file-ok", content=small)) + + result = json.loads(skill_manage( + action="write_file", + name="file-ok", + file_path="references/normal.md", + file_content="# Normal\n\n" + ("x" * 5000), + )) + assert result["success"] is True + + +class TestHandPlacedSkillsNoLimit: + """Skills dropped directly on disk are not constrained.""" + + def test_oversized_handplaced_skill_loads(self, isolate_skills, tmp_path): + """A hand-placed 200k skill can still be read via skill_view.""" + from tools.skills_tool import skill_view + + skill_dir = tmp_path / "skills" / "manual-giant" + skill_dir.mkdir(parents=True) + huge = _make_skill_content(200_000) + huge = huge.replace("name: test-skill", "name: manual-giant") + (skill_dir / "SKILL.md").write_text(huge, encoding="utf-8") + + result = json.loads(skill_view("manual-giant")) + assert "content" in result + # The full content is returned — no truncation at the storage layer + assert len(result["content"]) > MAX_SKILL_CONTENT_CHARS diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index 8507a6d1346f..6899ba51ceb5 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -82,6 +82,8 @@ SKILLS_DIR = HERMES_HOME / "skills" MAX_NAME_LENGTH = 64 MAX_DESCRIPTION_LENGTH = 1024 +MAX_SKILL_CONTENT_CHARS = 100_000 # ~36k tokens at 2.75 chars/token +MAX_SKILL_FILE_BYTES = 1_048_576 # 1 MiB per supporting file # Characters allowed in skill names (filesystem-safe, URL-friendly) VALID_NAME_RE = re.compile(r'^[a-z0-9][a-z0-9._-]*$') @@ -177,6 +179,21 @@ def _validate_frontmatter(content: str) -> Optional[str]: return None +def _validate_content_size(content: str, label: str = "SKILL.md") -> Optional[str]: + """Check that content doesn't exceed the character limit for agent writes. + + Returns an error message or None if within bounds. + """ + if len(content) > MAX_SKILL_CONTENT_CHARS: + return ( + f"{label} content is {len(content):,} characters " + f"(limit: {MAX_SKILL_CONTENT_CHARS:,}). " + f"Consider splitting into a smaller SKILL.md with supporting files " + f"in references/ or templates/." + ) + return None + + def _resolve_skill_dir(name: str, category: str = None) -> Path: """Build the directory path for a new skill, optionally under a category.""" if category: @@ -275,6 +292,10 @@ def _create_skill(name: str, content: str, category: str = None) -> Dict[str, An if err: return {"success": False, "error": err} + err = _validate_content_size(content) + if err: + return {"success": False, "error": err} + # Check for name collisions across all directories existing = _find_skill(name) if existing: @@ -318,6 +339,10 @@ def _edit_skill(name: str, content: str) -> Dict[str, Any]: if err: return {"success": False, "error": err} + err = _validate_content_size(content) + if err: + return {"success": False, "error": err} + existing = _find_skill(name) if not existing: return {"success": False, "error": f"Skill '{name}' not found. Use skills_list() to see available skills."} @@ -401,6 +426,12 @@ def _patch_skill( new_content = content.replace(old_string, new_string) if replace_all else content.replace(old_string, new_string, 1) + # Check size limit on the result + target_label = "SKILL.md" if not file_path else file_path + err = _validate_content_size(new_content, label=target_label) + if err: + return {"success": False, "error": err} + # If patching SKILL.md, validate frontmatter is still intact if not file_path: err = _validate_frontmatter(new_content) @@ -455,6 +486,21 @@ def _write_file(name: str, file_path: str, file_content: str) -> Dict[str, Any]: if not file_content and file_content != "": return {"success": False, "error": "file_content is required."} + # Check size limits + content_bytes = len(file_content.encode("utf-8")) + if content_bytes > MAX_SKILL_FILE_BYTES: + return { + "success": False, + "error": ( + f"File content is {content_bytes:,} bytes " + f"(limit: {MAX_SKILL_FILE_BYTES:,} bytes / 1 MiB). " + f"Consider splitting into smaller files." + ), + } + err = _validate_content_size(file_content, label=file_path) + if err: + return {"success": False, "error": err} + existing = _find_skill(name) if not existing: return {"success": False, "error": f"Skill '{name}' not found. Create it first with action='create'."} diff --git a/tools/skills_hub.py b/tools/skills_hub.py index c818261d7cb4..56c89ba713a2 100644 --- a/tools/skills_hub.py +++ b/tools/skills_hub.py @@ -2525,6 +2525,22 @@ def install_from_quarantine( if install_dir.exists(): shutil.rmtree(install_dir) + # Warn (but don't block) if SKILL.md is very large + skill_md = quarantine_path / "SKILL.md" + if skill_md.exists(): + try: + skill_size = skill_md.stat().st_size + if skill_size > 100_000: + logger.warning( + "Skill '%s' has a large SKILL.md (%s chars). " + "Large skills consume significant context when loaded. " + "Consider asking the author to split it into smaller files.", + safe_skill_name, + f"{skill_size:,}", + ) + except OSError: + pass + install_dir.parent.mkdir(parents=True, exist_ok=True) shutil.move(str(quarantine_path), str(install_dir))