mirror of
https://github.com/magnus919/agent-skills.git
synced 2026-09-11 19:47:12 +03:00
feat(validation): add SKILL.md token-budget gate to validate-skills.rb
Issue #382: the 500-line cap alone lets dense prose slip through, so bodies are now measured in characters (~4 chars/token proxy for the ~5,000-token budget) after stripping YAML frontmatter. Bodies over 20,000 characters hard-error with split-into-references remediation, mirroring the existing oversized-reference gate (#277). Adds ReferenceFileScan.oversized_skill_md_errors next to oversized_reference_errors, wired into the per-skill loop, with Minitest coverage: under-limit passes, over-limit fails with path and size, exactly-at-limit passes (strict >), huge-frontmatter/tiny-body passes, missing-frontmatter stays silent (reported elsewhere), and missing SKILL.md is ignored. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
parent
9e40609768
commit
4006940e86
@@ -209,3 +209,90 @@ class ReferenceFileScanTest < Minitest::Test
|
||||
File.write(path, content)
|
||||
end
|
||||
end
|
||||
|
||||
# Regression tests for the SKILL.md token-budget gate (#382). The gate strips
|
||||
# YAML frontmatter, measures the body in characters (~4 chars/token proxy for
|
||||
# the ~5,000-token budget), and must not fire on frontmatter size or at
|
||||
# exactly the limit.
|
||||
class OversizedSkillMdTest < Minitest::Test
|
||||
FRONTMATTER = "---\nname: test-skill\n---\n"
|
||||
|
||||
def test_body_under_limit_passes
|
||||
with_fixture do |root|
|
||||
write_skill_md(root, FRONTMATTER + ("a" * 19_999))
|
||||
assert_empty ReferenceFileScan.oversized_skill_md_errors(root, "test-skill")
|
||||
end
|
||||
end
|
||||
|
||||
def test_body_over_limit_fails_with_path_and_size
|
||||
with_fixture do |root|
|
||||
write_skill_md(root, FRONTMATTER + ("a" * 20_001))
|
||||
errors = ReferenceFileScan.oversized_skill_md_errors(root, "test-skill")
|
||||
assert_equal 1, errors.length
|
||||
assert_includes errors.first, "test-skill/SKILL.md"
|
||||
assert_includes errors.first, "20001"
|
||||
end
|
||||
end
|
||||
|
||||
def test_body_exactly_at_limit_passes
|
||||
with_fixture do |root|
|
||||
write_skill_md(root, FRONTMATTER + ("a" * 20_000))
|
||||
errors = ReferenceFileScan.oversized_skill_md_errors(root, "test-skill")
|
||||
assert_equal 20_000, ReferenceFileScan::MAX_SKILL_MD_BODY_CHARS
|
||||
assert_empty errors
|
||||
end
|
||||
end
|
||||
|
||||
def test_gate_does_not_count_frontmatter
|
||||
with_fixture do |root|
|
||||
# Huge frontmatter (well past the cap) with a tiny body must pass: only
|
||||
# the body after the closing --- counts toward the budget.
|
||||
frontmatter = "---\nname: test-skill\ndescription: #{'y' * 30_000}\n---\n"
|
||||
write_skill_md(root, frontmatter + ("x" * 100))
|
||||
text = File.read(File.join(root, "test-skill/SKILL.md"))
|
||||
match = text.match(/\A---\n.*?\n---\n/m)
|
||||
assert_operator match.end(0), :>, 25_000
|
||||
assert_empty ReferenceFileScan.oversized_skill_md_errors(root, "test-skill")
|
||||
end
|
||||
end
|
||||
|
||||
def test_error_mentions_references_remediation
|
||||
with_fixture do |root|
|
||||
write_skill_md(root, FRONTMATTER + ("a" * 20_001))
|
||||
errors = ReferenceFileScan.oversized_skill_md_errors(root, "test-skill")
|
||||
assert_equal 1, errors.length
|
||||
assert_includes errors.first, "references/"
|
||||
assert_includes errors.first, "triggers + workflow skeleton"
|
||||
end
|
||||
end
|
||||
|
||||
def test_missing_frontmatter_is_skipped_not_flagged_here
|
||||
# validate-skills.rb reports missing YAML frontmatter itself; the body
|
||||
# gate has nothing meaningful to measure and stays silent.
|
||||
with_fixture do |root|
|
||||
write_skill_md(root, "# No frontmatter\n" + ("a" * 30_000))
|
||||
assert_empty ReferenceFileScan.oversized_skill_md_errors(root, "test-skill")
|
||||
end
|
||||
end
|
||||
|
||||
def test_missing_skill_md_is_ignored
|
||||
with_fixture do |root|
|
||||
FileUtils.mkdir_p(File.join(root, "empty-skill"))
|
||||
assert_empty ReferenceFileScan.oversized_skill_md_errors(root, "empty-skill")
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def with_fixture(&block)
|
||||
Dir.mktmpdir do |directory|
|
||||
block.call(directory)
|
||||
end
|
||||
end
|
||||
|
||||
def write_skill_md(root, content)
|
||||
path = File.join(root, "test-skill", "SKILL.md")
|
||||
FileUtils.mkdir_p(File.dirname(path))
|
||||
File.write(path, content)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -58,6 +58,13 @@ module ReferenceFileScan
|
||||
# context; files past the cap must be split and re-indexed in SKILL.md.
|
||||
MAX_REFERENCE_CHARS = 60_000
|
||||
|
||||
# Character cap for the SKILL.md body (issue #382), excluding YAML
|
||||
# frontmatter. ~20,000 characters is a proxy for the ~5,000-token skill-body
|
||||
# budget (~4 chars/token): a line-based cap alone lets dense prose with long
|
||||
# lines slip through. Bodies past the cap must move detail into focused
|
||||
# references/ files loaded on demand.
|
||||
MAX_SKILL_MD_BODY_CHARS = 20_000
|
||||
|
||||
module_function
|
||||
|
||||
# Returns error strings for stale prose backtick references to nonexistent
|
||||
@@ -99,6 +106,29 @@ module ReferenceFileScan
|
||||
errors
|
||||
end
|
||||
|
||||
# Returns error strings for SKILL.md bodies (frontmatter stripped) under
|
||||
# <root>/<skill_rel>/ that exceed MAX_SKILL_MD_BODY_CHARS characters.
|
||||
# Shares oversized_reference_errors's remediation shape: split detail into
|
||||
# focused files and keep only the skeleton in SKILL.md.
|
||||
def oversized_skill_md_errors(root, skill_rel)
|
||||
errors = []
|
||||
skill_md = File.join(root, skill_rel, "SKILL.md")
|
||||
return errors unless File.file?(skill_md)
|
||||
|
||||
text = File.read(skill_md)
|
||||
match = text.match(/\A---\n.*?\n---\n/m)
|
||||
# Missing frontmatter is reported by validate-skills.rb's own check; the
|
||||
# body gate has nothing meaningful to measure without it.
|
||||
return errors unless match
|
||||
|
||||
body = text[match.end(0)..]
|
||||
size = body.length
|
||||
return errors unless size > MAX_SKILL_MD_BODY_CHARS
|
||||
|
||||
errors << "#{skill_rel}/SKILL.md: #{size} characters — SKILL.md body must be <= #{MAX_SKILL_MD_BODY_CHARS} characters (~5,000-token budget); move detail into focused files under references/ and leave triggers + workflow skeleton in SKILL.md"
|
||||
errors
|
||||
end
|
||||
|
||||
# Skill names named on a line, either in prose ("the X skill") or via a
|
||||
# relative SKILL.md link to a sibling skill.
|
||||
def named_skills(line)
|
||||
|
||||
@@ -132,6 +132,9 @@ skills.each do |skill|
|
||||
# Phase 1: references/*.md files must stay within the per-file character cap
|
||||
# (oversized files are split into focused files and re-indexed in SKILL.md).
|
||||
errors.concat(ReferenceFileScan.oversized_reference_errors(ROOT, skill_dir))
|
||||
# Issue #382: SKILL.md bodies (frontmatter excluded) must stay within the
|
||||
# ~5,000-token character budget; dense prose moves into references/ files.
|
||||
errors.concat(ReferenceFileScan.oversized_skill_md_errors(ROOT, skill_dir))
|
||||
end
|
||||
|
||||
if errors.empty?
|
||||
|
||||
Reference in New Issue
Block a user