Defame1297/holocron
0
0
Fork 0
Code Issues 14 Pull Requests Actions Packages Projects Releases 3 Wiki Activity

refactor(skills): retrofit the corpus to the ADR-0020 context contract (#129)

Browse Source 
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>
This commit was merged in pull request #129.
This commit is contained in:
Claude Code AI - Gitea MCP
2026-09-01 13:47:46 +00:00
committed by Defame1297
parent 0e91a3ae66
commit 598a7c326a
420 changed files with 15303 additions and 4740 deletions
Download Patch File Download Diff File Expand all files Collapse all files

522
scripts/skill-size-check.sh
View File

@@ -34,26 +34,44 @@ set -euo pipefail
# as a proxy (Python's str.split(), the same primitive
# skill-audit/scripts/validate.sh applies to these two constants; `wc -w`
# disagrees with it on Unicode separators, which is why the awk pass that used
# to live in the loop below is gone). Measured over this repo's 39 in-scope SKILL.md files, characters per
# word runs min 5.97 / median 6.79 / mean 6.77 / max 7.22. At the standard
# ~4-characters-per-token English approximation that is 1.49 / 1.70 / 1.69 /
# 1.81 tokens per word.
# to live in the loop below is gone).
#
# THE MEASUREMENT BASIS, stated because the previous re-measure drifted onto a
# different one and the numbers moved without the prose noticing: characters
# per word is len(text) / len(text.split()) over the WHOLE FILE, whitespace
# included, on plugins/*/.apm/skills/*/SKILL.md. Counting only non-whitespace
# characters gives a materially lower figure (4.90 / 5.52 / 5.54 / 6.19 today)
# and is not the basis MAX_WORDS is calibrated against.
#
# Measured over this repo's 39 in-scope SKILL.md files (2026-08-31, after the
# ADR-0020 retrofit), characters per word runs min 5.93 / median 6.67 / mean
# 6.63 / max 7.34. At the standard ~4-characters-per-token English
# approximation that is 1.48 / 1.67 / 1.66 / 1.84 tokens per word.
#
# MAX_WORDS=2770 is therefore calibrated to the corpus WORST case rather than
# its median: 2770 words at the densest observed 7.22 chars/word is ~20,000
# characters, or ~5,000 tokens at the 4-characters-per-token approximation. So
# what this gate guarantees is "under 5,000 tokens even for the densest prose
# its median: 2770 words at the densest observed 7.34 chars/word is ~20,300
# characters, or ~5,090 tokens at the 4-characters-per-token approximation. So
# what this gate guarantees is "about 5,000 tokens even for the densest prose
# the corpus has produced" — the earlier median-calibrated MAX_WORDS=2900 let
# such a file sit at exactly the ceiling and still spend ~5,240 tokens. A
# median-density file at 2770 words spends ~4,700 tokens, so typical prose
# gives up ~130 words of headroom to close that gap. The largest SKILL.md in
# the repo is 2,760 words whole-file (skill-author), twelve words under the
# ceiling — this is a gate two files have already grown into, not headroom.
# such a file sit at exactly the ceiling and spend ~5,320 tokens. A
# median-density file at 2770 words spends ~4,620 tokens, so typical prose
# gives up ~140 words of headroom to close that gap. Densest file today:
# git-commits at 7.34 chars/word.
#
# THE CORPUS IS NOWHERE NEAR THIS CEILING ANY MORE, and the note that used to
# stand here — "a gate two files have already grown into" — described the
# pre-retrofit corpus and is now wrong by a factor of three. The largest
# SKILL.md is write-docs at 914 whole-file words, then vale-run at 874;
# skill-author, the old high-water mark at 2,760, is down to 661. MAX_WORDS is
# a spec-conformance backstop with roughly 1,850 words of slack, and the gate
# that actually bites is ADR-0020's 900-word body budget below it. Do not read
# the two as redundant: they measure different spans, and a file can sit well
# inside one while failing the other.
#
# It is a one-sided proxy in the useful direction — nothing under the word
# ceiling is wildly over the token ceiling — but it is not exact BPE
# tokenization and does not replace one. Re-measure the corpus before treating
# any of these numbers as still current.
# tokenization and does not replace one. Re-measure the corpus, on the basis
# stated above, before treating any of these numbers as still current.
#
# python3 AND PyYAML are required for the ADR-0020 half, and both are hard
# dependencies rather than best-effort: python3 because pre-commit (which is how
@@ -88,40 +106,52 @@ BODY_MAX_WORDS=900
FAIL=0
for f in "$@"; do
# NOT a silent skip — see the matching note on the Python side. A broken
# symlink named SKILL.md is storable in git and a directory named SKILL.md
# reaches this hook the same way; both used to make the whole run exit 0 with
# no output at all, which is the one thing this script must never do.
if [[ ! -f "$f" ]]; then
if [[ -d "$f" ]]; then
why="is a directory, not a file"
elif [[ -L "$f" ]]; then
why="is a symlink that does not resolve to a file"
elif [[ -e "$f" ]]; then
why="is not a regular file"
else
why="does not exist"
fi
echo "ERROR: $f $why, so the line and word ceilings could not be measured. A path this hook was handed and could not read does not get to pass in silence." >&2
FAIL=1
continue
fi
# ZERO ARGUMENTS IS A USAGE ERROR, exit 2 — not a clean run.
#
# This hook is `pass_filenames: true` in both .pre-commit-config.yaml and
# .pre-commit-hooks.yaml, and pre-commit skips a filename-passing hook entirely
# when nothing matches its `files:` pattern, so it never invokes this script
# with an empty argument list. Every no-argument invocation therefore comes from
# somewhere else — a hand-run command, a wrapper, or a `files:` pattern edited
# into matching nothing — and printing nothing and exiting 0 made all three
# indistinguishable from a clean corpus. A mis-scoped pattern would have
# silently disabled the whole ADR-0020 gate family while every hook reported
# green.
#
# Exit 2, not 1, for the same reason a8cd5e8 split validate-adapter.sh's usage
# exits out: {0,1} are this script's verdict codes (clean / findings), and a
# caller that reads a non-zero exit as "the SKILL.md needs editing" must be able
# to tell a broken invocation from a real finding.
if [[ $# -eq 0 ]]; then
echo "usage: skill-size-check.sh <SKILL.md> [SKILL.md ...]" >&2
echo " Measures the agentskills.io spec ceilings and the ADR-0020 context" >&2
echo " budget for each SKILL.md named on the command line." >&2
echo " No paths were given. This is a usage error, not a clean run: a hook" >&2
echo " whose files: pattern matches nothing would otherwise be" >&2
echo " indistinguishable from a corpus with no findings." >&2
exit 2
fi
# The MAX_LINES / MAX_WORDS ceilings are NOT measured here. They used to be,
# in a single awk pass, and that pass was wrong twice over:
# * `read -r lines words <<< "$(awk ...)"` discarded awk's exit status, so a
# file awk could not read yielded empty variables, bash arithmetic read
# them as 0, and both ceilings passed in total silence — the one outcome
# this script forbids itself.
# * awk's NR/NF do not agree with the Python splitlines()/split() that
# skill-audit/scripts/validate.sh uses for the SAME two constants.
# splitlines() also breaks on \x0b \x0c \x1c \x1d \x1e \x85 U+2028 U+2029
# and split() on every Unicode space, so a body padded with U+2028 read as
# 6 lines here and 606 lines there — hook green, audit FAIL.
# One implementation now owns both: the Python block below already reads every
# file (with a real diagnostic on failure), so it counts there.
done
# An unreadable path — a broken symlink named SKILL.md is storable in git, and a
# directory named SKILL.md reaches this hook the same way — is diagnosed ONCE,
# in the Python per-file loop below. There used to be a bash pre-loop here doing
# exactly the same stat dance and printing exactly the same sentence, so every
# such path was reported twice with two ERROR lines for one broken file. It is
# still NOT a silent skip; the diagnosis simply lives where the file is read.
#
# The MAX_LINES / MAX_WORDS ceilings are not measured in bash either. They used
# to be, in a single awk pass, and that pass was wrong twice over:
# * `read -r lines words <<< "$(awk ...)"` discarded awk's exit status, so a
# file awk could not read yielded empty variables, bash arithmetic read
# them as 0, and both ceilings passed in total silence — the one outcome
# this script forbids itself.
# * awk's NR/NF do not agree with the Python splitlines()/split() that
# skill-audit/scripts/validate.sh uses for the SAME two constants.
# splitlines() also breaks on \x0b \x0c \x1c \x1d \x1e \x85 U+2028 U+2029
# and split() on every Unicode space, so a body padded with U+2028 read as
# 6 lines here and 606 lines there — hook green, audit FAIL.
# One implementation now owns both: the Python block below already reads every
# file (with a real diagnostic on failure), so it counts there.
if ! command -v python3 > /dev/null 2>&1; then
echo "ERROR: python3 is required for the ADR-0020 description/body/boundary-target gates but was not found on PATH." >&2
@@ -147,6 +177,23 @@ import sys
import yaml
# Output is UTF-8 for the same reason input is: under LC_ALL=C the streams
# default to ASCII, and this script's own message text carries em dashes (the
# ADR-0020 boundary SUGGESTION is one). Pinning only the reads moved the crash
# from the read to the write — a UnicodeEncodeError raised while PRINTING, after
# every check has already run, which loses the whole report. read_text() in the
# shared resolver block below pins the reads; this pins the writes.
#
# Deliberately OUTSIDE the ADR-0020 shared boundary resolver block: all three
# scripts in the family need this, but tests/test-adr0020-contract.sh hashes
# that block for byte-identity, so shared-looking edits belong beside it, not
# inside it.
for _stream in (sys.stdout, sys.stderr):
try:
_stream.reconfigure(encoding='utf-8')
except AttributeError: # pragma: no cover — Python < 3.7
pass
DESC_SUGGEST_CHARS = int(sys.argv[1])
DESC_MAX_CHARS = int(sys.argv[2])
BODY_SUGGEST_WORDS = int(sys.argv[3])
@@ -263,9 +310,27 @@ def _collect_package(pkg_dir, names):
safe_dir = glob.escape(pkg_dir)
for sub in ('.apm/skills/*/', 'skills/*/'):
for path in glob.glob(os.path.join(safe_dir, sub)):
names.add(os.path.basename(path.rstrip('/')).lower())
# A directory is a skill only if it HOLDS a SKILL.md. An empty
# leftover — a deleted skill whose directory survived, a scaffolding
# stub, an editor's stray mkdir — is untracked by git, so it exists
# on the machine that made it and nowhere else. Counting it made a
# boundary target resolve locally and dangle in a fresh clone: the
# same install-dependence the deployed-tree rule above exists to
# remove, arriving through a different door.
if os.path.isfile(os.path.join(path, 'SKILL.md')):
names.add(os.path.basename(path.rstrip('/')).lower())
for sub in ('.apm/agents/*.md', 'agents/*.md'):
for path in glob.glob(os.path.join(safe_dir, sub)):
# The same rule one directory over, which until now had no
# counterpart here at all: the skills branch above tests for a
# SKILL.md, the agents branch took every glob hit on trust. A
# DIRECTORY named `ghost-agent.md` matches `*.md` and glob does not
# tell the two apart, so a leftover of that shape resolved a routing
# target on the machine holding it and dangled everywhere else —
# identical install-dependence, arriving through the one door
# nobody guarded.
if not os.path.isfile(path):
continue
base = os.path.basename(path)
if base.endswith('.agent.md'):
base = base[:-len('.agent.md')]
@@ -456,8 +521,17 @@ def known_targets(start_dir):
# condition, pc-run's "run pre-commit hooks" reads as a route to a
# non-existent `pre-commit` skill.
# * A BARE arrow target counts only in ADR-0020's compressed boundary form,
# `Not <thing> -> <skill-name>`. Without that, diagnose's process chain
# "fix -> regression-test" reads as a route to `regression-test`.
# `Not <thing> -> <skill-name>`. The example that motivated it is gone:
# diagnose's process chain "fix -> regression-test", which without the
# gate read as a route to a non-existent `regression-test` skill, was cut
# when issue #99 retrofitted that description. So the gate is currently
# UNEXERCISED — gating and not gating produce the same verdict corpus-wide.
# Keep it anyway. It is a false-positive guard against prose no one has
# written yet, and any new process chain re-arms it. Unexercised is not the
# same as unnecessary, and the branch it guards is still load-bearing: the
# bare-arrow rule is the sole extractor for three real targets in
# kyberforge's audit skills (agent-audit -> agent-author, agent-audit ->
# skill-audit, skill-audit -> skill-author), all written unbackticked.
# * A backticked hyphenated token counts only inside a boundary sentence.
# Unconditionally, `pre-push` or `commit-msg` in a TRIGGER clause is a hard
# FAIL with no escape hatch. Gating it costs nothing (measured over this
@@ -535,6 +609,66 @@ def known_targets(start_dir):
# ambiguity to resolve, and an author who wants a route checked unconditionally
# has two ways to say so.
#
# BOTH FORMS ARE SWEPT FOR ON THEIR OWN, and that is a repair of the promise
# above rather than a widening of it. Until the sweeps existed, notation was
# only ever seen as the OBJECT OF A ROUTE VERB (`use
# /name`) or as the tail of a `not ... ->` clause with no `;` or sentence end in
# between. Every one of these therefore exited 0 in total silence — no ERROR, no
# SUGGESTION, not even the target's name:
# Do not use for Y — /no-such-skill instead.
# Do not use for Y; /no-such-skill handles that.
# Do not use for Y (/no-such-skill covers it).
# Do not use for Y — that is /no-such-skill's job.
# Do not use for Y — defer to /no-such-skill.
# Do not use for Y — /no-such-skill.
# Do not use for Y; -> no-such-skill covers it.
# For W, /no-such-skill is the right entry point.
# The target was never EXTRACTED, so the notation-first rule in _add() had
# nothing to apply itself to and the "always blocks" promise was false for the
# ordinary way an author writes the thing. The SUGGESTION tier made it worse
# than a gap: its printed remedy tells the author to "write it as `/name` or
# `-> name` and it will be checked properly", and taking that advice turned a
# visible SUGGESTION into silence — the gate teaching the one edit that blinds
# it.
#
# THE TWO SWEEPS ARE GATED DIFFERENTLY, and the asymmetry is the whole point.
# `/name` is Claude Code's invocation syntax and nothing else — no English
# sentence contains one by accident — so the ADR-0020 amendment and
# docs/spec/gates.md both promise it blocks UNCONDITIONALLY, for any name. So
# NOTATION_SLASH is swept over every sentence, boundary marker or not. Gating it
# on BOUNDARY_MARKER made that promise false for the last sentence of
# Do not use for Z — use /real-skill instead.
# For W, /no-such-skill is the right entry point.
# which exited 0 in total silence: the boundary clause is one sentence up, so
# the sweep never looked at the sentence carrying the broken route. Extraction is
# per-sentence by design (corroboration is scoped to one sentence), which is
# exactly what made the gap invisible.
#
# NOTATION_ARROW stays gated on BOUNDARY_MARKER, and so does the backtick sweep.
# Neither form is unambiguous: `-> name` is also how a process chain is written
# ("reproduce -> minimise -> regression-test") and a code span is how a tool, a
# file and a skill are all cited. Ungating either would fire on prose that
# carries no routing intent at all — the false-positive class this whole
# extractor is tuned against.
#
# BOTH `/name` PATTERNS REFUSE A TOKEN THAT IS PART OF A PATH: a following `/`,
# or a `.` followed by a non-space, means `references/foo.md`, `docs/a/b.md` or
# `https://x/y`, not a route. A sentence's closing `.` is not followed by a
# non-space, so `— /no-such-skill.` still counts.
#
# THAT GUARD IS WRITTEN `(?![\w-])` AND NOT `\b`, because `\b` is not a guard at
# all here: it holds after a hyphen, so when the trailing lookahead rejected the
# full segment the engine simply backtracked to a shorter hyphen-terminated
# prefix and reported THAT as a route. Every one of these was a hard blocking
# ERROR naming a skill nobody had written:
# the config lives at /opt-tools/bin/thing. -> 'opt'
# see /api-docs/v2.md for the schema. -> 'api' AND 'api-docs'
# the file /no-such-skill.md documents it. -> 'no-such'
# `(?![\w-])` forbids the shortened prefix outright, so the whole segment is
# rejected as the path it is. MARKED_TARGET carries the same guard: it had no
# trailing lookahead whatsoever, so `see /api-docs/v2.md` raised the second of
# the two errors above through the route-verb path rather than the sweep.
#
# NAMESPACE: `plugin:skill` is live in this repo (native user-scope installs
# still resolve `gitea:gitea-prs`), so the patterns admit an optional
# `<plugin>:` prefix and normalize_target() strips it before resolution.
@@ -545,7 +679,8 @@ ROUTE_VERB = (r"(?:use|uses|using|run|runs|invoke|invokes|invoking|try|see"
r"|that'?s|compose|composes|call|calls"
r"|routes?\s+to|delegates?\s+to|prefers?|switch(?:es)?\s+to"
r"|hands?\s+off\s+to)")
MARKED_TARGET = r"(?:`/?(%s)`|(?<![\w./*-])/(%s)\b)" % (NAME_ANY, NAME_ANY)
MARKED_TARGET = (r"(?:`/?(%s)`|(?<![\w./*-])/(%s)(?![\w-])(?!/|\.\S))"
% (NAME_ANY, NAME_ANY))
ANY_TARGET = r"(?:%s|(%s)\b)" % (MARKED_TARGET, NAME_HYPH)
ROUTE_MARKED = re.compile(r"\b%s\s+(?:the\s+|an?\s+)?%s" % (ROUTE_VERB, MARKED_TARGET), re.I)
ROUTE_ANY = re.compile(r"\b%s\s+(?:the\s+|an?\s+)?%s" % (ROUTE_VERB, ANY_TARGET), re.I)
@@ -558,12 +693,53 @@ ROUTE_ANY = re.compile(r"\b%s\s+(?:the\s+|an?\s+)?%s" % (ROUTE_VERB, ANY_TARGET)
CONT_MARKED = re.compile(r"\s*(?:or|and|/|,)\s*%s" % MARKED_TARGET, re.I)
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET, re.I)
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET, re.I)
ARROW_BOUNDARY = re.compile(r"\bnot\b[^.;]*?(?:->|→)\s*(%s)\b" % NAME_HYPH, re.I)
# The two EXPLICIT ROUTE NOTATION sweeps. NOTATION_SLASH runs over EVERY
# sentence; NOTATION_ARROW is scoped to a boundary sentence by its caller (see
# the asymmetry note in the header). NOTATION_SLASH is deliberately not a reuse
# of MARKED_TARGET's `/name` alternative: that one only ever runs behind a route
# verb or an arrow, and it may match a namespaced or path-adjacent token in
# positions this free-standing sweep must refuse.
# NOTATION_ARROW is ARROW_BOUNDARY minus its leading `\bnot\b%s*?`, which is
# what made `Do not use for Y; -> no-such-skill covers it.` invisible:
# CLAUSE_BODY cannot cross the `;`, so the clause's own punctuation disarmed the
# check. Dropping that prefix costs the one false positive the bare-arrow bullet
# above names — a process chain ending in a hyphenated word, `Instead, reproduce
# -> minimise -> regression-test.` — and costs it only in a sentence that already
# carries a BOUNDARY_MARKER. That exposure is neither new nor larger: the same
# chain written `Do not use for X — reproduce -> regression-test.` was already a
# hard ERROR under ARROW_BOUNDARY, so this changes which boundary words reach the
# arrow, not whether prose can. An author who means the chain and not a route
# writes it in its own sentence, where neither pattern looks.
NOTATION_SLASH = re.compile(
r"(?<![\w./*-])/(%s)(?![\w-])(?!/|\.\S)" % NAME_ANY, re.I)
NOTATION_ARROW = re.compile(r"(?:->|→)\s*(%s)\b" % NAME_HYPH, re.I)
# CLAUSE_BODY is what may sit between `Not` and the arrow, and it is NOT
# `[^.;]`. That class cannot cross a `.`, so every boundary clause naming a
# DOTTED FILENAME between the two — `.pre-commit-config.yaml`, `AGENTS.md`,
# `.vale.ini` — was invisible to both patterns below, and the two resulting
# failures were different sizes (issue #110):
# * with a BACKTICKED target the clause was MISDIAGNOSED. The backtick sweep
# still extracted the target, so the route was checked, but the gate
# reported "no boundary clause" on a clause that was present and working.
# Three authors in two retrofit waves reworded a correct clause to satisfy
# the regex, one of them stripping the very filename that discriminates the
# skill from its neighbour.
# * with a BARE target the clause was UNCHECKED. ARROW_BOUNDARY is the only
# extractor for a bare arrow target, so `Not AGENTS.md -> no-such-skill`
# produced no target, no dangling report and no missing-clause SUGGESTION.
# Silence, not noise — the worse of the two failure modes.
# A dot inside a filename is followed by a non-space; a sentence-ending dot is
# followed by whitespace or by end of string. So the class admits a `.` only
# when the next character is not whitespace, which crosses `AGENTS.md` and
# still stops at a real sentence end.
CLAUSE_BODY = r"(?:[^.;]|\.(?=\S))"
ARROW_BOUNDARY = re.compile(
r"\bnot\b%s*?(?:->|→)\s*(%s)\b" % (CLAUSE_BODY, NAME_HYPH), re.I)
BACKTICK = re.compile(r"`(%s)`" % NAME_HYPH, re.I)
# A boundary clause takes two shapes and BOTH count: the prose markers, and
# ADR-0020's compressed arrow form `Not <thing> -> <name>`.
BOUNDARY_MARKER = re.compile(r"\b(?:do\s+not|instead|rather\s+than|not\s+for)\b", re.I)
BOUNDARY_ARROW = re.compile(r"\bnot\b[^.;]*?(?:->|→)", re.I)
BOUNDARY_ARROW = re.compile(r"\bnot\b%s*?(?:->|→)" % CLAUSE_BODY, re.I)
# Sentence boundaries decide the CORROBORATION scope above, so getting one wrong
# is not cosmetic — it moves a target between SUGGESTION and blocking ERROR. Two
# shapes common in these descriptions defeat the naive "period, space, capital"
@@ -583,9 +759,17 @@ BOUNDARY_ARROW = re.compile(r"\bnot\b[^.;]*?(?:->|→)", re.I)
# a lowercase letter. Verified zero-delta on the current corpus (37 ERROR / 58
# SUGGESTION / 2 dangling before and after) — this protects the descriptions
# issue #99 is about to rewrite, not the ones already measured.
# re.I here too, and NOT as a tidy-up: this was the one pattern in the file
# built without it, contradicting the uniformity note on CONT_*/ARROW_* above.
# Without the flag `E.g.` and `I.e.` — the sentence-initial spellings, which is
# where an abbreviation most often lands — matched none of the lookbehinds, so
# the clause split at the abbreviation, the corroborating target was stranded on
# the far side of the cut, and a genuinely dangling target silently demoted from
# blocking ERROR to SUGGESTION. That is the OVER-SPLIT failure described
# directly above, still live for exactly the capitalised half of the input.
SENTENCE_SPLIT = re.compile(
u'(?<!\\be\\.g\\.)(?<!\\bi\\.e\\.)(?<!\\betc\\.)(?<!\\bvs\\.)(?<!\\bcf\\.)'
u'(?<=[.!?])\\s+(?=[A-Za-z`"“(])')
u'(?<=[.!?])\\s+(?=[A-Za-z`"“(])', re.I)
# The token that may follow a route target without turning it into a compound
# modifier: punctuation, end of sentence, a conjunction, a boundary word, or a
@@ -644,11 +828,26 @@ def _notation(text, start, arrow):
def _add(out, text, name, start, end, strict=None, arrow=False):
"""Record one target as (name, may_dangle, notation).
NOTATION IS DECIDED FIRST, and when it is set the follower test is skipped.
The header above promises that route notation "always blocks", and for the
`/name` form that was false: `-> name` reached this function with
strict=True from its two call sites, but `/name` did not, so it fell to
_terminal() and a follower outside FOLLOWER_OK set may_dangle=False. The
target then reached unresolved_targets() unblockable — and, before the
companion fix there, unreported as well. `... use /no-such-skill
afterwards.` exited 0 in total silence, on the one form ADR-0020 offers an
author who wants a route checked unconditionally.
"""
if not name:
return
notation = _notation(text, start, arrow)
if strict is None and notation:
strict = True
out.append((name,
_terminal(text, end) if strict is None else strict,
_notation(text, start, arrow)))
notation))
def _scan(text, route_re, cont_re, out):
@@ -688,7 +887,19 @@ def _extract_sentence(sentence):
for match in ARROW_BOUNDARY.finditer(sentence):
_add(out, sentence, match.group(1), match.start(1), match.end(1),
strict=True, arrow=True)
# `/name` wherever it sits, in ANY sentence — not only where a route verb or
# an arrow happens to precede it, and NOT only inside a boundary sentence.
# See the EXPLICIT ROUTE NOTATION note in the header for the eight phrasings
# this recovers and for why silence was the failure mode. The sweep takes no
# follower test: _add() reads the notation first and marks it.
for match in NOTATION_SLASH.finditer(sentence):
_add(out, sentence, match.group(1), match.start(1), match.end(1))
if boundary:
# The arrow and backtick forms are ambiguous in ordinary prose, so they
# stay scoped to a sentence that carries a boundary marker.
for match in NOTATION_ARROW.finditer(sentence):
_add(out, sentence, match.group(1), match.start(1), match.end(1),
strict=True, arrow=True)
for match in BACKTICK.finditer(sentence):
_add(out, sentence, match.group(1), match.start(1), match.end(1))
return out
@@ -707,6 +918,85 @@ def boundary_targets(description):
return sorted({name for name, _, _ in _extract(description)})
def _arrow_targets(description):
"""Names extracted from ARROW notation specifically.
Kept apart from boundary_targets() because the arrow form is the one shape
that ALWAYS names a target: ADR-0020's `Not <thing> -> <name>`. A clause
written that way from which nothing could be extracted is a parse failure
that deserves its own message, and telling it apart needs the arrow targets
alone rather than every target in the description.
"""
out = []
for sentence in SENTENCE_SPLIT.split(description):
for match in ARROW_MARKED.finditer(sentence):
name, _, _ = _first(match)
if name:
out.append(name)
for match in ARROW_BOUNDARY.finditer(sentence):
out.append(match.group(1))
return out
def boundary_clause_status(description):
"""'absent', 'unparsed' or 'present' — three outcomes, not two.
Issue #110's standing request: the gate must distinguish "no boundary
clause" from "boundary clause I could not parse". Reporting the first for
the second sends the author hunting for a problem that is not there, and
three of them reworded a correct clause to satisfy a regex instead.
'unparsed' is the narrow, certain case: an ADR-0020 arrow clause was
detected and NO target came out of it. The arrow form always names one, so
zero targets means the name is written in a shape the extractor cannot see
— a single-word bare target (`Not X -> forge`, which has to be written
`` `forge` `` or `/forge`) is the live example, since single-word names are
deliberately not matchable bare.
A PROSE clause yielding no target is NOT reported: "Do not use for anything
else" is a complete and legitimate boundary clause that names nowhere to go.
"""
if BOUNDARY_ARROW.search(description) and not _arrow_targets(description):
return 'unparsed'
if has_boundary_clause(description):
return 'present'
return 'absent'
def multi_target_arrow_clauses(description):
"""[(first, second)] for arrow clauses naming more than one target.
Issue #107: only the FIRST target after an arrow is resolved. The
conjunction continuation (CONT_*) is wired to the prose route verbs and
never to arrows, so `Not X -> a or b` resolved `a`, left `b` neither
resolved nor reported, and then printed "1 of 1 boundary target(s) resolve"
on a clause naming two — a gate under-reporting its own coverage, which is
the one failure mode ADR-0020 says a gate must not have.
The clause is REJECTED rather than the arrow scan extended. Extending it
would widen the resolver's deliberately conservative false-positive tuning
across every arrow in the corpus; rejecting costs nothing and makes the
one-arrow-per-target convention — already what every retrofitted gitea
skill does in practice — explicit instead of folkloric. The caller emits a
SUGGESTION telling the author to split.
"""
hits = []
for sentence in SENTENCE_SPLIT.split(description):
matches = (list(ARROW_MARKED.finditer(sentence))
+ list(ARROW_BOUNDARY.finditer(sentence)))
for match in matches:
first, _, _ = _first(match)
if not first:
continue
cont = CONT_ANY.match(sentence, match.end())
if not cont:
continue
second, _, _ = _first(cont)
if second:
hits.append((first, second))
return hits
def unresolved_targets(description, known):
"""Targets resolving to nothing, split into (blocking, reported).
@@ -723,6 +1013,17 @@ def unresolved_targets(description, known):
Everything else is reported and left alone. `known` is the resolved
universe from known_targets(); passing an empty set is not meaningful —
callers check for that first and decline out loud instead.
A NON-TERMINAL target is reported, never dropped. FOLLOWER_OK is a closed
whitelist of maybe eighty words, so the follower rule says "this token is
outside a list I keep" and not "this is prose" — and the old `continue`
turned that into invisibility at every tier. The gate then failed OPEN on
its own unfamiliarity: any target followed by a word nobody thought to
enumerate was neither blocked nor mentioned, so the check that did not run
said nothing about not running. The follower rule may withdraw the power to
BLOCK a commit — that is what it was added for, and the ATTRIBUTIVE USE note
above is the argument for it — but it may not withdraw visibility, which is
the same rule the corroboration tier already follows.
"""
blocking, reported = set(), set()
for sentence in SENTENCE_SPLIT.split(description):
@@ -731,7 +1032,10 @@ def unresolved_targets(description, known):
if normalize_target(name) in known}
for name, may_dangle, notation in found:
key = normalize_target(name)
if key in known or not may_dangle:
if key in known:
continue
if not may_dangle:
reported.add(name)
continue
if notation or (resolved - {key}):
blocking.add(name)
@@ -811,6 +1115,47 @@ def description_value(fm_text):
return re.sub(r'\s+', ' ', value).strip()
def hand_invoked(fm_text):
"""True when the frontmatter marks this file as reached only by hand.
`disable-model-invocation: true` removes a skill from the model-visible
listing entirely — it is not preloaded, and the Skill tool refuses to call
it — so its description is never matched against user intent. ADR-0020 and
skill-author's contract give such a skill ONE plain human-facing sentence:
no trigger list, no boundary clause. No validator knew the field existed
(issue #108), so the boundary-clause SUGGESTION fired on exactly the shape
the contract mandates, and its remedy — "add a boundary clause 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 followed the advice made the file worse.
Only the ROUTING rules are lifted. The body word budget still applies: the
body is loaded on invocation like any other, and competes with the caller's
live conversation the same way. So does the 400-character description FAIL —
a hand-invoked description is not preloaded, but it is still the one line
the user reads when choosing from the `/` menu, and the ceiling is the
outlier stop rather than the style target.
A parse failure returns False rather than raising. This is a MODIFIER on
other checks, not a check of its own: the frontmatter's validity is decided,
and failed, by description_value() on the same text, and raising a second
exception here would report one broken file twice with two different
diagnoses.
"""
try:
data = yaml.safe_load(fm_text)
except Exception:
return False
if not isinstance(data, dict):
return False
value = data.get('disable-model-invocation')
if isinstance(value, str):
# PyYAML already resolves the unquoted YAML 1.1 booleans, so this only
# catches a QUOTED "true" — which a host reads as truthy and which no
# gate should treat as opting back in to the routing rules.
return value.strip().lower() in ('true', 'yes', 'on')
return value is True
# --- Body-shape checks (skills only; agents have no references/ dir) -------
# Deterministic and countable, so they are enforced here. Whether a given
# gotcha is WARRANTED is semantic and stays the auditor's judgment, which is why
@@ -925,7 +1270,15 @@ def missing_reference_pointers(body, skill_dir):
end = masked.find('\n', match.end())
if end < 0:
end = len(masked)
if REFERENCE_PAST.search(masked[start:end]):
# The pointer's OWN SPAN is excised before the sweep. Run over the
# whole line, the past-tense test matched the very path it was judging,
# so a file exempted itself by its NAME: `references/deprecated-api.md`,
# `references/removed-flags.md` and `references/gone.md` produced no
# ERROR at all, while `references/missing.md` — an identical break —
# errored. The exemption is about what the SENTENCE says about the
# pointer, never about what the pointer is called.
line = masked[start:match.start()] + masked[match.end():end]
if REFERENCE_PAST.search(line):
continue
if REFERENCE_QUALIFIER.search(masked[start:match.start()]):
continue
@@ -942,7 +1295,9 @@ for path in files:
# so pre-commit really can hand one to this hook, and a directory named
# SKILL.md reaches it the same way — both used to exit 0 with zero
# output, which is precisely the "stay quiet about a measurement it did
# not take" failure this script forbids itself two screens up.
# not take" failure this script forbids itself two screens up. This is
# the ONLY place that diagnosis is made; the bash pre-loop that used to
# duplicate it printed a second ERROR line for the same broken file.
if os.path.isdir(path):
why = "is a directory, not a file"
elif os.path.islink(path):
@@ -964,8 +1319,8 @@ for path in files:
# SPEC CONFORMANCE (family 1). Whole file, frontmatter included, counted
# with the SAME primitives skill-audit/scripts/validate.sh uses for these
# two constants — see the note in the bash loop above for what the previous
# awk pass got wrong.
# two constants — see the note in bash above for what the previous awk pass
# got wrong.
lines = len(raw.splitlines())
words = len(raw.split())
if lines > MAX_LINES:
@@ -1000,6 +1355,9 @@ for path in files:
body = content[fm_match.end():]
skill_dir = os.path.dirname(os.path.abspath(path))
# ADR-0020's hand-invocation carve-out. See hand_invoked() for what it lifts
# and, more importantly, what it does not (issue #108).
by_hand = hand_invoked(fm_match.group(1))
# An absent or empty description is an ERROR here too, not a silent skip.
# All three ADR-0020 scripts have to agree on this input: the description is
@@ -1021,7 +1379,12 @@ for path in files:
"enumeration, output-format detail, composition notes and implementation "
"detail to the body or README.md."
% (path, dlen, DESC_MAX_CHARS))
elif dlen > DESC_SUGGEST_CHARS:
elif dlen > DESC_SUGGEST_CHARS and not by_hand:
# The 250-character TARGET is a routing-quality budget: it exists to
# keep the preloaded listing small and the trigger clause sharp. A
# hand-invoked description is in no listing, so there is no budget to
# spend and no shape to enforce. The 400-character FAIL above still
# applies — see hand_invoked().
suggest("%s: description is %d characters, over the %d-character target "
"(ADR-0020, hard fail at %d)."
% (path, dlen, DESC_SUGGEST_CHARS, DESC_MAX_CHARS))
@@ -1064,15 +1427,40 @@ for path in files:
round(100.0 * section_words / body_words),
round(100.0 * GOTCHA_MAX_BODY_FRACTION)))
# Missing boundary clause. SUGGESTION, not ERROR: detecting the absence is
# Boundary clause. SUGGESTION, not ERROR: detecting the absence is
# deterministic, but whether this particular skill warrants one is the
# auditor's call. Both accepted shapes count — the prose markers and
# ADR-0020's compressed `Not <thing> -> <name>` arrow.
if desc and not has_boundary_clause(desc):
suggest("%s: description has no boundary clause (ADR-0020). Add the prose form "
"(\"Do not use for X — use `y` instead\") or the compressed form "
"(\"Not X -> y\") so the router knows where NOT to send this skill."
% path)
#
# THREE outcomes, not two. Reporting "no boundary clause" for a clause that
# is present and merely unparsed is a wrong finding, not a strict one, and
# it cost three authors a reworded clause before it was diagnosed (#110).
#
# Skipped entirely for a hand-invoked skill: the contract gives it one plain
# sentence with no boundary clause, so the finding is wrong and its remedy
# names a router that cannot see the skill (#108).
if desc and not by_hand:
status = boundary_clause_status(desc)
if status == 'absent':
suggest("%s: description has no boundary clause (ADR-0020). Add the prose form "
"(\"Do not use for X — use `y` instead\") or the compressed form "
"(\"Not X -> y\") so the router knows where NOT to send this skill."
% path)
elif status == 'unparsed':
suggest("%s: description has an arrow boundary clause (\"Not X -> y\") from which "
"no target could be read, so the dangling-target check did not run on it "
"(ADR-0020). The clause is present — this is a PARSE failure, not a "
"missing clause. Most often the target is a single word, which is "
"deliberately not matchable bare because `research`, `triage` and `forge` "
"are all ordinary English: write it as `name` or /name." % path)
# One arrow, one target. A second name after the arrow is resolved by
# nothing and reported by nothing, so the clause claims coverage it does
# not have (#107).
for first, second in multi_target_arrow_clauses(desc):
suggest("%s: an arrow boundary clause names more than one target ('%s', then "
"'%s'), and only the first is resolved — the second is checked by "
"nothing (ADR-0020). Split it into one arrow per target: "
"\"Not X -> %s. Not Y -> %s.\"" % (path, first, second, first, second))
targets = boundary_targets(desc)
if targets: