Skip to content

fix(skill-validator): preserve blank lines inside block scalars in parse_frontmatter#238

Merged
potiuk merged 1 commit into
apache:mainfrom
andreahlert:fix/skill-validator-frontmatter-blank-lines
May 20, 2026
Merged

fix(skill-validator): preserve blank lines inside block scalars in parse_frontmatter#238
potiuk merged 1 commit into
apache:mainfrom
andreahlert:fix/skill-validator-frontmatter-blank-lines

Conversation

@andreahlert
Copy link
Copy Markdown
Collaborator

What

Fixes a latent correctness bug in skill-validator's parse_frontmatter, surfaced by a code review.

The bug

parse_frontmatter treated any blank line as a terminator for the current key. A YAML block scalar (| or >) with a paragraph break (a blank line between two indented paragraphs) silently lost everything after the first paragraph.

Two consequences:

  1. validate_frontmatter measures len(fm["description"]) + len(fm["when_to_use"]) against MAX_METADATA_CHARS (the Claude Code truncation budget). With the dropped paragraphs the measurement was too small, so frontmatter that actually exceeded the budget could pass the check.
  2. validate_principle_compliance and validate_trigger_preservation only saw the truncated string, missing forbidden patterns or trigger phrasing that lived in the dropped paragraphs.

Why latent

No existing SKILL.md frontmatter currently uses a paragraph break in a block scalar, so nothing in-tree exercised the bug. But init_skill.py scaffolding emits multi-line description and when_to_use blocks, and any author writing a two-paragraph description would have been silently mis-validated.

The fix

In real YAML, a blank line inside a block scalar is part of the value, not a terminator. Only a new top-level key finalises the current value. The parser now appends blank lines to the value lines and lets .strip() at finalisation discard leading/trailing blanks so single-line values are unaffected.

Changes

  • src/skill_validator/__init__.pyparse_frontmatter: blank line is content, not terminator
  • tests/test_validator.py — regression test for a | block scalar with an internal blank line

Validation

  • pytest: 77 passed
  • ruff check / ruff format / mypy: clean
  • prek including skill-validate (.claude/skills/**): all hooks pass — every existing SKILL.md still validates after the parser change

…rse_frontmatter

`parse_frontmatter` treated any blank line as a terminator for the
current key, so a block scalar with a paragraph break (a blank line
between two indented paragraphs in a `|` or `>` value) silently lost
everything after the first paragraph.

Two consequences:

1. `validate_frontmatter` measures `len(fm["description"]) +
   len(fm["when_to_use"])` against `MAX_METADATA_CHARS` (the
   Claude Code truncation budget). With the dropped paragraphs the
   measurement was too small, so a frontmatter that actually exceeds
   the budget could pass the check.

2. `validate_principle_compliance` and `validate_trigger_preservation`
   only saw the truncated string, missing forbidden patterns or
   trigger phrasing that lived in the dropped paragraphs.

No existing SKILL.md hit the bug, so it was latent. The
`init_skill.py` scaffolding emits multi-line description and
when_to_use blocks, and any author writing a two-paragraph
description would have been silently mis-validated.

Fix: in real YAML, a blank line inside a block scalar is part of the
value, not a terminator. Only a new top-level key finalises the
current value. Append blank lines to the value lines and let
`.strip()` at finalisation discard leading/trailing blanks so
single-line values are unaffected.

Adds a regression test that asserts a `|` block scalar with an
internal blank line keeps both paragraphs.
@andreahlert andreahlert requested a review from potiuk May 20, 2026 04:43
@andreahlert andreahlert self-assigned this May 20, 2026
@potiuk potiuk merged commit 87ee836 into apache:main May 20, 2026
13 checks passed
@andreahlert andreahlert deleted the fix/skill-validator-frontmatter-blank-lines branch May 20, 2026 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants