|
|
|
|
@@ -1,197 +0,0 @@
|
|
|
|
|
---
|
|
|
|
|
source_keys:
|
|
|
|
|
- agentskills-best-practices
|
|
|
|
|
- agentskills-optimizing-descriptions
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
# Retrofitting a skill to the ADR-0020 contract
|
|
|
|
|
|
|
|
|
|
Read this when `references/improve.md` Step 4 sends you here: the skill you are editing is over
|
|
|
|
|
the description or body budget and has to come into contract before any other change can be
|
|
|
|
|
committed. The gates are hot and carry no baseline file, so a one-line fix to a non-compliant
|
|
|
|
|
skill is blocked until this is done.
|
|
|
|
|
|
|
|
|
|
Measure first. Do not guess which gate fired: run `/skill-audit` on the directory and read its
|
|
|
|
|
`### Structure` dimension, which reports the description characters and the **body-only** word
|
|
|
|
|
count separately from the whole-file spec backstop. Retrofit against the number that actually
|
|
|
|
|
fired — a skill can sit a thousand words inside the whole-file backstop while failing the body
|
|
|
|
|
budget.
|
|
|
|
|
|
|
|
|
|
**Validate in place.** Audit the skill's real directory inside its package. Never audit a copy in a
|
|
|
|
|
scratch directory, and never move a skill out to work on it: the boundary-target universe is built
|
|
|
|
|
by walking up *from the file being checked*, so a copy with no authoring root above it resolves
|
|
|
|
|
against nothing and the check declines rather than running —
|
|
|
|
|
|
|
|
|
|
```text
|
|
|
|
|
INFO boundary-target resolution DID NOT RUN — no skill universe could be determined for
|
|
|
|
|
this path ... Unchecked target(s): totally-fake-target
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
The run still exits 0, so that line reads as a pass and is not one. Treat `DID NOT RUN` as **not
|
|
|
|
|
checked**, always. A retrofit signed off on a scratch copy carries an unverified boundary target
|
|
|
|
|
into the corpus, which is precisely the failure this gate exists to catch.
|
|
|
|
|
|
|
|
|
|
## Cut in this order
|
|
|
|
|
|
|
|
|
|
Work the list top down and stop as soon as the gate clears. The order is by ratio of tokens
|
|
|
|
|
removed to behaviour lost — inverting it is how a retrofit ends up deleting the one instruction
|
|
|
|
|
the skill existed to carry.
|
|
|
|
|
|
|
|
|
|
1. **Gotchas that paraphrase a step in the body below.** Zero information, and already a FAIL on
|
|
|
|
|
its own. Delete the Gotcha, keep the step.
|
|
|
|
|
2. **Spec restatements** — text that repeats a published specification, a tool's `--help`, or a
|
|
|
|
|
ceiling the validator already enforces. The agent gets this right without it. Delete, or move
|
|
|
|
|
the table to `references/` if a flow genuinely needs to look it up.
|
|
|
|
|
3. **Capability enumeration** — in a description, the feature list after the trigger clause; in a
|
|
|
|
|
body, the paragraph that recites what the skill can do. One capability clause survives in the
|
|
|
|
|
description; the rest belongs in `README.md`.
|
|
|
|
|
4. **Per-flow prose** — anything only one branch of the procedure ever reaches. This is the
|
|
|
|
|
largest single win in most bodies, and it is a *move*, not a delete: each flow gets its own
|
|
|
|
|
self-contained `references/` file, wired from a dispatch table.
|
|
|
|
|
|
|
|
|
|
If the body is still over after all four, the skill is doing two jobs. Split it, and say so
|
|
|
|
|
rather than compressing prose until it stops being readable.
|
|
|
|
|
|
|
|
|
|
## What "mutually exclusive flows" means
|
|
|
|
|
|
|
|
|
|
Two or more flows that a single invocation cannot both take. The three-way test, copied verbatim
|
|
|
|
|
from the body-discipline rubric `/skill-audit` judges against — nothing to load, it is quoted in
|
|
|
|
|
full here:
|
|
|
|
|
|
|
|
|
|
> separate subcommands, separate input types, separate lifecycle stages
|
|
|
|
|
|
|
|
|
|
Any one of the three is enough. Two flows that differ only in a parameter value are one flow.
|
|
|
|
|
At two or more mutually exclusive flows a dispatch table is **mandatory** regardless of word
|
|
|
|
|
count, because every invocation otherwise pays for every branch it did not take.
|
|
|
|
|
|
|
|
|
|
## Reference-file conventions
|
|
|
|
|
|
|
|
|
|
The create flow owns these rules, and this flow is forbidden from reading `references/create.md`,
|
|
|
|
|
so what a retrofit needs is restated here:
|
|
|
|
|
|
|
|
|
|
- **One topic per file.** A file mixing two concerns gets loaded for one of them and spends the
|
|
|
|
|
caller's context on the other.
|
|
|
|
|
- **Kebab-case filenames**, named after the topic rather than the flow that reads it —
|
|
|
|
|
`body-discipline.md`, not `step-3.md`.
|
|
|
|
|
- **Wire every file with the literal conditional form** ``If <condition>, read
|
|
|
|
|
`references/<file>.md` ``. A generic pointer ("see `references/` for details") is a Vale error.
|
|
|
|
|
- **Two hops from `SKILL.md`, never three.** A flow file may route on to a shared contract file;
|
|
|
|
|
a file reachable only through two intermediates is rarely loaded when it is needed.
|
|
|
|
|
- **`source_keys` frontmatter.** If the content you are moving drew on a research source, the new
|
|
|
|
|
file needs top-level `source_keys:` frontmatter listing those slugs, and every slug must already
|
|
|
|
|
exist as an `## <slug>` heading in `references/sources.md`. Moving sourced content out of
|
|
|
|
|
`SKILL.md` without carrying its slugs across breaks the provenance chain, and `/skill-audit`
|
|
|
|
|
reports the new file as an INFO with no `source_keys`.
|
|
|
|
|
|
|
|
|
|
## Collateral is mandatory, not optional
|
|
|
|
|
|
|
|
|
|
Moving content out of a `SKILL.md` leaves three files describing a structure that no longer
|
|
|
|
|
exists. `/skill-audit`'s provenance check exits clean on all three of these, so nothing catches
|
|
|
|
|
them for you. After every retrofit that adds, removes or renames a file:
|
|
|
|
|
|
|
|
|
|
- [ ] **`README.md` file table** — a row for every new `references/` file, and no row left for a
|
|
|
|
|
file that is gone. Say what triggers the load, not just what the file contains.
|
|
|
|
|
- [ ] **`references/README.md`**, where the skill has one — same update, same reason.
|
|
|
|
|
- [ ] **`references/sources.md` → `Contributing files`** — add the new file to every slug whose
|
|
|
|
|
content moved into it, and remove any file the retrofit deleted. This is the one that gets
|
|
|
|
|
missed: `sources.md` keeps citing sections of `SKILL.md` that no longer exist, the
|
|
|
|
|
provenance check still exits 0, and the stale claim survives review.
|
|
|
|
|
- [ ] **Reachability of every relocated gate.** For each Gotcha or gate the retrofit moved out of
|
|
|
|
|
the body, list the flows that need it and confirm each one reaches the surviving copy. A gate
|
|
|
|
|
that lands in a single flow file is invisible to every other branch, and no gate detects
|
|
|
|
|
that: `/skill-audit` reads whichever file it was handed, and the word counts improve either
|
|
|
|
|
way. Where more than one flow needs it, the copy belongs in the body's common-gates section,
|
|
|
|
|
not in a flow file. Grep the skill for the gate's key term and check every branch that hits
|
|
|
|
|
zero.
|
|
|
|
|
- [ ] Re-run `/skill-audit` and confirm its `### Provenance` dimension does not report the new
|
|
|
|
|
file as missing `source_keys`.
|
|
|
|
|
|
|
|
|
|
## Compression must not add authority the source text didn't have
|
|
|
|
|
|
|
|
|
|
This one is **not** part of the checklist above, and deliberately so: it fires on a wording change
|
|
|
|
|
with no file change at all, so a retrofit that adds and removes nothing still owes it.
|
|
|
|
|
|
|
|
|
|
The `sources.md` bullet above is about an entry going *stale* — Contributing files left uncited
|
|
|
|
|
after content moves. This is a distinct failure: a compression or rewrite pass that upgrades an
|
|
|
|
|
honest hedge in a Description into an unsupported confident claim, without the underlying source
|
|
|
|
|
having changed at all — "no forge-specific content drawn directly from it beyond that" quietly
|
|
|
|
|
becoming "Grounds Step 2's dispatch table."
|
|
|
|
|
|
|
|
|
|
`/skill-audit`'s provenance script does now notice this class: it diffs each slug's `Description`
|
|
|
|
|
and `Contributing files` text against a base ref and raises an **INFO** when the wording changed.
|
|
|
|
|
That is a prompt, not a verdict — it reports only *that* the claim moved, never whether the new
|
|
|
|
|
claim is true, because a bash script can verify an entry is internally consistent and nothing more.
|
|
|
|
|
Answering it is this flow's job: if a retrofit strengthens or otherwise changes the wording of a
|
|
|
|
|
provenance claim, re-read the upstream research doc first and confirm the stronger wording is
|
|
|
|
|
actually still true before committing it.
|
|
|
|
|
|
|
|
|
|
## Versioning a retrofitted skill
|
|
|
|
|
|
|
|
|
|
`SKILL.md` Step 4 says to bump the **patch** version on improve, which presumes there is a version
|
|
|
|
|
to bump. A pre-ADR-0020 skill often carries none — `metadata.version` only became mandatory under
|
|
|
|
|
ADR-0022, and this flow is exactly where those skills surface.
|
|
|
|
|
|
|
|
|
|
A skill with no `metadata.version` is **seeded at `"1.0.0"`, not bumped**. `"0.1.0"` is reserved
|
|
|
|
|
for a skill created new by the create flow: it means "created and never yet revised", which
|
|
|
|
|
understates a skill that has been through retrofit and audit passes without tracking a version.
|
|
|
|
|
Add the field in this retrofit — the `skill-frontmatter` pre-commit hook blocks the commit without
|
|
|
|
|
it.
|
|
|
|
|
|
|
|
|
|
## Worked example — a description retrofit
|
|
|
|
|
|
|
|
|
|
`gitea-issues` before, 827 characters, the single most common shape in the corpus:
|
|
|
|
|
|
|
|
|
|
```text
|
|
|
|
|
Use when reading or writing Gitea issues: listing repo issues, getting a single issue's details/
|
|
|
|
|
comments/labels, creating an issue, updating its state, adding or editing comments, applying
|
|
|
|
|
labels via issue_write, or searching issues/PRs across repositories. Triggers on "create an
|
|
|
|
|
issue", "what issues are open", "get issue #N", "close issue #N", "comment on issue #N", "search
|
|
|
|
|
issues for X" — even when the user doesn't say "Gitea" explicitly. Composes gitea-labels-
|
|
|
|
|
milestones for all label inference/resolution and milestone lookup — do not use this skill to
|
|
|
|
|
manage label or milestone definitions themselves (create/edit/delete a label, create/close a
|
|
|
|
|
milestone), that's gitea-labels-milestones directly. Do not use for pull requests (use gitea-prs)
|
|
|
|
|
or for local git branch/commit work (use gitea-branches or git-branches).
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
After, the 290 characters that shipped:
|
|
|
|
|
|
|
|
|
|
```text
|
|
|
|
|
Use when reading or writing Gitea issues — "create an issue", "what issues are open", "close
|
|
|
|
|
issue #N", "comment on issue #N", "search issues for X" — even when the user does not say
|
|
|
|
|
"Gitea". Not pull requests -> `gitea-prs`. Not label or milestone definitions ->
|
|
|
|
|
`gitea-labels-milestones`.
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
The retrofit kept the quoted-phrasing register and dropped the verb list, not the other way round.
|
|
|
|
|
Either register is admissible — what is banned is carrying both. Choose whichever routes better
|
|
|
|
|
for the skill in hand; here the quoted user phrasings do, because they are how people actually ask.
|
|
|
|
|
|
|
|
|
|
What came out, and why:
|
|
|
|
|
|
|
|
|
|
| Removed | Why |
|
|
|
|
|
|---|---|
|
|
|
|
|
| The second trigger register — `Triggers on "create an issue", "what issues are open", …` | The same triggers restated as quoted user phrasings. Two registers of one trigger list is a FAIL, not a suggestion. |
|
|
|
|
|
| `applying labels via issue_write` | Implementation detail. The router does not choose a skill by which MCP call it makes. |
|
|
|
|
|
| `Composes gitea-labels-milestones for all label inference/resolution and milestone lookup` | A composition note. It changes no routing decision and belongs in `README.md`. |
|
|
|
|
|
| The parenthetical `(create/edit/delete a label, create/close a milestone)` | Capability enumeration inside a boundary clause. The boundary needs the target, not its feature list. |
|
|
|
|
|
| The `gitea-branches` / `git-branches` boundary | Dropped entirely. Neither was ever going to win an issue request, so the clause defended against nothing — an invented boundary costs characters and buys no routing accuracy. |
|
|
|
|
|
| `Do not use for pull requests (use gitea-prs)` prose form | Kept, but rewritten as `Not pull requests -> \`gitea-prs\`.` The rewrite buys characters, one uniform shape for the router, **and** a stricter check: an unresolved arrow target is a blocking ERROR, while an unresolved prose target is only a SUGGESTION unless another target in the same sentence resolves. The prose form does not dangle as loudly. |
|
|
|
|
|
|
|
|
|
|
What stayed: one trigger clause, one capability clause, the indirect trigger (genuinely warranted
|
|
|
|
|
here — people say "create an issue", not "create a Gitea issue"), and the boundary clauses.
|
|
|
|
|
|
|
|
|
|
## Two rules the gates enforce but the prose does not spell out
|
|
|
|
|
|
|
|
|
|
**Boundary clauses may be plural.** Write one per genuine near-miss — the example above carries
|
|
|
|
|
two, because two different skills could each steal activations. "A boundary clause" in the
|
|
|
|
|
contract means *at least one*, not *exactly one*. What is banned is a boundary clause invented for
|
|
|
|
|
a skill that was never going to compete, not a second real one.
|
|
|
|
|
|
|
|
|
|
**Never let a hyphenated routing target wrap across lines in a folded `>` scalar.** YAML folding
|
|
|
|
|
replaces the newline with a space, so `gitea-labels-` at the end of one line and `milestones` at
|
|
|
|
|
the start of the next fold into `gitea-labels- milestones`. The gate then reads the target as
|
|
|
|
|
`gitea-labels`, finds no such skill, and reports a dangling boundary target. This is not
|
|
|
|
|
hypothetical — it is how `gitea-labels-milestones` broke (issue #100). It is fixed: the corpus
|
|
|
|
|
carries no dangling target today, and the repo's test suite pins that set as empty, so a
|
|
|
|
|
reintroduction fails the suite rather than joining a backlog. Reflow the line so the whole name
|
|
|
|
|
sits on one of them. The same applies to any backticked skill or agent name in a description.
|
