Skip to content

Commit 8e7fed7

Browse files
Brad KinnardBrad Kinnard
authored andcommitted
fix: YAML type coercion detection, reference depth dedup, README accuracy
Unsolved problem solved: - Added frontmatter.name.type and frontmatter.description.type rules that catch YAML safe_load silently converting bare values (true → bool, 123 → int, null → None). Provides clear fix: quote the value. Bugs fixed: - check_reference_depth emitted duplicate diagnostics for ../../ paths (both depth-exceeded AND traverses-above). Changed to elif. - README Rules table said 'Body exceeds' for sizing rules but code counts full file lines/tokens. Fixed to say 'File exceeds'. Cleanup: - Standardized on collections.abc.Callable (was typing.Callable in 2 files) - Added action/entrypoint.py to CI compile check - 171 tests passing (was 160)
1 parent fa2ada6 commit 8e7fed7

9 files changed

Lines changed: 205 additions & 9 deletions

File tree

.github/workflows/ci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ jobs:
6161
python -m py_compile src/skillcheck/rules/frontmatter.py
6262
python -m py_compile src/skillcheck/rules/references.py
6363
python -m py_compile src/skillcheck/rules/sizing.py
64+
python -m py_compile action/entrypoint.py
6465
6566
package:
6667
runs-on: ubuntu-latest

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1818
- **Agent-scoped checks**`--target-agent {claude,vscode,all}` scopes compatibility diagnostics to a specific agent.
1919
- **Skip flags**`--skip-dirname-check` and `--skip-ref-check` for CI environments where filesystem context is unavailable.
2020
- **`-q`/`--quiet` flag** — suppresses all output; exit code only.
21+
- **YAML type coercion detection**`frontmatter.name.type` and `frontmatter.description.type` catch when `yaml.safe_load` silently converts bare values like `true`, `123`, or `null` into non-string types. Provides clear fix advice (quote the value).
2122
- **YAML anchor detection**`frontmatter.yaml-anchors` warns when YAML anchors/aliases silently copy values in frontmatter.
2223
- **Symlink escape detection**`references.escape` errors when a file reference resolves outside the skill directory (CWE-59).
2324
- **GitHub Actions CI workflow** — test matrix across Python 3.10–3.13 on Ubuntu, macOS, and Windows; compile check; package build verification.
@@ -28,6 +29,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2829
### Changed
2930
- `KNOWN_FRONTMATTER_FIELDS` expanded to include `model`, `context`, `agent`, `hooks`, `user-invocable`, `disable-model-invocation`, `skills`, `mode`, `tags`, `version`, `author`.
3031
- Token estimation uses a word-run + punctuation-run heuristic (~15% error) with optional `tiktoken` for ~5% error.
32+
- Standardized on `collections.abc.Callable` across all modules (was `typing.Callable` in some).
33+
34+
### Fixed
35+
- `check_reference_depth` emitted duplicate diagnostics for `../../` paths (both depth-exceeded and traverses-above). Changed to `elif` so only the most specific warning fires.
36+
- README Rules table described sizing rules as "Body exceeds..." but the code counts full file lines/tokens. Table now says "File exceeds...".
3137

3238
## [0.1.0] - 2026-03-10
3339

