feat(skills): publish SKILL.md under its frontmatter identity - #4044
Open
twishabansal wants to merge 6 commits into
Open
twishabansal wants to merge 6 commits into
twishabansal wants to merge 6 commits into
Conversation
SEP-2640 fixes a SKILL.md as text/markdown and takes its name and description from frontmatter rather than the file backing it. A file resource would otherwise be listed as "SKILL.md", which tells a client nothing about which skill it belongs to. Discovery already parses and validates the frontmatter, so this wraps the SKILL.md resource with the identity it declares instead of re-reading it. Wrapping rather than setting the fields in each resource type keeps SKILL.md's shape out of file, text, and later directory, and applies on every protocol version -- a client that has not negotiated the skills extension still sees the files as ordinary resources, and should see a sensibly named one. Frozen until the next reload, which is sound here: validation ties frontmatter.name to the final URI segment, so a name can only change by renaming the directory, which changes URIs and needs a reload regardless. Only description can drift, and nothing verifies it. The wire name still comes from the resources map key, so the name reaches a client once #4014 wires it to GetName(). Verified by simulating that change locally: resources/list then reports analytics-guide rather than the configured key. mimeType and description take effect already.
twishabansal
added this pull request to stack #4016
September 15, 2026 15:37
twishabansal
marked this pull request as draft
September 15, 2026 15:37
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces functionality to publish SKILL.md resources under the identity (name and description) declared in their frontmatter, rather than using their backing resource configuration names. It implements a skillDoc wrapper in internal/skills/metadata.go and integrates it into the server's configuration initialization. Tests are also added to verify this behavior. The review feedback suggests making the description field optional in the frontmatter so that a skill is still wrapped and published under its frontmatter name even if a description is not provided.
Every existing case builds a map holding a single skill, so nothing pins that each SKILL.md takes its own frontmatter identity rather than another skill's. Two skills plus a supporting file close that gap.
WithDocMetadata skipped an entry whose frontmatter name or description was missing or not a string. Discover cannot produce one, since Entry.Validate rejects it first, so the branch was unreachable from the only caller. It was still the wrong behaviour to fall back on: a skip drops the SKILL.md from the result, and with it the mimeType fix, saying nothing about why. It now returns an error. Reusing requiredString keeps the message identical to the one Entry.Validate would have produced for the same frontmatter, so a caller assembling entries by hand reads the same diagnostic wherever the check fires.
Contributor
Author
|
TODO (rebase onto #4014): collapse the apply loop in #4014 keys the runtime resource map by URI ( for key, r := range resourcesMap {
if doc, ok := docs[r.GetURI()]; ok {
resourcesMap[key] = doc
}
}
It reduces to maps.Copy(resourcesMap, docs).
Flagging it because
#4014 edits server.go:250 and this edits 260-274 — the rebase will likely
merge clean and leave the redundant loop in place. |
twishabansal
marked this pull request as ready for review
September 15, 2026 18:26
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
3 participants
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.
Description
SEP-2640 fixes a SKILL.md as
text/markdown, and takes itsnameanddescriptionfrom the frontmatter, not from the resource that backs it. Otherwise a client sees a resource calledSKILL.md, which does not say which skill it belongs to.Discovery already parses and validates the frontmatter, so this wraps the SKILL.md resource with the identity it declares. A wrapper keeps SKILL.md's shape out of
file,text, and laterdirectory, and applies on every protocol version.The metadata is fixed until the next reload. Validation ties
frontmatter.nameto the final URI segment, so onlydescriptioncan drift.Depends on #4014