test: add failing tests for BOM frontmatter and missing SkillName#1583
Merged
Aaronontheweb merged 5 commits intoJul 6, 2026
Merged
Conversation
… all issues - Strip UTF-8 BOM (\uFEFF) before checking for YAML frontmatter delimiter - Populate SkillName on all SkillScanIssue records created in SkillScanner - Fix BOM check in ParseSkillFile and ParseFlatSkillFile for issue kind determination Fixes netclaw-dev#1582
Aaronontheweb
added
skills
…ormalize issue SkillNames
Review follow-ups on the BOM/SkillName change:
- ExtractFrontmatter threw ArgumentOutOfRangeException on a degenerate
"---\n---" block (empty YAML body): the opening line's newline is also
the closing delimiter, so the slice computed a negative-length range.
Because Scan's parse calls are unguarded, this propagated out and aborted
the entire skill-discovery pass (no skills loaded at all). Since
File.ReadAllText strips the BOM, a plain "---\n---" SKILL.md on disk hit
this too. Now guarded to return null (reported as invalid frontmatter).
- ResourceEnumerationFailed derived SkillName from the parent directory
(Path.GetDirectoryName(skillDirectory)), yielding the container name
("files") instead of the skill name. Fixed to use the leaf directory.
- Normalized all error-path SkillName derivations via NormalizeSkillName so
errored rows render the canonical lowercased name, consistent with
accepted skills.
- Removed dead caller-side content.TrimStart('') no-ops: content comes
from File.ReadAllText which already strips the BOM, so BOM tolerance lives
solely in ExtractFrontmatter.
Adds regression tests: degenerate-block returns null (does not throw), Scan
survives a degenerate SKILL.md and keeps discovering healthy siblings, and
issue SkillNames are normalized from mixed-case directories.
Aaronontheweb
enabled auto-merge (squash)
July 5, 2026 20:08
This was referenced Jul 7, 2026
Merged
Aaronontheweb
added a commit
that referenced
this pull request
Jul 8, 2026
) * test: add failing tests for BOM frontmatter parsing and missing SkillName in issues * fix: handle UTF-8 BOM in ExtractFrontmatter and populate SkillName on all issues - Strip UTF-8 BOM (\uFEFF) before checking for YAML frontmatter delimiter - Populate SkillName on all SkillScanIssue records created in SkillScanner - Fix BOM check in ParseSkillFile and ParseFlatSkillFile for issue kind determination Fixes #1582 * fix(skills): prevent scan-aborting crash on degenerate frontmatter; normalize issue SkillNames Review follow-ups on the BOM/SkillName change: - ExtractFrontmatter threw ArgumentOutOfRangeException on a degenerate "---\n---" block (empty YAML body): the opening line's newline is also the closing delimiter, so the slice computed a negative-length range. Because Scan's parse calls are unguarded, this propagated out and aborted the entire skill-discovery pass (no skills loaded at all). Since File.ReadAllText strips the BOM, a plain "---\n---" SKILL.md on disk hit this too. Now guarded to return null (reported as invalid frontmatter). - ResourceEnumerationFailed derived SkillName from the parent directory (Path.GetDirectoryName(skillDirectory)), yielding the container name ("files") instead of the skill name. Fixed to use the leaf directory. - Normalized all error-path SkillName derivations via NormalizeSkillName so errored rows render the canonical lowercased name, consistent with accepted skills. - Removed dead caller-side content.TrimStart('') no-ops: content comes from File.ReadAllText which already strips the BOM, so BOM tolerance lives solely in ExtractFrontmatter. Adds regression tests: degenerate-block returns null (does not throw), Scan survives a degenerate SKILL.md and keeps discovering healthy siblings, and issue SkillNames are normalized from mixed-case directories.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the two
SkillScannerbugs from #1582 — and, following review, hardens a related crash path. This PR ships both the tests and the fixes (all green).close #1582
Fixes
UTF-8 BOM frontmatter parsing — SKILL.md files saved with a UTF-8 BOM were rejected by
ExtractFrontmatterbecausecontent.StartsWith("---")failed when the BOM preceded the delimiter.ExtractFrontmatternow strips the leading BOM before the delimiter check, so BOM-prefixed skills parse correctly. (Note: on the disk-scan pathFile.ReadAllTextalready strips the BOM; the strip inExtractFrontmattercovers direct-string API callers.)Missing SkillName on issues — every
SkillScanIssuenow populatesSkillName, sonetclaw skill issuescan identify which skill is broken. Names are canonicalized viaNormalizeSkillNameso errored rows render the same lowercased name as accepted skills.Review-driven hardening
Scan-aborting crash on degenerate frontmatter — a degenerate
---\n---block (empty YAML body) madeExtractFrontmatterslice a negative-length range and throwArgumentOutOfRangeException. BecauseScan's parse calls are unguarded, this propagated out and aborted the entire skill-discovery pass (no skills loaded at all) — reachable on disk sinceFile.ReadAllTextstrips the BOM down to a plain---\n---. Now guarded to returnnull, reported as invalid frontmatter.ResourceEnumerationFailedSkillName — was derived from the parent directory (Path.GetDirectoryName(skillDirectory)), yielding the container name (e.g.files) instead of the skill name. Fixed to use the leaf directory.Dead-code cleanup — removed no-op caller-side
content.TrimStart('')guards; BOM tolerance now lives solely inExtractFrontmatter.Tests
ExtractFrontmatterhandles UTF-8 BOMSkillScanIssuepopulatesSkillNamefor broken frontmatter, and normalizes it from a mixed-case directoryExtractFrontmatterreturnsnull(does not throw) for degenerate blocks — plain, BOM-prefixed, and no-trailing-newlineScandoes not abort on a degenerate-frontmatter skill and keeps discovering healthy siblingsAll 45
SkillScannerTestspass;dotnet slopwatch analyzereports 0 issues.