Skip to content

fix(agents): parse frontmatter on the --- delimiter line, not any --- substring#3590

Merged
mnriem merged 1 commit into
github:mainfrom
chuenchen309:fix/parse-frontmatter-delimiter-line
Jul 21, 2026
Merged

fix(agents): parse frontmatter on the --- delimiter line, not any --- substring#3590
mnriem merged 1 commit into
github:mainfrom
chuenchen309:fix/parse-frontmatter-delimiter-line

Conversation

@chuenchen309

Copy link
Copy Markdown
Contributor

Description

CommandRegistrar.parse_frontmatter finds the closing frontmatter delimiter with content.find("---", 3) — a raw substring search (src/specify_cli/agents.py). It stops at the first --- anywhere after the opening, including one embedded in a frontmatter value or inside an indented YAML literal block, rather than a --- that occupies its own line (the actual YAML document-separator).

Any command source file whose frontmatter contains --- in a value gets a corrupted description and a corrupted body written into the generated agent files, silently:

from specify_cli.agents import CommandRegistrar

content = """---
description: Separate sections with --- markers
argument-hint: "[name]"
---
Real body starts here.
"""
CommandRegistrar.parse_frontmatter(content)
  • Before: frontmatter {'description': 'Separate sections with'} — description truncated, argument-hint silently dropped, and the rest (markers\nargument-hint: ...\n---\n...) spills into the body.
  • After: {'description': 'Separate sections with --- markers', 'argument-hint': '[name]'} and body Real body starts here.

The sibling frontmatter handler in this codebase, VibeIntegration._inject_frontmatter_flag (src/specify_cli/integrations/vibe/__init__.py), already scans for a line-anchored ---; parse_frontmatter was the only frontmatter reader using a substring find. This change makes them consistent. parse_frontmatter is called before rendering every command into each agent's directory (register_commands, extensions, and preset/core commands), so the fix covers all of those paths.

Testing

  • uv sync && uv run pytesttests/test_extensions.py + tests/test_presets.py724 passed; no regression.
  • Added test_parse_frontmatter_dash_in_value (in TestParseFrontmatter), which fails on main (assert 'Separate sections with' == '...--- markers') and passes with the fix.
  • ruff check src/specify_cli/agents.py → clean.
  • Also verified the standard, no-frontmatter, unterminated-frontmatter, and indented-literal-block cases still behave correctly.

AI Disclosure

  • No AI tools were used in preparing this PR.
  • AI tools were used — this change was authored by an AI coding agent (Claude Code / Claude Opus 4.8) running on this account: it found the bug, wrote the repro, wrote the test, and drafted this description. The human account holder reviews every change and is accountable for it. The verification above is real and re-runnable from the diff. If this isn't the kind of contribution you want, say so and I'll close it.

… substring

CommandRegistrar.parse_frontmatter located the closing delimiter with
content.find("---", 3), a raw substring search. It stopped at the first
"---" anywhere after the opening — including one embedded in a
frontmatter value (e.g. a description "Separate sections with ---
markers") or inside an indented literal block — which truncated the
frontmatter and spilled the remainder into the body, silently corrupting
both the parsed metadata and the rendered command body.

Match the closing "---" on line boundaries, mirroring the line-anchored
scan already used by VibeIntegration._inject_frontmatter_flag.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes YAML frontmatter parsing so only line-delimited --- closes the block.

Changes:

  • Replaces substring matching with line-based delimiter detection.
  • Adds regression coverage for --- inside a frontmatter value.
Show a summary per file
File Description
src/specify_cli/agents.py Corrects closing-delimiter detection.
tests/test_extensions.py Adds regression test for embedded dashes.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Medium

@mnriem
mnriem merged commit 7873c44 into github:main Jul 21, 2026
12 checks passed
@mnriem

mnriem commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

mnriem pushed a commit that referenced this pull request Jul 23, 2026
…val (#3634)

* fix(extensions): parse SKILL.md on the --- delimiter line during removal

ExtensionManager._unregister_extension_skills verified an installed skill
before deleting it by reading metadata.source back from its SKILL.md with a
raw split("---", 2). That substring split stops at the first "---" anywhere
after the opening delimiter, including one embedded in a command description
(e.g. "Separate sections with --- markers"). The frontmatter was then
truncated mid-value, metadata.source parsed empty, the skill looked
unrelated, and its directory was left orphaned on uninstall.

Parse on the "---" delimiter *line* instead, reusing CommandRegistrar.
parse_frontmatter (the line-anchored parser from #3590) in both the fast
(registry-driven) and fallback (directory-scan) removal paths.

Add a regression test that installs an extension whose command description
contains "---", removes it, and asserts the skill directory is gone. Fails
before the fix (dir orphaned), passes after.

* test: cover the fallback scan branch for the --- SKILL.md parse

Copilot noted the new regression test only exercised the fast removal
path (skills_project keeps ai_skills enabled, so remove() resolves the
skills dir directly). Add test_skills_removed_with_dashes_via_fallback_scan,
which deletes init-options.json after install so _get_skills_dir() returns
None and removal takes the fallback directory-scan branch. That branch
re-reads metadata.source with an independently duplicated parser; reverting
it to the old substring split now fails this test (dir orphaned) while the
fast-path test still passes.
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.

3 participants