Repository navigation
fix(skills): readable, actionable SKILL.md frontmatter diagnostics - #244
Merged
Merged
Conversation
Inspect recovered YAML structure to avoid quoting hints for comments and block scalars. Escape parser control characters and tolerate absent error values. Cover both skill loaders and CLI diagnostic rendering.
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
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.
Brings the SKILL.md frontmatter diagnostic fix (reported and authored by @Hylouis233 in #236) onto
mainas a clean, reviewable branch.Fixes #236.
What changed
SKILL.md frontmatter syntax errors and the "missing description" error are now emitted as a single line that names the file, points at the line and column a user actually edits, states the parser reason, and adds a fix hint for the common unquoted-colon mistake.
Before:
After:
packages/coding-agent/src/utils/frontmatter.tsgainsdescribeFrontmatterError()andformatFrontmatterError():yamlpackage's multi-line message onto one line, dropping the quoted source snippet and caret, and reads the location fromlinePoswith a regex fallback on the headline.YAML_TO_FILE_LINE_OFFSET = 1, since the opening---is always line 1 and the YAML block always begins on line 2.": "inside thedescriptionscalar by inspecting the parser's recovered tree, so comments, quoted and block scalars,10:30-style values, URLs and flow collections are left alone.parseFrontmatter's normalization is factored intoextractFrontmatter, which the detector reuses so both see identical content.packages/agent-core/src/harness/skills.tsandpackages/coding-agent/src/core/skills.tsuseformatFrontmatterErrorforparse_failed, and theirvalidateDescriptionprefixes the file path.apps/cli/src/ui/interactive-mode.tsno longer prints the path twice: when a diagnostic message already starts with its path, the remainder is appended to the formatted path line instead of being emitted on a second indented line.Scope: which skills load or fail is unchanged — message text only. Severity stays
warningand theparse_failed/invalid_metadatacodes are unchanged, so consumers filtering on those codes still work.Provenance
Cherry-picked from the author's fork (
Hylouis233/Step-Code#3), rebased onto currentmain. Commit authorship is preserved. The fork branch also carried two unrelated commits (an i18nfeatand an ultracodefeat); those are deliberately excluded here so this branch contains only the #236 fix — 10 files, +490/−30.Verification
Reproduced the issue against this branch in an isolated
HOMEwith the exact three files from the report (two unquoted-colon descriptions, one with no frontmatter block), then confirmed the new output:All five reported problems are addressed: line number corrected, source snippet and caret dropped, message on one line, quoting hint added, path merged.
pnpm install --frozen-lockfileandpnpm build— clean.tsgo --noEmit— exit 0.biome checkon the 8 changed/added.tsfiles —Checked 8 files. No fixes applied.vitest --run test/frontmatter.test.ts test/skills.test.ts(coding-agent) — 55 passed.vitest --run test/harness/skills.test.ts(agent-core) — 11 passed.vitest --run test/interactive-mode-diagnostics.test.ts(apps/cli) — 4 passed.check:pinned-deps,check:ts-relative-imports,check:layer-direction,check:ui-layer,check:tui-no-ai,check:coding-agent-entry-freeze,check:contracts-deps-empty,check:derived-compat-only,check:no-provider-dispatch.10:30, URLs,#comments, flow collections, nested mappings, CRLF, BOM, non-ASCII) — no false positives or missed cases.Notes
Diagnostics keep
warningseverity and the existingparse_failed/invalid_metadatacodes, so nothing downstream breaks. Only the message text changes.