README.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ Validates against the [agentskills.io specification](https://agentskills.io/spec
1616
[![License: MIT](https://img.shields.io/badge/license-MIT-blue.svg)](LICENSE)
1717
[![Python 3.10+](https://img.shields.io/badge/python-3.10%2B-3776ab.svg)](https://python.org)
1818
[![PyYAML](https://img.shields.io/badge/deps-PyYAML-yellow.svg)](https://pyyaml.org)
19-
[![Tests](https://img.shields.io/badge/tests-160%20passed-brightgreen.svg)](#testing)
19+
[![Tests](https://img.shields.io/badge/tests-171%20passed-brightgreen.svg)](#testing)
2020

2121
</div>
2222

@@ -212,13 +212,15 @@ Rules marked **spec** are derived from the [agentskills.io specification](https:
212212
| Rule ID | Severity | Source | What it checks |
213213
|---|---|---|---|
214214
| `frontmatter.name.required` | error | spec | `name` field must exist |
215+
| `frontmatter.name.type` | error | advisory | `name` must be a string (catches YAML coercion of `true`, `123`, `null`) |
215216
| `frontmatter.name.max-length` | error | spec | Name must be 64 characters or fewer |
216217
| `frontmatter.name.invalid-chars` | error | spec | Lowercase, numbers, hyphens only |
217218
| `frontmatter.name.leading-trailing-hyphen` | error | spec | No leading or trailing hyphens |
218219
| `frontmatter.name.consecutive-hyphens` | error | spec | No consecutive hyphens |
219220
| `frontmatter.name.reserved-word` | error | advisory | Not a reserved word (`claude`, `anthropic`) |
220221
| `frontmatter.name.directory-mismatch` | error | spec | Name must match parent directory (VS Code requirement) |
221222
| `frontmatter.description.required` | error | spec | `description` field must exist |
223+
| `frontmatter.description.type` | error | advisory | `description` must be a string (catches YAML coercion) |
222224
| `frontmatter.description.empty` | error | spec | Description must not be blank |
223225
| `frontmatter.description.max-length` | error | spec | Description must be 1024 characters or fewer |
224226
| `frontmatter.description.xml-tags` | error | advisory | No XML/HTML tags in description |
@@ -227,8 +229,8 @@ Rules marked **spec** are derived from the [agentskills.io specification](https:
227229
| `frontmatter.yaml-anchors` | warning | advisory | YAML anchors/aliases can silently copy values |
228230
| `description.quality-score` | info | advisory | Scores description 0-100 for agent discoverability |
229231
| `description.min-score` | warning | advisory | Description score below `--min-desc-score` threshold |
230-
| `sizing.body.line-count` | warning | spec | Body exceeds line threshold |
231-
| `sizing.body.token-estimate` | warning | spec | Body exceeds token threshold |
232+
| `sizing.body.line-count` | warning | spec | File exceeds line threshold |
233+
| `sizing.body.token-estimate` | warning | spec | File exceeds token threshold |
232234
| `disclosure.metadata-budget` | warning | spec | Frontmatter exceeds ~100 token metadata budget |
233235
| `disclosure.body-budget` | warning | spec | Body exceeds 5000 token instruction budget |
234236
| `disclosure.body-bloat` | info | advisory | Large code blocks, tables, or base64 in body |

src/skillcheck/rules/__init__.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from __future__ import annotations
22

3-
from typing import Callable
3+
from collections.abc import Callable
44

55
from skillcheck import config
66
from skillcheck.parser import ParsedSkill
@@ -33,6 +33,8 @@
3333
check_name_max_length,
3434
check_name_required,
3535
check_name_reserved_words,
36+
check_name_type,
37+
check_description_type,
3638
check_unknown_fields,
3739
check_yaml_anchors,
3840
)
@@ -44,12 +46,14 @@
4446

4547
_FRONTMATTER_RULES: list[Callable[[ParsedSkill], list[Diagnostic]]] = [
4648
check_name_required,
49+
check_name_type,
4750
check_name_max_length,
4851
check_name_charset,
4952
check_name_leading_trailing_hyphen,
5053
check_name_consecutive_hyphens,
5154
check_name_reserved_words,
5255
check_description_required,
56+
check_description_type,
5357
check_description_non_empty,
5458
check_description_max_length,
5559
check_description_no_xml_tags,

src/skillcheck/rules/frontmatter.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,50 @@ def check_name_required(skill: ParsedSkill) -> list[Diagnostic]:
6060
return []
6161

6262

63+
def check_name_type(skill: ParsedSkill) -> list[Diagnostic]:
64+
"""Ensure ``name`` is a string, not a YAML-coerced boolean/number/null.
65+
66+
``yaml.safe_load`` silently converts bare values like ``true``, ``123``,
67+
and ``null`` into Python ``bool``, ``int``, and ``None``. When this
68+
happens, every downstream rule sees corrupted data (e.g., ``str(True)``
69+
becomes ``'True'`` and then fails charset checks for uppercase ``T``).
70+
"""
71+
name = skill.frontmatter.get("name")
72+
if name is None:
73+
return [] # handled by check_name_required
74+
if isinstance(name, str):
75+
return []
76+
yaml_type = type(name).__name__
77+
return [Diagnostic(
78+
rule="frontmatter.name.type",
79+
severity=Severity.ERROR,
80+
message=(
81+
f"Field 'name' must be a string but YAML parsed it as {yaml_type} "
82+
f"({name!r}). Quote the value: name: \"{name}\""
83+
),
84+
line=_field_line(skill.raw_text, "name"),
85+
)]
86+
87+
88+
def check_description_type(skill: ParsedSkill) -> list[Diagnostic]:
89+
"""Ensure ``description`` is a string, not a YAML-coerced type."""
90+
desc = skill.frontmatter.get("description")
91+
if desc is None:
92+
return [] # handled by check_description_required
93+
if isinstance(desc, str):
94+
return []
95+
yaml_type = type(desc).__name__
96+
return [Diagnostic(
97+
rule="frontmatter.description.type",
98+
severity=Severity.ERROR,
99+
message=(
100+
f"Field 'description' must be a string but YAML parsed it as {yaml_type} "
101+
f"({desc!r}). Quote the value: description: \"{desc}\""
102+
),
103+
line=_field_line(skill.raw_text, "description"),
104+
)]
105+
106+
63107
def check_name_max_length(skill: ParsedSkill) -> list[Diagnostic]:
64108
name = skill.frontmatter.get("name")
65109
if name is None:

src/skillcheck/rules/references.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,7 @@ def check_reference_depth(skill: ParsedSkill) -> list[Diagnostic]:
117117
f"Keep file references one level deep from SKILL.md."
118118
),
119119
))
120-
# Also flag parent traversal
121-
if ref.startswith(".."):
120+
elif ref.startswith(".."):
122121
diagnostics.append(Diagnostic(
123122
rule="references.depth-exceeded",
124123
severity=Severity.WARNING,

src/skillcheck/rules/sizing.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from __future__ import annotations
22

3-
from typing import Callable
3+
from collections.abc import Callable
44

55
from skillcheck.parser import ParsedSkill
66
from skillcheck.result import Diagnostic, Severity

tests/test_references.py

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,26 @@ def test_parent_traversal_flagged(tmp_path):
152152
)
153153
skill = parse(f)
154154
diagnostics = check_reference_depth(skill)
155-
assert len(diagnostics) >= 1
156-
assert any("traverses above" in d.message for d in diagnostics)
155+
# ../../other/file.py is depth 3 — the depth check catches it.
156+
# Only one diagnostic should fire (no duplicate from startswith check).
157+
assert len(diagnostics) == 1
158+
assert diagnostics[0].rule == "references.depth-exceeded"
159+
assert "3 levels deep" in diagnostics[0].message
160+
161+
162+
def test_single_dotdot_traversal_flagged(tmp_path):
163+
"""../file.txt has depth 1 — only the startswith('..') traversal warning fires."""
164+
skill_dir = tmp_path / "my-skill"
165+
skill_dir.mkdir()
166+
f = skill_dir / "SKILL.md"
167+
f.write_text(
168+
"---\nname: my-skill\ndescription: Single dotdot.\n---\n"
169+
"See [up](../notes.txt) for context.\n"
170+
)
171+
skill = parse(f)
172+
diagnostics = check_reference_depth(skill)
173+
assert len(diagnostics) == 1
174+
assert "traverses above" in diagnostics[0].message
157175

158176

159177
def test_shallow_ref_passes(tmp_path):

tests/test_yaml_types.py

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
"""Tests for YAML type coercion detection in frontmatter.
2+
3+
yaml.safe_load silently converts bare values:
4+
name: true → bool
5+
name: 123 → int
6+
name: 1.5 → float
7+
name: null → None
8+
9+
These produce confusing downstream errors. The type check rules catch
10+
them early with a clear fix (quote the value).
11+
"""
12+
13+
import pytest
14+
15+
from skillcheck.parser import parse
16+
from skillcheck.result import Severity
17+
from skillcheck.rules.frontmatter import check_description_type, check_name_type
18+
19+
20+
# ---------------------------------------------------------------------------
21+
# frontmatter.name.type
22+
# ---------------------------------------------------------------------------
23+
24+
25+
def test_name_boolean_detected(tmp_path):
26+
"""name: true → parsed as bool, should fire type error."""
27+
f = tmp_path / "SKILL.md"
28+
f.write_text("---\nname: true\ndescription: Boolean name.\n---\nBody.\n")
29+
skill = parse(f)
30+
diagnostics = check_name_type(skill)
31+
assert len(diagnostics) == 1
32+
assert diagnostics[0].rule == "frontmatter.name.type"
33+
assert diagnostics[0].severity == Severity.ERROR
34+
assert "bool" in diagnostics[0].message
35+
assert 'name: "True"' in diagnostics[0].message
36+
37+
38+
def test_name_integer_detected(tmp_path):
39+
"""name: 123 → parsed as int, should fire type error."""
40+
f = tmp_path / "SKILL.md"
41+
f.write_text("---\nname: 123\ndescription: Numeric name.\n---\nBody.\n")
42+
skill = parse(f)
43+
diagnostics = check_name_type(skill)
44+
assert len(diagnostics) == 1
45+
assert "int" in diagnostics[0].message
46+
47+
48+
def test_name_float_detected(tmp_path):
49+
"""name: 1.5 → parsed as float, should fire type error."""
50+
f = tmp_path / "SKILL.md"
51+
f.write_text("---\nname: 1.5\ndescription: Float name.\n---\nBody.\n")
52+
skill = parse(f)
53+
diagnostics = check_name_type(skill)
54+
assert len(diagnostics) == 1
55+
assert "float" in diagnostics[0].message
56+
57+
58+
def test_name_string_passes(tmp_path):
59+
"""name: my-skill → string, no type error."""
60+
f = tmp_path / "SKILL.md"
61+
f.write_text("---\nname: my-skill\ndescription: Valid.\n---\nBody.\n")
62+
skill = parse(f)
63+
assert check_name_type(skill) == []
64+
65+
66+
def test_name_none_skipped(tmp_path):
67+
"""name absent → handled by check_name_required, not type check."""
68+
f = tmp_path / "SKILL.md"
69+
f.write_text("---\ndescription: No name.\n---\nBody.\n")
70+
skill = parse(f)
71+
assert check_name_type(skill) == []
72+
73+
74+
def test_name_quoted_true_passes(tmp_path):
75+
"""name: "true" → stays as string, no type error."""
76+
f = tmp_path / "SKILL.md"
77+
f.write_text('---\nname: "true"\ndescription: Quoted boolean.\n---\nBody.\n')
78+
skill = parse(f)
79+
assert check_name_type(skill) == []
80+
81+
82+
# ---------------------------------------------------------------------------
83+
# frontmatter.description.type
84+
# ---------------------------------------------------------------------------
85+
86+
87+
def test_description_boolean_detected(tmp_path):
88+
"""description: true → parsed as bool, should fire type error."""
89+
f = tmp_path / "SKILL.md"
90+
f.write_text("---\nname: my-skill\ndescription: true\n---\nBody.\n")
91+
skill = parse(f)
92+
diagnostics = check_description_type(skill)
93+
assert len(diagnostics) == 1
94+
assert diagnostics[0].rule == "frontmatter.description.type"
95+
assert diagnostics[0].severity == Severity.ERROR
96+
assert "bool" in diagnostics[0].message
97+
98+
99+
def test_description_integer_detected(tmp_path):
100+
"""description: 42 → parsed as int, should fire type error."""
101+
f = tmp_path / "SKILL.md"
102+
f.write_text("---\nname: my-skill\ndescription: 42\n---\nBody.\n")
103+
skill = parse(f)
104+
diagnostics = check_description_type(skill)
105+
assert len(diagnostics) == 1
106+
assert "int" in diagnostics[0].message
107+
108+
109+
def test_description_string_passes(tmp_path):
110+
"""description: Some text → string, no type error."""
111+
f = tmp_path / "SKILL.md"
112+
f.write_text("---\nname: my-skill\ndescription: Validates things.\n---\nBody.\n")
113+
skill = parse(f)
114+
assert check_description_type(skill) == []
115+
116+
117+
def test_description_none_skipped(tmp_path):
118+
"""description absent → handled by check_description_required."""
119+
f = tmp_path / "SKILL.md"
120+
f.write_text("---\nname: my-skill\n---\nBody.\n")
121+
skill = parse(f)
122+
assert check_description_type(skill) == []

0 commit comments

Comments
 (0)