598a7c326a
Retrofits all 39 skills to ADR-0020's description/body context contract, then fixes what six rounds of independent review found in that retrofit — including four ways the hot gate itself failed open. Closes #99, #107, #108, #110, #111, #114, #115, #120. ## The retrofit (waves 1-5) | | Start | Now | |---|---|---| | Description FAILs (>400 chars) | 26 | **0** | | Body FAILs (>900 words, body-only) | 9 | **0** | | Dangling routing targets | 2 | **0** | | `Kyberforge.CompositionNote` | 10 | **0** | | Preload tax | 21,005 chars | **~10,500** | Under the 12,000-char success criterion. Per-wave detail is on #99. ## The review fixes **The gate failed open four ways, three of them found after the retrofit shipped.** An unrecognised follower token made a dangling target vanish. A skill directory with no `SKILL.md` resolved as a valid target, so a commit could be green locally and red in a fresh clone — three existing fixtures were relying on that, one of which made the install-leak A/B pass vacuously. Then the free-standing `/name` sweep turned out to be gated on the sentence carrying a boundary marker, so route notation in any other sentence was invisible — not an ERROR, not a SUGGESTION, not an INFO — which left the documented "`/name` always blocks" promise false from a second direction. All four fixed and pinned. **Two checks were silently not running.** `validate-provenance.sh` checks 7-8 were dead across nine skills. Waking them exposed a deeper problem: they assume `Research doc:` names a source index, but 30 of 121 entries point at topic content documents, so every new check-7 INFO was a false positive and check 8 was saved from a false-FAIL flood only by an *unannounced* skip. Checks 7/8 are now scoped to source indexes and every skip announces itself (#121). **The retrofit's own anti-goal, four times.** ADR-0020 warns that a blunt gate gets satisfied by deleting content rather than relocating it. `diagnose` and `skill-audit` relocated prose and then read it unconditionally; `prototype` and `vale-config` deleted rules outright that survived nowhere. All four addressed. ## Verification - `bash tests/run-tests.sh --strict` — 24 suites, 0 skipped, 0 failed - `bash tests/run-bats.sh` — 325 tests, 0 failures - `pre-commit run --all-files` — 17/17 - `pre-commit run --hook-stage pre-push --all-files` — 16/16, with `apm marketplace check` and `apm pack --check-clean` run against the remote, not skipped - `scripts/skill-size-check.sh` over all 39 skills — rc 0, 0 ERROR/FAIL, SUGGESTION-only - Preload tax measured at **10,498 chars**, max description 390 — both inside budget - Every new test proven non-vacuous by a deliberate mutation of the behaviour it covers **Per-commit sync, stated accurately:** the ten commits from the latest review round each pass `check-plugin-content-sync` in isolation, verified by checking each out in a detached worktree with a clean between. The earlier gitea window (`dfacf05..bedbd1d`, nine commits) does **not** — its mirror was regenerated in one batch at `bbc7300`. An earlier revision of this description claimed the property held for every commit; it does not, and a bisect through that window lands on a red commit. **Squash-merge** to collapse it, or accept that this range is not bisectable. ## Version bump Six plugins and the catalog take a **patch**, not a minor. The branch is **89 commits — 40 `fix` / 30 `refactor` / 12 `docs` / 5 `chore` / 2 `test` — zero `feat`, zero `!`, zero `BREAKING CHANGE`** — and adds no skill, agent, command or hook. (Two earlier revisions of this section cited a stale histogram, most recently 78 commits; the figures above are measured at HEAD.) Both rules this repo ships (`forge/references/version-bump.md`, landing in this PR, and `git-commits/references/conventional-commits-spec.md`) make that a patch, and the catalog set is unchanged at 7 entries. Not settled by that: four published files were removed from the installed tree, three moved, and `caveman` gained `disable-model-invocation`, retiring its old triggers. Under a strict reading those are major-class and currently ship under `refactor:` with no marker. Whether the deployed skill surface is a public contract is written down nowhere — worth deciding, but it outlives this PR. ## Deliberately not in scope #112 (cherry-pick ownership, now resolved in favour of `git-commits`), #113 (`rtk git` normalisation), #116 (research fan-out), #101 (audit-skill merge), #122 (non-spec skill-root files), #123 (no PRD producer) stay open. #117 is the one worth reading: the contract's remedy is to move prose into `references/`, which is exactly where neither the size gate nor Vale looks — and the blind spot is wider than #117 currently records, since there is no root `.vale.ini` at all, so every ADR, `CONTEXT.md` and `README.md` is unlinted too. That blind spot let this branch carry two `level: error` `Kyberforge.SentenceOpenerThereIs` violations into `references/` files it created — `provider-adapter-author/references/provider-matrix.md:31` and `agent-audit/references/finding-criteria.md:95`. Both are reworded in `afadaae`, confirmed by routing each file through the audit's own `vale-wrap.sh` (1 error each before, 0 after). Five further occurrences sit in `references/` files already on `main`; those are the pre-existing corpus and stay with #117, which is the real fix. Also unfixed and not this PR's: `apm install` appends a duplicate `SessionStart` entry to `.claude/settings.json`, so a fresh clone cannot get pre-push green without an edit AGENTS.md warns against. Reproduces identically on `main`. Co-authored-by: Defame1297 <gitea@rkdr.net> Reviewed-on: https://git.dev.rkdr.net/Defame1297/holocron/pulls/129 Co-authored-by: Claude Code AI - Gitea MCP <claude@noreply.git.dev.rkdr.net> Co-committed-by: Claude Code AI - Gitea MCP <claude@noreply.git.dev.rkdr.net>
122 lines
8.3 KiB
Markdown
122 lines
8.3 KiB
Markdown
---
|
||
source_keys:
|
||
- agentskills-spec
|
||
- agentskills-using-scripts
|
||
---
|
||
|
||
# Validation Scripts Reference
|
||
|
||
Read this when a Step 1 script fails, cannot run, or reports something that needs interpreting.
|
||
Nothing here is needed on a clean run.
|
||
|
||
## Report the gap, do not guess
|
||
|
||
If a script cannot run at all — Bash denied, `python3` unavailable, PyYAML not importable, `vale`
|
||
not installed — say so as an **INFO** finding naming the script and the missing dependency, then
|
||
fall back to the manual checks below. An INFO never changes PASS/FAIL. Silently omitting the
|
||
dimension a script would have covered reports a clean audit that checked less than it claims to
|
||
have checked, and the Step 4 coverage line then names a dimension nothing actually examined.
|
||
|
||
## Manual structural fallback
|
||
|
||
`validate.sh` needs `python3` **and** PyYAML, and refuses to start without either — the description
|
||
value has to be measured after YAML folding is resolved, so skipping the ADR-0020 gates would be a
|
||
vacuous pass rather than a partial one. The two are checked separately, so the message already names
|
||
the right one — report it verbatim rather than diagnosing further:
|
||
|
||
```text
|
||
Error: python3 is required but was not found on PATH.
|
||
Error: PyYAML is required but is not importable by python3.
|
||
```
|
||
|
||
Without them — or with Bash denied, or on a permission error — work this list
|
||
by hand and file the results under `### Structure` exactly as the script's output would have been:
|
||
|
||
- **`name`** present, 1–64 characters, kebab-case (lowercase letters, digits and hyphens; no
|
||
leading, trailing or doubled hyphen), and **matching the skill's directory name** exactly.
|
||
- **`description`** present and non-empty; no unfilled `FILL IN:` placeholder in it. An absent or
|
||
empty description is a **FAIL**, never a silent skip — it is the one field preloaded into every
|
||
session, so a skill without one can never be routed to.
|
||
- **Description length**, measured on the folded YAML value with newlines collapsed to single
|
||
spaces — not on the raw block scalar, which counts indentation. 250 characters SUGGESTION, 400
|
||
FAIL (ADR-0020), 1,024 FAIL (agentskills.io spec).
|
||
- **Body length**, counting everything after the frontmatter's closing `---`. 600 words
|
||
SUGGESTION, 900 FAIL (ADR-0020).
|
||
- **Whole-file ceilings**, counting the file including frontmatter: 500 lines FAIL, 2,770 words
|
||
FAIL (agentskills.io spec). These are a different measurement from the two above — report them
|
||
as separate findings, never merged.
|
||
- **A boundary clause is present** — either the prose form (`do not` / `instead` / `rather than` /
|
||
`not for`) or ADR-0020's compressed `Not <thing> -> <name>` arrow. **SUGGESTION**, not FAIL:
|
||
the absence is deterministic, but whether this skill warrants one is the auditor's call.
|
||
- **Boundary targets resolve** — **FAIL** on a name that resolves to nothing. See the section
|
||
below; resolving these by hand is the one item on this list with a procedure of its own.
|
||
- **Every `references/<file>.md` named in the body exists on disk** — **FAIL**, not a suggestion.
|
||
A dispatch table or "read X" trigger naming a missing file sends the agent nowhere. Ignore
|
||
mentions inside fenced code blocks, and ignore a mention whose own line says the file is gone
|
||
(`removed`, `deleted`, `renamed`, `superseded`, `replaced`, `obsolete`, `deprecated`, `former`,
|
||
`gone`, `no longer`, `used to`) — that is a historical note, not a dispatch entry.
|
||
- **Gotchas discipline**, both **SUGGESTION**. Locate the section by a heading that *is* Gotchas
|
||
(`## Common Gotchas` counts; `## Gotcha handling` and `## Why gotchas matter` do not), running to
|
||
the next heading at the same level or shallower. More than five top-level entries is one
|
||
suggestion; a section over 25% of the body word count is a second, independent one. Count
|
||
entries at column 0 only — an indented child bullet is not an entry — and ignore fenced code
|
||
blocks for both.
|
||
- **No unfilled `FILL IN:` placeholder** anywhere in the body.
|
||
- **Every file in `scripts/`** carries the executable bit and contains no interactive prompt —
|
||
no bare `read`, no `select`, nothing that blocks on a TTY.
|
||
|
||
## Resolving boundary targets by hand
|
||
|
||
Targets are read from **both** boundary forms. The compressed `Not <thing> -> <name>` arrow and the
|
||
prose form are each parsed *and* target-checked, so a typo in prose phrasing fails exactly as an
|
||
arrow typo does — do not check only the names after an arrow.
|
||
|
||
Build the universe by walking up **from the `SKILL.md` under audit**, never from the validator's own
|
||
location. The nearest ancestor holding `plugins/*/.apm/skills/` or `plugins/*/.apm/agents/` is the
|
||
authoring root, falling back to the nearest ancestor holding `.git`. When one is found the universe
|
||
is every skill and agent under `<root>/plugins/*/`, plus the skill's own apm package, plus the
|
||
packages that package declares in its `apm.yml` under `dependencies.apm`. Deployed `.claude/` and
|
||
`.agents/` trees are consulted **only** when no authoring root exists — they are gitignored
|
||
`apm install` output, and reading them would make a fresh clone and a developer machine disagree.
|
||
|
||
Three ways to read the result wrong:
|
||
|
||
- **A hyphenated name used attributively is not a dangling target.** "Use pre-commit hooks instead
|
||
of ad-hoc scripts" reads as a route to `pre-commit` on wording alone. What separates a route from
|
||
prose is grammar: a route target is terminal — followed by punctuation, a conjunction, or a
|
||
boundary word — whereas a compound modifier is followed by the noun it modifies. A name followed
|
||
by an ordinary noun still *confirms* a route when it exists, but never raises a FAIL on its own.
|
||
- **A SUGGESTION-tier unresolved target is not a FAIL you may promote.** Terminal position alone is
|
||
not evidence of a route: "run `pre-commit` instead", "see `commit-msg`" and "use the clean-up
|
||
instead" are all terminal and all prose. A prose-form target earns a FAIL only when its own
|
||
sentence names another target that *does* resolve; otherwise the script reports it and moves on,
|
||
and so should you. Route notation — `/name` and `-> name` — is exempt and always FAILs, and it is
|
||
the fix to recommend when the author did mean a route.
|
||
- **`INFO boundary-target resolution DID NOT RUN` is not a pass.** The script prints it, and exits
|
||
0, when no universe could be determined for that path — the usual cause being a skill copy
|
||
audited outside its package. Report it as an INFO naming the unchecked targets and re-run against
|
||
the real directory; filing it as clean signs off targets nothing verified.
|
||
|
||
## Script-specific failures
|
||
|
||
- **`validate-provenance.sh` printed nothing *and exited 0*.** That is a pass, not a skip — it
|
||
exits 0 silently when the skill has no `source_keys` and no `references/sources.md`, and nothing
|
||
to validate is not a finding. Check the exit code before you believe the silence: a target that
|
||
is not a directory, a directory holding no `SKILL.md`, a missing or extra argument, and an absent
|
||
`python3` all exit **2** with a message on stderr. Exit 2 means the script never ran — report it
|
||
as an unaudited dimension, never as a pass and never as a finding. Exit 1 is findings.
|
||
- **`vale` reports `0 files`.** Treat the pass as NOT RUN, not as clean, and fall back to full
|
||
Step 3 judgment for the dimensions it would have covered. The bundled `Kyberforge` style is
|
||
scoped by glob in `assets/vale/.vale.ini`; a file outside those globs is silently not linted.
|
||
- **`E100 Runtime error ... does not exist` (exit 2) from `vale-wrap.sh`.** An explicit relative
|
||
`--config` was passed. Pass none: the wrapper locates its own `assets/vale/.vale.ini` from its
|
||
own path, so a resolved script path plus an unresolved config path produces exactly this. Do not
|
||
read this exit code as vale being unavailable — that misreading sends the audit down the
|
||
fallback path while vale was installed and working the whole time.
|
||
- **The `vale` binary is genuinely absent** (`command not found`). Report one INFO naming it, then
|
||
fall back to full Step 3 judgment for the description, body-discipline and patterns dimensions —
|
||
the prefilter's whole coverage. Judge those by rubric rather than dropping them.
|
||
- **A path argument that does not exist is a hard error** in `vale-wrap.sh`, deliberately: bare
|
||
`vale` would fall back to reading stdin and print a clean-looking `0 errors ... in stdin`, which
|
||
the `0 files` guard above does not catch.
|