Skip to content

fix(scanner): read skill frontmatter values as YAML scalars - #173

Merged
RealZST merged 1 commit into
mainfrom
fix/skill-frontmatter-yaml-scalars
Nov 1, 2026
Merged

RealZST merged 1 commit into
mainfrom
fix/skill-frontmatter-yaml-scalars

Conversation

@RealZST

@RealZST RealZST commented Nov 1, 2026

Copy link
Copy Markdown
Owner

Summary

SKILL.md frontmatter is YAML, but parse_skill_frontmatter copied the text after name: / description: verbatim. A quoted value such as name: "imagegen" therefore became a skill named "imagegen", quotes included. All of OpenAI's bundled Codex skills (imagegen, pdf, playwright, speech, spreadsheet, doc, …) quote their names, and every agent that reads them with a YAML parser sees imagegen.

What that did in HarnessKit, reproduced on origin/main:

Before After
hk list "imagegen" imagegen
hk info imagegen / hk disable imagegen Error: Extension not found: imagegen works
Install paths that name the target directory after the parsed name create a folder literally called "imagegen" imagegen
Hermes native disable (#172) the quoted name written to skills.disabled never matches Hermes' own name matches
Directory skill without a frontmatter name, e.g. my.skill/ named my (file_stem) named my.skill, as every agent does

skill_locations already carried a one-off patch for this ("strip surrounding quotes if present"), which masked the symptom in one code path only. It is removed.

Changes

  • yaml_scalar: each frontmatter value is parsed with serde_yaml and used when it is a string, so quotes, escapes and single vs. double quoting resolve the way YAML defines them. Text that YAML would not read as a plain string (an unquoted colon, a bare number, a boolean) is kept verbatim, so hand-written frontmatter such as description: Use when: foo keeps working. The whole block is deliberately not handed to a strict YAML parser for that reason. bins items go through the same helper instead of trimming quote characters by hand.
  • skill_entry_name: a directory skill falls back to its whole folder name, a standalone .md to its stem (notes.md.disabled → notes). The scanner, find_skill_by_id and find_skill_content each had their own file_stem fallback; they now share this one, and the scanner also uses it as the skills CLI lockfile key instead of recomputing it.
  • Four tests, written failing first: quoted scalars resolve; plain values YAML would reject are kept; a my.skill/ directory is named my.skill; find_skill_by_id resolves the id the scanner produced for it (this one guards the finders against drifting from the scanner again).

Verification

  • cargo test --workspace: 813 passed, 0 failed.
  • cargo clippy --workspace --all-targets: no new diagnostics in touched files.
  • rustfmt --check compared hunk-by-hunk against origin/main: no new drift in either file.
  • Multi-agent regression check (Claude directory + loose skill, Codex, Gemini, Cursor, Grok flat + nested, a five-agent skill in ~/.agents/skills): hk list --json, hk status, and disable/enable round-trips produce byte-identical output with the origin/main binary and this branch's binary.

Upgrade notes

  • A skill's id derives from its name, so rows for previously quoted names are re-keyed on the first scan after upgrading; a HarnessKit-side disable on one of them is reset once.
  • Directories that earlier versions created with quotes in their name (e.g. ~/.claude/skills/"imagegen"/) are left alone. HarnessKit now identifies them by their frontmatter name, so they keep working; renaming the folder or reinstalling the skill gives it a clean directory name. No automatic rename, since a same-named directory may already exist.

🤖 Generated with Claude Code

`parse_skill_frontmatter` took the text after `name:` / `description:`
verbatim, so a quoted value such as `name: "imagegen"` became the skill
name with its quotes. SKILL.md frontmatter is YAML; every agent reads the
value as `imagegen`. In HarnessKit the quoted name showed in listings,
`hk info imagegen` / `hk disable imagegen` failed with "Extension not
found", install paths that name the target directory after the parsed
name created a folder literally called `"imagegen"`, and a quoted name
written to Hermes' `skills.disabled` would never match.

- parse each value through serde_yaml: quotes and escapes are syntax;
  text YAML would not read as a plain string (an unquoted colon, a bare
  number) is kept verbatim so hand-written frontmatter keeps working;
  `bins` items go through the same helper instead of trimming quotes
- a skill without a frontmatter `name` is named after its whole
  directory name (`my.skill`, not `my`); one `skill_entry_name` serves
  the scanner, `find_skill_by_id` and `find_skill_content`, which each
  had their own `file_stem` fallback, and doubles as the lockfile key
- drop the quote-stripping workaround in `skill_locations`

Existing rows for such skills are re-keyed on the next scan (their id
derives from the name), so a HarnessKit-side disable on one of them is
reset once. Directories already created with quotes in their name are
left as they are.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@RealZST
RealZST merged commit fdbe22b into main Nov 1, 2026
4 checks passed
@RealZST
RealZST deleted the fix/skill-frontmatter-yaml-scalars branch November 1, 2026 05:58
RealZST added a commit that referenced this pull request Nov 1, 2026
Four places each decided on their own what counts as a skill entry in a
skills directory, where its file is, whether it is disabled and what it
is called: the scanner, the id lookup behind delete/detail, the content
reader behind the Web and desktop panels, and the name lookup behind
toggling. They drifted twice (#170: only the scanner missed a standalone
`.md.disabled`; #173: three different name fallbacks). `skill_entries`
now answers that question once and all four consume it; `enabled_skill_file`
gives the toggle-stable path that the scanner's `source_path` and the
toggle's rename target both need.

Two behaviours fall out of the unification and are pinned by tests:

- `skill_locations` matches a skill by its name only; the folder it lives
  in is no longer a second handle, since a different skill could be named
  after that folder.
- `skill_locations` includes standalone `.md` skills (when the adapter
  lists them), so a loose skill present under several agents is disabled
  and re-enabled on every agent, like directory skills already were.

The content reader no longer re-reads the file it just matched, and the
id-lookup drift guard added in #173 is dropped: with one implementation
there is nothing left to drift.

Co-authored-by: RealZST <zst9626@126.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
RealZST added a commit that referenced this pull request Nov 1, 2026
An extension id is derived from its name, so #173 reading quoted skill names differently made the next scan report the same file under a new id. The old marketplace row, whose file was still on disk, was kept as a scan gap and listed as a duplicate. A file now scanned for the same agent under another id is pruned and its install_meta moves to the new row unless it has its own; matching is on kind, agent and path for skills and plugins only. The two stale-prune loops are merged into one helper.

Co-authored-by: RealZST <zst9626@126.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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