docs: document what qualifies as a skill - #92
Closed
MajorLift wants to merge 1 commit into
Closed
Conversation
Two admission questions that review currently answers ad hoc, and that no automated check can answer. The first is whether the thing is a skill at all. Skills shape generation; linters, hooks, and fitness functions enforce. A skill that substitutes for enforcement is unsafe, because an agent can ignore any context it is given. One that restates what a deterministic check already verifies is wasteful, spending context on every invocation to duplicate what CI produces for free. One that teaches the upstream pattern so enforcement rarely fires is the right shape, and should name the layer it pairs with. The second is whether it earns its context budget. Frontmatter for every installed skill loads at agent startup, so a skill that is never selected still costs its description on every run. Adapted from the review of ADR 0057 (MetaMask/decisions#162).
5 tasks
MajorLift
added a commit
that referenced
this pull request
Jul 31, 2026
## Summary Acceptance criteria for skill contributions, in both halves — a CI validator for the deterministic properties, and the two questions in `CONTRIBUTING.md` that no check can answer. From the [ADR 0057 review](MetaMask/decisions#162), where acceptance criteria were raised as an open item. ## The validator `yarn lint:skills` checks directory layout, `skill.md` path and name pattern, frontmatter keys and maturity values, `mms-` prefix misuse, repo-overlay names, and description length. `tools/skill-schema.mjs` is the single source of truth, imported by the linter so the documented and enforced schemas cannot drift. **Description budget is 1,536** — a repo budget, not an operator limit. An earlier draft used 1,024, justified as the per-operator minimum citing OpenCode. That does not hold up: `tools/install` emits the description verbatim with no truncation anywhere in the tooling, and six skills over 1,024 — up to 1,344 — install and load in Claude Code today. It also never bound anything, since the 46 skills on `main` have a median description of 45 characters and a maximum of 928. Enforcing it had a real cost. A description is the discovery surface and the only part of a skill carrying its own trigger cues, so trimming one to fit makes the skill less likely to be selected when it is relevant. The budget still exists — always-on context should be bounded — but at a number that reflects a deliberate choice rather than an unverified external claim, and the comment now asks for an operator and version before anyone tightens it. ## What running it against `main` turned up Three errors, two of which were defects in the repo rather than in the check: - **`workflows/` never shipped.** 14 files across two `web3-tools` skills, referenced **22 times** from their bodies, absent from the installer's bundle list — so every installed copy carried 22 dangling links and nothing reported it. Added to `copy_bundle_dirs`; all 22 now resolve, 42 files delivered. This is the third instance of one root cause, after `pr-validate`'s `hooks/` and domain `knowledge/`: a directory that exists in source and is not in the copy list. - **`metadata` was rejected** as unknown frontmatter, though README documents it as preserved through install. Now in the optional set. - **`performance` description was 1,078 chars.** Under the 1,536 budget, so it stays as authored — an earlier trim to 928 was reverted along with the ceiling that motivated it. Validator now reports **0 errors across 46 skills**. ## So the list stops being written twice Two tests read `tools/install` directly: - `BUNDLE_DIRS` must match the directories it copies. - Every key it reads via `frontmatter_value` must be declared in the schema. The comment asking a human to keep Bash and JS in sync is now checked. Knowledge files also get their own declared keys, since they take `domain` rather than the installer-behaviour keys skills use. ## And the half a linter cannot check `CONTRIBUTING.md` gains two questions: **Is it a skill, or does it belong in an enforcement layer?** Skills shape generation; linters, hooks, and fitness functions enforce. A skill that *substitutes* for enforcement is unsafe — an agent can ignore any context it is given. One that *restates* a deterministic check is wasteful. One that *teaches the upstream pattern* so enforcement rarely fires is the right shape, and should name the layer it pairs with. **Does it earn its context budget?** Frontmatter for every installed skill loads at agent startup, so a skill that is never selected still costs its `description` on every run. ## Keeping the number honest Two tests, because the failure mode here was documentation drifting from enforcement in both directions: - The boundary case derives its length from `DESCRIPTION_MAX` rather than hardcoding one, so it keeps testing the boundary when the budget moves. The previous hardcoded 1,100 silently stopped exercising anything the moment the ceiling rose. - A new check fails if `README.md`, `CONTRIBUTING.md`, or `.github/SKILL_TEMPLATE.md` state a figure the schema does not enforce. All three had said 1,536 while the schema said 1,024; reconciling them by editing the docs down was the wrong direction, and this makes that disagreement fail rather than get silently resolved. ## Test plan - [x] `node .github/scripts/lint-skill-entry.mjs` — 0 errors, 46 skills - [x] `test/lint-skill-entry.test.mjs` — 10 pass, including both drift checks - [x] `test/cli.test.mjs` 11 pass, `test/unit.test.mjs` 30 pass - [x] Real install of `web3-tools`: 22/22 `workflows/` references resolve, 42 files delivered - [x] 84 remaining warnings are all missing recommended sections — advisory, non-blocking ## Notes - Supersedes #92, folded here: the check handles what it can, the prose says where to spend review attention instead. - 84 warnings suggest either the recommended sections are wrong for this corpus or most skills predate them. Worth a follow-up decision; not blocking, since warnings do not fail CI.
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
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.
Summary
Adds two admission questions to
CONTRIBUTING.mdthat review currently answers ad hoc, and that no automated check can answer. Adapted from the ADR 0057 review, where acceptance criteria were raised as an open item.Is it a skill, or does it belong in an enforcement layer?
Skills shape generation. Linters, hooks, and fitness functions enforce. Both should exist for any pattern load-bearing enough to encode — the section adds a table of the four layers and where each sits.
Three ways a proposal fails:
The review question becomes "is this doing generation-time work the linter cannot?" rather than "is this redundant with the linter?"
Does it earn its context budget?
Frontmatter for every installed skill loads at agent startup, as fixed overhead growing linearly with the catalogue — a skill that is never selected still costs its
descriptionon every run. Criteria: not a duplicate, actionable rather than aspirational, scoped so a reader can tell when it applies, description within the documented budget.Notes
CHANGELOG.mdentry.yarn test, so review attention goes to the two questions no check can answer.