fix(gates): close the /name fail-open and stop the path guard inventing targets
Two defects in the routing-target resolver, both latent in the corpus but hot for anything written next. The free-standing `/name` sweep sat inside `if boundary:`, so route notation in a sentence carrying no boundary marker was never extracted at all — not an ERROR, not a SUGGESTION, not an INFO. That contradicted ADR-0020's amendment and gates.md, which both promise `/name` blocks unconditionally. The sweep now runs over every sentence. `-> name` and backticked forms stay gated deliberately: an arrow also writes a process chain and a code span cites tools, files and skills alike, so ungating either fires on ordinary prose. The path guard used `\b`, which still holds after a hyphen, so the engine backtracked to a shorter hyphen-terminated prefix whenever the lookahead rejected the full segment. `/api-docs/v2.md` in a boundary clause raised blocking ERRORs for 'api' and 'api-docs' — names no author wrote, with no corroboration escape. `(?![\w-])` forbids the shortened prefix outright; MARKED_TARGET, which had no trailing guard at all, gained one. Zero arguments now exits 2 rather than 0, so a mis-scoped `files:` pattern is no longer indistinguishable from a clean corpus. Both hook manifests pass filenames and pre-commit skips a filename-passing hook when nothing matches, so the hook never sees an empty argv — that contract is now asserted by a test rather than left in prose. Deleting the sweep entirely used to leave every suite green. It now kills eight assertions. The suite also gains its first slash-path and URL fixtures, in both directions. Refs: #107, #110, #124 ADR: 0020
This commit is contained in:
@@ -106,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
|
||||
@@ -165,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])
|
||||
@@ -580,9 +609,9 @@ 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 inside a boundary sentence, 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
|
||||
# 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:
|
||||
@@ -593,6 +622,7 @@ def known_targets(start_dir):
|
||||
# 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
|
||||
@@ -601,12 +631,43 @@ def known_targets(start_dir):
|
||||
# visible SUGGESTION into silence — the gate teaching the one edit that blinds
|
||||
# it.
|
||||
#
|
||||
# The sweeps are gated on the sentence carrying a BOUNDARY_MARKER, the same gate
|
||||
# the backtick sweep uses, and NOTATION_SLASH refuses 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.
|
||||
# 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
|
||||
@@ -618,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)
|
||||
@@ -631,10 +693,12 @@ 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)
|
||||
# The two EXPLICIT ROUTE NOTATION sweeps, scoped to a boundary sentence by their
|
||||
# caller. 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 the
|
||||
# trailing lookahead here is the part that makes a FREE-STANDING sweep safe.
|
||||
# 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
|
||||
@@ -646,7 +710,8 @@ ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET, re.I)
|
||||
# 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)\b(?!/|\.\S)" % NAME_ANY, re.I)
|
||||
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
|
||||
@@ -822,14 +887,16 @@ 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:
|
||||
# Route notation wherever it sits in the clause, not only where a route
|
||||
# verb or an arrow happens to precede it. See the EXPLICIT ROUTE
|
||||
# NOTATION note in the header for the seven phrasings this recovers and
|
||||
# for why silence was the failure mode. Neither sweep takes the follower
|
||||
# test: _add() reads the notation first and both forms reach it marked.
|
||||
for match in NOTATION_SLASH.finditer(sentence):
|
||||
_add(out, sentence, match.group(1), match.start(1), match.end(1))
|
||||
# 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)
|
||||
@@ -1228,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):
|
||||
@@ -1250,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:
|
||||
|
||||
Reference in New Issue
Block a user