docs: amend ADR-0020 and correct the gate reference to match what ships
The ADR said a /slash target behind a route verb takes the follower test; the implementation decides notation first and skips it. Recorded as a dated amendment rather than a silent edit, per the ADR-0016/0017 convention. Its Enforcement table called itself exhaustive 'because the failure this ADR is most exposed to is a rule filed under Enforcement that no validator implements'. Three shipped behaviours were missing, including the disable-model-invocation carve-out that removes two rows. Also de-pins the 'zoom-out is the one carrier' claim, which caveman falsified, and the stale dangling-target statuses. gates.md's ERROR row made terminality a conjunct for route notation, telling an author a form is safe that exits 1. It now documents the references/ Vale blind spot and its two independent causes -- AGENTS.md trimmed to the operative rule per the split gates.md itself states -- plus the undocumented skill-frontmatter hook and the second scope exclusion. CONTEXT.md glosses 'routing target'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJJrm5YmacbwMdzZpXcoti
This commit is contained in:
@@ -36,7 +36,7 @@ Fall back to raw shell only when no skill covers it.
|
|||||||
- **Do not add repo-owned keys to `.claude/settings.json`.** apm treats it as its own deployed artifact and `apm audit --ci` replays the install and diffs, so anything apm would not have written is permanent drift that fails the `apm-audit-ci` pre-push hook. A hook you want here is authored in `plugins/<name>/.apm/hooks/` and deployed by apm, never hand-written into that file. The `SessionStart` entry already in it is exactly that: kyberforge authors it in `plugins/kyberforge/.apm/hooks/hooks.json` and apm merges it in, so it is apm's own output, it is what the replay expects, and it belongs in the commit — do not strip it (ADR-0019). Machine-specific settings go in the gitignored `.claude/settings.local.json`; shared enforcement goes in `.pre-commit-config.yaml`.
|
- **Do not add repo-owned keys to `.claude/settings.json`.** apm treats it as its own deployed artifact and `apm audit --ci` replays the install and diffs, so anything apm would not have written is permanent drift that fails the `apm-audit-ci` pre-push hook. A hook you want here is authored in `plugins/<name>/.apm/hooks/` and deployed by apm, never hand-written into that file. The `SessionStart` entry already in it is exactly that: kyberforge authors it in `plugins/kyberforge/.apm/hooks/hooks.json` and apm merges it in, so it is apm's own output, it is what the replay expects, and it belongs in the commit — do not strip it (ADR-0019). Machine-specific settings go in the gitignored `.claude/settings.local.json`; shared enforcement goes in `.pre-commit-config.yaml`.
|
||||||
- **`apm.lock.yaml` turning up modified is expected, not a bug.** kyberforge's `SessionStart` hook runs `apm outdated` at startup and `apm update --yes` when something is behind, which rewrites the lock. Commit or discard it deliberately.
|
- **`apm.lock.yaml` turning up modified is expected, not a bug.** kyberforge's `SessionStart` hook runs `apm outdated` at startup and `apm update --yes` when something is behind, which rewrites the lock. Commit or discard it deliberately.
|
||||||
- **A `.apm/` edit is not live in this session until it is pushed.** The six dependencies resolve from the holocron remote, unpinned against the default branch. `apm install` deploys from the lock; `apm update` is what re-resolves refs.
|
- **A `.apm/` edit is not live in this session until it is pushed.** The six dependencies resolve from the holocron remote, unpinned against the default branch. `apm install` deploys from the lock; `apm update` is what re-resolves refs.
|
||||||
- **The ADR-0020 skill gates ship hot, with no baseline — and the corpus is now clean.** All 39 skills clear both FAIL tiers: no description over 400 characters, no body over 900 words (counted body-only). Retrofitted plugin by plugin under #99 (see `docs/spec/gates.md`). Because nothing is grandfathered, the gates now bite on first commit — a new skill, or an edit that pushes a description past 400, is blocked until it complies. **No routing target dangles**, and `tests/test-adr0020-targets.sh` pins that set as empty, so a new boundary clause naming a non-existent skill fails the suite rather than joining a backlog. Two blind spots survive: `skill-size-check` does not cover the Vale half, so `Kyberforge.CompositionNote` fires nowhere today but any new description can reintroduce it; and every `references/` file is unlinted — which matters because the contract's own remedy is to move prose *into* `references/`, out of the prose gate's reach. That blind spot has **two** independent causes and closing either alone changes nothing: the `Kyberforge` style is scoped `[**/SKILL.md]` (the cause #117 records), *and* the `vale-audit-prefilter-skill` hook filters on `files: '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$'`, so a reference file is never handed to Vale whatever the style says. Check both gates: `pre-commit run --all-files`.
|
- **The ADR-0020 skill gates ship hot, with no baseline — and the corpus is now clean.** All 39 skills clear both FAIL tiers: no description over 400 characters, no body over 900 words (counted body-only). Retrofitted plugin by plugin under #99 (see `docs/spec/gates.md`). Because nothing is grandfathered, the gates now bite on first commit — a new skill, or an edit that pushes a description past 400, is blocked until it complies. **No routing target dangles**, and `tests/test-adr0020-targets.sh` pins that set as empty, so a new boundary clause naming a non-existent skill fails the suite rather than joining a backlog. Two blind spots survive: `skill-size-check` does not cover the Vale half, so `Kyberforge.CompositionNote` fires nowhere today but any new description can reintroduce it; and no `references/` file is linted by anything, so prose relocated out of a body to satisfy the word gate lands outside the prose gate. It has two independent causes and closing either alone changes nothing — `docs/spec/gates.md` has both, issue #117 tracks it. Check both gates: `pre-commit run --all-files`.
|
||||||
- **Run `bash tests/run-tests.sh --strict` before considering any change done.** Keep the flag: without it a suite whose dependency is missing exits 77 and is counted SKIPPED rather than failed, so the run goes green having verified less than it claims.
|
- **Run `bash tests/run-tests.sh --strict` before considering any change done.** Keep the flag: without it a suite whose dependency is missing exits 77 and is counted SKIPPED rather than failed, so the run goes green having verified less than it claims.
|
||||||
- **Before pushing, rehearse the gate locally:** `pre-commit run --hook-stage pre-push --all-files`. It runs the 14 pre-push hooks this repo authors itself plus pre-commit's 2 `meta` hooks, so it prints 16; `check-release-needed` passes without checking anything, because it needs a real push to `main`. `docs/spec/gates.md` reconciles both.
|
- **Before pushing, rehearse the gate locally:** `pre-commit run --hook-stage pre-push --all-files`. It runs the 14 pre-push hooks this repo authors itself plus pre-commit's 2 `meta` hooks, so it prints 16; `check-release-needed` passes without checking anything, because it needs a real push to `main`. `docs/spec/gates.md` reconciles both.
|
||||||
- **Pushing without a network** needs `SKIP=apm-marketplace-check,apm-pack-check-clean git push` — those two resolve a remote marketplace entry via `git ls-remote`. Skip only those two; the rest are real local checks, and adding one to `SKIP` disarms it silently.
|
- **Pushing without a network** needs `SKIP=apm-marketplace-check,apm-pack-check-clean git push` — those two resolve a remote marketplace entry via `git ls-remote`. Skip only those two; the rest are real local checks, and adding one to `SKIP` disarms it silently.
|
||||||
|
|||||||
@@ -26,6 +26,14 @@ trigger clause, at most one capability clause, and a boundary clause, and nothin
|
|||||||
and the target-resolution walk: `docs/spec/gates.md`.
|
and the target-resolution walk: `docs/spec/gates.md`.
|
||||||
_Avoid_: skill budget, size limit
|
_Avoid_: skill budget, size limit
|
||||||
|
|
||||||
|
**Routing target**:
|
||||||
|
The skill or agent name a boundary clause sends work to. It **resolves** when a skill or agent of
|
||||||
|
that name is reachable from the file being checked, and **dangles** when none is — a route the router
|
||||||
|
cannot take. Dangling is a blocking ERROR in route notation (`/name`, `→ name`) and a SUGGESTION for
|
||||||
|
a bare name nothing else in the sentence corroborates. Verdicts and the resolution walk:
|
||||||
|
`docs/spec/gates.md`.
|
||||||
|
_Avoid_: route, pointer, cross-reference
|
||||||
|
|
||||||
**Dispatch body**:
|
**Dispatch body**:
|
||||||
The body pattern a skill with two or more mutually exclusive flows must use — the body carries only
|
The body pattern a skill with two or more mutually exclusive flows must use — the body carries only
|
||||||
the dispatch table and the gates common to every branch, and each flow lives in its own
|
the dispatch table and the gates common to every branch, and each flow lives in its own
|
||||||
|
|||||||
@@ -203,8 +203,14 @@ with no trigger list.
|
|||||||
|
|
||||||
Verified end-to-end rather than assumed: `plugins/bin/.apm/skills/zoom-out/SKILL.md:4` carries the
|
Verified end-to-end rather than assumed: `plugins/bin/.apm/skills/zoom-out/SKILL.md:4` carries the
|
||||||
flag, apm passes it through verbatim to both `.claude/skills/zoom-out/SKILL.md:4` and the flat mirror
|
flag, apm passes it through verbatim to both `.claude/skills/zoom-out/SKILL.md:4` and the flat mirror
|
||||||
at `plugins/bin/skills/zoom-out/SKILL.md:4`, and `zoom-out` is the one installed skill absent from
|
at `plugins/bin/skills/zoom-out/SKILL.md:4`, and `zoom-out` was — at the time of that check, when it
|
||||||
the model-visible skill listing in a live session. It remains invocable as `/zoom-out`.
|
was the only carrier — the one installed skill absent from the model-visible skill listing in a live
|
||||||
|
session. It remains invocable as `/zoom-out`. `caveman` has since taken the flag as well, so the
|
||||||
|
corpus now has **two** carriers. Do not read a carrier list off this page; re-derive it:
|
||||||
|
|
||||||
|
```
|
||||||
|
grep -l '^disable-model-invocation: true' plugins/*/.apm/skills/*/SKILL.md
|
||||||
|
```
|
||||||
|
|
||||||
### Merging siblings
|
### Merging siblings
|
||||||
|
|
||||||
@@ -219,10 +225,13 @@ rather than the core job.
|
|||||||
skills still exist separately, and this change made the split deeper rather than shallower: retrofit
|
skills still exist separately, and this change made the split deeper rather than shallower: retrofit
|
||||||
to the dispatch pattern took `skill-audit` from 3 reference files to 7 and `agent-audit` from 4 to 8,
|
to the dispatch pattern took `skill-audit` from 3 reference files to 7 and `agent-audit` from 4 to 8,
|
||||||
and their two same-named `references/description-quality.md` files now differ on 100 of ~120 lines
|
and their two same-named `references/description-quality.md` files now differ on 100 of ~120 lines
|
||||||
after normalising `skill`/`agent`, where before they were closer. The merge stays the decision; it
|
after normalising `skill`/`agent`, where before they were closer. It has kept deepening since: the
|
||||||
reopens ADR-0008 (agent-audit's single-file invocation contract) and touches every call site in
|
#99 retrofit added `finding-criteria.md` to `skill-audit`, drawing it level with `agent-audit`. Both
|
||||||
`skill-author`, `agent-author` and `forge`, which is why it is its own change and not a rider on
|
figures move with the next retrofit, so measure rather than quote —
|
||||||
this one. Recorded here rather than dropped, so the gap between the rule and the tree is deliberate
|
`ls plugins/kyberforge/.apm/skills/<name>/references/ | grep -c '\.md$'`. The merge stays the
|
||||||
|
decision; it reopens ADR-0008 (agent-audit's single-file invocation contract) and touches every call
|
||||||
|
site in `skill-author`, `agent-author` and `forge`, which is why it is its own change and not a rider
|
||||||
|
on this one. Recorded here rather than dropped, so the gap between the rule and the tree is deliberate
|
||||||
and dated instead of discovered later.
|
and dated instead of discovered later.
|
||||||
|
|
||||||
### Enforcement and rollout
|
### Enforcement and rollout
|
||||||
@@ -233,11 +242,13 @@ which tier each rule is in, because the failure this ADR is most exposed to is a
|
|||||||
|
|
||||||
| Check | Applies to | Tier | Home |
|
| Check | Applies to | Tier | Home |
|
||||||
|---|---|---|---|
|
|---|---|---|---|
|
||||||
| description characters (250 SUGGESTION / 400 FAIL) | skills, agents | deterministic | `scripts/skill-size-check.sh`; constants mirrored in `skill-audit/scripts/validate.sh` and `agent-audit/scripts/validate.sh` |
|
| description characters (250 SUGGESTION † / 400 FAIL) | skills, agents | deterministic | `scripts/skill-size-check.sh`; constants mirrored in `skill-audit/scripts/validate.sh` and `agent-audit/scripts/validate.sh` |
|
||||||
| body-only words (600 SUGGESTION / 900 FAIL) | skills | deterministic | `skill-size-check.sh`, `skill-audit/scripts/validate.sh` |
|
| body-only words (600 SUGGESTION / 900 FAIL) | skills | deterministic | `skill-size-check.sh`, `skill-audit/scripts/validate.sh` |
|
||||||
| description present and non-empty (ERROR) | skills, agents | deterministic | same |
|
| description present and non-empty (ERROR) | skills, agents | deterministic | same |
|
||||||
| boundary target resolves to a real skill or agent (ERROR when written as `/name` or `-> name`, or when its own sentence names another target that resolves; SUGGESTION otherwise) | skills, agents | deterministic | same |
|
| boundary target resolves to a real skill or agent (ERROR when written in route notation — `/name`, or any arrow form; or when a *terminal* bare name's own sentence names another target that resolves; SUGGESTION otherwise) | skills, agents | deterministic | same |
|
||||||
| boundary clause absent (SUGGESTION) | skills, agents | deterministic | same |
|
| boundary clause absent — `absent` (SUGGESTION) † | skills, agents | deterministic | same |
|
||||||
|
| an arrow clause is present but no target can be read out of it — `unparsed` (SUGGESTION) † | skills, agents | deterministic | same |
|
||||||
|
| one arrow clause naming two or more targets, of which only the first is resolved (SUGGESTION, issue #107) † | skills, agents | deterministic | same |
|
||||||
| Gotchas entry count over five (SUGGESTION) | skills | deterministic | same |
|
| Gotchas entry count over five (SUGGESTION) | skills | deterministic | same |
|
||||||
| Gotchas over 25% of the body (SUGGESTION) | skills | deterministic | same |
|
| Gotchas over 25% of the body (SUGGESTION) | skills | deterministic | same |
|
||||||
| every `references/<file>.md` a body names exists (ERROR) | skills | deterministic | same |
|
| every `references/<file>.md` a body names exists (ERROR) | skills | deterministic | same |
|
||||||
@@ -254,6 +265,21 @@ that guessed at them would be a worse gate than no gate, because it would be bel
|
|||||||
enforced, they are reviewed, and this table exists so that distinction is written down rather than
|
enforced, they are reviewed, and this table exists so that distinction is written down rather than
|
||||||
inferred from whether a validator happens to have been written yet.
|
inferred from whether a validator happens to have been written yet.
|
||||||
|
|
||||||
|
**† These four, and only these four, are lifted for a hand-invoked file** — one whose frontmatter
|
||||||
|
carries `disable-model-invocation: true`, read as a boolean by `hand_invoked()` in all three scripts.
|
||||||
|
No validator knew the field existed (issue **#108**), so every routing SUGGESTION above fired on
|
||||||
|
exactly the shape the *Invocation as a design axis* section mandates, and the boundary-clause
|
||||||
|
remedy — "so the router knows where NOT to send this skill" — was addressed to a router that cannot
|
||||||
|
see the skill at all. An author who took the advice made the file worse.
|
||||||
|
|
||||||
|
What does **not** lift is the point of the carve-out. Both body word tiers stand: the body is still
|
||||||
|
loaded on invocation and still competes with the caller's live conversation. The 400-character
|
||||||
|
description FAIL stands: that description is not preloaded, but it is the one line a user reads when
|
||||||
|
choosing from the `/` menu, and the ceiling is an outlier stop rather than a routing-quality budget —
|
||||||
|
which is exactly why the 250-character *target* is the tier that lifts. And a target the description
|
||||||
|
does happen to name is still resolved and can still dangle as a blocking ERROR. Mechanics, and the
|
||||||
|
reason the field is read as a boolean rather than as a mention of the key: `docs/spec/gates.md`.
|
||||||
|
|
||||||
Two of the deterministic rows are tuned for **false positives over recall**, and what they decline to
|
Two of the deterministic rows are tuned for **false positives over recall**, and what they decline to
|
||||||
see is part of the contract. On target extraction: a bare hyphenated name counts only inside a
|
see is part of the contract. On target extraction: a bare hyphenated name counts only inside a
|
||||||
boundary sentence, and a single-word name is never matchable bare — `research`, `triage`, `forge`,
|
boundary sentence, and a single-word name is never matchable bare — `research`, `triage`, `forge`,
|
||||||
@@ -264,7 +290,8 @@ raise an error: one followed by an ordinary lowercase noun is a compound **modif
|
|||||||
confirm-only — it still resolves and still counts as a route when the name exists, but it can never
|
confirm-only — it still resolves and still counts as a route when the name exists, but it can never
|
||||||
dangle. Only a *terminal* target can. The compressed arrow form `→ <name>` is exempt from that
|
dangle. Only a *terminal* target can. The compressed arrow form `→ <name>` is exempt from that
|
||||||
follower test and is always error-eligible, because nothing reads as a compound modifier after an
|
follower test and is always error-eligible, because nothing reads as a compound modifier after an
|
||||||
arrow; a `/slash` target reached through a route verb is **not** exempt and takes the same test. The
|
arrow; a `/slash` target reached through a route verb is **not** exempt and takes the same test.
|
||||||
|
*Amended 2026-08-31 — the `/slash` half is reversed: it is exempt too. See the amendment below.* The
|
||||||
simpler rule — "only marked targets may dangle" — was available and would have been wrong here: both
|
simpler rule — "only marked targets may dangle" — was available and would have been wrong here: both
|
||||||
live true positives are bare, `research`'s "(use neuledge-context)" and the `gitea-labels-` /
|
live true positives are bare, `research`'s "(use neuledge-context)" and the `gitea-labels-` /
|
||||||
`milestones` fold. On the body-shape checks: a `## Gotchas` heading must *end* in "gotchas", not
|
`milestones` fold. On the body-shape checks: a `## Gotchas` heading must *end* in "gotchas", not
|
||||||
@@ -296,6 +323,46 @@ Three pre-existing contradictions are fixed in the same change, because they are
|
|||||||
- `description-quality.md:45-50` has no FAIL condition for internal-mechanics content, which is why
|
- `description-quality.md:45-50` has no FAIL condition for internal-mechanics content, which is why
|
||||||
`skill-author/SKILL.md:102` never bit.
|
`skill-author/SKILL.md:102` never bit.
|
||||||
|
|
||||||
|
## Amendment (2026-08-31): route notation short-circuits the follower test, `/name` included
|
||||||
|
|
||||||
|
The Enforcement section above exempts the arrow form from the follower test and then withholds the
|
||||||
|
same exemption from `/name`: "a `/slash` target reached through a route verb is **not** exempt and
|
||||||
|
takes the same test." That half is reversed. **Both spellings of route notation are exempt, and the
|
||||||
|
exemption is decided before the follower test rather than weighed against it.**
|
||||||
|
|
||||||
|
Three things make the original call wrong rather than merely strict.
|
||||||
|
|
||||||
|
**It contradicted the promise the same paragraph makes.** Route notation is offered to an author as
|
||||||
|
the way to get a target checked unconditionally — the SUGGESTION text on an unpromoted target says
|
||||||
|
so in as many words: "write it as `/name` or `-> name` and it will be checked properly." Under the
|
||||||
|
original rule that was true of one of the two spellings. `-> name` reached `_add()` with
|
||||||
|
`strict=True` from both its call sites; `/name` did not, so it fell through to `_terminal()` and any
|
||||||
|
follower outside `FOLLOWER_OK` demoted it. `Do not use for Y — use /no-such-skill afterwards.` exited
|
||||||
|
0 — and, before the companion visibility fix, in total silence.
|
||||||
|
|
||||||
|
**The follower test's own justification does not reach `/name`.** That test exists for *prose*: a
|
||||||
|
bare hyphenated token followed by an ordinary lowercase noun is a compound modifier, "pre-commit
|
||||||
|
hooks" and "pull-request template". A leading slash is Claude Code's invocation syntax and occurs in
|
||||||
|
no English compound, so there is no attributive reading to protect. The exemption was withheld from
|
||||||
|
the one shape the rule it protects against cannot describe.
|
||||||
|
|
||||||
|
**`FOLLOWER_OK` is a closed whitelist of roughly eighty words, and a closed list is the wrong thing
|
||||||
|
to hang a blocking gate on.** Leaving `/name` under it made *whether a commit is blocked* depend on
|
||||||
|
whether someone had thought to enumerate the next word — the gate failing open on its own
|
||||||
|
unfamiliarity. The bare-target path keeps the follower test precisely because it needs a brake it can
|
||||||
|
justify; the notation path asked for one and was given the same brake by accident.
|
||||||
|
|
||||||
|
What is unchanged: the **corroboration** branch. A *bare* terminal name still earns its blocking
|
||||||
|
ERROR only from a resolving sibling in the same sentence, and a compound modifier still cannot
|
||||||
|
dangle at all. The conservative tuning that decision rests on is untouched — this amendment moves one
|
||||||
|
explicitly-marked spelling out from under it, not the prose path.
|
||||||
|
|
||||||
|
Verified on fixtures inside a synthetic plugin tree: `… Do not use for Y — use /no-such-skill
|
||||||
|
afterwards.` exits 1, while the same sentence with the bare `no-such-skill` exits 0 at SUGGESTION,
|
||||||
|
and rises to a blocking ERROR the moment a resolving sibling joins it. The reasoning is recorded at
|
||||||
|
the point of enforcement in `_add()`'s docstring in `scripts/skill-size-check.sh` and its two
|
||||||
|
mirrored copies, and the verdict table in `docs/spec/gates.md` states the corrected shape.
|
||||||
|
|
||||||
## Consequences
|
## Consequences
|
||||||
|
|
||||||
**Editing any non-compliant skill now requires retrofitting it first.** At decision time, 30 of 39
|
**Editing any non-compliant skill now requires retrofitting it first.** At decision time, 30 of 39
|
||||||
@@ -342,8 +409,8 @@ and `git-*` families — where every sibling shares a keyword and boundary claus
|
|||||||
— are the ones most likely to sit at the FAIL tier permanently. If the retrofit shows that family
|
— are the ones most likely to sit at the FAIL tier permanently. If the retrofit shows that family
|
||||||
routing degrades, the tier is the first thing to revisit.
|
routing degrades, the tier is the first thing to revisit.
|
||||||
|
|
||||||
**Four broken routing targets were found; two are fixed here and two are live.** Tracked as issue
|
**Four broken routing targets were found; two were fixed here and two shortly after.** Tracked as
|
||||||
#100.
|
issue #100.
|
||||||
|
|
||||||
- `skill-audit` routed to `/skill-improve` twice in its description plus `README.md:10`, and no such
|
- `skill-audit` routed to `/skill-improve` twice in its description plus `README.md:10`, and no such
|
||||||
skill exists — the real target is `skill-author`. **Fixed here**, as a side effect of retrofitting
|
skill exists — the real target is `skill-author`. **Fixed here**, as a side effect of retrofitting
|
||||||
@@ -353,14 +420,24 @@ routing degrades, the tier is the first thing to revisit.
|
|||||||
detectable by the resolvable-target check and never will be: "examine agent files manually" names
|
detectable by the resolvable-target check and never will be: "examine agent files manually" names
|
||||||
no target, and a check that resolves names cannot see a name that is absent. A misroute to nowhere
|
no target, and a check that resolves names cannot see a name that is absent. A misroute to nowhere
|
||||||
is a review finding, not a gate finding.
|
is a review finding, not a gate finding.
|
||||||
- `research` routes to `neuledge-context`, which exists only inside that string. **Live.**
|
- `research` routes to `neuledge-context`, which exists only inside that string. Was **live**;
|
||||||
|
**fixed under #99** — the retrofitted description names no such target.
|
||||||
- `gitea-issues` carries the literal string `gitea-labels- milestones` in its folded description, a
|
- `gitea-issues` carries the literal string `gitea-labels- milestones` in its folded description, a
|
||||||
stray space introduced by YAML wrapping mid-token, breaking the skill name in preloaded text.
|
stray space introduced by YAML wrapping mid-token, breaking the skill name in preloaded text. Was
|
||||||
**Live** — the check reports it as a dangling `gitea-labels`.
|
**live**, reported as a dangling `gitea-labels`; **fixed under #99** — the name now folds intact.
|
||||||
|
|
||||||
So the check fires on 3 of the 4 against the base commit and on 2 at the tip of this change, and
|
So the check fired on 3 of the 4 against the base commit and on 2 at the tip of the change that
|
||||||
`tests/test-skill-size-check.sh` probes exactly those three by name rather than asserting a count, so
|
carried this ADR. **The corpus dangling set is now empty**, and that is asserted rather than
|
||||||
it degrades to SKIP as #100 lands rather than going stale.
|
observed: `tests/test-adr0020-targets.sh` pins the set as empty, so a new boundary clause naming a
|
||||||
|
non-existent skill fails the suite instead of joining a backlog. `tests/test-skill-size-check.sh`
|
||||||
|
probed the three original names rather than asserting a count; as each was retrofitted its probe was
|
||||||
|
**removed, not skipped**, because a `pass "SKIP: …"` branch is an assertion-free result counted in
|
||||||
|
the totals and makes the suite look one test stronger than it is. That file's commentary survives the
|
||||||
|
probes and states the rule. Re-derive the current set — never read it off this page:
|
||||||
|
|
||||||
|
```
|
||||||
|
bash scripts/skill-size-check.sh plugins/*/.apm/skills/*/SKILL.md | grep 'does not resolve'
|
||||||
|
```
|
||||||
|
|
||||||
**Duplication between `skill-author` and `agent-author` survives un-gated.** The merge rule
|
**Duplication between `skill-author` and `agent-author` survives un-gated.** The merge rule
|
||||||
deliberately excludes the author pair, so the commit-verification argument in four near-copies, the
|
deliberately excludes the author pair, so the commit-verification argument in four near-copies, the
|
||||||
|
|||||||
@@ -98,11 +98,57 @@ loudly (`Error: jq is required but not installed`).
|
|||||||
## Skill and agent context gates (ADR-0020)
|
## Skill and agent context gates (ADR-0020)
|
||||||
|
|
||||||
The `skill-size-check` pre-commit hook, scoped to `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$`,
|
The `skill-size-check` pre-commit hook, scoped to `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$`,
|
||||||
runs `scripts/skill-size-check.sh`. That scope means it never lints the
|
runs `scripts/skill-size-check.sh`. It is also shipped to external repos as
|
||||||
`plugins/kyberforge/docs/research/examples/` reference skills. It is also shipped to external repos
|
`kyberforge-skill-size-check` (see
|
||||||
as `kyberforge-skill-size-check` (see
|
|
||||||
[External consumers](#external-consumers-the-root-pre-commit-hooksyaml)).
|
[External consumers](#external-consumers-the-root-pre-commit-hooksyaml)).
|
||||||
|
|
||||||
|
**Two things fall outside that scope, both deliberately.** The `[^/]+/SKILL\.md$` tail admits only a
|
||||||
|
`SKILL.md` sitting directly in a skill directory under `.apm/skills/`:
|
||||||
|
|
||||||
|
- the `plugins/kyberforge/docs/research/examples/` reference skills, which are vendored upstream
|
||||||
|
corpus and not this repo's to gate;
|
||||||
|
- `plugins/kyberforge/.apm/skills/skill-author/assets/templates/SKILL.md` — inside `.apm/skills/`,
|
||||||
|
but two directories deeper. It is the `FILL IN:` scaffold `skill-author` copies, so its
|
||||||
|
`description: >` is a comment block rather than a description and every ADR-0020 measurement over
|
||||||
|
it would be meaningless. A reader adjusting the pattern needs to know it is there.
|
||||||
|
|
||||||
|
Everything else it matches exactly, with nothing over- or under-caught. Re-derive both halves:
|
||||||
|
|
||||||
|
```
|
||||||
|
git ls-files | grep -cE '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$' # the real skills
|
||||||
|
git ls-files | grep -E '^plugins/[^/]+/\.apm/skills/.*SKILL\.md$' \
|
||||||
|
| grep -vE '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$' # the scaffold only
|
||||||
|
```
|
||||||
|
|
||||||
|
The first count equals the number of skill directories (`ls -d plugins/*/.apm/skills/*/ | wc -l`);
|
||||||
|
the second returns exactly the template. The remaining unmatched `SKILL.md` files in the tree are the
|
||||||
|
generated flat mirror, which is excluded by the `.apm/` segment on purpose — a mirror edit is drift,
|
||||||
|
not an authoring change.
|
||||||
|
|
||||||
|
### `skill-frontmatter`, the other hook on that scope
|
||||||
|
|
||||||
|
A second `repo: local` pre-commit hook, `skill-frontmatter`, runs on the **same** `files:` pattern at
|
||||||
|
the same stage. It is a short shell loop: for each file, `grep -q "^name:"` and
|
||||||
|
`grep -q "^description:"`, failing with "missing required frontmatter fields" if either is absent.
|
||||||
|
|
||||||
|
**It overlaps ADR-0020's "description present and non-empty" FAIL, and the overlap is not clean.**
|
||||||
|
The ADR (`:95-101`) requires that question be decided on the **YAML-folded value** and nowhere else,
|
||||||
|
precisely because a line regex gets it wrong in both directions. Measured on fixtures:
|
||||||
|
|
||||||
|
| Frontmatter | `skill-frontmatter` | `skill-size-check` |
|
||||||
|
|---|---|---|
|
||||||
|
| `description:` with no value, then `model: sonnet` | passes — the key is on a line | ERROR, "missing or empty" |
|
||||||
|
| `"description": …` (quoted key, valid YAML) | **fails** — `^description:` does not match | passes, description read normally |
|
||||||
|
|
||||||
|
So the grep is not a second opinion on presence. It is blind to the shape ADR-0020 was written
|
||||||
|
against, and it is the only one of the two that objects to a quoted key. Neither disagreement is
|
||||||
|
currently live in the corpus, and the honest reading is that presence is `skill-size-check`'s
|
||||||
|
question — the grep's contribution to it is noise on one shape and silence on the other.
|
||||||
|
|
||||||
|
What the grep does add is the `name:` key, which **no** ADR-0020 check reads: a `SKILL.md` with no
|
||||||
|
`name:` passes `skill-size-check` at exit 0. That is its real and only unique coverage, and the
|
||||||
|
reason not to fold it into the size gate on the grounds of redundancy.
|
||||||
|
|
||||||
### Two independent gate families, neither replaced the other
|
### Two independent gate families, neither replaced the other
|
||||||
|
|
||||||
**Family 1 — agentskills.io spec backstop** (unchanged, conformance not quality):
|
**Family 1 — agentskills.io spec backstop** (unchanged, conformance not quality):
|
||||||
@@ -141,7 +187,7 @@ A boundary-clause target that resolves to no skill or agent has **three** possib
|
|||||||
| Verdict | When |
|
| Verdict | When |
|
||||||
|---|---|
|
|---|---|
|
||||||
| **SUGGESTION** — the default | the target does not resolve and neither promotion condition below holds |
|
| **SUGGESTION** — the default | the target does not resolve and neither promotion condition below holds |
|
||||||
| **blocking ERROR** | the target is **terminal** (not a compound modifier) **and** either written in route notation (`/name` for any name; a bare `-> name` only when the name is hyphenated, a backticked `` -> `name` `` for any — see the gap below) **or** corroborated by another target in the same sentence that *does* resolve |
|
| **blocking ERROR** | the target is written in **route notation** — `/name` for any name, or any arrow form (a bare `-> name` only when the name is hyphenated, a backticked `` -> `name` `` for any — see the gap below); **or** it is a bare **terminal** name (not a compound modifier) **corroborated** by another target in the same sentence that *does* resolve |
|
||||||
| **INFO, "DID NOT RUN"** | no skill universe could be determined for the path at all — the targets are named and left unchecked, exit 0 |
|
| **INFO, "DID NOT RUN"** | no skill universe could be determined for the path at all — the targets are named and left unchecked, exit 0 |
|
||||||
|
|
||||||
The default is deliberately soft because a hyphenated word in a boundary clause is as likely to be a
|
The default is deliberately soft because a hyphenated word in a boundary clause is as likely to be a
|
||||||
@@ -149,6 +195,17 @@ tool, a file format or an English compound as a route: "pre-commit hooks" is pro
|
|||||||
never reaches the check at all, being a compound modifier rather than a terminal name. The
|
never reaches the check at all, being a compound modifier rather than a terminal name. The
|
||||||
SUGGESTION text says how to opt in — write it as `/name` or `-> name` and it gets checked properly.
|
SUGGESTION text says how to opt in — write it as `/name` or `-> name` and it gets checked properly.
|
||||||
|
|
||||||
|
**The two promotion conditions are not symmetric, and the order matters.** `_add()` decides
|
||||||
|
**notation first**: when the name is written `/name`, or reached through any arrow form, the target
|
||||||
|
is marked error-eligible there and the terminal test is never run. Terminality gates only the *bare*
|
||||||
|
path — a name in prose earns its error from corroboration, and a compound modifier can never dangle.
|
||||||
|
Reading the row as "terminal AND (notation OR corroborated)" gets the notation half backwards: it
|
||||||
|
predicts that `` … Do not use for Y — use /no-such-skill afterwards. `` is a SUGGESTION, because
|
||||||
|
`afterwards` is a follower outside `FOLLOWER_OK`. It exits 1. That was the defect — `-> name` reached
|
||||||
|
`_add()` with `strict=True` from both its call sites and `/name` did not, so the one spelling
|
||||||
|
ADR-0020 offers an author who wants a route checked unconditionally was the one spelling a stray
|
||||||
|
follower could silence.
|
||||||
|
|
||||||
**Known gap: a BARE arrow target must be hyphenated.** Target extraction is built on `NAME_HYPH` in
|
**Known gap: a BARE arrow target must be hyphenated.** Target extraction is built on `NAME_HYPH` in
|
||||||
`scripts/skill-size-check.sh`, which requires at least one hyphen, and `ARROW_BOUNDARY` inherits
|
`scripts/skill-size-check.sh`, which requires at least one hyphen, and `ARROW_BOUNDARY` inherits
|
||||||
that. So `Not X -> gitea-prs` is extracted and checked, while `Not X -> triage` yields no target.
|
that. So `Not X -> gitea-prs` is extracted and checked, while `Not X -> triage` yields no target.
|
||||||
@@ -558,6 +615,35 @@ passing one explicit file per invocation. The two manifests scope **differently
|
|||||||
Narrowing a `.vale.ini` glob to a `plugins/`-shaped path to "tighten" it breaks the consumer case,
|
Narrowing a `.vale.ini` glob to a `plugins/`-shaped path to "tighten" it breaks the consumer case,
|
||||||
and `check-vale-style-sync`'s probe set is built to catch exactly that.
|
and `check-vale-style-sync`'s probe set is built to catch exactly that.
|
||||||
|
|
||||||
|
### The blind spot: `references/` is unlinted, for two independent reasons
|
||||||
|
|
||||||
|
Every `references/*.md` file in the corpus is outside the prose gate. Count them with
|
||||||
|
`git ls-files | grep -cE '^plugins/[^/]+/\.apm/skills/[^/]+/references/.*\.md$'` rather than reading
|
||||||
|
a figure here; it moves with every retrofit. This is the gap that matters most, because the context
|
||||||
|
contract's own remedy for an over-long body is to move prose **into** `references/` — the gate pushes
|
||||||
|
text across its own boundary and then stops watching it.
|
||||||
|
|
||||||
|
**Closing either cause alone changes nothing.** There are two, and they are independent:
|
||||||
|
|
||||||
|
| Cause | Where | Effect on a `references/` file |
|
||||||
|
|---|---|---|
|
||||||
|
| the `Kyberforge` style is scoped `[**/SKILL.md]` | `skill-audit/assets/vale/.vale.ini` | matches no section, so Vale lints 0 files and exits 0 |
|
||||||
|
| the hook's `files:` regex is `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$` | `vale-audit-prefilter-skill` in `.pre-commit-config.yaml` | the file is never handed to Vale at all |
|
||||||
|
|
||||||
|
Verified both ways. Handing skill-audit's `vale-wrap.sh` a reference file directly — bypassing
|
||||||
|
pre-commit entirely, so only the style scope is in play — prints `0 errors … in 0 files` and exits 0,
|
||||||
|
where the same wrapper on a `SKILL.md` reports `in 1 file`. And the hook's `files:` regex, applied to
|
||||||
|
`git ls-files`, selects only the skill-directory `SKILL.md` files scoped at the top of this page, so
|
||||||
|
pre-commit never hands Vale a reference file to begin with. Widening the glob to `[**/*.md]` would
|
||||||
|
still lint nothing through the hook; widening the hook's `files:` alone would hand Vale files its own
|
||||||
|
config declines to match, which is the [0-file NOT RUN](#a-0-file-vale-run-is-not-run) shape — a
|
||||||
|
green run that measured nothing. **Issue #117** records the style-scope half; the hook half has to
|
||||||
|
land in the same change or the fix is cosmetic.
|
||||||
|
|
||||||
|
The consumer manifest is a third axis and does not rescue this either: `.pre-commit-hooks.yaml`'s
|
||||||
|
`(^|/)SKILL\.md$` is layout-agnostic but still filename-shaped, so an external repo running
|
||||||
|
`kyberforge-vale-audit-skill` has the same gap.
|
||||||
|
|
||||||
### `vale-wrap.sh`, never bare `vale`
|
### `vale-wrap.sh`, never bare `vale`
|
||||||
|
|
||||||
Both audit skills' Step 1 and both pre-commit hooks call **each copy's own**
|
Both audit skills' Step 1 and both pre-commit hooks call **each copy's own**
|
||||||
|
|||||||
Reference in New Issue
Block a user