feat(skill): read and write SKILL.md, the Agent Skills format - #435
Merged
Conversation
Adds skill/skillmd, a codec for the Agent Skills specification (agentskills.io): the file format now conformed to by Claude Code, Codex, Cursor, Copilot, Gemini CLI and roughly forty more clients. This is the seam that makes the existing ecosystem an input and makes our own skills usable by harnesses we do not control. Parse is tolerant and Validate is strict, which is the central design choice. Import has to swallow files written by clients with no shared validator, against a specification that is silent on what an unrecognised frontmatter key means; export has to emit something a conformant reader cannot reject. So Parse keeps undefined keys in Doc.Unknown rather than dropping or refusing them, preserving their original order so re-exporting a foreign pack does not rewrite it, and Validate applies the specification's constraints to documents we are about to publish. No YAML dependency. The frontmatter is a closed set of six keys, five scalars and one flat string map, and a general YAML engine would bring anchors, aliases, merge keys and implicit coercion to the job of reading files fetched from public registries. The grammar here covers what the format uses and refuses the rest by construction. Extensions go where the specification sanctions them: metadata for "properties not defined by the Agent Skills spec", and compatibility for runtime requirements. No custom top-level keys. allowed-tools is parsed but documented as a claim, never a grant. It is a skill declaring which tools it would like pre-approved, it is marked experimental, and the specification says nothing about trust, so honouring it from an unauthored pack would let a downloaded text file widen its own permissions. Tested with a table over the grammar, a round-trip property, a second-round-trip fixed-point property, and a fuzz target; skillmd is added to the rigor gate's fuzz-required list. The property test caught an undefined key with an empty value being silently dropped. 10.6M fuzz executions found no further round-trip violation. Signed-off-by: Ion Alpha <damienrch@gmail.com>
Two defects in the capture path, both of which would surface the moment a bundled craft pack shares a scope with the learning loop. Curate keyed the upsert on (scope, slug) and only read the previous record to carry uses/wins forward. A distilled lesson whose title slugified onto an authored skill's slug therefore replaced that skill's body, check and tags with no signal that curated content had just been lost. Capture now looks the slug up once, and refuses to store when an existing skill in the scope lacks the learned-provenance tag, recording the refusal in Captured.Skipped so the caller can tell "learned nothing new" from "would have clobbered an authored skill". A learned skill re-captures itself as before, since it carries the tag. The guard runs before the verifier, so a capture that will be skipped costs no sandbox time. slugify did not bound length, so a long lesson title produced a slug over the Agent Skills name limit and a title with no alphanumerics fell back to the literal "skill". Neither is a conformant name, so a learned skill was not exportable as a SKILL.md, quietly breaking the claim that authored and learned skills are one record type. slugify now caps at skillmd.MaxNameLen where the name is minted, and a property test asserts every slug it produces passes skillmd.ValidateName. Signed-off-by: Ion Alpha <damienrch@gmail.com> Signed-off-by: Ion Alpha <contact@ionalpha.io>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Bring skill/skillmd to full statement coverage: an edge table for the block-scalar chomping, interior blank lines, the double-quote escapes and the metadata-block boundaries, a rejection table for the bare and never-closed openers, an inline metadata value, and the quote failures, plus a direct check that Validate runs the name rule. Rewrite indentOf as a branch-free leading-space count. The all-spaces return was unreachable through Parse, since every all-whitespace line is filtered as skippable before indentOf sees it; the one-liner is identical in behaviour and carries no dead branch. Signed-off-by: Ion Alpha <contact@ionalpha.io>
The mutation gate kept posting a red advisory check on this PR. The cause was not the code under test: every skill/skillmd mutant was being KILLED. gremlins gives each mutant a test timeout of (baseline suite duration x timeout-coefficient), and the default coefficient let a single INCREMENT_DECREMENT mutant - which flips a loop counter's ++ to --, turning the suite into an infinite loop - stall the run for ~70s. Several such runaways ran in parallel and starved the runner, which was reclaimed mid-run (~63% through the file). A runner-level cancellation reports the job's check as failed regardless of exit status, and it kills the shell before the step's `|| echo` guard can swallow it, so the advisory signal went red for a result it never produced. Bound it three ways: cap timeout-coefficient to 3 so a runaway dies at a small multiple of the suite time, cap workers to 2 so timed-out mutants can't pile up and exhaust the runner, and add MUTATE_TIMEOUT so dev/mutate wraps gremlins in `timeout`. The wrapper ends an over-long run ourselves with a catchable non-zero status the `|| echo` swallows, instead of leaving the runner to be reclaimed - the only failure mode the existing guard could not catch. Signed-off-by: Ion Alpha <contact@ionalpha.io>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
What
Adds
skill/skillmd: a codec for the Agent SkillsSKILL.mdformat.Parse,Format,Validate,ValidateName, and aDoccarrying every field the specification defines (name,description,license,compatibility,metadata,allowed-tools). Registers the package on the rigor gate'sFuzzRequiredlist.Why
SKILL.mdis a governed open standard (agentskills.io), conformed to by Codex CLI, Cursor, Copilot, Gemini CLI and others. A readable and writable codec makes the external ecosystem a supported input and makes our output usable by harnesses we do not control. This lands the codec half; thefs.FSloader and directory writer follow separately.How to verify
go test ./skill/skillmd/...covers the grammar (plain, quoted and block scalars, CRLF, BOM, bodies containing their own---, one nesting level undermetadata), every rejection path, a lossless round-trip property, a second-round-trip fixed-point property over awkward values, and a fuzz target over the frontmatter parser.Notes for reviewers
Design decisions, with reasons so they are not relitigated:
Doc.Unknownin original order so re-exporting a foreign pack does not rewrite it.Validateis the only place that judges them.allowed-toolsis parsed as a claim, never a grant. Honouring it from an unauthored pack would let a downloaded text file widen its own permissions. The admission decision belongs elsewhere.MaxDocSizeof 1 MiB is ours: the format sets no ceiling and a loader over an untrusted tree needs one. Malformed frontmatter fails loudly rather than producing an empty skill.