mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
feat(skills): size limits for agent writes + fuzzy matching for patch (#4414)
* feat(skills): add content size limits for agent-created skills 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 * feat(skills): add fuzzy matching to skill patch _patch_skill now uses the same 8-strategy fuzzy matching engine (tools/fuzzy_match.py) as the file patch tool. Handles whitespace normalization, indentation differences, escape sequences, and block-anchor matching. Eliminates exact-match failures when agents patch skills with minor formatting mismatches.
This commit is contained in:
parent
70744add15
commit
f6ada27d1c
5 changed files with 464 additions and 18 deletions
174
tests/tools/test_skill_improvements.py
Normal file
174
tests/tools/test_skill_improvements.py
Normal file
|
|
@ -0,0 +1,174 @@
|
|||
"""Tests for skill fuzzy patching via tools.fuzzy_match."""
|
||||
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.skill_manager_tool import (
|
||||
_create_skill,
|
||||
_patch_skill,
|
||||
_write_file,
|
||||
skill_manage,
|
||||
)
|
||||
|
||||
|
||||
SKILL_CONTENT = """\
|
||||
---
|
||||
name: test-skill
|
||||
description: A test skill for unit testing.
|
||||
---
|
||||
|
||||
# Test Skill
|
||||
|
||||
Step 1: Do the thing.
|
||||
Step 2: Do another thing.
|
||||
Step 3: Final step.
|
||||
"""
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fuzzy patching
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestFuzzyPatchSkill:
|
||||
@pytest.fixture(autouse=True)
|
||||
def setup_skills(self, tmp_path, monkeypatch):
|
||||
skills_dir = tmp_path / "skills"
|
||||
skills_dir.mkdir()
|
||||
monkeypatch.setattr("tools.skill_manager_tool.SKILLS_DIR", skills_dir)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
self.skills_dir = skills_dir
|
||||
|
||||
def test_exact_match_still_works(self):
|
||||
_create_skill("test-skill", SKILL_CONTENT)
|
||||
result = _patch_skill("test-skill", "Step 1: Do the thing.", "Step 1: Done!")
|
||||
assert result["success"] is True
|
||||
content = (self.skills_dir / "test-skill" / "SKILL.md").read_text()
|
||||
assert "Step 1: Done!" in content
|
||||
|
||||
def test_whitespace_trimmed_match(self):
|
||||
"""Patch with extra leading whitespace should still find the target."""
|
||||
skill = """\
|
||||
---
|
||||
name: ws-skill
|
||||
description: Whitespace test
|
||||
---
|
||||
|
||||
# Commands
|
||||
|
||||
def hello():
|
||||
print("hi")
|
||||
"""
|
||||
_create_skill("ws-skill", skill)
|
||||
# Agent sends patch with no leading whitespace (common LLM behaviour)
|
||||
result = _patch_skill("ws-skill", "def hello():\n print(\"hi\")", "def hello():\n print(\"hello world\")")
|
||||
assert result["success"] is True
|
||||
content = (self.skills_dir / "ws-skill" / "SKILL.md").read_text()
|
||||
assert 'print("hello world")' in content
|
||||
|
||||
def test_indentation_flexible_match(self):
|
||||
"""Patch where only indentation differs should succeed."""
|
||||
skill = """\
|
||||
---
|
||||
name: indent-skill
|
||||
description: Indentation test
|
||||
---
|
||||
|
||||
# Steps
|
||||
|
||||
1. First step
|
||||
2. Second step
|
||||
3. Third step
|
||||
"""
|
||||
_create_skill("indent-skill", skill)
|
||||
# Agent sends with different indentation
|
||||
result = _patch_skill(
|
||||
"indent-skill",
|
||||
"1. First step\n2. Second step",
|
||||
"1. Updated first\n2. Updated second"
|
||||
)
|
||||
assert result["success"] is True
|
||||
content = (self.skills_dir / "indent-skill" / "SKILL.md").read_text()
|
||||
assert "Updated first" in content
|
||||
|
||||
def test_multiple_matches_blocked_without_replace_all(self):
|
||||
"""Multiple fuzzy matches should return an error without replace_all."""
|
||||
skill = """\
|
||||
---
|
||||
name: dup-skill
|
||||
description: Duplicate test
|
||||
---
|
||||
|
||||
# Steps
|
||||
|
||||
word word word
|
||||
"""
|
||||
_create_skill("dup-skill", skill)
|
||||
result = _patch_skill("dup-skill", "word", "replaced")
|
||||
assert result["success"] is False
|
||||
assert "match" in result["error"].lower()
|
||||
|
||||
def test_replace_all_with_fuzzy(self):
|
||||
skill = """\
|
||||
---
|
||||
name: dup-skill
|
||||
description: Duplicate test
|
||||
---
|
||||
|
||||
# Steps
|
||||
|
||||
word word word
|
||||
"""
|
||||
_create_skill("dup-skill", skill)
|
||||
result = _patch_skill("dup-skill", "word", "replaced", replace_all=True)
|
||||
assert result["success"] is True
|
||||
content = (self.skills_dir / "dup-skill" / "SKILL.md").read_text()
|
||||
assert "word" not in content
|
||||
assert "replaced" in content
|
||||
|
||||
def test_no_match_returns_preview(self):
|
||||
_create_skill("test-skill", SKILL_CONTENT)
|
||||
result = _patch_skill("test-skill", "this does not exist anywhere", "replacement")
|
||||
assert result["success"] is False
|
||||
assert "file_preview" in result
|
||||
|
||||
def test_fuzzy_patch_on_supporting_file(self):
|
||||
"""Fuzzy matching should also work on supporting files."""
|
||||
_create_skill("test-skill", SKILL_CONTENT)
|
||||
ref_content = " function hello() {\n console.log('hi');\n }"
|
||||
_write_file("test-skill", "references/code.js", ref_content)
|
||||
# Patch with stripped indentation
|
||||
result = _patch_skill(
|
||||
"test-skill",
|
||||
"function hello() {\nconsole.log('hi');\n}",
|
||||
"function hello() {\nconsole.log('hello world');\n}",
|
||||
file_path="references/code.js"
|
||||
)
|
||||
assert result["success"] is True
|
||||
content = (self.skills_dir / "test-skill" / "references" / "code.js").read_text()
|
||||
assert "hello world" in content
|
||||
|
||||
def test_patch_preserves_frontmatter_validation(self):
|
||||
"""Fuzzy matching should still run frontmatter validation on SKILL.md."""
|
||||
_create_skill("test-skill", SKILL_CONTENT)
|
||||
# Try to destroy the frontmatter via patch
|
||||
result = _patch_skill("test-skill", "---\nname: test-skill", "BROKEN")
|
||||
assert result["success"] is False
|
||||
assert "structure" in result["error"].lower() or "frontmatter" in result["error"].lower()
|
||||
|
||||
def test_skill_manage_patch_uses_fuzzy(self):
|
||||
"""The dispatcher should route to the fuzzy-matching patch."""
|
||||
_create_skill("test-skill", SKILL_CONTENT)
|
||||
raw = skill_manage(
|
||||
action="patch",
|
||||
name="test-skill",
|
||||
old_string=" Step 1: Do the thing.", # extra leading space
|
||||
new_string="Step 1: Updated.",
|
||||
)
|
||||
result = json.loads(raw)
|
||||
# Should succeed via line-trimmed or indentation-flexible matching
|
||||
assert result["success"] is True
|
||||
|
|
@ -271,7 +271,7 @@ class TestPatchSkill:
|
|||
_create_skill("my-skill", VALID_SKILL_CONTENT)
|
||||
result = _patch_skill("my-skill", "this text does not exist", "replacement")
|
||||
assert result["success"] is False
|
||||
assert "not found" in result["error"]
|
||||
assert "not found" in result["error"].lower() or "could not find" in result["error"].lower()
|
||||
|
||||
def test_patch_ambiguous_match_rejected(self, tmp_path):
|
||||
content = """\
|
||||
|
|
@ -288,7 +288,7 @@ word word
|
|||
_create_skill("my-skill", content)
|
||||
result = _patch_skill("my-skill", "word", "replaced")
|
||||
assert result["success"] is False
|
||||
assert "matched" in result["error"]
|
||||
assert "match" in result["error"].lower()
|
||||
|
||||
def test_patch_replace_all(self, tmp_path):
|
||||
content = """\
|
||||
|
|
|
|||
215
tests/tools/test_skill_size_limits.py
Normal file
215
tests/tools/test_skill_size_limits.py
Normal file
|
|
@ -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
|
||||
Loading…
Add table
Add a link
Reference in a new issue