mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-22 13:34:17 +02:00
Switch skills frontmatter parsing to yaml.BaseLoader
safe_load's implicit scalar typing on the public SkillInfo.frontmatter
mapping was backwards-incompatible (version: 1.10 -> 1.1, enabled: yes
-> True, dates -> datetime.date). BaseLoader keeps block-scalar support
without coercing scalars, so the description-only recovery branch is
no longer needed.
🤖 Generated with Claude Code
This commit is contained in:
parent
0310ba6c7d
commit
7659b5c545
2 changed files with 35 additions and 29 deletions
|
|
@ -66,9 +66,16 @@ def _parse_frontmatter_line_based(frontmatter_text: str) -> dict[str, Any]:
|
|||
def parse_frontmatter(content: str) -> tuple[dict[str, Any], str]:
|
||||
"""Parse YAML frontmatter from markdown content.
|
||||
|
||||
Prefers full YAML parsing so multiline block scalars (`|` / `>`) work.
|
||||
If YAML parsing fails (invalid YAML, recursion limit, etc.), falls back to
|
||||
a line-based key:value parser so plain frontmatter is not discarded.
|
||||
Uses `yaml.BaseLoader` so multiline block scalars (`|` / `>`) work while
|
||||
every scalar value is returned as a plain string. `SkillInfo.frontmatter`
|
||||
is a public mapping; letting a full loader (e.g. `safe_load`) apply
|
||||
implicit scalar typing would silently turn `version: 1.10` into the float
|
||||
`1.1`, `enabled: yes` into `True`, or a date-like value into
|
||||
`datetime.date` for every skill author, which is a backwards-incompatible
|
||||
surprise rather than an internal detail. `BaseLoader` has no such
|
||||
constructors, so values round-trip as written. If YAML parsing fails
|
||||
(invalid YAML, recursion limit, etc.), falls back to a line-based
|
||||
key:value parser so plain frontmatter is not discarded.
|
||||
|
||||
Args:
|
||||
content: Markdown content potentially starting with ---
|
||||
|
|
@ -93,7 +100,7 @@ def parse_frontmatter(content: str) -> tuple[dict[str, Any], str]:
|
|||
remaining = content[3 + end_match.end() :]
|
||||
|
||||
try:
|
||||
parsed = yaml.safe_load(frontmatter_text)
|
||||
parsed = yaml.load(frontmatter_text, Loader=yaml.BaseLoader)
|
||||
except (yaml.YAMLError, RecursionError):
|
||||
# Prefer partial recovery over discarding every key (issue #4416 review).
|
||||
return _parse_frontmatter_line_based(frontmatter_text), remaining
|
||||
|
|
@ -101,17 +108,6 @@ def parse_frontmatter(content: str) -> tuple[dict[str, Any], str]:
|
|||
if not isinstance(parsed, dict):
|
||||
return {}, remaining
|
||||
|
||||
# YAML may type `description: true` / dates as non-str, but SkillInfo.description
|
||||
# is a str field that callers truthiness-check.
|
||||
if "description" in parsed and parsed["description"] is not None:
|
||||
if not isinstance(parsed["description"], str):
|
||||
# YAML 1.1 coerces bare words like `yes` to non-str; recover the
|
||||
# original text via the line-based parser instead.
|
||||
raw = _parse_frontmatter_line_based(frontmatter_text).get("description")
|
||||
parsed["description"] = (
|
||||
raw if isinstance(raw, str) else str(parsed["description"])
|
||||
)
|
||||
|
||||
return parsed, remaining
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -130,27 +130,37 @@ Body
|
|||
assert frontmatter == {}
|
||||
assert body == "Body\n"
|
||||
|
||||
def test_frontmatter_numeric_scalars_are_typed(self):
|
||||
"""yaml.safe_load applies YAML typing to unquoted scalars.
|
||||
def test_frontmatter_scalars_are_preserved_as_strings(self):
|
||||
"""`SkillInfo.frontmatter` is public; it must not silently retype values.
|
||||
|
||||
`version` is never read downstream (only `description` is, in
|
||||
skill_provider.py), so this typed behavior is safe today, but it IS
|
||||
a change from the old parser, which always kept every value as a
|
||||
string. This pins the actual coerced types so a regression is
|
||||
caught if that assumption ever changes.
|
||||
`yaml.BaseLoader` parses block scalars (unlike the old line parser)
|
||||
but applies no implicit scalar resolution, so every value comes back
|
||||
exactly as written instead of being coerced to float/bool/date the
|
||||
way a full loader (e.g. `safe_load`) would.
|
||||
|
||||
The `description: null` case looks surprising but is intentional:
|
||||
under `BaseLoader`, YAML's null token resolves to the *string*
|
||||
`"null"`, not Python `None`. This matches the pre-existing line-based
|
||||
parser (which did `value.strip()` on the raw text and also produced
|
||||
`"null"`), so this restores original behavior rather than inventing
|
||||
a new one. Do not "fix" this back to `None`.
|
||||
"""
|
||||
content = """---
|
||||
version: 1.0
|
||||
enabled: true
|
||||
version: 1.10
|
||||
enabled: yes
|
||||
released: 2024-01-01
|
||||
description: null
|
||||
---
|
||||
|
||||
Body
|
||||
"""
|
||||
frontmatter, body = parse_frontmatter(content)
|
||||
assert frontmatter["version"] == 1.0
|
||||
assert isinstance(frontmatter["version"], float)
|
||||
assert frontmatter["enabled"] is True
|
||||
assert isinstance(frontmatter["enabled"], bool)
|
||||
assert frontmatter["version"] == "1.10"
|
||||
assert frontmatter["enabled"] == "yes"
|
||||
assert frontmatter["released"] == "2024-01-01"
|
||||
assert frontmatter["description"] == "null"
|
||||
for value in frontmatter.values():
|
||||
assert isinstance(value, str)
|
||||
|
||||
def test_frontmatter_description_bool_recovers_original_text(self):
|
||||
content = """---
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue