test: pin the nine ADR-0020 gate defects that shipped untested
Every defect fixed in f7cc279 was reachable because nothing asserted against it.
The gate had 43 assertions and none of them covered a consumer repo, a non-string
description, an unclosed fence, or the two spec ceilings. Each case below fails
against the pre-fix code and passes against the current one; every one was proved
non-vacuous by mutating a scratch copy of the script and watching the test go red,
independently twice.
The two that mattered most had no fixture anywhere. A consumer repo WITH .git is
the shape the resolver exists to serve, and only the no-.git case had ever been
tested, which is exactly why the blocker was invisible. And ADR-0020 says the
walk-up runs in two passes specifically so a nested .git cannot beat a plugins/
root further up — no fixture had ever placed a .git inside a plugin.
test-adr0020-differential.sh loses _non_adr_hook_error(). It excluded MAX_LINES and
MAX_WORDS from the cross-script comparison on the untested assumption that awk and
splitlines() agree. They do not, and the divergence stayed invisible for exactly as
long as the exclusion stood. The ceilings are now compared like any other rule.
Two existing assertions were repairs, not additions. The skill-improve probe had
been fixed by this very branch, so its iteration permanently took an
assertion-free SKIP that still counted as a pass; both branches now fail loudly and
each names the other file's pin so the two stay in step. And the yaml-none fixture
emitted `---/---`, which never matched the frontmatter pattern at all — it passed on
the bare word "frontmatter", present in both messages, while never reaching the
branch it was named for. Needles throughout that file now name their branch.
Refs: #99
ADR: 0020
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015W3iwF9ncfRZddGBxsMCYi
This commit is contained in:
@@ -15,14 +15,20 @@
|
||||
# ready to ship and the commit hook then rejects it, or worse, the reverse. So the
|
||||
# comparison here is over VERDICTS on files, not over source text.
|
||||
#
|
||||
# Scope: the ADR-0020 axes the two scripts share — description length and tier,
|
||||
# body word count and tier, dangling routing targets, missing references/
|
||||
# pointers, the two Gotchas suggestions, the missing-boundary-clause suggestion,
|
||||
# a declined resolution, and an empty description. The two scripts legitimately
|
||||
# differ elsewhere (validate.sh also checks name/directory agreement, script
|
||||
# executability and the 1024-char spec backstop; the hook checks whole-file lines
|
||||
# and words), and those lines are ignored rather than being forced into a shared
|
||||
# shape they were never meant to have.
|
||||
# Scope: every axis the two scripts share. The ADR-0020 ones — description
|
||||
# length and tier, body word count and tier, dangling routing targets, missing
|
||||
# references/ pointers, the two Gotchas suggestions, the missing-boundary-clause
|
||||
# suggestion, a declined resolution, an empty description — plus the two
|
||||
# agentskills.io spec ceilings, MAX_LINES and MAX_WORDS.
|
||||
#
|
||||
# Those last two were EXCLUDED from this comparison until a real divergence
|
||||
# shipped behind the exclusion. The header used to say "the hook checks
|
||||
# whole-file lines and words" as if the auditor did not; it does, from its own
|
||||
# copy of the same two constants, and the two implementations disagreed on
|
||||
# Unicode whitespace for as long as nobody compared them. An axis both scripts
|
||||
# measure is in scope by definition — the only lines still ignored are the ones
|
||||
# a single script owns outright (validate.sh's name/directory agreement, script
|
||||
# executability and 1024-char description backstop).
|
||||
#
|
||||
# Run over the real 39-skill corpus AND over purpose-built fixtures that sit ON
|
||||
# each boundary. The corpus alone is not enough — it happens not to contain a
|
||||
@@ -145,6 +151,48 @@ make_fx gotchas-fraction "$CLEAN" 0
|
||||
python3 -c "print(' '.join(['word'] * 70))"
|
||||
} >> "$FX/gotchas-fraction/SKILL.md"
|
||||
|
||||
# The agentskills.io spec ceilings, measured over Unicode whitespace.
|
||||
#
|
||||
# These two are in the comparison at all because they used to be excluded from
|
||||
# it — `_non_adr_hook_error()` waved a spec-ceiling exit through as "not a
|
||||
# disagreement", and that exclusion is exactly why the divergence below stayed
|
||||
# invisible. The hook counted lines and words in a single awk pass (NR / NF)
|
||||
# while skill-audit counted them with Python's splitlines() / split(). The two
|
||||
# primitives do not agree: splitlines() also breaks on U+2028, U+2029, \x0b,
|
||||
# \x0c, \x1c-\x1e and \x85, and split() breaks on every Unicode space. Same
|
||||
# constants, same file, different verdict — hook green, audit FAIL, which is the
|
||||
# precise failure mode ("passes its own audit, blocked by the commit hook",
|
||||
# inverted) this whole suite exists to catch.
|
||||
#
|
||||
# One fixture per primitive, each sitting just past its ceiling on the Python
|
||||
# measurement and nowhere near it on the awk one.
|
||||
python3 - "$FX" <<'PY'
|
||||
import os
|
||||
import sys
|
||||
|
||||
fx = sys.argv[1]
|
||||
# Spelled as escapes, never as literals. An invisible separator pasted into a
|
||||
# source file is unreviewable and one editor round-trip away from becoming an
|
||||
# ordinary space, which would silently turn both fixtures into nothing.
|
||||
SEP_LINE = '\u2028' # LINE SEPARATOR: splitlines() breaks on it, awk's NR does not
|
||||
SEP_WORD = '\u00a0' # NO-BREAK SPACE: split() breaks on it, awk's NF does not
|
||||
head = ('---\nname: %s\n'
|
||||
'description: Use when doing the thing. Do not use for anything else.\n'
|
||||
'---\n\n')
|
||||
# 600 U+2028-separated segments: 605 lines to splitlines(), 6 to awk's NR.
|
||||
# Word count stays far below the 2,770 ceiling, so this fixture isolates lines.
|
||||
cases = {
|
||||
'spec-lines-u2028': SEP_LINE.join(['word'] * 600),
|
||||
# 2,800 U+00A0-separated words: 2,816 words to split(), 17 to awk's NF.
|
||||
'spec-words-u00a0': SEP_WORD.join(['word'] * 2800),
|
||||
}
|
||||
for name, body in cases.items():
|
||||
d = os.path.join(fx, name)
|
||||
os.makedirs(d, exist_ok=True)
|
||||
with open(os.path.join(d, 'SKILL.md'), 'w', encoding='utf-8') as fh:
|
||||
fh.write(head % name + body + '\n')
|
||||
PY
|
||||
|
||||
# Empty description — the shape that used to exit 0 in silence.
|
||||
mkdir -p "$FX/empty-desc"
|
||||
printf -- '---\nname: empty-desc\ndescription:\nmodel: sonnet\n---\n\nDo the thing.\n' \
|
||||
@@ -219,6 +267,18 @@ RULES = (
|
||||
('NO_BOUNDARY_CLAUSE', re.compile(r'(description has no boundary clause)')),
|
||||
('RESOLUTION_DECLINED', re.compile(r'(boundary-target resolution DID NOT RUN)')),
|
||||
('DESC_EMPTY', re.compile(r'(description field is missing or empty)')),
|
||||
# The agentskills.io spec ceilings. These were EXCLUDED from the comparison
|
||||
# until the awk/Python divergence shipped, on the reasoning that "the hook
|
||||
# checks whole-file lines and words" and the auditor did not. It does — with
|
||||
# the same two constants — so the exclusion was never a scope decision, only
|
||||
# an untested assumption, and it hid a real disagreement. Both scripts spell
|
||||
# the finding differently, so the patterns match either wording and capture
|
||||
# only the MEASUREMENT:
|
||||
# hook: "... has 605 lines, exceeding the 500-line ceiling ..."
|
||||
# audit: "SKILL.md line count 605 — exceeds 500-line limit"
|
||||
('SPEC_LINES', re.compile(r'(?:has|line count) (\d+)(?: lines,)? (?:exceeding|—)')),
|
||||
('SPEC_WORDS', re.compile(
|
||||
r'(?:has|word count) (\d+)(?: words \(proxy for tokens\),)? (?:exceeding|—)')),
|
||||
)
|
||||
|
||||
|
||||
@@ -227,9 +287,15 @@ def verdict(output):
|
||||
|
||||
Lines that match no rule are dropped rather than compared: the two scripts
|
||||
legitimately check different things outside ADR-0020 (name/directory
|
||||
agreement, script executability, the 1024-char spec backstop, whole-file
|
||||
line and word ceilings), and forcing those into the comparison would report
|
||||
a difference that is not a disagreement.
|
||||
agreement, script executability, the 1024-char spec backstop), and forcing
|
||||
those into the comparison would report a difference that is not a
|
||||
disagreement.
|
||||
|
||||
The whole-file line and word ceilings are NOT in that list. They were
|
||||
excluded once, on the untested assumption that awk and splitlines() agree;
|
||||
they do not, and the divergence was invisible for exactly as long as the
|
||||
exclusion stood. SPEC_LINES/SPEC_WORDS are compared like any other rule —
|
||||
see the file header. Do not re-add an exclusion for them.
|
||||
"""
|
||||
found = set()
|
||||
for raw in output.splitlines():
|
||||
@@ -278,9 +344,15 @@ def compare(label, skill_dir):
|
||||
problems.append('the hook reported an ADR-0020 ERROR but exited 0')
|
||||
if audit_err and audit_rc == 0:
|
||||
problems.append('skill-audit reported an ADR-0020 FAIL but exited 0')
|
||||
if not hook_err and hook_rc != 0 and not _non_adr_hook_error(hook_out):
|
||||
problems.append('the hook exited %d with no ADR-0020 ERROR and no spec-ceiling ERROR'
|
||||
% hook_rc)
|
||||
# No escape hatch here any more. There used to be one — a
|
||||
# `_non_adr_hook_error()` helper that waved through a non-zero hook exit
|
||||
# explained by MAX_LINES / MAX_WORDS, on the grounds that those two were
|
||||
# outside the comparison. They are inside it now (see SPEC_LINES /
|
||||
# SPEC_WORDS in RULES), so every ERROR the hook can raise is a token this
|
||||
# comparison holds both scripts to.
|
||||
if not hook_err and hook_rc != 0:
|
||||
problems.append('the hook exited %d with no compared ERROR at all — it has an '
|
||||
'ERROR source this comparison does not know about' % hook_rc)
|
||||
|
||||
if problems:
|
||||
bad('%s: %s' % (label, '; '.join(problems)))
|
||||
@@ -289,16 +361,6 @@ def compare(label, skill_dir):
|
||||
return False
|
||||
|
||||
|
||||
def _non_adr_hook_error(output):
|
||||
"""True if the hook failed on a spec ceiling rather than an ADR-0020 gate.
|
||||
|
||||
MAX_LINES / MAX_WORDS are the hook's other ERROR sources and are outside
|
||||
this comparison, so a non-zero exit explained by one of them is not a
|
||||
disagreement.
|
||||
"""
|
||||
return bool(re.search(r'ERROR: .*(-line ceiling|-word ceiling \(~5,000 tokens)', output))
|
||||
|
||||
|
||||
# --- The real corpus -------------------------------------------------------
|
||||
corpus = sorted(glob.glob(os.path.join(repo_root, 'plugins', '*', '.apm', 'skills', '*')))
|
||||
corpus = [d for d in corpus if os.path.isfile(os.path.join(d, 'SKILL.md'))]
|
||||
@@ -359,6 +421,31 @@ else:
|
||||
ok('every one of the %d compared axes was exercised by at least one fixture'
|
||||
% len(expected_tokens))
|
||||
|
||||
# --- The Unicode-whitespace fixtures, named and asserted directly -----------
|
||||
# The two comparisons above would catch this divergence, but only as "fixture
|
||||
# spec-lines-u2028 disagreed" — one line among 65. Spelled out here so the
|
||||
# failure names the primitive, and so the ceiling is asserted to FIRE in both
|
||||
# scripts rather than merely to be reported the same way by both.
|
||||
print("")
|
||||
print("--- both scripts break the spec ceilings on the same Unicode whitespace ---")
|
||||
for name, token, expected in (('spec-lines-u2028', 'SPEC_LINES', '605'),
|
||||
('spec-words-u00a0', 'SPEC_WORDS', '2816')):
|
||||
skill_dir = os.path.join(fixture_dir, name)
|
||||
_, h_out = run(['bash', hook, os.path.join(skill_dir, 'SKILL.md')])
|
||||
_, a_out = run(['bash', validate, skill_dir])
|
||||
want = ('ERROR', token, expected)
|
||||
missing = [who for who, v in (('the hook', verdict(h_out)),
|
||||
('skill-audit', verdict(a_out)))
|
||||
if want not in v]
|
||||
if missing:
|
||||
bad('%s: %s did not report %s=%s. The two scripts must count with the '
|
||||
'same primitive — Python splitlines()/split(), not awk NR/NF, which '
|
||||
'does not break on this character' % (name, ' and '.join(missing),
|
||||
token, expected))
|
||||
else:
|
||||
ok('%s: both scripts measure %s=%s and raise the ceiling ERROR'
|
||||
% (name, token, expected))
|
||||
|
||||
print("")
|
||||
print("Results: %d passed, %d failed" % (passes, failures))
|
||||
sys.exit(1 if failures else 0)
|
||||
|
||||
Reference in New Issue
Block a user