fix(copilot): represent the documented skill frontmatter fields and keep unmodeled rule frontmatter - #2580
Merged
Merged
Conversation
…ing rule frontmatter on import The copilot skill schema carried only name/description/license/allowed-tools, so four documented fields — argument-hint, user-invocable, disable-model-invocation and context — could not be expressed for the target even though the sibling copilotcli already carries three of them and both write the same SKILL.md. All four are now in the tool schema and the copilot section of the canonical skill frontmatter, in both directions, and the two invocation gates read the top-level defaults through the shared resolvers (a section value, including false, still wins). copilotcli reads those defaults the same way now, so the siblings do not disagree about the same key. CopilotRuleFrontmatterSchema was a closed z.object, so importing a hand-written .github/instructions/*.instructions.md parsed fine but lost any field beyond the schema, and the next generate wrote the file back without it. It is a looseObject now and the rest-fields ride the tool-scoped copilot section, matching copilot-command.ts and copilot-subagent.ts and the project's own frontmatter guideline.
…ecedence Review follow-up: copilot and copilotcli write the same SKILL.md at both scopes and copilotcli writes last, so a value set in only one section does not survive when both targets are generated; context has no copilotcli counterpart. Also records that description/applyTo written in the copilot rule section lose to the canonical values, and adds the copilotcli counterpart of the top-level-defaults test.
The generation order follows the order the targets are listed in, not the tuple order, so which of copilot/copilotcli writes last is not fixed.
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
2 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.
Fixes the two oversights recorded in the 2026-07-30 re-check comment on #2401. The open design items (gaps 5 and 7, and
chat.permissions.default) are untouched, so the issue stays open.1. Four documented skill frontmatter fields were unrepresentable for
copilotVS Code documents
name,description,argument-hint,user-invocable,disable-model-invocationandcontext: fork;CopilotSkillFrontmatterSchemaemitted onlyname/description/license/allowed-tools, and thecopilot:section of the canonical skill frontmatter accepted onlylicense/allowed-tools. As the comment notes, this is an oversight rather than a scope call — the siblingcopilotclialready carries three of the four and both targets write the sameSKILL.md.All four are now in the tool schema and in the canonical
copilot:section, in both directions.The two invocation gates are resolved through the shared
resolveUserInvocable/resolveDisableModelInvocationhelpers, so the top-leveluser-invocable/disable-model-invocationdefaults apply and a section value — including an explicitfalse— still wins.copilotcliread only its own section before; it now goes through the same resolvers, so the two siblings cannot disagree about the same key for one skill. The docstrings and the docs list both targets accordingly.2.
CopilotRuleFrontmatterSchemasilently dropped frontmatter on importIt was a closed
z.object, so importing a hand-written.github/instructions/*.instructions.mdcarrying any field beyond the schema parsed fine but lost it, and the nextgeneratewrote the file back without it — the one Copilot adapter violating the project's ownlooseObjectfrontmatter guideline.It is a
z.looseObjectnow, andtoRulesyncRulecarries every field other thandescription/applyTo(which have canonical homes) into the tool-scopedcopilot:section, whichfromRulesyncRulespreads back out. That is the shapecopilot-command.tsandcopilot-subagent.tsalready use. The canonicalcopilot:rule section was already alooseObject, so nothing else had to change. One existing test asserted the old lossy behavior and now asserts the field survives.Verification
New tests cover the skill round-trip of all four fields, the top-level-default resolution with a section override, and an instructions file with an unmodeled field surviving import → generate. Full
pnpm cicheckis green, pluse2e-skills.spec.tsande2e-rules.spec.ts.Part of #2401
🤖 Generated with Claude Code