Skip to content

fix(runtime): parse skill frontmatter as YAML - #59

Merged
locez merged 1 commit into
mainfrom
fix/skill-frontmatter-metadata
Sep 16, 2026
Merged

locez merged 1 commit into
mainfrom
fix/skill-frontmatter-metadata

Conversation

@locez

@locez locez commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Why

SKILL.md frontmatter was scanned line by line, so any metadata: map with indented keys produced invalid frontmatter line and the whole skill was dropped from the catalog with only a load warning. Skills that ship structured metadata were therefore invisible to the model.

Real frontmatter that failed before, and now loads:

---
name: sample-tool
description: Use when the task needs the sample tool.
metadata:
  cli_version: ">=1.2.3"
  category: product
  requires:
    bins:
      - sample-cli
---

What changed

  • Parse with serde_norway. The frontmatter block is deserialized into a typed { name, description } document; unknown top-level keys are ignored on purpose, so metadata, license, or tool-policy sections no longer hide a skill.
  • Wider accepted surface. Plain multi-line scalars, |/> block scalars with -/+ chomping, quoted values, trailing comments, CRLF line endings, and a leading byte-order mark all behave as their authors expect.
  • One normalization owner. SkillMetadata::new trims name, collapses description whitespace, and rejects control characters or line breaks; the stable prefix and the CLI completion list therefore share the same single-line invariant instead of relying on the renderer.
  • Typed failures. FrontmatterError and SkillLoadWarningReason replace String messages, and SkillLoadWarning::message() becomes reason() with a Display implementation. The unused SkillError::RootNotFound variant is removed.
  • Module split. Frontmatter extraction and parsing move to skill/frontmatter.rs (skill.rs 856 to 727 lines), following the existing context.rs plus context/ facade pattern.
  • Deduplication. memory.rs and context/projection.rs now use the shared text::collapse_whitespace instead of three copies of the same split_whitespace().collect().join(" ") conversion.

Verification

  • cargo fmt --all --check - clean
  • cargo clippy --all-targets --all-features -- -D warnings - clean
  • cargo test --all - exit 0, 58 test targets test result: ok, no failures
  • cargo metadata --format-version 1 --no-deps - exit 0
  • git diff --check - clean

Focused coverage added: loads_skill_frontmatter_from_roots (nested metadata, multi-line description, BOM plus CRLF end to end), skips_invalid_skill_files_with_typed_warnings (four warnings asserted by path and reason), warns_about_duplicate_skill_names, normalizes_description_to_one_line, rejects_multi_line_names, plus parser tests for block scalars, comments, quoted values, and missing or null fields.

Notes for review

  • Stable-prefix effect. Tool definitions and their order are untouched. A skill whose description spanned lines, or had extra whitespace, renders differently in the cacheable stable prefix once and then stays stable; skills that were previously dropped now appear in it.
  • Dependency addition. serde_norway 0.9.42 is a direct dependency of merry-runtime only, and brings unsafe-libyaml-norway 0.2.15. The lockfile diff adds exactly those two packages with no version drift elsewhere. The workspace unsafe_code = "forbid" lint still covers Merry's own code only.
  • Deliberate YAML semantics. Non-string scalars such as name: 123 are read as text, description: holding a nested map stays a load warning, an empty block (--- immediately followed by ---) reports a missing name, and multi-document frontmatter is not supported.
  • The earlier fixtures that used real third-party skill content were replaced with neutral sample-tool and demo-skill files.

Skill frontmatter was scanned line by line, so a `metadata:` map with
indented keys failed with `invalid frontmatter line` and the whole skill
was dropped from the catalog. Real skill files carry structured
metadata, so that silently hid them from the model.

- Read frontmatter with serde_norway and keep only the typed `name` and
  `description` fields; unknown top-level keys are ignored on purpose.
- Accept plain multi-line scalars, `|`/`>` blocks, quoted values,
  trailing comments, CRLF line endings, and a leading byte-order mark.
- Normalize name and description in `SkillMetadata::new`, so the stable
  prefix and the CLI completion list share one single-line invariant
  instead of depending on the renderer.
- Type loader failures as `FrontmatterError` and
  `SkillLoadWarningReason` instead of `String` messages.
- Move frontmatter parsing into `skill/frontmatter.rs` and share
  `text::collapse_whitespace` with memory canonicalization.

Verified with cargo fmt --all --check, cargo clippy --all-targets
--all-features -- -D warnings, and cargo test --all.
@locez
locez merged commit b5d6318 into main Sep 16, 2026
8 checks passed
@locez
locez deleted the fix/skill-frontmatter-metadata branch September 16, 2026 19:24
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.

1 participant